Allow fast null checks in no-unused-expression rule (#1638)

src/rules/noUnusedExpressionRule.ts

@@ -18,6 +18,9 @@
 import * as ts from "typescript";
 
 import * as Lint from "../index";
+import { unwrapParentheses } from "../language/utils";
+
+const ALLOW_FAST_NULL_CHECKS = "allow-fast-null-checks";
 
 export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
@@ -29,9 +32,21 @@ export class Rule extends Lint.Rules.AbstractRule {
             (and thus usually no-ops).`,
         rationale: Lint.Utils.dedent`
             Detects potential errors where an assignment or function call was intended.`,
-        optionsDescription: "Not configurable.",
-        options: null,
-        optionExamples: ["true"],
+        optionsDescription: Lint.Utils.dedent`
+            One argument may be optionally provided:
+
+            * \`${ALLOW_FAST_NULL_CHECKS}\` allows to use logical operators to perform fast null checks and perform
+            method or function calls for side effects (e.g. \`e && e.preventDefault()\`).`,
+        options: {
+            type: "array",
+            items: {
+                type: "string",
+                enum: [ALLOW_FAST_NULL_CHECKS],
+            },
+            minLength: 0,
+            maxLength: 1,
+        },
+        optionExamples: ["true", `[true, "${ALLOW_FAST_NULL_CHECKS}"]`],
         type: "functionality",
         typescriptOnly: false,
     };
@@ -108,6 +123,15 @@ export class NoUnusedExpressionWalker extends Lint.RuleWalker {
             case ts.SyntaxKind.GreaterThanGreaterThanGreaterThanEqualsToken:
                 this.expressionIsUnused = false;
                 break;
+            case ts.SyntaxKind.AmpersandAmpersandToken:
+            case ts.SyntaxKind.BarBarToken:
+                if (this.hasOption(ALLOW_FAST_NULL_CHECKS) && isTopLevelExpression(node)) {
+                    this.expressionIsUnused = !hasCallExpression(node.right);
+                    break;
+                } else {
+                    this.expressionIsUnused = true;
+                    break;
+                }
             default:
                 this.expressionIsUnused = true;
         }
@@ -177,3 +201,32 @@ export class NoUnusedExpressionWalker extends Lint.RuleWalker {
         }
     }
 }
+
+function hasCallExpression(node: ts.Expression): boolean {
+    const nodeToCheck = unwrapParentheses(node);
+
+    if (nodeToCheck.kind === ts.SyntaxKind.CallExpression) {
+        return true;
+    }
+
+    if (nodeToCheck.kind === ts.SyntaxKind.BinaryExpression) {
+        const operatorToken = (nodeToCheck as ts.BinaryExpression).operatorToken;
+
+        if (operatorToken.kind === ts.SyntaxKind.AmpersandAmpersandToken ||
+            operatorToken.kind === ts.SyntaxKind.BarBarToken) {
+            return hasCallExpression((nodeToCheck as ts.BinaryExpression).right);
+        }
+    }
+
+    return false;
+}
+
+function isTopLevelExpression(node: ts.Expression): boolean {
+    let nodeToCheck = node.parent as ts.Node;
+
+    while (nodeToCheck.kind === ts.SyntaxKind.ParenthesizedExpression) {
+        nodeToCheck = nodeToCheck.parent as ts.Node;
+    }
+
+    return nodeToCheck.kind === ts.SyntaxKind.ExpressionStatement;
+}

test/rules/no-unused-expression/test.js.lint → test/rules/no-unused-expression/allow-fast-null-checks/test.js.lint

@@ -109,4 +109,22 @@ a => fun2(a);
 "use strct";
 ~~~~~~~~~~~~ [0]
 
+afterEach((el) => {
+   el && el.remove();
+});
+
+checkParams((a, b) => {
+   (a || required('a')) && (b || required('b'));
+});
+
+checkParams((a, b) => {
+   ((a && b) || required('a, b'));
+});
+
+function interactionHandler(e) {
+    // fails in all cases since logical NOT operator is redundant
+    e && !e.preventDefault();
+    ~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+}
+
 [0]: expected an assignment or function call

