fix(eslint-plugin): [no-unnecessary-boolean-literal-compare] fixer sh…

…ould handle parentheses (#6569) * fix(eslint-plugin): [no-unnecessary-boolean-literal-compare] fixer should handle parentheses * fix: simplify code and add missing handling for additional cases * fix: small code refactor
typescript-eslint · Mar 13, 2023 · 2d8c196 · 2d8c196
1 parent cb12682
commit 2d8c196
Show file tree

Hide file tree

Showing 3 changed files with 113 additions and 8 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
@@ -24,7 +24,6 @@ interface BooleanComparison {
   literalBooleanInComparison: boolean;
   forTruthy: boolean;
   negated: boolean;
-  range: [number, number];
 }
 
 interface BooleanComparisonWithTypeInformation extends BooleanComparison {
@@ -82,6 +81,7 @@ export default util.createRule<Options, MessageIds>({
   create(context, [options]) {
     const parserServices = util.getParserServices(context);
     const checker = parserServices.program.getTypeChecker();
+    const sourceCode = context.getSourceCode();
 
     function getBooleanComparison(
       node: TSESTree.BinaryExpression,
@@ -185,10 +185,6 @@ export default util.createRule<Options, MessageIds>({
           forTruthy: literalBooleanInComparison ? !negated : negated,
           expression,
           negated,
-          range:
-            expression.range[0] < against.range[0]
-              ? [expression.range[1], against.range[1]]
-              : [against.range[0], expression.range[0]],
         };
       }
 
@@ -227,7 +223,10 @@ export default util.createRule<Options, MessageIds>({
 
         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`
@@ -237,6 +236,10 @@ export default util.createRule<Options, MessageIds>({
             ) {
               if (!comparison.forTruthy) {
                 yield fixer.insertTextBefore(node, '!');
+                if (!util.isStrongPrecedenceNode(comparison.expression)) {
+                  yield fixer.insertTextBefore(node, '(');
+                  yield fixer.insertTextAfter(node, ')');
+                }
               }
               return;
             }

diff --git 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
@@ -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,107 @@ 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')) {
+        }
+      `,
+    },
+    {
+      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)) {
+        }
+      `,
+    },
   ],
 });