Skip to content

Commit

Permalink
feat(eslint-plugin): add no-unnecessary-boolean-literal-compare (#242)
Browse files Browse the repository at this point in the history
Co-authored-by: Brad Zacher <brad.zacher@gmail.com>
  • Loading branch information
Josh Goldberg and bradzacher committed Jan 29, 2020
1 parent b835ec2 commit 6bebb1d
Show file tree
Hide file tree
Showing 8 changed files with 372 additions and 61 deletions.
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
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
@@ -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
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
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
@@ -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
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;
}
}
@@ -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) { }
`,
},
],
});

0 comments on commit 6bebb1d

Please sign in to comment.