From 96f64d9a40ce1fbe8e7bf0bce6723dc2ae2cca4f Mon Sep 17 00:00:00 2001 From: Chris Barr Date: Tue, 24 Oct 2017 10:00:54 -0400 Subject: [PATCH] Improve failure message & docs for ban-comma-operator rule (#3380) (#3384) --- src/rules/banCommaOperatorRule.ts | 30 +++++++++++++++++++--- test/rules/ban-comma-operator/test.ts.lint | 18 +++++++++++-- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/src/rules/banCommaOperatorRule.ts b/src/rules/banCommaOperatorRule.ts index 7e2c694c80a..d0f76af44ef 100644 --- a/src/rules/banCommaOperatorRule.ts +++ b/src/rules/banCommaOperatorRule.ts @@ -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); diff --git a/test/rules/ban-comma-operator/test.ts.lint b/test/rules/ban-comma-operator/test.ts.lint index 945908b6c47..0b9cbf34493 100644 --- a/test/rules/ban-comma-operator/test.ts.lint +++ b/test/rules/ban-comma-operator/test.ts.lint @@ -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.