Skip to content

feat(eslint-plugin): add no-unnecessary-boolean-literal-compare #242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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
121 changes: 61 additions & 60 deletions packages/eslint-plugin/README.md

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion packages/eslint-plugin/ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th
| [`newline-per-chained-call`] | 🌟 | [`newline-per-chained-call`][newline-per-chained-call] |
| [`new-parens`] | 🌟 | [`new-parens`][new-parens] |
| [`no-angle-bracket-type-assertion`] | ✅ | [`@typescript-eslint/consistent-type-assertions`] |
| [`no-boolean-literal-compare`] | 🛑 | N/A |
| [`no-boolean-literal-compare`] | | [`@typescript-eslint/no-unnecessary-boolean-literal-compare`] |
| [`no-consecutive-blank-lines`] | 🌟 | [`no-multiple-empty-lines`][no-multiple-empty-lines] |
| [`no-irregular-whitespace`] | 🌟 | [`no-irregular-whitespace`][no-irregular-whitespace] with `skipStrings: false` |
| [`no-parameter-properties`] | ✅ | [`@typescript-eslint/no-parameter-properties`] |
Expand Down Expand Up @@ -600,6 +600,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint-
[`@typescript-eslint/type-annotation-spacing`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/type-annotation-spacing.md
[`@typescript-eslint/typedef`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/typedef.md
[`@typescript-eslint/unified-signatures`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/unified-signatures.md
[`@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
[`@typescript-eslint/no-misused-new`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-misused-new.md
[`@typescript-eslint/no-this-alias`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-this-alias.md
[`@typescript-eslint/no-throw-literal`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-throw-literal.md
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Flags unnecessary equality comparisons against boolean literals (`no-unnecessary-boolean-literal-compare`)

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.

## Rule Details

This rule ensures that you do not include unnecessary comparisons with boolean literals.
A comparison is considered unnecessary if it checks a boolean literal against any variable with just the `boolean` type.
A comparison is **_not_** considered unnecessary if the type is a union of booleans (`string | boolean`, `someObject | boolean`).

Examples of **incorrect** code for this rule:

```ts
declare const someCondition: boolean;
if (someCondition === true) {
}
```

Examples of **correct** code for this rule

```ts
declare const someCondition: boolean;
if (someCondition) {
}

declare const someObjectBoolean: boolean | Record<string, unknown>;
if (someObjectBoolean === true) {
}

declare const someStringBoolean: boolean | string;
if (someStringBoolean === true) {
}

declare const someUndefinedCondition: boolean | undefined;
if (someUndefinedCondition === false) {
}
```

## Related to

- TSLint: [no-boolean-literal-compare](https://palantir.github.io/tslint/rules/no-boolean-literal-compare)
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"@typescript-eslint/no-this-alias": "error",
"@typescript-eslint/no-throw-literal": "error",
"@typescript-eslint/no-type-alias": "error",
"@typescript-eslint/no-unnecessary-boolean-literal-compare": "error",
"@typescript-eslint/no-unnecessary-condition": "error",
"@typescript-eslint/no-unnecessary-qualifier": "error",
"@typescript-eslint/no-unnecessary-type-arguments": "error",
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import noRequireImports from './no-require-imports';
import noThisAlias from './no-this-alias';
import noThrowLiteral from './no-throw-literal';
import noTypeAlias from './no-type-alias';
import noUnnecessaryBooleanLiteralCompare from './no-unnecessary-boolean-literal-compare';
import noUnnecessaryCondition from './no-unnecessary-condition';
import noUnnecessaryQualifier from './no-unnecessary-qualifier';
import useDefaultTypeParameter from './no-unnecessary-type-arguments';
Expand Down Expand Up @@ -132,6 +133,7 @@ export default {
'no-this-alias': noThisAlias,
'no-type-alias': noTypeAlias,
'no-throw-literal': noThrowLiteral,
'no-unnecessary-boolean-literal-compare': noUnnecessaryBooleanLiteralCompare,
'no-unnecessary-condition': noUnnecessaryCondition,
'no-unnecessary-qualifier': noUnnecessaryQualifier,
'no-unnecessary-type-arguments': useDefaultTypeParameter,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';
import * as tsutils from 'tsutils';
import * as ts from 'typescript';
import * as util from '../util';

type MessageIds = 'direct' | 'negated';

interface BooleanComparison {
expression: TSESTree.Expression;
forTruthy: boolean;
negated: boolean;
range: [number, number];
}

export default util.createRule<[], MessageIds>({
name: 'no-unnecessary-boolean-literal-compare',
meta: {
docs: {
description:
'Flags unnecessary equality comparisons against boolean literals',
category: 'Stylistic Issues',
recommended: false,
requiresTypeChecking: true,
},
fixable: 'code',
messages: {
direct:
'This expression unnecessarily compares a boolean value to a boolean instead of using it directly',
negated:
'This expression unnecessarily compares a boolean value to a boolean instead of negating it.',
},
schema: [],
type: 'suggestion',
},
defaultOptions: [],
create(context) {
const parserServices = util.getParserServices(context);
const checker = parserServices.program.getTypeChecker();

function getBooleanComparison(
node: TSESTree.BinaryExpression,
): BooleanComparison | undefined {
const comparison = deconstructComparison(node);
if (!comparison) {
return undefined;
}

const expressionType = checker.getTypeAtLocation(
parserServices.esTreeNodeToTSNodeMap.get(comparison.expression),
);

if (
!tsutils.isTypeFlagSet(
expressionType,
ts.TypeFlags.Boolean | ts.TypeFlags.BooleanLiteral,
)
) {
return undefined;
}

return comparison;
}

function deconstructComparison(
node: TSESTree.BinaryExpression,
): BooleanComparison | undefined {
const comparisonType = util.getEqualsKind(node.operator);
if (!comparisonType) {
return undefined;
}

for (const [against, expression] of [
[node.right, node.left],
[node.left, node.right],
]) {
if (
against.type !== AST_NODE_TYPES.Literal ||
typeof against.value !== 'boolean'
) {
continue;
}

const { value } = against;
const negated = node.operator.startsWith('!');

return {
forTruthy: value ? !negated : negated,
expression,
negated,
range:
expression.range[0] < against.range[0]
? [expression.range[1], against.range[1]]
: [against.range[1], expression.range[1]],
};
}

return undefined;
}

return {
BinaryExpression(node): void {
const comparison = getBooleanComparison(node);

if (comparison) {
context.report({
fix: function*(fixer) {
yield fixer.removeRange(comparison.range);

if (!comparison.forTruthy) {
yield fixer.insertTextBefore(node, '!');
}
},
messageId: comparison.negated ? 'negated' : 'direct',
node,
});
}
},
};
},
});
36 changes: 36 additions & 0 deletions packages/eslint-plugin/src/util/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,3 +254,39 @@ export function getTokenAtPosition(
}
return current!;
}

export interface EqualsKind {
isPositive: boolean;
isStrict: boolean;
}

export function getEqualsKind(operator: string): EqualsKind | undefined {
switch (operator) {
case '==':
return {
isPositive: true,
isStrict: false,
};

case '===':
return {
isPositive: true,
isStrict: true,
};

case '!=':
return {
isPositive: false,
isStrict: false,
};

case '!==':
return {
isPositive: true,
isStrict: true,
};

default:
return undefined;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import rule from '../../src/rules/no-unnecessary-boolean-literal-compare';
import { RuleTester, getFixturesRootDir } from '../RuleTester';

const rootDir = getFixturesRootDir();
const ruleTester = new RuleTester({
parser: '@typescript-eslint/parser',
parserOptions: {
ecmaVersion: 2015,
tsconfigRootDir: rootDir,
project: './tsconfig.json',
},
});

ruleTester.run('boolean-literal-compare', rule, {
valid: [
`
declare const varAny: any;
varAny === true;
`,
`
declare const varAny: any;
varAny == false;
`,
`
declare const varString: string;
varString === false;
`,
`
declare const varString: string;
varString === true;
`,
`
declare const varObject: {};
varObject === true;
`,
`
declare const varObject: {};
varObject == false;
`,
`
declare const varBooleanOrString: boolean | undefined;
varBooleanOrString === false;
`,
`
declare const varBooleanOrString: boolean | undefined;
varBooleanOrString == true;
`,
`
declare const varBooleanOrUndefined: boolean | undefined;
varBooleanOrUndefined === true;
`,
`'false' === true;`,
`'true' === false;`,
],

invalid: [
{
code: `true === true`,
errors: [
{
messageId: 'direct',
},
],
output: `true`,
},
{
code: `false !== true`,
errors: [
{
messageId: 'negated',
},
],
output: `!false`,
},
{
code: `
declare const varBoolean: boolean;
if (varBoolean !== false) { }
`,
errors: [
{
messageId: 'negated',
},
],
output: `
declare const varBoolean: boolean;
if (varBoolean) { }
`,
},
{
code: `
declare const varTrue: true;
if (varTrue !== true) { }
`,
errors: [
{
messageId: 'negated',
},
],
output: `
declare const varTrue: true;
if (!varTrue) { }
`,
},
],
});