ban-comma-operator: ignore for-loop incrementors #3485
Conversation
|
Thanks for your interest in palantir/tslint, @vilchik-elena! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request. |
src/rules/banCommaOperatorRule.ts
Outdated
|
|
||
| function isForLoopIncrementor(node: ts.Node) { | ||
| const parent = node.parent; | ||
| return parent && parent.kind === ts.SyntaxKind.ForStatement && (parent as ts.ForStatement).incrementor === node; |
There was a problem hiding this comment.
node.parent will never be undefined here. You can simply use const parent = node.parent!
That also fixes the lint error on this line, because parent is not a boolean expression.
src/rules/banCommaOperatorRule.ts
Outdated
| function isForLoopIncrementor(node: ts.Node) { | ||
| const parent = node.parent; | ||
| return parent && parent.kind === ts.SyntaxKind.ForStatement && (parent as ts.ForStatement).incrementor === node; | ||
| } |
There was a problem hiding this comment.
linter is complaining about missing newline before return
|
Just to make sure: We probably don't want to whitelist the for loop initializer? let i: number, arr: string[], len: number;
for (i = 0, arr = doStuff(), len = arr.length; i < arr.length; ++i) {
} |
|
Indeed, I think it's better avoid comma in initiallizer, you better put declaration instead. |
|
ci failed again. Is it FP? Locally it's green for me |
|
CI failure is unrelated. Seems like |
|
CI will be fixed by #3488 |
|
thanks @vilchik-elena! |
PR checklist
Overview of change:
Rule
ban-comma-operatorraises an issue on every usage of comma operator, while using it inside for-loop incrementor IMO should be authorised.CHANGELOG.md entry:
[enhancement]
ban-comma-operatorignores comma operator inside for-loop incrementor