Skip to content

Commit

Permalink
Improve failure message & docs for ban-comma-operator rule (palantir#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ChrisMBarr authored and ajafff committed Oct 24, 2017
1 parent f810a5a commit 1365c3c
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 6 deletions.
30 changes: 26 additions & 4 deletions src/rules/banCommaOperatorRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,41 @@ import * as ts from "typescript";
import * as Lint from "../index";

export class Rule extends Lint.Rules.AbstractRule {
/* tslint:disable:object-literal-sort-keys */
/* tslint:disable:object-literal-sort-keys max-line-length */
public static metadata: Lint.IRuleMetadata = {
ruleName: "ban-comma-operator",
description: "Bans the comma operator.",
description: "Disallows the comma operator to be used.",
descriptionDetails: "[Read more about the comma operator here](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Comma_Operator).",
rationale: Lint.Utils.dedent`
Using the comma operator can create a potential for many non-obvious bugs or lead to misunderstanding of code.
### Examples
\`\`\`
foo((bar, baz)); // evaluates to 'foo(baz)' because of the extra parens - confusing and not obvious
\`\`\`
\`\`\`
switch (foo) {
case 1, 2: // equals 'case 2' - probably intended 'case 1: case2:'
return true;
case 3:
return false;
}
\`\`\`
\`\`\`
let x = (y = 1, z = 2); // x is equal to 2 - this may not be immediately obvious.
\`\`\`
`,
options: null,
optionsDescription: "",
optionExamples: [true],
type: "typescript",
typescriptOnly: true,
};
/* tslint:enable:object-literal-sort-keys */
/* tslint:enable:object-literal-sort-keys max-line-length */

public static FAILURE_STRING = "Don't use the comma operator.";
public static FAILURE_STRING = "Do not use comma operator here because it can be easily misunderstood or lead to unintended bugs.";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithFunction(sourceFile, walk);
Expand Down
18 changes: 16 additions & 2 deletions test/rules/ban-comma-operator/test.ts.lint
Original file line number Diff line number Diff line change
@@ -1,6 +1,20 @@
let x = (y = 1, z = 2);
~~~~~~~~~~~~ [Don't use the comma operator.]
~~~~~~~~~~~~ [0]

// Error prone: forgot to add parens around arguments.
(x, y => x + y)(a, b);
~~~~~~~~~~~~~ [Don't use the comma operator.]
~~~~~~~~~~~~~ [0]

foo((bar, baz));
~~~~~~~~ [0]

switch (blah) {
case 1, 2: // equals `case 2` - probably intended `case 1: case2:`
~~~~ [0]
return true;
case 3:
return false;
}


[0]: Do not use comma operator here because it can be easily misunderstood or lead to unintended bugs.

0 comments on commit 1365c3c

Please sign in to comment.