diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 91eaf17711e..21846de67c1 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -166,30 +166,48 @@ In these cases, we create what we call an extension rule; a rule within our plug **Key**: :heavy_check_mark: = recommended, :wrench: = fixable, :thought_balloon: = requires type information -| Name | Description | :heavy_check_mark: | :wrench: | :thought_balloon: | -| --------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | -------- | ----------------- | -| [`@typescript-eslint/brace-style`](./docs/rules/brace-style.md) | Enforce consistent brace style for blocks | | :wrench: | | -| [`@typescript-eslint/comma-spacing`](./docs/rules/comma-spacing.md) | Enforces consistent spacing before and after commas | | :wrench: | | -| [`@typescript-eslint/default-param-last`](./docs/rules/default-param-last.md) | Enforce default parameters to be last | | | | -| [`@typescript-eslint/func-call-spacing`](./docs/rules/func-call-spacing.md) | Require or disallow spacing between function identifiers and their invocations | | :wrench: | | -| [`@typescript-eslint/indent`](./docs/rules/indent.md) | Enforce consistent indentation | | :wrench: | | -| [`@typescript-eslint/no-array-constructor`](./docs/rules/no-array-constructor.md) | Disallow generic `Array` constructors | :heavy_check_mark: | :wrench: | | -| [`@typescript-eslint/no-empty-function`](./docs/rules/no-empty-function.md) | Disallow empty functions | :heavy_check_mark: | | | -| [`@typescript-eslint/no-extra-parens`](./docs/rules/no-extra-parens.md) | Disallow unnecessary parentheses | | :wrench: | | -| [`@typescript-eslint/no-extra-semi`](./docs/rules/no-extra-semi.md) | Disallow unnecessary semicolons | | :wrench: | | -| [`@typescript-eslint/no-magic-numbers`](./docs/rules/no-magic-numbers.md) | Disallow magic numbers | | | | -| [`@typescript-eslint/no-unused-expressions`](./docs/rules/no-unused-expressions.md) | Disallow unused expressions | | | | -| [`@typescript-eslint/no-unused-vars`](./docs/rules/no-unused-vars.md) | Disallow unused variables | :heavy_check_mark: | | | -| [`@typescript-eslint/no-use-before-define`](./docs/rules/no-use-before-define.md) | Disallow the use of variables before they are defined | :heavy_check_mark: | | | -| [`@typescript-eslint/no-useless-constructor`](./docs/rules/no-useless-constructor.md) | Disallow unnecessary constructors | | | | -| [`@typescript-eslint/quotes`](./docs/rules/quotes.md) | Enforce the consistent use of either backticks, double, or single quotes | | :wrench: | | -| [`@typescript-eslint/require-await`](./docs/rules/require-await.md) | Disallow async functions which have no `await` expression | :heavy_check_mark: | | :thought_balloon: | -| [`@typescript-eslint/return-await`](./docs/rules/return-await.md) | Enforces consistent returning of awaited values | | | :thought_balloon: | -| [`@typescript-eslint/semi`](./docs/rules/semi.md) | Require or disallow semicolons instead of ASI | | :wrench: | | -| [`@typescript-eslint/space-before-function-paren`](./docs/rules/space-before-function-paren.md) | Enforces consistent spacing before function parenthesis | | :wrench: | | - - - -## Contributing - -[See the contributing guide here](../../CONTRIBUTING.md). +| Name | Description | :heavy_check_mark: | :wrench: | +| --------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ | -------- | +| [`@typescript-eslint/adjacent-overload-signatures`](./docs/rules/adjacent-overload-signatures.md) | Require that member overloads be consecutive (`adjacent-overload-signatures` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/array-type`](./docs/rules/array-type.md) | Requires using either `T[]` or `Array` for arrays (`array-type` from TSLint) | :heavy_check_mark: | :wrench: | +| [`@typescript-eslint/ban-types`](./docs/rules/ban-types.md) | Enforces that types will not to be used (`ban-types` from TSLint) | :heavy_check_mark: | :wrench: | +| [`@typescript-eslint/no-unnecessary-boolean-literal-compare`](./docs/rules/no-unnecessary-boolean-literal-compare.md) | Flags unnecessary equality comparisons against booleans (`no-boolean-literal-compare` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/camelcase`](./docs/rules/camelcase.md) | Enforce camelCase naming convention | :heavy_check_mark: | | +| [`@typescript-eslint/class-name-casing`](./docs/rules/class-name-casing.md) | Require PascalCased class and interface names (`class-name` from TSLint) | :heavy_check_mark: | | +| [`@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 (`member-access` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/generic-type-naming`](./docs/rules/generic-type-naming.md) | Enforces naming of generic type variables | | | +| [`@typescript-eslint/indent`](./docs/rules/indent.md) | Enforce consistent indentation (`indent` from TSLint) | :heavy_check_mark: | :wrench: | +| [`@typescript-eslint/interface-name-prefix`](./docs/rules/interface-name-prefix.md) | Require that interface names be prefixed with `I` (`interface-name` from TSLint) | :heavy_check_mark: | | +| [`@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-naming`](./docs/rules/member-naming.md) | Enforces naming conventions for class members by visibility. | | | +| [`@typescript-eslint/member-ordering`](./docs/rules/member-ordering.md) | Require a consistent member declaration order (`member-ordering` from TSLint) | | | +| [`@typescript-eslint/no-angle-bracket-type-assertion`](./docs/rules/no-angle-bracket-type-assertion.md) | Enforces the use of `as Type` assertions instead of `` assertions (`no-angle-bracket-type-assertion` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/no-array-constructor`](./docs/rules/no-array-constructor.md) | Disallow generic `Array` constructors | :heavy_check_mark: | :wrench: | +| [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces (`no-empty-interface` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type (`no-any` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces (`no-unnecessary-class` from TSLint) | | | +| [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop (`no-for-in-array` from TSLint) | | | +| [`@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. (`no-inferrable-types` from TSLint) | :heavy_check_mark: | :wrench: | +| [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor`. (`no-misused-new` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md) | Disallow the use of custom TypeScript modules and namespaces (`no-namespace` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator (`no-non-null-assertion` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/no-object-literal-type-assertion`](./docs/rules/no-object-literal-type-assertion.md) | Forbids an object literal to appear in a type assertion expression (`no-object-literal-type-assertion` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/no-parameter-properties`](./docs/rules/no-parameter-properties.md) | Disallow the use of parameter properties in class constructors. (`no-parameter-properties` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/no-require-imports`](./docs/rules/no-require-imports.md) | Disallows invocation of `require()` (`no-require-imports` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` (`no-this-assignment` from TSLint) | | | +| [`@typescript-eslint/no-triple-slash-reference`](./docs/rules/no-triple-slash-reference.md) | Disallow `/// ` comments (`no-reference` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases (`interface-over-type-literal` from TSLint) | | | +| [`@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 (`no-unnecessary-type-assertion` from TSLint) | | :wrench: | +| [`@typescript-eslint/no-unused-vars`](./docs/rules/no-unused-vars.md) | Disallow unused variables (`no-unused-variable` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/no-use-before-define`](./docs/rules/no-use-before-define.md) | Disallow the use of variables before they are defined | :heavy_check_mark: | | +| [`@typescript-eslint/no-useless-constructor`](./docs/rules/no-useless-constructor.md) | Disallow unnecessary constructors | | | +| [`@typescript-eslint/no-var-requires`](./docs/rules/no-var-requires.md) | Disallows the use of require statements except in import statements (`no-var-requires` from TSLint) | :heavy_check_mark: | | +| [`@typescript-eslint/prefer-function-type`](./docs/rules/prefer-function-type.md) | Use function types instead of interfaces with call signatures (`callable-types` from TSLint) | | :wrench: | +| [`@typescript-eslint/prefer-interface`](./docs/rules/prefer-interface.md) | Prefer an interface declaration over a type literal (type T = { ... }) (`interface-over-type-literal` from TSLint) | :heavy_check_mark: | :wrench: | +| [`@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. (`no-internal-module` from TSLint) | :heavy_check_mark: | :wrench: | +| [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async. (`promise-function-async` from TSLint) | :heavy_check_mark: | | +| [`@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. (`restrict-plus-operands` from TSLint) | | | +| [`@typescript-eslint/type-annotation-spacing`](./docs/rules/type-annotation-spacing.md) | Require consistent spacing around type annotations (`typedef-whitespace` from TSLint) | :heavy_check_mark: | :wrench: | + + diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index 20661b22ea0..213cda29f2d 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -158,8 +158,8 @@ It lists all TSLint rules along side rules from the ESLint ecosystem that are th | [`newline-before-return`] | 🌟 | [`padding-line-between-statements`][padding-line-between-statements] [1] | | [`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-angle-bracket-type-assertion`] | ✅ | [`@typescript-eslint/no-angle-bracket-type-assertion`] | +| [`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`] | 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..34e56f7c8d8 --- /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 (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/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..14b4c1ab9aa --- /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) { } + `, + }, + ], +});