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

rewrite no-unused-expression again #2645

Merged
merged 1 commit into from
Apr 29, 2017
Merged

rewrite no-unused-expression again #2645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
168 changes: 99 additions & 69 deletions src/rules/noUnusedExpressionRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@

import {
isAssignmentKind, isBinaryExpression, isConditionalExpression, isExpressionStatement,
isIdentifier, isNumericLiteral, isPrefixUnaryExpression, isVoidExpression,
isIdentifier, isNumericLiteral, isParenthesizedExpression, isVoidExpression,
} from "tsutils";
import * as ts from "typescript";

import * as Lint from "../index";
import { unwrapParentheses } from "../language/utils";

const ALLOW_FAST_NULL_CHECKS = "allow-fast-null-checks";
const ALLOW_NEW = "allow-new";
Expand Down Expand Up @@ -55,10 +54,10 @@ export class Rule extends Lint.Rules.AbstractRule {
type: "array",
items: {
type: "string",
enum: [ALLOW_FAST_NULL_CHECKS],
enum: [ALLOW_FAST_NULL_CHECKS, ALLOW_NEW, ALLOW_TAGGED_TEMPLATE],
},
minLength: 0,
maxLength: 1,
maxLength: 3,
},
optionExamples: [true, [true, ALLOW_FAST_NULL_CHECKS]],
type: "functionality",
Expand All @@ -69,87 +68,118 @@ export class Rule extends Lint.Rules.AbstractRule {
public static FAILURE_STRING = "unused expression, expected an assignment or function call";

public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
return this.applyWithWalker(new NoUnusedExpressionWalker(sourceFile, this.ruleName, {
return this.applyWithFunction(sourceFile, walk, {
allowFastNullChecks: this.ruleArguments.indexOf(ALLOW_FAST_NULL_CHECKS) !== -1,
allowNew: this.ruleArguments.indexOf(ALLOW_NEW) !== -1,
allowTaggedTemplate: this.ruleArguments.indexOf(ALLOW_TAGGED_TEMPLATE) !== -1,
}));
});
}
}

