From 6bebb1dc47897ee0e1f075d7e5dd89d8b0590f31 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 29 Jan 2020 12:44:20 -0500 Subject: [PATCH] feat(eslint-plugin): add no-unnecessary-boolean-literal-compare (#242) Co-authored-by: Brad Zacher --- packages/eslint-plugin/README.md | 121 ++++++++--------- packages/eslint-plugin/ROADMAP.md | 3 +- .../no-unnecessary-boolean-literal-compare.md | 41 ++++++ packages/eslint-plugin/src/configs/all.json | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + .../no-unnecessary-boolean-literal-compare.ts | 123 ++++++++++++++++++ packages/eslint-plugin/src/util/types.ts | 36 +++++ ...nnecessary-boolean-literal-compare.test.ts | 106 +++++++++++++++ 8 files changed, 372 insertions(+), 61 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/no-unnecessary-boolean-literal-compare.md create mode 100644 packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts create mode 100644 packages/eslint-plugin/tests/rules/no-unnecessary-boolean-literal-compare.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 744f2513d9e..911d2f6e12c 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -92,66 +92,67 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int **Key**: :heavy_check_mark: = recommended, :wrench: = fixable, :thought_balloon: = requires type information -| Name | Description | :heavy_check_mark: | :wrench: | :thought_balloon: | -| --------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | ------------------ | -------- | ----------------- | -| [`@typescript-eslint/adjacent-overload-signatures`](./docs/rules/adjacent-overload-signatures.md) | Require that member overloads be consecutive | :heavy_check_mark: | | | -| [`@typescript-eslint/array-type`](./docs/rules/array-type.md) | Requires using either `T[]` or `Array` for arrays | | :wrench: | | -| [`@typescript-eslint/await-thenable`](./docs/rules/await-thenable.md) | Disallows awaiting a value that is not a Thenable | :heavy_check_mark: | | :thought_balloon: | -| [`@typescript-eslint/ban-ts-comment`](./docs/rules/ban-ts-comment.md) | Bans `// @ts-` comments from being used | | | | -| [`@typescript-eslint/ban-types`](./docs/rules/ban-types.md) | Bans specific types from being used | :heavy_check_mark: | :wrench: | | -| [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | :heavy_check_mark: | | | -| [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | | :wrench: | | -| [`@typescript-eslint/explicit-function-return-type`](./docs/rules/explicit-function-return-type.md) | Require explicit return types on functions and class methods | :heavy_check_mark: | | | -| [`@typescript-eslint/explicit-member-accessibility`](./docs/rules/explicit-member-accessibility.md) | Require explicit accessibility modifiers on class properties and methods | | | | -| [`@typescript-eslint/explicit-module-boundary-types`](./docs/rules/explicit-module-boundary-types.md) | Require explicit return and argument types on exported functions' and classes' public class methods | | | | -| [`@typescript-eslint/member-delimiter-style`](./docs/rules/member-delimiter-style.md) | Require a specific member delimiter style for interfaces and type literals | :heavy_check_mark: | :wrench: | | -| [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order | | | | -| [`@typescript-eslint/naming-convention`](./docs/rules/naming-convention.md) | Enforces naming conventions for everything across a codebase | | | :thought_balloon: | -| [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Disallow the delete operator with computed key expressions | | :wrench: | | -| [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces | :heavy_check_mark: | :wrench: | | -| [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :heavy_check_mark: | :wrench: | | -| [`@typescript-eslint/no-extra-non-null-assertion`](./docs/rules/no-extra-non-null-assertion.md) | Disallow extra non-null assertion | | | | -| [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces | | | | -| [`@typescript-eslint/no-floating-promises`](./docs/rules/no-floating-promises.md) | Requires Promise-like values to be handled appropriately | | | :thought_balloon: | -| [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop | :heavy_check_mark: | | :thought_balloon: | -| [`@typescript-eslint/no-implied-eval`](./docs/rules/no-implied-eval.md) | Disallow the use of `eval()`-like methods | | | :thought_balloon: | -| [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean | :heavy_check_mark: | :wrench: | | -| [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor` | :heavy_check_mark: | | | -| [`@typescript-eslint/no-misused-promises`](./docs/rules/no-misused-promises.md) | Avoid using promises in places not designed to handle them | :heavy_check_mark: | | :thought_balloon: | -| [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md) | Disallow the use of custom TypeScript modules and namespaces | :heavy_check_mark: | | | -| [`@typescript-eslint/no-non-null-asserted-optional-chain`](./docs/rules/no-non-null-asserted-optional-chain.md) | Disallows using a non-null assertion after an optional chain expression | | | | -| [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator | :heavy_check_mark: | | | -| [`@typescript-eslint/no-parameter-properties`](./docs/rules/no-parameter-properties.md) | Disallow the use of parameter properties in class constructors | | | | -| [`@typescript-eslint/no-require-imports`](./docs/rules/no-require-imports.md) | Disallows invocation of `require()` | | | | -| [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` | :heavy_check_mark: | | | -| [`@typescript-eslint/no-throw-literal`](./docs/rules/no-throw-literal.md) | Disallow throwing literals as exceptions | | | :thought_balloon: | -| [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases | | | | -| [`@typescript-eslint/no-unnecessary-condition`](./docs/rules/no-unnecessary-condition.md) | Prevents conditionals where the type is always truthy or always falsy | | :wrench: | :thought_balloon: | -| [`@typescript-eslint/no-unnecessary-qualifier`](./docs/rules/no-unnecessary-qualifier.md) | Warns when a namespace qualifier is unnecessary | | :wrench: | :thought_balloon: | -| [`@typescript-eslint/no-unnecessary-type-arguments`](./docs/rules/no-unnecessary-type-arguments.md) | Enforces that type arguments will not be used if not required | | :wrench: | :thought_balloon: | -| [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression | :heavy_check_mark: | :wrench: | :thought_balloon: | -| [`@typescript-eslint/no-unused-vars-experimental`](./docs/rules/no-unused-vars-experimental.md) | Disallow unused variables and arguments | | | :thought_balloon: | -| [`@typescript-eslint/no-var-requires`](./docs/rules/no-var-requires.md) | Disallows the use of require statements except in import statements | :heavy_check_mark: | | | -| [`@typescript-eslint/prefer-as-const`](./docs/rules/prefer-as-const.md) | Prefer usage of `as const` over literal type | | :wrench: | | -| [`@typescript-eslint/prefer-for-of`](./docs/rules/prefer-for-of.md) | Prefer a ‘for-of’ loop over a standard ‘for’ loop if the index is only used to access the array being iterated | | | | -| [`@typescript-eslint/prefer-function-type`](./docs/rules/prefer-function-type.md) | Use function types instead of interfaces with call signatures | | :wrench: | | -| [`@typescript-eslint/prefer-includes`](./docs/rules/prefer-includes.md) | Enforce `includes` method over `indexOf` method | :heavy_check_mark: | :wrench: | :thought_balloon: | -| [`@typescript-eslint/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md) | Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules | :heavy_check_mark: | :wrench: | | -| [`@typescript-eslint/prefer-nullish-coalescing`](./docs/rules/prefer-nullish-coalescing.md) | Enforce the usage of the nullish coalescing operator instead of logical chaining | | :wrench: | :thought_balloon: | -| [`@typescript-eslint/prefer-optional-chain`](./docs/rules/prefer-optional-chain.md) | Prefer using concise optional chain expressions instead of chained logical ands | | :wrench: | | -| [`@typescript-eslint/prefer-readonly`](./docs/rules/prefer-readonly.md) | Requires that private members are marked as `readonly` if they're never modified outside of the constructor | | :wrench: | :thought_balloon: | -| [`@typescript-eslint/prefer-regexp-exec`](./docs/rules/prefer-regexp-exec.md) | Enforce that `RegExp#exec` is used instead of `String#match` if no global flag is provided | :heavy_check_mark: | | :thought_balloon: | -| [`@typescript-eslint/prefer-string-starts-ends-with`](./docs/rules/prefer-string-starts-ends-with.md) | Enforce the use of `String#startsWith` and `String#endsWith` instead of other equivalent methods of checking substrings | :heavy_check_mark: | :wrench: | :thought_balloon: | -| [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async | | | :thought_balloon: | -| [`@typescript-eslint/require-array-sort-compare`](./docs/rules/require-array-sort-compare.md) | Requires `Array#sort` calls to always provide a `compareFunction` | | | :thought_balloon: | -| [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md) | When adding two variables, operands must both be of type number or of type string | | | :thought_balloon: | -| [`@typescript-eslint/restrict-template-expressions`](./docs/rules/restrict-template-expressions.md) | Enforce template literal expressions to be of string type | | | :thought_balloon: | -| [`@typescript-eslint/strict-boolean-expressions`](./docs/rules/strict-boolean-expressions.md) | Restricts the types allowed in boolean expressions | | | :thought_balloon: | -| [`@typescript-eslint/triple-slash-reference`](./docs/rules/triple-slash-reference.md) | Sets preference level for triple slash directives versus ES6-style import declarations | :heavy_check_mark: | | | -| [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations | :heavy_check_mark: | :wrench: | | -| [`@typescript-eslint/typedef`](./docs/rules/typedef.md) | Requires type annotations to exist | | | | -| [`@typescript-eslint/unbound-method`](./docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | :heavy_check_mark: | | :thought_balloon: | -| [`@typescript-eslint/unified-signatures`](./docs/rules/unified-signatures.md) | Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter | | | | +| Name | Description | :heavy_check_mark: | :wrench: | :thought_balloon: | +| --------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------- | ------------------ | -------- | ----------------- | +| [`@typescript-eslint/adjacent-overload-signatures`](./docs/rules/adjacent-overload-signatures.md) | Require that member overloads be consecutive | :heavy_check_mark: | | | +| [`@typescript-eslint/array-type`](./docs/rules/array-type.md) | Requires using either `T[]` or `Array` for arrays | | :wrench: | | +| [`@typescript-eslint/await-thenable`](./docs/rules/await-thenable.md) | Disallows awaiting a value that is not a Thenable | :heavy_check_mark: | | :thought_balloon: | +| [`@typescript-eslint/ban-ts-comment`](./docs/rules/ban-ts-comment.md) | Bans `// @ts-` comments from being used | | | | +| [`@typescript-eslint/ban-types`](./docs/rules/ban-types.md) | Bans specific types from being used | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | :heavy_check_mark: | | | +| [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | | :wrench: | | +| [`@typescript-eslint/explicit-function-return-type`](./docs/rules/explicit-function-return-type.md) | Require explicit return types on functions and class methods | :heavy_check_mark: | | | +| [`@typescript-eslint/explicit-member-accessibility`](./docs/rules/explicit-member-accessibility.md) | Require explicit accessibility modifiers on class properties and methods | | | | +| [`@typescript-eslint/explicit-module-boundary-types`](./docs/rules/explicit-module-boundary-types.md) | Require explicit return and argument types on exported functions' and classes' public class methods | | | | +| [`@typescript-eslint/member-delimiter-style`](./docs/rules/member-delimiter-style.md) | Require a specific member delimiter style for interfaces and type literals | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order | | | | +| [`@typescript-eslint/naming-convention`](./docs/rules/naming-convention.md) | Enforces naming conventions for everything across a codebase | | | :thought_balloon: | +| [`@typescript-eslint/no-dynamic-delete`](./docs/rules/no-dynamic-delete.md) | Disallow the delete operator with computed key expressions | | :wrench: | | +| [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/no-extra-non-null-assertion`](./docs/rules/no-extra-non-null-assertion.md) | Disallow extra non-null assertion | | | | +| [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces | | | | +| [`@typescript-eslint/no-floating-promises`](./docs/rules/no-floating-promises.md) | Requires Promise-like values to be handled appropriately | | | :thought_balloon: | +| [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop | :heavy_check_mark: | | :thought_balloon: | +| [`@typescript-eslint/no-implied-eval`](./docs/rules/no-implied-eval.md) | Disallow the use of `eval()`-like methods | | | :thought_balloon: | +| [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor` | :heavy_check_mark: | | | +| [`@typescript-eslint/no-misused-promises`](./docs/rules/no-misused-promises.md) | Avoid using promises in places not designed to handle them | :heavy_check_mark: | | :thought_balloon: | +| [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md) | Disallow the use of custom TypeScript modules and namespaces | :heavy_check_mark: | | | +| [`@typescript-eslint/no-non-null-asserted-optional-chain`](./docs/rules/no-non-null-asserted-optional-chain.md) | Disallows using a non-null assertion after an optional chain expression | | | | +| [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator | :heavy_check_mark: | | | +| [`@typescript-eslint/no-parameter-properties`](./docs/rules/no-parameter-properties.md) | Disallow the use of parameter properties in class constructors | | | | +| [`@typescript-eslint/no-require-imports`](./docs/rules/no-require-imports.md) | Disallows invocation of `require()` | | | | +| [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` | :heavy_check_mark: | | | +| [`@typescript-eslint/no-throw-literal`](./docs/rules/no-throw-literal.md) | Disallow throwing literals as exceptions | | | :thought_balloon: | +| [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases | | | | +| [`@typescript-eslint/no-unnecessary-boolean-literal-compare`](./docs/rules/no-unnecessary-boolean-literal-compare.md) | Flags unnecessary equality comparisons against boolean literals | | :wrench: | :thought_balloon: | +| [`@typescript-eslint/no-unnecessary-condition`](./docs/rules/no-unnecessary-condition.md) | Prevents conditionals where the type is always truthy or always falsy | | :wrench: | :thought_balloon: | +| [`@typescript-eslint/no-unnecessary-qualifier`](./docs/rules/no-unnecessary-qualifier.md) | Warns when a namespace qualifier is unnecessary | | :wrench: | :thought_balloon: | +| [`@typescript-eslint/no-unnecessary-type-arguments`](./docs/rules/no-unnecessary-type-arguments.md) | Enforces that type arguments will not be used if not required | | :wrench: | :thought_balloon: | +| [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression | :heavy_check_mark: | :wrench: | :thought_balloon: | +| [`@typescript-eslint/no-unused-vars-experimental`](./docs/rules/no-unused-vars-experimental.md) | Disallow unused variables and arguments | | | :thought_balloon: | +| [`@typescript-eslint/no-var-requires`](./docs/rules/no-var-requires.md) | Disallows the use of require statements except in import statements | :heavy_check_mark: | | | +| [`@typescript-eslint/prefer-as-const`](./docs/rules/prefer-as-const.md) | Prefer usage of `as const` over literal type | | :wrench: | | +| [`@typescript-eslint/prefer-for-of`](./docs/rules/prefer-for-of.md) | Prefer a ‘for-of’ loop over a standard ‘for’ loop if the index is only used to access the array being iterated | | | | +| [`@typescript-eslint/prefer-function-type`](./docs/rules/prefer-function-type.md) | Use function types instead of interfaces with call signatures | | :wrench: | | +| [`@typescript-eslint/prefer-includes`](./docs/rules/prefer-includes.md) | Enforce `includes` method over `indexOf` method | :heavy_check_mark: | :wrench: | :thought_balloon: | +| [`@typescript-eslint/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md) | Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/prefer-nullish-coalescing`](./docs/rules/prefer-nullish-coalescing.md) | Enforce the usage of the nullish coalescing operator instead of logical chaining | | :wrench: | :thought_balloon: | +| [`@typescript-eslint/prefer-optional-chain`](./docs/rules/prefer-optional-chain.md) | Prefer using concise optional chain expressions instead of chained logical ands | | :wrench: | | +| [`@typescript-eslint/prefer-readonly`](./docs/rules/prefer-readonly.md) | Requires that private members are marked as `readonly` if they're never modified outside of the constructor | | :wrench: | :thought_balloon: | +| [`@typescript-eslint/prefer-regexp-exec`](./docs/rules/prefer-regexp-exec.md) | Enforce that `RegExp#exec` is used instead of `String#match` if no global flag is provided | :heavy_check_mark: | | :thought_balloon: | +| [`@typescript-eslint/prefer-string-starts-ends-with`](./docs/rules/prefer-string-starts-ends-with.md) | Enforce the use of `String#startsWith` and `String#endsWith` instead of other equivalent methods of checking substrings | :heavy_check_mark: | :wrench: | :thought_balloon: | +| [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async | | | :thought_balloon: | +| [`@typescript-eslint/require-array-sort-compare`](./docs/rules/require-array-sort-compare.md) | Requires `Array#sort` calls to always provide a `compareFunction` | | | :thought_balloon: | +| [`@typescript-eslint/restrict-plus-operands`](./docs/rules/restrict-plus-operands.md) | When adding two variables, operands must both be of type number or of type string | | | :thought_balloon: | +| [`@typescript-eslint/restrict-template-expressions`](./docs/rules/restrict-template-expressions.md) | Enforce template literal expressions to be of string type | | | :thought_balloon: | +| [`@typescript-eslint/strict-boolean-expressions`](./docs/rules/strict-boolean-expressions.md) | Restricts the types allowed in boolean expressions | | | :thought_balloon: | +| [`@typescript-eslint/triple-slash-reference`](./docs/rules/triple-slash-reference.md) | Sets preference level for triple slash directives versus ES6-style import declarations | :heavy_check_mark: | | | +| [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations | :heavy_check_mark: | :wrench: | | +| [`@typescript-eslint/typedef`](./docs/rules/typedef.md) | Requires type annotations to exist | | | | +| [`@typescript-eslint/unbound-method`](./docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | :heavy_check_mark: | | :thought_balloon: | +| [`@typescript-eslint/unified-signatures`](./docs/rules/unified-signatures.md) | Warns for any two overloads that could be unified into one by using a union or an optional/rest parameter | | | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index 20661b22ea0..c4f99f947af 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -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`] | @@ -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 diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-boolean-literal-compare.md b/packages/eslint-plugin/docs/rules/no-unnecessary-boolean-literal-compare.md new file mode 100644 index 00000000000..81b4a5c67f6 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-boolean-literal-compare.md @@ -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; +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) diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index e4b69b7764c..987fc94a3d7 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -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", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index abaa31db3b3..49ca742af3e 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -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'; @@ -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, diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts b/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts new file mode 100644 index 00000000000..f0ebc0292ce --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts @@ -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, + }); + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index e53bfb35531..eae3519ea21 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -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; + } +} diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-boolean-literal-compare.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-boolean-literal-compare.test.ts new file mode 100644 index 00000000000..1c6cfebeecc --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-boolean-literal-compare.test.ts @@ -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) { } + `, + }, + ], +});