From bdf04c3cf0af18f8d0a1f98b155ef89d6ce18398 Mon Sep 17 00:00:00 2001 From: Armano Date: Sat, 4 Mar 2023 19:52:45 +0100 Subject: [PATCH 1/3] fix(eslint-plugin): [no-unnecessary-boolean-literal-compare] fixer should handle parentheses --- .../no-unnecessary-boolean-literal-compare.ts | 32 +++++++-- .../src/util/getWrappingFixer.ts | 2 +- ...nnecessary-boolean-literal-compare.test.ts | 70 ++++++++++++++++++- 3 files changed, 98 insertions(+), 6 deletions(-) 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 index f874905b738..97c94bab7c7 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts @@ -1,5 +1,9 @@ import type { TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { + AST_NODE_TYPES, + AST_TOKEN_TYPES, + ASTUtils, +} from '@typescript-eslint/utils'; import * as tsutils from 'tsutils'; import * as ts from 'typescript'; @@ -82,6 +86,7 @@ export default util.createRule({ create(context, [options]) { const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); + const sourceCode = context.getSourceCode(); function getBooleanComparison( node: TSESTree.BinaryExpression, @@ -180,6 +185,17 @@ export default util.createRule({ const { value: literalBooleanInComparison } = against; const negated = !comparisonType.isPositive; + const nodes = + expression.range[0] < against.range[0] + ? [expression, against] + : [against, expression]; + + const tokens = sourceCode.getFirstTokenBetween(nodes[0], nodes[1], { + filter: token => + token.type === AST_TOKEN_TYPES.Punctuator && + (token.value === '(' || token.value === ')'), + }); + return { literalBooleanInComparison, forTruthy: literalBooleanInComparison ? !negated : negated, @@ -187,8 +203,8 @@ export default util.createRule({ negated, range: expression.range[0] < against.range[0] - ? [expression.range[1], against.range[1]] - : [against.range[0], expression.range[0]], + ? [tokens?.range[1] ?? expression.range[1], against.range[1]] + : [against.range[0], tokens?.range[0] ?? expression.range[0]], }; } @@ -236,7 +252,15 @@ export default util.createRule({ comparison.literalBooleanInComparison ) { if (!comparison.forTruthy) { - yield fixer.insertTextBefore(node, '!'); + if ( + !util.isStrongPrecedenceNode(comparison.expression) && + !ASTUtils.isParenthesized(comparison.expression, sourceCode) + ) { + yield fixer.insertTextBefore(node, '!('); + yield fixer.insertTextAfter(node, ')'); + } else { + yield fixer.insertTextBefore(node, '!'); + } } return; } diff --git a/packages/eslint-plugin/src/util/getWrappingFixer.ts b/packages/eslint-plugin/src/util/getWrappingFixer.ts index 0f867033b04..73203cf8cdf 100644 --- a/packages/eslint-plugin/src/util/getWrappingFixer.ts +++ b/packages/eslint-plugin/src/util/getWrappingFixer.ts @@ -69,7 +69,7 @@ export function getWrappingFixer( /** * Check if a node will always have the same precedence if it's parent changes. */ -function isStrongPrecedenceNode(innerNode: TSESTree.Node): boolean { +export function isStrongPrecedenceNode(innerNode: TSESTree.Node): boolean { return ( innerNode.type === AST_NODE_TYPES.Literal || innerNode.type === AST_NODE_TYPES.Identifier || 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 index a519e5f11a0..ce0d8e82c60 100644 --- 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 @@ -1,5 +1,5 @@ import rule from '../../src/rules/no-unnecessary-boolean-literal-compare'; -import { getFixturesRootDir, RuleTester } from '../RuleTester'; +import { getFixturesRootDir, noFormat, RuleTester } from '../RuleTester'; const rootDir = getFixturesRootDir(); const ruleTester = new RuleTester({ @@ -258,5 +258,73 @@ ruleTester.run('no-unnecessary-boolean-literal-compare', rule, { } `, }, + { + code: noFormat` + declare const x; + if ((x instanceof Error) === false) { + } + `, + errors: [ + { + messageId: 'direct', + }, + ], + output: ` + declare const x; + if (!(x instanceof Error)) { + } + `, + }, + { + code: noFormat` + declare const x; + if (false === (x instanceof Error)) { + } + `, + errors: [ + { + messageId: 'direct', + }, + ], + output: ` + declare const x; + if (!(x instanceof Error)) { + } + `, + }, + { + code: ` + declare const x; + if (x instanceof Error === false) { + } + `, + errors: [ + { + messageId: 'direct', + }, + ], + output: ` + declare const x; + if (!(x instanceof Error)) { + } + `, + }, + { + code: noFormat` + declare const x; + if (typeof x === 'string' === false) { + } + `, + errors: [ + { + messageId: 'direct', + }, + ], + output: ` + declare const x; + if (!(typeof x === 'string')) { + } + `, + }, ], }); From 768cc365846fb1aeb5d60d89d5b4bac3c0481a1d Mon Sep 17 00:00:00 2001 From: Armano Date: Sat, 4 Mar 2023 20:43:59 +0100 Subject: [PATCH 2/3] fix: simplify code and add missing handling for additional cases --- .../no-unnecessary-boolean-literal-compare.ts | 32 ++++------------- ...nnecessary-boolean-literal-compare.test.ts | 34 +++++++++++++++++++ 2 files changed, 40 insertions(+), 26 deletions(-) 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 index 97c94bab7c7..c3a3ca036d3 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts @@ -1,9 +1,5 @@ import type { TSESTree } from '@typescript-eslint/utils'; -import { - AST_NODE_TYPES, - AST_TOKEN_TYPES, - ASTUtils, -} from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as tsutils from 'tsutils'; import * as ts from 'typescript'; @@ -28,7 +24,6 @@ interface BooleanComparison { literalBooleanInComparison: boolean; forTruthy: boolean; negated: boolean; - range: [number, number]; } interface BooleanComparisonWithTypeInformation extends BooleanComparison { @@ -185,26 +180,11 @@ export default util.createRule({ const { value: literalBooleanInComparison } = against; const negated = !comparisonType.isPositive; - const nodes = - expression.range[0] < against.range[0] - ? [expression, against] - : [against, expression]; - - const tokens = sourceCode.getFirstTokenBetween(nodes[0], nodes[1], { - filter: token => - token.type === AST_TOKEN_TYPES.Punctuator && - (token.value === '(' || token.value === ')'), - }); - return { literalBooleanInComparison, forTruthy: literalBooleanInComparison ? !negated : negated, expression, negated, - range: - expression.range[0] < against.range[0] - ? [tokens?.range[1] ?? expression.range[1], against.range[1]] - : [against.range[0], tokens?.range[0] ?? expression.range[0]], }; } @@ -243,7 +223,10 @@ export default util.createRule({ context.report({ fix: function* (fixer) { - yield fixer.removeRange(comparison.range); + yield fixer.replaceText( + node, + sourceCode.getText(comparison.expression), + ); // if the expression `exp` isn't nullable, or we're comparing to `true`, // we can just replace the entire comparison with `exp` or `!exp` @@ -252,10 +235,7 @@ export default util.createRule({ comparison.literalBooleanInComparison ) { if (!comparison.forTruthy) { - if ( - !util.isStrongPrecedenceNode(comparison.expression) && - !ASTUtils.isParenthesized(comparison.expression, sourceCode) - ) { + if (!util.isStrongPrecedenceNode(comparison.expression)) { yield fixer.insertTextBefore(node, '!('); yield fixer.insertTextAfter(node, ')'); } else { 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 index ce0d8e82c60..24dce35c799 100644 --- 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 @@ -326,5 +326,39 @@ ruleTester.run('no-unnecessary-boolean-literal-compare', rule, { } `, }, + { + code: noFormat` + declare const x; + if (x instanceof Error === (false)) { + } + `, + errors: [ + { + messageId: 'direct', + }, + ], + output: ` + declare const x; + if (!(x instanceof Error)) { + } + `, + }, + { + code: noFormat` + declare const x; + if ((false) === x instanceof Error) { + } + `, + errors: [ + { + messageId: 'direct', + }, + ], + output: ` + declare const x; + if (!(x instanceof Error)) { + } + `, + }, ], }); From 79de5c29678a0f287f9508df557cfdfe04adab97 Mon Sep 17 00:00:00 2001 From: Armano Date: Sat, 4 Mar 2023 20:46:27 +0100 Subject: [PATCH 3/3] fix: small code refactor --- .../src/rules/no-unnecessary-boolean-literal-compare.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 index c3a3ca036d3..f14b44fec27 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-boolean-literal-compare.ts @@ -235,11 +235,10 @@ export default util.createRule({ comparison.literalBooleanInComparison ) { if (!comparison.forTruthy) { + yield fixer.insertTextBefore(node, '!'); if (!util.isStrongPrecedenceNode(comparison.expression)) { - yield fixer.insertTextBefore(node, '!('); + yield fixer.insertTextBefore(node, '('); yield fixer.insertTextAfter(node, ')'); - } else { - yield fixer.insertTextBefore(node, '!'); } } return;