test/rules/no-unused-expression/test.ts.lint → test/rules/no-unused-expression/allow-fast-null-checks/test.ts.lint

@@ -115,4 +115,22 @@ a => fun2(a);
 "use strct";
 ~~~~~~~~~~~~ [0]
 
+afterEach((el) => {
+   el && el.remove();
+});
+
+checkParams((a, b) => {
+   (a || required('a')) && (b || required('b'));
+});
+
+checkParams((a, b) => {
+   ((a && b) || required('a, b'));
+});
+
+function interactionHandler(e) {
+    // fails in all cases since logical NOT operator is redundant
+    e && !e.preventDefault();
+    ~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+}
+
 [0]: expected an assignment or function call

test/rules/no-unused-expression/allow-fast-null-checks/tslint.json

@@ -0,0 +1,8 @@
+{
+  "rules": {
+    "no-unused-expression": [true, "allow-fast-null-checks"]
+  },
+  "jsRules": {
+    "no-unused-expression": [true, "allow-fast-null-checks"]
+  }
+}

test/rules/no-unused-expression/default/test.js.lint

@@ -0,0 +1,133 @@
+"use strict";
+'use asm';
+"ngInject";
+'';
+
+function fun1() {
+    "use strict";
+    'someOtherDirective';
+    return 0;
+}
+
+(function() { "directive";
+'foo'
+'directive2'
+console.log('foo');
+'notdirective';
+~~~~~~~~~~~~~~~ [0]
+})();
+
+const a = () => {
+'use strict'; "use cool"; "use lint"; var a = 1; "notdirective"; }
+                                                 ~~~~~~~~~~~~~~~ [0]
+
+function fun2(a) {
+    return 0;
+}
+
+function fun3(a, b) {
+    return 0;
+}
+
+class Foo {
+    constructor() {
+        "ngInject";
+        var a = 1;
+        'notdirective';
+        ~~~~~~~~~~~~~~~ [0]
+    }
+
+    bar() {
+        'use strict';
+    }
+
+    get baz() {
+        'use asm';
+    }
+
+    set baz(newValue) {
+        "use asm";
+    }
+}
+
+// valid code:
+
+var i;
+var j = 3;
+i = 1 + 2;
+j = fun1();
+fun1();
+fun2(2);
+fun3(2, fun1());
+i++;
+i += 2;
+--i;
+i <<= 2;
+i = fun1() + fun1();
+j = (j === 0 ? 5 : 6);
+(j === 0 ? fun1() : fun2(j));
+(a => 5)(4);
+var obj = {};
+delete obj.key;
+function* g() {
+    for (let i = 0; i < 100; i++) {
+        yield i;
+    }
+}
+
+async function f(foo) {
+    await foo;
+    return 0;
+}
+
+new Foo();
+
+// invalid code:
+
+5;
+~~ [0]
+i;
+~~ [0]
+3 + 5;
+~~~~~~ [0]
+fun1() + fun1();
+~~~~~~~~~~~~~~~~ [0]
+fun2(i) + fun3(4,7);
+~~~~~~~~~~~~~~~~~~~~ [0]
+fun1() + 4;
+~~~~~~~~~~~ [0]
+4 + fun2(j);
+~~~~~~~~~~~~ [0]
+(j === 0 ? fun1() : 5);
+~~~~~~~~~~~~~~~~~~~~~~~ [0]
+(j === 0 ? i : fun2(j));
+~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+a => fun2(a);
+~~~~~~~~~~~~~ [0]
+() => {return fun1();};
+~~~~~~~~~~~~~~~~~~~~~~~ [0]
+"use strct";
+~~~~~~~~~~~~ [0]
+
+afterEach((el) => {
+   el && el.remove();
+   ~~~~~~~~~~~~~~~~~~ [0]
+});
+
+checkParams((a, b) => {
+   (a || required('a')) && (b || required('b'));
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+});
+
+checkParams((a, b) => {
+   ((a && b) || required('a, b'));
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+});
+
+function interactionHandler(e) {
+    // fails in all cases since logical NOT operator is redundant
+    e && !e.preventDefault();
+    ~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
+}
+
+[0]: expected an assignment or function call