From 757d9e6d3b499bc581f564bddada70d7fc2d2516 Mon Sep 17 00:00:00 2001 From: Noam Date: Sat, 19 Jan 2019 11:41:38 +0200 Subject: [PATCH 1/9] add new rule implementation --- src/configs/all.ts | 1 + src/rules/noTautologyExpressionRule.ts | 52 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 src/rules/noTautologyExpressionRule.ts diff --git a/src/configs/all.ts b/src/configs/all.ts index bb75db71b73..93441b76875 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -129,6 +129,7 @@ export const rules = { "no-string-throw": true, "no-sparse-arrays": true, "no-submodule-imports": true, + "no-tautology-expression": true, "no-unbound-method": true, "no-unnecessary-class": [true, "allow-empty-class"], "no-unsafe-any": true, diff --git a/src/rules/noTautologyExpressionRule.ts b/src/rules/noTautologyExpressionRule.ts new file mode 100644 index 00000000000..3978b5826bb --- /dev/null +++ b/src/rules/noTautologyExpressionRule.ts @@ -0,0 +1,52 @@ +/** + * @license + * Copyright 2013 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 * as tsutils from "tsutils"; +import * as ts from "typescript"; + +import * as Lint from "../index"; + +const TAUTOLOGY_DISCOVERED_ERROR_STRING = "Expression is a tautology. Comparison is redundant."; +export class Rule extends Lint.Rules.AbstractRule { + public static metadata: Lint.IRuleMetadata = { + description: "Enforces that two equal variables or literals are not compared.", + optionExamples: [true], + options: null, + optionsDescription: "Not configurable.", + rationale: `Clean redundant code and unnecessary comparison of objects and literals.`, + ruleName: "no-tautology-expression", + type: "functionality", + typescriptOnly: false, + }; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithFunction(sourceFile, walk); + } +} + +function walk(context: Lint.WalkContext) { + const cb = (node: ts.Node): void => { + if (tsutils.isBinaryExpression(node)) { + // Simple textual comparison of both sides + if (node.left.getText() === node.right.getText()) { + context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); + } + } + return ts.forEachChild(node, cb); + }; + return ts.forEachChild(context.sourceFile, cb); +} From 2d139225a96e19c9ee350a44fe78ede458c19f9d Mon Sep 17 00:00:00 2001 From: Noam Date: Sat, 19 Jan 2019 14:44:18 +0200 Subject: [PATCH 2/9] add some basic unit tests --- src/rules/noTautologyExpressionRule.ts | 12 ++++++-- .../no-tautology-expression/test.ts.lint | 30 +++++++++++++++++++ .../rules/no-tautology-expression/tslint.json | 5 ++++ 3 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 test/rules/no-tautology-expression/test.ts.lint create mode 100644 test/rules/no-tautology-expression/tslint.json diff --git a/src/rules/noTautologyExpressionRule.ts b/src/rules/noTautologyExpressionRule.ts index 3978b5826bb..2d38420fac8 100644 --- a/src/rules/noTautologyExpressionRule.ts +++ b/src/rules/noTautologyExpressionRule.ts @@ -41,12 +41,18 @@ export class Rule extends Lint.Rules.AbstractRule { function walk(context: Lint.WalkContext) { const cb = (node: ts.Node): void => { if (tsutils.isBinaryExpression(node)) { - // Simple textual comparison of both sides - if (node.left.getText() === node.right.getText()) { - context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); + if (isLiteral(node.left) && isLiteral(node.right)) { + // Simple textual comparison of both sides + if (node.left.getText() === node.right.getText()) { + context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); + } } } return ts.forEachChild(node, cb); }; return ts.forEachChild(context.sourceFile, cb); } + +function isLiteral(node: ts.Node): boolean { + return tsutils.isStringLiteral(node) || tsutils.isNumericLiteral(node); +} diff --git a/test/rules/no-tautology-expression/test.ts.lint b/test/rules/no-tautology-expression/test.ts.lint new file mode 100644 index 00000000000..70ea61c71ac --- /dev/null +++ b/test/rules/no-tautology-expression/test.ts.lint @@ -0,0 +1,30 @@ +const expr = "someStr" === "someStr"; + ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] +const expr = 123 === 123; + ~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] +const someVar = 100; +const expr = someVar === someVar; + ~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + +const someFunc = () => { + if ("SomeStr" === "SomeStr") { + ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + return true; + } +} + +const someFunc = () => { + if (100 === 100) { + ~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + return true; + } +} + +const someFunc = () => { + let someVar = 100; + if (someVar === someVar) { + ~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + return true; + } +} + diff --git a/test/rules/no-tautology-expression/tslint.json b/test/rules/no-tautology-expression/tslint.json new file mode 100644 index 00000000000..82111395df2 --- /dev/null +++ b/test/rules/no-tautology-expression/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-tautology-expression": true + } +} From 726a3d320972621a5de9c36aee88fc7614d66251 Mon Sep 17 00:00:00 2001 From: Noam Date: Sat, 19 Jan 2019 19:49:13 +0200 Subject: [PATCH 3/9] fix some issues in rule --- src/configs/all.ts | 1 - src/rules/noTautologyExpressionRule.ts | 58 ------------------- .../no-tautology-expression/test.ts.lint | 30 ---------- .../rules/no-tautology-expression/tslint.json | 5 -- 4 files changed, 94 deletions(-) delete mode 100644 src/rules/noTautologyExpressionRule.ts delete mode 100644 test/rules/no-tautology-expression/test.ts.lint delete mode 100644 test/rules/no-tautology-expression/tslint.json diff --git a/src/configs/all.ts b/src/configs/all.ts index 93441b76875..bb75db71b73 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -129,7 +129,6 @@ export const rules = { "no-string-throw": true, "no-sparse-arrays": true, "no-submodule-imports": true, - "no-tautology-expression": true, "no-unbound-method": true, "no-unnecessary-class": [true, "allow-empty-class"], "no-unsafe-any": true, diff --git a/src/rules/noTautologyExpressionRule.ts b/src/rules/noTautologyExpressionRule.ts deleted file mode 100644 index 2d38420fac8..00000000000 --- a/src/rules/noTautologyExpressionRule.ts +++ /dev/null @@ -1,58 +0,0 @@ -/** - * @license - * Copyright 2013 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 * as tsutils from "tsutils"; -import * as ts from "typescript"; - -import * as Lint from "../index"; - -const TAUTOLOGY_DISCOVERED_ERROR_STRING = "Expression is a tautology. Comparison is redundant."; -export class Rule extends Lint.Rules.AbstractRule { - public static metadata: Lint.IRuleMetadata = { - description: "Enforces that two equal variables or literals are not compared.", - optionExamples: [true], - options: null, - optionsDescription: "Not configurable.", - rationale: `Clean redundant code and unnecessary comparison of objects and literals.`, - ruleName: "no-tautology-expression", - type: "functionality", - typescriptOnly: false, - }; - - public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { - return this.applyWithFunction(sourceFile, walk); - } -} - -function walk(context: Lint.WalkContext) { - const cb = (node: ts.Node): void => { - if (tsutils.isBinaryExpression(node)) { - if (isLiteral(node.left) && isLiteral(node.right)) { - // Simple textual comparison of both sides - if (node.left.getText() === node.right.getText()) { - context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); - } - } - } - return ts.forEachChild(node, cb); - }; - return ts.forEachChild(context.sourceFile, cb); -} - -function isLiteral(node: ts.Node): boolean { - return tsutils.isStringLiteral(node) || tsutils.isNumericLiteral(node); -} diff --git a/test/rules/no-tautology-expression/test.ts.lint b/test/rules/no-tautology-expression/test.ts.lint deleted file mode 100644 index 70ea61c71ac..00000000000 --- a/test/rules/no-tautology-expression/test.ts.lint +++ /dev/null @@ -1,30 +0,0 @@ -const expr = "someStr" === "someStr"; - ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] -const expr = 123 === 123; - ~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] -const someVar = 100; -const expr = someVar === someVar; - ~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] - -const someFunc = () => { - if ("SomeStr" === "SomeStr") { - ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] - return true; - } -} - -const someFunc = () => { - if (100 === 100) { - ~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] - return true; - } -} - -const someFunc = () => { - let someVar = 100; - if (someVar === someVar) { - ~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] - return true; - } -} - diff --git a/test/rules/no-tautology-expression/tslint.json b/test/rules/no-tautology-expression/tslint.json deleted file mode 100644 index 82111395df2..00000000000 --- a/test/rules/no-tautology-expression/tslint.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "rules": { - "no-tautology-expression": true - } -} From f89be3285a4c61022f4f134af500b8a667b42298 Mon Sep 17 00:00:00 2001 From: Noam Date: Fri, 25 Jan 2019 09:07:03 +0200 Subject: [PATCH 4/9] add noTautologyExpressionRule, add unit tests --- src/configs/all.ts | 1 + src/rules/noTautologyExpressionRule.ts | 80 +++++++++++++++++++ .../no-tautology-expression/test.ts.lint | 33 ++++++++ .../rules/no-tautology-expression/tslint.json | 5 ++ 4 files changed, 119 insertions(+) create mode 100644 src/rules/noTautologyExpressionRule.ts create mode 100644 test/rules/no-tautology-expression/test.ts.lint create mode 100644 test/rules/no-tautology-expression/tslint.json diff --git a/src/configs/all.ts b/src/configs/all.ts index bb75db71b73..93441b76875 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -129,6 +129,7 @@ export const rules = { "no-string-throw": true, "no-sparse-arrays": true, "no-submodule-imports": true, + "no-tautology-expression": true, "no-unbound-method": true, "no-unnecessary-class": [true, "allow-empty-class"], "no-unsafe-any": true, diff --git a/src/rules/noTautologyExpressionRule.ts b/src/rules/noTautologyExpressionRule.ts new file mode 100644 index 00000000000..0ff297567f1 --- /dev/null +++ b/src/rules/noTautologyExpressionRule.ts @@ -0,0 +1,80 @@ +/** + * @license + * Copyright 2013 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 * as tsutils from "tsutils"; +import * as ts from "typescript"; + +import * as Lint from "../index"; + +const TAUTOLOGY_DISCOVERED_ERROR_STRING = "Expression is a tautology. Comparison is redundant."; +export class Rule extends Lint.Rules.AbstractRule { + public static metadata: Lint.IRuleMetadata = { + description: Lint.Utils.dedent` + Enforces that two equal variables or literals are not compared. Expression like 3 === 3 or + someVar === someVar will are tautologies, and will produce an error. + `, + optionExamples: [true], + options: null, + optionsDescription: "Not configurable.", + rationale: `Clean redundant code and unnecessary comparison of objects and literals.`, + ruleName: "no-tautology-expression", + type: "functionality", + typescriptOnly: false, + }; + + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithFunction(sourceFile, walk); + } +} + +function walk(context: Lint.WalkContext) { + const cb = (node: ts.Node): void => { + if (tsutils.isBinaryExpression(node) && isRationalOrEqualityOperator(node.operatorToken)) { + if (isLiteral(node.left) && isLiteral(node.right)) { + if (node.left.text === node.right.text) { + context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); + } + } else if ( + tsutils.isPropertyAccessExpression(node.left) && + tsutils.isPropertyAccessExpression(node.right) + ) { + if (node.left.name.text === node.right.name.text) { + context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); + } + } + } + return ts.forEachChild(node, cb); + }; + return ts.forEachChild(context.sourceFile, cb); +} + +function isLiteral(node: ts.Node): node is ts.StringLiteral | ts.NumericLiteral { + return (node as ts.StringLiteral) !== undefined || (node as ts.NumericLiteral) !== undefined; +} + +function isRationalOrEqualityOperator(operator: ts.BinaryOperatorToken): boolean { + return new Set([ + ts.SyntaxKind.LessThanToken, + ts.SyntaxKind.GreaterThanToken, + ts.SyntaxKind.LessThanEqualsToken, + ts.SyntaxKind.GreaterThanEqualsToken, + ts.SyntaxKind.EqualsEqualsToken, + ts.SyntaxKind.EqualsEqualsEqualsToken, + ts.SyntaxKind.ExclamationEqualsToken, + ts.SyntaxKind.ExclamationEqualsEqualsToken, + ]).has(operator.kind); +} diff --git a/test/rules/no-tautology-expression/test.ts.lint b/test/rules/no-tautology-expression/test.ts.lint new file mode 100644 index 00000000000..b320b59eda0 --- /dev/null +++ b/test/rules/no-tautology-expression/test.ts.lint @@ -0,0 +1,33 @@ +const expr = "someStr" === "someStr"; + ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] +const expr = 123 === 123; + ~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] +const someVar = 100; +const expr = someVar === someVar; + ~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + +const someFunc = () => { + if ("SomeStr" === "SomeStr") { + ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + return true; + } +} + +const someFunc = () => { + if (100 === 100) { + ~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + return true; + } +} + +const someFunc = () => { + let someVar = 100; + if (someVar === someVar) { + ~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + return true; + } +} + +const someVar = 3 + 3; + + diff --git a/test/rules/no-tautology-expression/tslint.json b/test/rules/no-tautology-expression/tslint.json new file mode 100644 index 00000000000..82111395df2 --- /dev/null +++ b/test/rules/no-tautology-expression/tslint.json @@ -0,0 +1,5 @@ +{ + "rules": { + "no-tautology-expression": true + } +} From f5c6eeaebaf6f63c42d708d24417b46c1ccf9e26 Mon Sep 17 00:00:00 2001 From: Noam Date: Tue, 29 Jan 2019 21:39:33 +0200 Subject: [PATCH 5/9] add examples to rule description, limit rule to validate only relational/logical expressions, add tests --- src/rules/noTautologyExpressionRule.ts | 16 ++-- .../no-tautology-expression/test.ts.lint | 89 +++++++++++++++++-- 2 files changed, 93 insertions(+), 12 deletions(-) diff --git a/src/rules/noTautologyExpressionRule.ts b/src/rules/noTautologyExpressionRule.ts index 0ff297567f1..4d4fa30761c 100644 --- a/src/rules/noTautologyExpressionRule.ts +++ b/src/rules/noTautologyExpressionRule.ts @@ -20,12 +20,13 @@ import * as ts from "typescript"; import * as Lint from "../index"; -const TAUTOLOGY_DISCOVERED_ERROR_STRING = "Expression is a tautology. Comparison is redundant."; +const TAUTOLOGY_DISCOVERED_ERROR_STRING = + "Expression is either a tautology or a contradiction. Binary expression is redundant."; export class Rule extends Lint.Rules.AbstractRule { public static metadata: Lint.IRuleMetadata = { description: Lint.Utils.dedent` - Enforces that two equal variables or literals are not compared. Expression like 3 === 3 or - someVar === someVar will are tautologies, and will produce an error. + Enforces that relational/equality binary operators does not take two equal variables/literals as operands. + Expression like 3 === 3, someVar === someVar, "1" > "1" are either a tautology or contradiction, and will produce an error. `, optionExamples: [true], options: null, @@ -43,7 +44,10 @@ export class Rule extends Lint.Rules.AbstractRule { function walk(context: Lint.WalkContext) { const cb = (node: ts.Node): void => { - if (tsutils.isBinaryExpression(node) && isRationalOrEqualityOperator(node.operatorToken)) { + if ( + tsutils.isBinaryExpression(node) && + isRelationalOrOrLogicalOperator(node.operatorToken) + ) { if (isLiteral(node.left) && isLiteral(node.right)) { if (node.left.text === node.right.text) { context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); @@ -66,7 +70,7 @@ function isLiteral(node: ts.Node): node is ts.StringLiteral | ts.NumericLiteral return (node as ts.StringLiteral) !== undefined || (node as ts.NumericLiteral) !== undefined; } -function isRationalOrEqualityOperator(operator: ts.BinaryOperatorToken): boolean { +function isRelationalOrOrLogicalOperator(operator: ts.BinaryOperatorToken): boolean { return new Set([ ts.SyntaxKind.LessThanToken, ts.SyntaxKind.GreaterThanToken, @@ -76,5 +80,7 @@ function isRationalOrEqualityOperator(operator: ts.BinaryOperatorToken): boolean ts.SyntaxKind.EqualsEqualsEqualsToken, ts.SyntaxKind.ExclamationEqualsToken, ts.SyntaxKind.ExclamationEqualsEqualsToken, + ts.SyntaxKind.AmpersandAmpersandToken, + ts.SyntaxKind.BarBarToken, ]).has(operator.kind); } diff --git a/test/rules/no-tautology-expression/test.ts.lint b/test/rules/no-tautology-expression/test.ts.lint index b320b59eda0..1fd258e4c02 100644 --- a/test/rules/no-tautology-expression/test.ts.lint +++ b/test/rules/no-tautology-expression/test.ts.lint @@ -1,21 +1,22 @@ + const expr = "someStr" === "someStr"; - ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] const expr = 123 === 123; - ~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + ~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] const someVar = 100; const expr = someVar === someVar; - ~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + ~~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] const someFunc = () => { if ("SomeStr" === "SomeStr") { - ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] return true; } } const someFunc = () => { if (100 === 100) { - ~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + ~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] return true; } } @@ -23,11 +24,85 @@ const someFunc = () => { const someFunc = () => { let someVar = 100; if (someVar === someVar) { - ~~~~~~~~~~~~~~~~~~~ [Expression is a tautology. Comparison is redundant.] + ~~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + return true; + } +} + +const someFunc = () => { + if (1 !== 1) { + ~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + return true; + } +} + +const someFunc = () => { + if (1 > 1) { + ~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + return true; + } +} + +const someFunc = () => { + const someVar = 123; + if (someVar < someVar) { + ~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + return true; + } +} + +const someFunc = () => { + if ("str" == "str") { + ~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + return true; + } +} + +const someFunc = () => { + if (123 != 123) { + ~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] return true; } } -const someVar = 3 + 3; +const someFunc = () => { + if ("str" <= "str") { + ~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + return true; + } +} + +const someFunc = () => { + if ("str" >= "str") { + ~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + return true; + } +} + +const someFunc = () => { + const someVar = true; + if (someVar || someVar) { + ~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + return true; + } +} + +const someFunc = () => { + const someVar = true; + if (someVar && someVar) { + ~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + return true; + } +} + +const someVar1 = 3 + 3; +const someVar2 = 3 - 3; +const someVar3 = 3 * 3; +const someVar4 = 3 % 3; +const someVar5 = 3 / 3; + + + + From e948a3c67c25e7944dbd7dc690c218af4b4dee32 Mon Sep 17 00:00:00 2001 From: Noam Date: Fri, 1 Mar 2019 21:39:43 +0200 Subject: [PATCH 6/9] fix typo in isRelationalOrLogicalOperator --- src/rules/noTautologyExpressionRule.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/rules/noTautologyExpressionRule.ts b/src/rules/noTautologyExpressionRule.ts index 4d4fa30761c..3219dfca97d 100644 --- a/src/rules/noTautologyExpressionRule.ts +++ b/src/rules/noTautologyExpressionRule.ts @@ -44,10 +44,7 @@ export class Rule extends Lint.Rules.AbstractRule { function walk(context: Lint.WalkContext) { const cb = (node: ts.Node): void => { - if ( - tsutils.isBinaryExpression(node) && - isRelationalOrOrLogicalOperator(node.operatorToken) - ) { + if (tsutils.isBinaryExpression(node) && isRelationalOrLogicalOperator(node.operatorToken)) { if (isLiteral(node.left) && isLiteral(node.right)) { if (node.left.text === node.right.text) { context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); @@ -70,7 +67,7 @@ function isLiteral(node: ts.Node): node is ts.StringLiteral | ts.NumericLiteral return (node as ts.StringLiteral) !== undefined || (node as ts.NumericLiteral) !== undefined; } -function isRelationalOrOrLogicalOperator(operator: ts.BinaryOperatorToken): boolean { +function isRelationalOrLogicalOperator(operator: ts.BinaryOperatorToken): boolean { return new Set([ ts.SyntaxKind.LessThanToken, ts.SyntaxKind.GreaterThanToken, From 211314e3def312f58b8c50e56fb009a6b3ba1033 Mon Sep 17 00:00:00 2001 From: Noam Date: Fri, 1 Mar 2019 23:18:06 +0200 Subject: [PATCH 7/9] modify isLiteral to use tsutils modified walk to cover additional cases add property access unit tests --- src/rules/noTautologyExpressionRule.ts | 12 ++++++-- .../no-tautology-expression/test.ts.lint | 29 +++++++++++++++---- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/rules/noTautologyExpressionRule.ts b/src/rules/noTautologyExpressionRule.ts index 3219dfca97d..bc15cf2219c 100644 --- a/src/rules/noTautologyExpressionRule.ts +++ b/src/rules/noTautologyExpressionRule.ts @@ -49,12 +49,18 @@ function walk(context: Lint.WalkContext) { if (node.left.text === node.right.text) { context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); } + } else if (tsutils.isIdentifier(node.left) && tsutils.isIdentifier(node.right)) { + if (node.left.text === node.right.text) { + context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); + } } else if ( tsutils.isPropertyAccessExpression(node.left) && tsutils.isPropertyAccessExpression(node.right) ) { - if (node.left.name.text === node.right.name.text) { - context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); + if (node.left.expression.getText() === node.right.expression.getText()) { + if (node.left.name.text === node.right.name.text) { + context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); + } } } } @@ -64,7 +70,7 @@ function walk(context: Lint.WalkContext) { } function isLiteral(node: ts.Node): node is ts.StringLiteral | ts.NumericLiteral { - return (node as ts.StringLiteral) !== undefined || (node as ts.NumericLiteral) !== undefined; + return tsutils.isStringLiteral(node) || tsutils.isNumericLiteral(node); } function isRelationalOrLogicalOperator(operator: ts.BinaryOperatorToken): boolean { diff --git a/test/rules/no-tautology-expression/test.ts.lint b/test/rules/no-tautology-expression/test.ts.lint index 1fd258e4c02..fd6b48f2fbc 100644 --- a/test/rules/no-tautology-expression/test.ts.lint +++ b/test/rules/no-tautology-expression/test.ts.lint @@ -95,14 +95,31 @@ const someFunc = () => { } } +const someFunc = () => { + const someObj = { "name" : "moses the great" }; + if (someObj.name === someObj.name) { + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + return true; + } +} + +const someFunc = () => { + const someObj = { "name" : "moses the great", "address" : "king's road 10" }; + if (someObj.name === someObj.address) { + return true; + } +} + +const someFunc = () => { + const objOne = { "name" : "moses the great" }; + const objTwo = { "name" : "moses the great" }; + if (objOne.name === objTwo.name) { + return true; + } +} + const someVar1 = 3 + 3; const someVar2 = 3 - 3; const someVar3 = 3 * 3; const someVar4 = 3 % 3; const someVar5 = 3 / 3; - - - - - - From 6c3905385771b003ff5cd81f027a0ec65c3490c0 Mon Sep 17 00:00:00 2001 From: Noam Date: Fri, 1 Mar 2019 23:22:35 +0200 Subject: [PATCH 8/9] modify error message --- src/rules/noTautologyExpressionRule.ts | 2 +- .../no-tautology-expression/test.ts.lint | 32 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/rules/noTautologyExpressionRule.ts b/src/rules/noTautologyExpressionRule.ts index bc15cf2219c..d2d13d887ed 100644 --- a/src/rules/noTautologyExpressionRule.ts +++ b/src/rules/noTautologyExpressionRule.ts @@ -21,7 +21,7 @@ import * as ts from "typescript"; import * as Lint from "../index"; const TAUTOLOGY_DISCOVERED_ERROR_STRING = - "Expression is either a tautology or a contradiction. Binary expression is redundant."; + "Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction."; export class Rule extends Lint.Rules.AbstractRule { public static metadata: Lint.IRuleMetadata = { description: Lint.Utils.dedent` diff --git a/test/rules/no-tautology-expression/test.ts.lint b/test/rules/no-tautology-expression/test.ts.lint index fd6b48f2fbc..f8980c43a08 100644 --- a/test/rules/no-tautology-expression/test.ts.lint +++ b/test/rules/no-tautology-expression/test.ts.lint @@ -1,22 +1,22 @@ const expr = "someStr" === "someStr"; - ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] const expr = 123 === 123; - ~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] const someVar = 100; const expr = someVar === someVar; - ~~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] const someFunc = () => { if ("SomeStr" === "SomeStr") { - ~~~~~~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } const someFunc = () => { if (100 === 100) { - ~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } @@ -24,21 +24,21 @@ const someFunc = () => { const someFunc = () => { let someVar = 100; if (someVar === someVar) { - ~~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } const someFunc = () => { if (1 !== 1) { - ~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } const someFunc = () => { if (1 > 1) { - ~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } @@ -46,35 +46,35 @@ const someFunc = () => { const someFunc = () => { const someVar = 123; if (someVar < someVar) { - ~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } const someFunc = () => { if ("str" == "str") { - ~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } const someFunc = () => { if (123 != 123) { - ~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } const someFunc = () => { if ("str" <= "str") { - ~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } const someFunc = () => { if ("str" >= "str") { - ~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } @@ -82,7 +82,7 @@ const someFunc = () => { const someFunc = () => { const someVar = true; if (someVar || someVar) { - ~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } @@ -90,7 +90,7 @@ const someFunc = () => { const someFunc = () => { const someVar = true; if (someVar && someVar) { - ~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } @@ -98,7 +98,7 @@ const someFunc = () => { const someFunc = () => { const someObj = { "name" : "moses the great" }; if (someObj.name === someObj.name) { - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Expression is either a tautology or a contradiction. Binary expression is redundant.] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] return true; } } From 704df1c45c648ad380ecad38f4b62f85eafb55c4 Mon Sep 17 00:00:00 2001 From: Noam Date: Thu, 7 Mar 2019 22:06:43 +0200 Subject: [PATCH 9/9] add checks for ts.NullLiteral, ts.BooleanLiteral add unit tests --- src/rules/noTautologyExpressionRule.ts | 20 ++++++++--- .../no-tautology-expression/test.ts.lint | 34 +++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/rules/noTautologyExpressionRule.ts b/src/rules/noTautologyExpressionRule.ts index d2d13d887ed..8ec352836ae 100644 --- a/src/rules/noTautologyExpressionRule.ts +++ b/src/rules/noTautologyExpressionRule.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2013 Palantir Technologies, Inc. + * Copyright 2019 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. @@ -45,7 +45,10 @@ export class Rule extends Lint.Rules.AbstractRule { function walk(context: Lint.WalkContext) { const cb = (node: ts.Node): void => { if (tsutils.isBinaryExpression(node) && isRelationalOrLogicalOperator(node.operatorToken)) { - if (isLiteral(node.left) && isLiteral(node.right)) { + if ( + (tsutils.isStringLiteral(node.left) && tsutils.isStringLiteral(node.right)) || + (tsutils.isNumericLiteral(node.left) && tsutils.isNumericLiteral(node.right)) + ) { if (node.left.text === node.right.text) { context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); } @@ -62,6 +65,11 @@ function walk(context: Lint.WalkContext) { context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); } } + } else if ( + (isBooleanLiteral(node.left) && isBooleanLiteral(node.right)) || + (isNullLiteral(node.left) && isNullLiteral(node.right)) + ) { + context.addFailureAtNode(node, TAUTOLOGY_DISCOVERED_ERROR_STRING); } } return ts.forEachChild(node, cb); @@ -69,8 +77,12 @@ function walk(context: Lint.WalkContext) { return ts.forEachChild(context.sourceFile, cb); } -function isLiteral(node: ts.Node): node is ts.StringLiteral | ts.NumericLiteral { - return tsutils.isStringLiteral(node) || tsutils.isNumericLiteral(node); +function isNullLiteral(node: ts.Node): node is ts.NullLiteral { + return node.kind === ts.SyntaxKind.NullKeyword; +} + +function isBooleanLiteral(node: ts.Node): node is ts.BooleanLiteral { + return node.kind === ts.SyntaxKind.TrueKeyword || node.kind === ts.SyntaxKind.FalseKeyword; } function isRelationalOrLogicalOperator(operator: ts.BinaryOperatorToken): boolean { diff --git a/test/rules/no-tautology-expression/test.ts.lint b/test/rules/no-tautology-expression/test.ts.lint index f8980c43a08..09a5057a7cf 100644 --- a/test/rules/no-tautology-expression/test.ts.lint +++ b/test/rules/no-tautology-expression/test.ts.lint @@ -118,6 +118,40 @@ const someFunc = () => { } } +const someFunc = () => { + if (true === true) { + ~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] + return true; + } +} + +const someFunc = () => { + if (false === false) { + ~~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] + return true; + } +} + +const someFunc = () => { + if (true === false) { + ~~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] + return true; + } +} + +const someFunc = () => { + if (null === null) { + ~~~~~~~~~~~~~ [Both sides of this equality comparison are the same, so the expression is either a tautology or a contradiction.] + return true; + } +} + +const someFunc = () => { + if (null === false) { + return true; + } +} + const someVar1 = 3 + 3; const someVar2 = 3 - 3; const someVar3 = 3 * 3;