diff --git a/src/rules/noUnusedExpressionRule.ts b/src/rules/noUnusedExpressionRule.ts index b0789d6a652..953652beedb 100644 --- a/src/rules/noUnusedExpressionRule.ts +++ b/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; +} diff --git a/test/rules/no-unused-expression/test.js.lint b/test/rules/no-unused-expression/allow-fast-null-checks/test.js.lint similarity index 81% rename from test/rules/no-unused-expression/test.js.lint rename to test/rules/no-unused-expression/allow-fast-null-checks/test.js.lint index 80e8e7bd054..b2386eac84a 100644 --- a/test/rules/no-unused-expression/test.js.lint +++ b/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 diff --git a/test/rules/no-unused-expression/test.ts.lint b/test/rules/no-unused-expression/allow-fast-null-checks/test.ts.lint similarity index 83% rename from test/rules/no-unused-expression/test.ts.lint rename to test/rules/no-unused-expression/allow-fast-null-checks/test.ts.lint index e88ea48cc3d..c72dcbf6ad3 100644 --- a/test/rules/no-unused-expression/test.ts.lint +++ b/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 diff --git a/test/rules/no-unused-expression/allow-fast-null-checks/tslint.json b/test/rules/no-unused-expression/allow-fast-null-checks/tslint.json new file mode 100644 index 00000000000..c8a2a238f40 --- /dev/null +++ b/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"] + } +} diff --git a/test/rules/no-unused-expression/default/test.js.lint b/test/rules/no-unused-expression/default/test.js.lint new file mode 100644 index 00000000000..936dd446696 --- /dev/null +++ b/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 diff --git a/test/rules/no-unused-expression/default/test.ts.lint b/test/rules/no-unused-expression/default/test.ts.lint new file mode 100644 index 00000000000..66ec5fadbe9 --- /dev/null +++ b/test/rules/no-unused-expression/default/test.ts.lint @@ -0,0 +1,139 @@ +"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: number) { + return 0; +} + +function fun3(a: number, b: number) { + return 0; +} + +namespace Fam { 'use strict'; 'use cool'; } +module Bam { 'use strict'; 'use cool'; } +namespace Az.Bz.Cz.Dz { + 'ngInject'; +} + +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: number; +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(): Iterable { + for (let i = 0; i < 100; i++) { + yield i; + } +} + +async function f(foo: Promise): Promise { + 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 diff --git a/test/rules/no-unused-expression/tslint.json b/test/rules/no-unused-expression/default/tslint.json similarity index 100% rename from test/rules/no-unused-expression/tslint.json rename to test/rules/no-unused-expression/default/tslint.json