Add the ban-comma-operator rule (#3250)

src/configs/all.ts

@@ -88,6 +88,7 @@ export const rules = {
     // Functionality
     "await-promise": true,
     // "ban": no sensible default
+    "ban-comma-operator": true,
     "curly": true,
     "forin": true,
     // "import-blacklist": no sensible default

src/configs/latest.ts

@@ -69,6 +69,7 @@ export const rules = {
     "jsdoc-format": {
         options: "check-multiline-start",
     },
+    "ban-comma-operator": true,
 };
 // tslint:enable object-literal-sort-keys
 

src/rules/banCommaOperatorRule.ts

@@ -0,0 +1,50 @@
+/**
+ * @license
+ * Copyright 2017 Palantir Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { isBinaryExpression } from "tsutils";
+import * as ts from "typescript";
+
+import * as Lint from "../index";
+
+export class Rule extends Lint.Rules.AbstractRule {
+    /* tslint:disable:object-literal-sort-keys */
+    public static metadata: Lint.IRuleMetadata = {
+        ruleName: "ban-comma-operator",
+        description: "Bans the comma operator.",
+        options: null,
+        optionsDescription: "",
+        optionExamples: [true],
+        type: "typescript",
+        typescriptOnly: true,
+    };
+    /* tslint:enable:object-literal-sort-keys */
+
+    public static FAILURE_STRING = "Don't use the comma operator.";
+
+    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        return this.applyWithFunction(sourceFile, walk);
+    }
+}
+
+function walk(ctx: Lint.WalkContext<void>) {
+    return ts.forEachChild(ctx.sourceFile, function cb(node: ts.Node): void {
+        if (isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.CommaToken) {
+            ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
+        }
+        return ts.forEachChild(node, cb);
+    });
+}

src/rules/noConditionalAssignmentRule.ts

@@ -26,7 +26,7 @@ export class Rule extends Lint.Rules.AbstractRule {
         ruleName: "no-conditional-assignment",
         description: "Disallows any type of assignment in conditionals.",
         descriptionDetails: "This applies to `do-while`, `for`, `if`, and `while` statements and conditional (ternary) expressions.",
-        rationale: Lint.Utils.dedent `
+        rationale: Lint.Utils.dedent`
             Assignments in conditionals are often typos:
             for example \`if (var1 = var2)\` instead of \`if (var1 == var2)\`.
             They also can be an indicator of overly clever code which decreases maintainability.`,
@@ -56,30 +56,36 @@ function walk(ctx: Lint.WalkContext<void>) {
         }
         switch (kind) {
             case ts.SyntaxKind.ConditionalExpression:
-                return check((node as ts.ConditionalExpression).condition),
-                       cb((node as ts.ConditionalExpression).whenTrue),
-                       cb((node as ts.ConditionalExpression).whenFalse);
+                check((node as ts.ConditionalExpression).condition);
+                cb((node as ts.ConditionalExpression).whenTrue);
+                cb((node as ts.ConditionalExpression).whenFalse);
+                return;
             case ts.SyntaxKind.IfStatement:
-                return check((node as ts.IfStatement).expression),
-                       cb((node as ts.IfStatement).thenStatement),
-                       maybeCallback(cb, (node as ts.IfStatement).elseStatement);
+                check((node as ts.IfStatement).expression);
+                cb((node as ts.IfStatement).thenStatement);
+                maybeCallback(cb, (node as ts.IfStatement).elseStatement);
+                return;
             case ts.SyntaxKind.DoStatement:
             case ts.SyntaxKind.WhileStatement:
-                return check((node as ts.DoStatement | ts.WhileStatement).expression),
-                       cb((node as ts.IterationStatement).statement);
+                check((node as ts.DoStatement | ts.WhileStatement).expression);
+                cb((node as ts.IterationStatement).statement);
+                return;
             case ts.SyntaxKind.ForStatement:
-                return maybeCallback(cb, (node as ts.ForStatement).initializer),
-                       maybeCallback(check, (node as ts.ForStatement).condition),
-                       maybeCallback(cb, (node as ts.ForStatement).incrementor),
-                       cb((node as ts.ForStatement).statement);
+                maybeCallback(cb, (node as ts.ForStatement).initializer);
+                maybeCallback(check, (node as ts.ForStatement).condition);
+                maybeCallback(cb, (node as ts.ForStatement).incrementor);
+                cb((node as ts.ForStatement).statement);
+                return;
         }
         if (checking !== 0) {
             switch (kind) {
                 case ts.SyntaxKind.BinaryExpression:
                     if (isAssignmentKind((node as ts.BinaryExpression).operatorToken.kind)) {
                         ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
                     }
-                    return cb((node as ts.BinaryExpression).left), cb((node as ts.BinaryExpression).right);
+                    cb((node as ts.BinaryExpression).left);
+                    cb((node as ts.BinaryExpression).right);
+                    return;
                 case ts.SyntaxKind.ParenthesizedExpression:
                 case ts.SyntaxKind.NonNullExpression:
                 case ts.SyntaxKind.AsExpression:
@@ -109,6 +115,6 @@ function walk(ctx: Lint.WalkContext<void>) {
 
 function maybeCallback(cb: (node: ts.Node) => void, node?: ts.Node) {
     if (node !== undefined) {
-        return cb(node);
+        cb(node);
     }
 }

test/rules/ban-comma-operator/test.ts.lint

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

test/rules/ban-comma-operator/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "ban-comma-operator": true
+  }
+}