Add 'binary-expression-operand-order' rule (palantir#2805)

src/configs/all.ts

@@ -165,6 +165,7 @@ export const rules = {
     "array-type": [true, "array-simple"],
     "arrow-parens": true,
     "arrow-return-shorthand": [true, "multiline"],
+    "binary-expression-operand-order": true,
     "callable-types": true,
     "class-name": true,
     "comment-format": [

src/language/utils.ts

@@ -16,7 +16,7 @@
  */
 
 import * as path from "path";
-import { isBlockScopedVariableDeclarationList } from "tsutils";
+import { isBlockScopedVariableDeclarationList, isPrefixUnaryExpression } from "tsutils";
 import * as ts from "typescript";
 
 import {IDisabledInterval, RuleFailure} from "./rule/rule"; // tslint:disable-line deprecation
@@ -425,3 +425,9 @@ export function getEqualsKind(node: ts.BinaryOperatorToken): EqualsKind | undefi
             return undefined;
     }
 }
+
+export function isNegativeNumberLiteral(node: ts.Node): node is ts.PrefixUnaryExpression & { operand: ts.NumericLiteral } {
+    return isPrefixUnaryExpression(node) &&
+        node.operator === ts.SyntaxKind.MinusToken &&
+        node.operand.kind === ts.SyntaxKind.NumericLiteral;
+}

src/rules/alignRule.ts

@@ -169,7 +169,7 @@ class AlignWalker extends Lint.AbstractWalker<Options> {
             if (line !== pos.line && pos.character !== alignToColumn) {
                 const diff = alignToColumn - pos.character;
                 let fix: Lint.Fix | undefined;
-                if (0 < diff) {
+                if (diff >= 0) {
                     fix = Lint.Replacement.appendText(start, " ".repeat(diff));
                 } else if (node.pos <= start + diff && /^\s+$/.test(sourceFile.text.substring(start + diff, start))) {
                     // only delete text if there is only whitespace

src/rules/binaryExpressionOperandOrderRule.ts

@@ -0,0 +1,93 @@
+/**
+ * @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";
+import { isNegativeNumberLiteral } from "../language/utils";
+
+export class Rule extends Lint.Rules.AbstractRule {
+    /* tslint:disable:object-literal-sort-keys */
+    public static metadata: Lint.IRuleMetadata = {
+        ruleName: "binary-expression-operand-order",
+        description: Lint.Utils.dedent`
+            In a binary expression, a literal should always be on the right-hand side if possible.
+            For example, prefer 'x + 1' over '1 + x'.`,
+        optionsDescription: "Not configurable.",
+        options: null,
+        optionExamples: [true],
+        type: "style",
+        typescriptOnly: false,
+    };
+    /* tslint:enable:object-literal-sort-keys */
+
+    public static FAILURE_STRING = "Literal expression should be on the right-hand side of a binary expression.";
+
+    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        return this.applyWithFunction(sourceFile, walk);
+    }
+}
+
+function walk(ctx: Lint.WalkContext<void>): void {
+    ts.forEachChild(ctx.sourceFile, function cb(node) {
+        if (isBinaryExpression(node) && isLiteral(node.left) && !isAllowedOrderedOperator(node)) {
+            ctx.addFailureAtNode(node, Rule.FAILURE_STRING);
+        }
+        ts.forEachChild(node, cb);
+    });
+}
+
+/** Allows certain inherently ordered operators that can't easily be written with the literal on the right. */
+function isAllowedOrderedOperator(node: ts.BinaryExpression): boolean {
+    switch (node.operatorToken.kind) {
+        case ts.SyntaxKind.PlusToken:
+            // Allow `"foo" + x` but not `1 + x`.
+            return node.left.kind === ts.SyntaxKind.StringLiteral;
+        case ts.SyntaxKind.MinusToken:
+        case ts.SyntaxKind.SlashToken:
+        case ts.SyntaxKind.PercentToken:
+        case ts.SyntaxKind.LessThanLessThanToken:
+        case ts.SyntaxKind.GreaterThanGreaterThanToken:
+        case ts.SyntaxKind.GreaterThanGreaterThanGreaterThanToken:
+        case ts.SyntaxKind.AsteriskAsteriskToken:
+        case ts.SyntaxKind.InKeyword:
+        case ts.SyntaxKind.CommaToken:
+            return true;
+        default:
+            return false;
+    }
+}
+
+function isLiteral(node: ts.Expression): boolean {
+    switch (node.kind) {
+        case ts.SyntaxKind.StringLiteral:
+        case ts.SyntaxKind.NumericLiteral:
+        case ts.SyntaxKind.TrueKeyword:
+        case ts.SyntaxKind.FalseKeyword:
+        case ts.SyntaxKind.NullKeyword:
+            return true;
+        case ts.SyntaxKind.Identifier:
+            return (node as ts.Identifier).originalKeywordKind === ts.SyntaxKind.UndefinedKeyword;
+        case ts.SyntaxKind.PrefixUnaryExpression:
+            return isNegativeNumberLiteral(node);
+        case ts.SyntaxKind.ParenthesizedExpression:
+            return isLiteral((node as ts.ParenthesizedExpression).expression);
+        default:
+            return false;
+    }
+}

src/rules/noMagicNumbersRule.ts

@@ -15,10 +15,10 @@
  * limitations under the License.
  */
 
-import { isPrefixUnaryExpression } from "tsutils";
 import * as ts from "typescript";
 
 import * as Lint from "../index";
+import { isNegativeNumberLiteral } from "../language/utils";
 
 export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
@@ -73,9 +73,7 @@ class NoMagicNumbersWalker extends Lint.AbstractWalker<Set<string>> {
             if (node.kind === ts.SyntaxKind.NumericLiteral) {
                 return this.checkNumericLiteral(node, (node as ts.NumericLiteral).text);
             }
-            if (isPrefixUnaryExpression(node) &&
-                node.operator === ts.SyntaxKind.MinusToken &&
-                node.operand.kind === ts.SyntaxKind.NumericLiteral) {
+            if (isNegativeNumberLiteral(node)) {
                 return this.checkNumericLiteral(node, `-${(node.operand as ts.NumericLiteral).text}`);
             }
             return ts.forEachChild(node, cb);

test/rules/binary-expression-operand-order/test.ts.lint

@@ -0,0 +1,39 @@
+"red" !== x;
+~~~~~~~~~~~ [0]
+
+true === x;
+~~~~~~~~~~ [0]
+
+false === x;
+~~~~~~~~~~~ [0]
+
+null == x;
+~~~~~~~~~ [0]
+
+undefined != x;
+~~~~~~~~~~~~~~ [0]
+
+5 < x;
+~~~~~ [0]
+
+-1 + x;
+~~~~~~ [0]
+
+(-1) + x;
+~~~~~~~~ [0]
+
+// Allows string literal left hand side.
+"foo" + x;
+
+// Allows certain ordered operators.
+1 - x;
+1 / x;
+1 % x;
+1 << x;
+1 >> x;
+1 >>> x;
+2 ** x;
+"key" in x;
+"foo", x;
+
+[0]: Literal expression should be on the right-hand side of a binary expression.

test/rules/binary-expression-operand-order/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "binary-expression-operand-order": true
+  }
+}