class NoUnusedExpressionWalker extends Lint.AbstractWalker<Options> {
public walk(sourceFile: ts.SourceFile) {
const cb = (node: ts.Node): void => {
if (isExpressionStatement(node)) {
if (!isDirective(node) && this.isUnusedExpression(node.expression)) {
this.reportFailure(node);
}
} else if (isVoidExpression(node)) {
// allow `void 0`
if (!isLiteralZero(node.expression) && this.isUnusedExpression(node.expression)) {
this.reportFailure(node.expression);
}
function walk(ctx: Lint.WalkContext<Options>) {
let checking = false;
let allowFastNullChecks = true;
return ts.forEachChild(ctx.sourceFile, cb);

function cb(node: ts.Node): boolean {
if (checking) {
if (isParenthesizedExpression(node) || isVoidExpression(node)) {
return cb(node.expression);
} else if (isConditionalExpression(node)) {
noCheck(node.condition, cb);
return both(node.whenTrue, node.whenFalse);
} else if (isBinaryExpression(node)) {
if (node.operatorToken.kind === ts.SyntaxKind.CommaToken) {
// allow indirect eval: `(0, eval)("code");`
if (!isIndirectEval(node) && this.isUnusedExpression(node.left)) {
this.reportFailure(node.left);
}
switch (node.operatorToken.kind) {
case ts.SyntaxKind.CommaToken:
if (isIndirectEval(node)) {
return false;
}
return both(node.left, node.right);
case ts.SyntaxKind.AmpersandAmpersandToken:
case ts.SyntaxKind.BarBarToken:
if (allowFastNullChecks) {
noCheck(node.left, cb);
return cb(node.right);
}
}
}
return ts.forEachChild(node, cb);
};
return ts.forEachChild(sourceFile, cb);
}

private reportFailure(node: ts.Node) {
const start = node.getStart(this.sourceFile);
const end = node.end;
// don't add a new failure if it is contained in another failure's span
for (const failure of this.failures) {
if (failure.getStartPosition().getPosition() <= start &&
failure.getEndPosition().getPosition() >= end) {
return;
noCheck(node, forEachChild);
return isUnusedExpression(node, ctx.options);
}
if (isExpressionStatement(node)) {
allowFastNullChecks = ctx.options.allowFastNullChecks;
if (!isDirective(node)) {
check(node.expression, node);
}
allowFastNullChecks = true;
return false;
} else if (isVoidExpression(node)) {
// allow `void 0` and `void(0)`
if (!isLiteralZero(isParenthesizedExpression(node.expression) ? node.expression.expression : node.expression)) {
check(node.expression);
}
return false;
} else if (isBinaryExpression(node) && node.operatorToken.kind === ts.SyntaxKind.CommaToken && !isIndirectEval(node)) {
check(node.left);
return cb(node.right);
}
this.addFailure(start, end, Rule.FAILURE_STRING);
return ts.forEachChild(node, cb);
}

private isUnusedExpression(expression: ts.Expression): boolean {
expression = unwrapParentheses(expression);
switch (expression.kind) {
case ts.SyntaxKind.CallExpression:
case ts.SyntaxKind.YieldExpression:
case ts.SyntaxKind.DeleteExpression:
case ts.SyntaxKind.AwaitExpression:
case ts.SyntaxKind.PostfixUnaryExpression:
return false;
case ts.SyntaxKind.NewExpression:
return !this.options.allowNew;
case ts.SyntaxKind.TaggedTemplateExpression:
return !this.options.allowTaggedTemplate;
default:
}
if (isPrefixUnaryExpression(expression) &&
(expression.operator === ts.SyntaxKind.PlusPlusToken || expression.operator === ts.SyntaxKind.MinusMinusToken)) {
return false;
}
if (isConditionalExpression(expression)) {
return this.isUnusedExpression(expression.whenTrue) || this.isUnusedExpression(expression.whenFalse);
function forEachChild(node: ts.Node): boolean {
return ts.forEachChild(node, cb);
}

function check(node: ts.Node, failNode?: ts.Node): void {
checking = true;
if (cb(node)) {
ctx.addFailureAtNode(failNode === undefined ? node : failNode, Rule.FAILURE_STRING);
}
if (isBinaryExpression(expression)) {
const operatorKind = expression.operatorToken.kind;
if (isAssignmentKind(operatorKind)) {
return false;
}
if (this.options.allowFastNullChecks &&
(operatorKind === ts.SyntaxKind.AmpersandAmpersandToken || operatorKind === ts.SyntaxKind.BarBarToken)) {
return this.isUnusedExpression(expression.right);
} else if (operatorKind === ts.SyntaxKind.CommaToken) {
return this.isUnusedExpression(expression.left) || this.isUnusedExpression(expression.right);
checking = false;
}

function noCheck(node: ts.Node, callback: (node: ts.Node) => boolean): void {
const old = allowFastNullChecks;
checking = false;
allowFastNullChecks = true;
callback(node);
allowFastNullChecks = old;
checking = true;
}

function both(one: ts.Node, two: ts.Node): boolean {
if (cb(one)) {
if (cb(two)) {
return true;
} else {
ctx.addFailureAtNode(one, Rule.FAILURE_STRING);
}
} else if (cb(two)) {
ctx.addFailureAtNode(two, Rule.FAILURE_STRING);
}
return true;
return false;
}
}

function isUnusedExpression(node: ts.Node, options: Options): boolean {
switch (node.kind) {
case ts.SyntaxKind.CallExpression:
case ts.SyntaxKind.YieldExpression:
case ts.SyntaxKind.DeleteExpression:
case ts.SyntaxKind.AwaitExpression:
case ts.SyntaxKind.PostfixUnaryExpression:
return false;
case ts.SyntaxKind.NewExpression:
return !options.allowNew;
case ts.SyntaxKind.TaggedTemplateExpression:
return !options.allowTaggedTemplate;
case ts.SyntaxKind.BinaryExpression:
return !isAssignmentKind((node as ts.BinaryExpression).operatorToken.kind);
case ts.SyntaxKind.PrefixUnaryExpression:
return (node as ts.PrefixUnaryExpression).operator !== ts.SyntaxKind.PlusPlusToken &&
(node as ts.PrefixUnaryExpression).operator !== ts.SyntaxKind.MinusMinusToken;
default:
return true;
}
}

Expand Down
10 changes: 8 additions & 2 deletions test/rules/no-unused-expression/allow-fast-null-checks/test.ts.lint
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ fun1() + 4;
4 + fun2(j);
~~~~~~~~~~~~ [0]
(j === 0 ? fun1() : 5);
~~~~~~~~~~~~~~~~~~~~~~~ [0]
~ [0]
(j === 0 ? i : fun2(j));
~~~~~~~~~~~~~~~~~~~~~~~~ [0]
~ [0]
a => fun2(a);
~~~~~~~~~~~~~ [0]
() => {return fun1();};
Expand All @@ -134,4 +134,10 @@ function interactionHandler(e) {
~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
}

a && b;
~~~~~~~ [0]
a() && b;
~~~~~~~~~ [0]
a() && b();

[0]: unused expression, expected an assignment or function call
6 changes: 3 additions & 3 deletions test/rules/no-unused-expression/allow-new/test.ts.lint
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ fun1() + 4;
4 + fun2(j);
~~~~~~~~~~~~ [0]
(j === 0 ? fun1() : 5);
~~~~~~~~~~~~~~~~~~~~~~~ [0]
~ [0]
(j === 0 ? i : fun2(j));
~~~~~~~~~~~~~~~~~~~~~~~~ [0]
~ [0]
a => fun2(a);
~~~~~~~~~~~~~ [0]
() => {return fun1();};
Expand Down Expand Up @@ -140,6 +140,6 @@ if (foo)
var1 = 1, var2 = 1;
else
var1 = 2, var2;
~~~~~~~~~~~~~~~ [0]
~~~~ [0]

[0]: unused expression, expected an assignment or function call
6 changes: 3 additions & 3 deletions test/rules/no-unused-expression/allow-tagged-template/test.ts.lint
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ fun1() + 4;
4 + fun2(j);
~~~~~~~~~~~~ [0]
(j === 0 ? fun1() : 5);
~~~~~~~~~~~~~~~~~~~~~~~ [0]
~ [0]
(j === 0 ? i : fun2(j));
~~~~~~~~~~~~~~~~~~~~~~~~ [0]
~ [0]
a => fun2(a);
~~~~~~~~~~~~~ [0]
() => {return fun1();};
Expand Down Expand Up @@ -142,7 +142,7 @@ if (foo)
var1 = 1, var2 = 1;
else
var1 = 2, var2;
~~~~~~~~~~~~~~~ [0]
~~~~ [0]

(0,eval)('"foobar";');
(foo,eval)('"foobar";');
Expand Down
39 changes: 35 additions & 4 deletions test/rules/no-unused-expression/default/test.ts.lint
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ fun1() + 4;
4 + fun2(j);
~~~~~~~~~~~~ [0]
(j === 0 ? fun1() : 5);
~~~~~~~~~~~~~~~~~~~~~~~ [0]
~ [0]
(j === 0 ? i : fun2(j));
~~~~~~~~~~~~~~~~~~~~~~~~ [0]
~ [0]
a => fun2(a);
~~~~~~~~~~~~~ [0]
() => {return fun1();};
Expand Down Expand Up @@ -137,31 +137,50 @@ function interactionHandler(e) {
~~~~~~~~~~~~~~~~~~~~~~~~~ [0]
}

a && b ? a : b;
~~~~~~~~~~~~~~~ [0]

a && b;
~~~~~~~ [0]
a() && b;
~~~~~~~~~ [0]
a() && b();
~~~~~~~~~~~ [0]

let var1, var2;
if (foo)
var1 = 1, var2 = 1;
else
var1 = 2, var2;
~~~~~~~~~~~~~~~ [0]
~~~~ [0]
var1, var2;
~~~~~~~~~~~ [0]

(0,eval)('"foobar";');
(foo,eval)('"foobar";');
~~~ [0]
(foo(),eval)('"foobar";');
(foo(), 0, eval)('"foobar";');
~ [0]
(0, 1, eval)('"foobar";');
~~~~ [0]
(1, eval)('"foobar";');
~ [0]
(0, foo)('"foobar":');
(0, foo)('"foobar";');
~ [0]
var2 = (0, 1, 2);
~~~~ [0]
(0, 1, 2);
~~~~~~~~~~ [0]
(0, 1, foo());
~~~~ [0]

var1 = void 0;
var1 = void(0);
var1 = void 1;
~ [0]
var1 = void(1);
~~~ [0]
var1 = void foo;
~~~ [0]
var1 = void foo === bar;
Expand All @@ -174,6 +193,18 @@ var1 = void foo.bar;
~~~~~~~ [0]
var1 = void foo();

void foo();
void (foo && foo());
~~~~~~~~~~~~~~~~~~~~ [0]
void 0;
~~~~~~~ [0]
void (foo = bar);
void 1;
~~~~~~~ [0]
foo ? bar = foo : void 0;
~~~~~~ [0]
var1 = foo ? void (foo && foo()) : void 0;

switch(l) {
case 0, 1:
~ [0]
Expand Down