8000 feat(eslint-plugin): add no-unnecessary-boolean-literal-compare (#242) · webmaster128/typescript-eslint@6bebb1d · GitHub
[go: up one dir, main page]
Skip to content

Commit 6bebb1d

Browse files
Josh Goldbergbradzacher
Josh Goldberg
and
bradzacher
authored
feat(eslint-plugin): add no-unnecessary-boolean-literal-compare (typescript-eslint#242)
Co-authored-by: Brad Zacher <brad.zacher@gmail.com>
1 parent b835ec2 commit 6bebb1d

File tree

8 files changed

+372
-61
lines changed

8 files changed

+372
-61
lines changed

‎packages/eslint-plugin/README.md

Lines changed: 61 additions & 60 deletions
Large diffs are not rendered by default.

‎packages/eslint-plugin/ROADMAP.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th
159159
| [`newline-per-chained-call`] | 🌟 | [`newline-per-chained-call`][newline-per-chained-call] |
160160
| [`new-parens`] | 🌟 | [`new-parens`][new-parens] |
161161
| [`no-angle-bracket-type-assertion`] | ✅ | [`@typescript-eslint/consistent-type-assertions`] |
162-
| [`no-boolean-literal-compare`] | 🛑 | N/A |
162+
| [`no-boolean-literal-compare`] | ✅ | [`@typescript-eslint/no-unnecessary-boolean-literal-compare`] |
163163
| [`no-consecutive-blank-lines`] | 🌟 | [`no-multiple-empty-lines`][no-multiple-empty-lines] |
164164
| [`no-irregular-whitespace`] | 🌟 | [`no-irregular-whitespace`][no-irregular-whitespace] with `skipStrings: false` |
165165
| [`no-parameter-properties`] | ✅ | [`@typescript-eslint/no-parameter-properties`] |
@@ -600,6 +600,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
600600
[`@typescript-eslint/type-annotation-spacing`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/type-annotation-spacing.md
601601
[`@typescript-eslint/typedef`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/typedef.md
602602
[`@typescript-eslint/unified-signatures`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/unified-signatures.md
603+
[`@typescript-eslint/no-unnecessary-boolean-literal-compare`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unnecessary-boolean-literal-compare.md
603604
[`@typescript-eslint/no-misused-new`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-misused-new.md
604605
[`@typescript-eslint/no-this-alias`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-this-alias.md
605606
[`@typescript-eslint/no-throw-literal`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-throw-literal.md

‎packages/eslint-plugin/docs/rules/no-unnecessary-boolean-literal-compare.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# Flags unnecessary equality comparisons against boolean literals (`no-unnecessary-boolean-literal-compare`)
2+
3+
Comparing boolean values to boolean literals is unnecessary, those comparisons result in the same booleans. Using the boolean values directly, or via a unary negation (`!value`), is more concise and clearer.
4+
5+
## Rule Details
6+
7+
This rule ensures that you do not include unnecessary comparisons with boolean literals.
8+
A comparison is considered unnecessary if it checks a boolean literal against any variable with just the `boolean` type.
9+
A comparison is **_not_** considered unnecessary if the type is a union of booleans (`string | boolean`, `someObject | boolean`).
10+
11+
Examples of **incorrect** code for this rule:
12+
13+
```ts
14+
declare const someCondition: boolean;
15+
if (someCondition === true) {
16+
}
17+
```
18+
19+
Examples of **correct** code for this rule
20+
21+
```ts
22+
declare const someCondition: boolean;
23+
if (someCondition) {
24+
}
25+
26+
declare const someObjectBoolean: boolean | Record<string, unknown>;
27+
if (someObjectBoolean === true) {
28+
}
29+
30+
declare const someStringBoolean: boolean | string;
31+
if (someStringBoolean === true) {
32+
}
33+
34+
declare const someUndefinedCondition: boolean | undefined;
35+
if (someUndefinedCondition === false) {
36+
}
37+
```
38+
39+
## Related to
40+
41+
- TSLint: [no-boolean-literal-compare](https://palantir.github.io/tslint/rules/no-boolean-literal-compare)

‎packages/eslint-plugin/src/configs/all.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
"@typescript-eslint/no-this-alias": "error",
5454
"@typescript-eslint/no-throw-literal": "error",
5555
"@typescript-eslint/no-type-alias": "error",
56+
"@typescript-eslint/no-unnecessary-boolean-literal-compare": "error",
5657
"@typescript-eslint/no-unnecessary-condition": "error",
5758
"@typescript-eslint/no-unnecessary-qualifier": "error",
5859
"@typescript-eslint/no-unnecessary-type-arguments": "error",

‎packages/eslint-plugin/src/rules/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import noRequireImports from './no-require-imports';
4646
import noThisAlias from './no-this-alias';
4747
import noThrowLiteral from './no-throw-literal';
4848
import noTypeAlias from './no-type-alias';
49+
import noUnnecessaryBooleanLiteralCompare from './no-unnecessary-boolean-literal-compare';
4950
import noUnnecessaryCondition from './no-unnecessary-condition';
5051
import noUnnecessaryQualifier from './no-unnecessary-qualifier';
5152
import useDefaultTypeParameter from './no-unnecessary-type-arguments';
@@ -132,6 +133,7 @@ export default {
132133
'no-this-alias': noThisAlias,
133134
'no-type-alias': noTypeAlias,
134135
'no-throw-literal': noThrowLiteral,
136+
'no-unnecessary-boolean-literal-compare': noUnnecessaryBooleanLiteralCompare,
135137
'no-unnecessary-condition': noUnnecessaryCondition,
136138
'no-unnecessary-qualifier': noUnnecessaryQualifier,
137139
'no-unnecessary-type-arguments': useDefaultTypeParameter,

‎packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import {
2+
AST_NODE_TYPES,
3+
TSESTree,
4+
} from '@typescript-eslint/experimental-utils';
5+
import * as tsutils from 'tsutils';
6+
import * as ts from 'typescript';
7+
import * as util from '../util';
8+
9+
type MessageIds = 'direct' | 'negated';
10+
11+
interface BooleanComparison {
12+
expression: TSESTree.Expression;
13+
forTruthy: boolean;
14+
negated: boolean;
15+
range: [number, number];
16+
}
17+
18+
export default util.createRule<[], MessageIds>({
19+
name: 'no-unnecessary-boolean-literal-compare',
20+
meta: {
21+
docs: {
22+
description:
23+
'Flags unnecessary equality comparisons against boolean literals',
24+
category: 'Stylistic Issues',
25+
recommended: false,
26+
requiresTypeChecking: true,
27+
},
28+
fixable: 'code',
29+
messages: {
30+
direct:
31+
'This expression unnecessarily compares a boolean value to a boolean instead of using it directly',
32+
negated:
33+
'This expression unnecessarily compares a boolean value to a boolean instead of negating it.',
34+
},
35+
schema: [],
36+
type: 'suggestion',
37+
},
38+
defaultOptions: [],
39+
create(context) {
40+
const parserServices = util.getParserServices(context);
41+
const checker = parserServices.program.getTypeChecker();
42+
43+
function getBooleanComparison(
44+
node: TSESTree.BinaryExpression,
45+
): BooleanComparison | undefined {
46+
const comparison = deconstructComparison(node);
47+
if (!comparison) {
48+
return undefined;
49+
}
50+
51+
const expressionType = checker.getTypeAtLocation(
52+
parserServices.esTreeNodeToTSNodeMap.get(comparison.expression),
53+
);
54+
55+
if (
56+
!tsutils.isTypeFlagSet(
57+
expressionType,
58+
ts.TypeFlags.Boolean | ts.TypeFlags.BooleanLiteral,
59+
)
60+
) {
61+
return undefined;
62+
}
63+
64+
return comparison;
65+
}
66+
67+
function deconstructComparison(
68+
node: TSESTree.BinaryExpression,
69+
): BooleanComparison | undefined {
70+
const comparisonType = util.getEqualsKind(node.operator);
71+
if (!comparisonType) {
72+
return undefined;
73+
}
74+
75+
for (const [against, expression] of [
76+
[node.right, node.left],
77+
[node.left, node.right],
78+
]) {
79+
if (
80+
against.type !== AST_NODE_TYPES.Literal ||
81+
typeof against.value !== 'boolean'
82+
) {
83+
continue;
84+
}
85+
86+
const { value } = against;
87+
const negated = node.operator.startsWith('!');
88+
89+
return {
90+
forTruthy: value ? !negated : negated,
91+
expression,
92+
negated,
93+
range:
94+
expression.range[0] < against.range[0]
95+
? [expression.range[1], against.range[1]]
96+
: [against.range[1], expression.range[1]],
97+
};
98+
}
99+
100+
return undefined;
101+
}
102+
103+
return {
104+
BinaryExpression(node): void {
105+
const comparison = getBooleanComparison(node);
106+
107+
if (comparison) {
108+
context.report({
109+
fix: function*(fixer) {
110+
yield fixer.removeRange(comparison.range);
111+
112+
if (!comparison.forTruthy) {
113+
yield fixer.insertTextBefore(node, '!');
114+
}
115+
},
116+
messageId: comparison.negated ? 'negated' : 'direct',
117+
node,
118+
});
119+
}
120+
},
121+
};
122+
},
123+
});

‎packages/eslint-plugin/src/util/types.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,3 +254,39 @@ export function getTokenAtPosition(
254254
}
255255
return current!;
256256
}
257+
258+
export interface EqualsKind {
259+
isPositive: boolean;
260+
isStrict: boolean;
261+
}
262+
263+
export function getEqualsKind(operator: string): EqualsKind | undefined {
264+
switch (operator) {
265+
case '==':
266+
return {
267+
isPositive: true,
268+
isStrict: false,
269+
};
270+
271+
case '===':
272+
return {
273+
isPositive: true,
274+
isStrict: true,
275+
};
276+
277+
case '!=':
278+
return {
279+
isPositive: false,
280+
isStrict: false,
281+
};
282+
283+
case '!==':
284+
return {
285+
isPositive: true,
286+
isStrict: true,
287+
};
288+
289+
default:
290+
return undefined;
291+
}
292+
}

‎packages/eslint-plugin/tests/rules/no-unnecessary-boolean-literal-compare.test.ts

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import rule from '../../src/rules/no-unnecessary-boolean-literal-compare';
2+
import { RuleTester, getFixturesRootDir } from '../RuleTester';
3+
4+
const rootDir = getFixturesRootDir();
5+
const ruleTester = new RuleTester({
6+
parser: '@typescript-eslint/parser',
7+
parserOptions: {
8+
ecmaVersion: 2015,
9+
tsconfigRootDir: rootDir,
10+
project: './tsconfig.json',
11+
},
12+
});
13+
14+
ruleTester.run('boolean-literal-compare', rule, {
15+
valid: [
16+
`
17+
declare const varAny: any;
18+
varAny === true;
19+
`,
20+
`
21+
declare const varAny: any;
22+
varAny == false;
23+
`,
24+
`
25+
declare const varString: string;
26+
varString === false;
27+
`,
28+
`
29+
declare const varString: string;
30+
varString === true;
31+
`,
32+
`
33+
declare const varObject: {};
34+
varObject === true;
35+
`,
36+
`
37+
declare const varObject: {};
38+
varObject == false;
39+
`,
40+
`
41+
declare const varBooleanOrString: boolean | undefined;
42+
varBooleanOrString === false;
43+
`,
44+
`
45+
declare const varBooleanOrString: boolean | undefined;
46+
varBooleanOrString == true;
47+
`,
48+
`
49+
declare const varBooleanOrUndefined: boolean | undefined;
50+
varBooleanOrUndefined === true;
51+
`,
52+
`'false' === true;`,
53+
`'true' === false;`,
54+
],
55+
56+
invalid: [
57+
{
58+
code: `true === true`,
59+
errors: [
60+
{
61+
messageId: 'direct',
62+
},
63+
],
64+
output: `true`,
65+
},
66+
{
67+
code: `false !== true`,
68+
errors: [
69+
{
70+
messageId: 'negated',
71+
},
72+
],
73+
output: `!false`,
74+
},
75+
{
76+
code: `
77+
declare const varBoolean: boolean;
78+
if (varBoolean !== false) { }
79+
`,
80+
errors: [
81+
{
82+
messageId: 'negated',
83+
},
84+
],
85+
output: `
86+
declare const varBoolean: boolean;
87+
if (varBoolean) { }
88+
`,
89+
},
90+
{
91+
code: `
92+
declare const varTrue: true;
93+
if (varTrue !== true) { }
94+
`,
95+
errors: [
96+
{
97+
messageId: 'negated',
98+
},
99+
],
100+
output: `
101+
4E58 declare const varTrue: true;
102+
if (!varTrue) { }
103+
`,
104+
},
105+
],
106+
});

0 commit comments

Comments
 (0)
0