Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 56 additions & 3 deletions src/rules/noUnusedExpressionRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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,
};
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"rules": {
"no-unused-expression": [true, "allow-fast-null-checks"]
},
"jsRules": {
"no-unused-expression": [true, "allow-fast-null-checks"]
}
}
133 changes: 133 additions & 0 deletions test/rules/no-unused-expression/default/test.js.lint
Original file line number Diff line number Diff line change
@@ -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
Loading