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

… fixer and add support for double negation (#6620)
typescript-eslint · Mar 21, 2023 · 81c8519 · 81c8519
1 parent 3b1e9db
commit 81c8519
Show file tree

Hide file tree

Showing 2 changed files with 148 additions and 36 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
@@ -22,7 +22,6 @@ type Options = [
 interface BooleanComparison {
   expression: TSESTree.Expression | TSESTree.PrivateIdentifier;
   literalBooleanInComparison: boolean;
-  forTruthy: boolean;
   negated: boolean;
 }
 
@@ -182,7 +181,6 @@ export default util.createRule<Options, MessageIds>({
 
         return {
           literalBooleanInComparison,
-          forTruthy: literalBooleanInComparison ? !negated : negated,
           expression,
           negated,
         };
@@ -223,48 +221,43 @@ export default util.createRule<Options, MessageIds>({
 
         context.report({
           fix: function* (fixer) {
+            // 1. isUnaryNegation - parent negation
+            // 2. literalBooleanInComparison - is compared to literal boolean
+            // 3. negated - is expression negated
+
+            const isUnaryNegation =
+              node.parent != null && nodeIsUnaryNegation(node.parent);
+
+            const shouldNegate =
+              comparison.negated !== comparison.literalBooleanInComparison;
+
+            const mutatedNode = isUnaryNegation ? node.parent! : node;
+
             yield fixer.replaceText(
-              node,
+              mutatedNode,
               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`
-            if (
-              !comparison.expressionIsNullableBoolean ||
-              comparison.literalBooleanInComparison
-            ) {
-              if (!comparison.forTruthy) {
-                yield fixer.insertTextBefore(node, '!');
-                if (!util.isStrongPrecedenceNode(comparison.expression)) {
-                  yield fixer.insertTextBefore(node, '(');
-                  yield fixer.insertTextAfter(node, ')');
-                }
-              }
-              return;
-            }
+            // if `isUnaryNegation === literalBooleanInComparison === !negated` is true - negate the expression
+            if (shouldNegate === isUnaryNegation) {
+              yield fixer.insertTextBefore(mutatedNode, '!');
 
-            // if we're here, then the expression is a nullable boolean and we're
-            // comparing to a literal `false`
-
-            // if we're doing `== false` or `=== false`, then we need to negate the expression
-            if (!comparison.negated) {
-              const { parent } = node;
-              // if the parent is a negation, we can instead just get rid of the parent's negation.
-              // i.e. instead of resulting in `!(!(exp))`, we can just result in `exp`
-              if (parent != null && nodeIsUnaryNegation(parent)) {
-                // remove from the beginning of the parent to the beginning of this node
-                yield fixer.removeRange([parent.range[0], node.range[0]]);
-                // remove from the end of the node to the end of the parent
-                yield fixer.removeRange([node.range[1], parent.range[1]]);
-              } else {
-                yield fixer.insertTextBefore(node, '!');
+              // if the expression `exp` is not a strong precedence node, wrap it in parentheses
+              if (!util.isStrongPrecedenceNode(comparison.expression)) {
+                yield fixer.insertTextBefore(mutatedNode, '(');
+                yield fixer.insertTextAfter(mutatedNode, ')');
               }
             }
 
-            // provide the default `true`
-            yield fixer.insertTextBefore(node, '(');
-            yield fixer.insertTextAfter(node, ' ?? true)');
+            // if the expression `exp` is nullable, and we're not comparing to `true`, insert `?? true`
+            if (
+              comparison.expressionIsNullableBoolean &&
+              !comparison.literalBooleanInComparison
+            ) {
+              // provide the default `true`
+              yield fixer.insertTextBefore(mutatedNode, '(');
+              yield fixer.insertTextAfter(mutatedNode, ' ?? true)');
+            }
           },
           messageId: comparison.expressionIsNullableBoolean
             ? comparison.literalBooleanInComparison

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
@@ -360,5 +360,124 @@ ruleTester.run('no-unnecessary-boolean-literal-compare', rule, {
         }
       `,
     },
+    {
+      code: `
+        declare const varBoolean: boolean;
+        if (!(varBoolean !== false)) {
+        }
+      `,
+      errors: [
+        {
+          messageId: 'negated',
+        },
+      ],
+      output: `
+        declare const varBoolean: boolean;
+        if (!varBoolean) {
+        }
+      `,
+    },
+    {
+      code: `
+        declare const varBoolean: boolean;
+        if (!(varBoolean === false)) {
+        }
+      `,
+      errors: [
+        {
+          messageId: 'direct',
+        },
+      ],
+      output: `
+        declare const varBoolean: boolean;
+        if (varBoolean) {
+        }
+      `,
+    },
+    {
+      code: `
+        declare const varBoolean: boolean;
+        if (!(varBoolean instanceof Event == false)) {
+        }
+      `,
+      errors: [
+        {
+          messageId: 'direct',
+        },
+      ],
+      output: `
+        declare const varBoolean: boolean;
+        if (varBoolean instanceof Event) {
+        }
+      `,
+    },
+    {
+      code: `
+        declare const varBoolean: boolean;
+        if (varBoolean instanceof Event == false) {
+        }
+      `,
+      errors: [
+        {
+          messageId: 'direct',
+        },
+      ],
+      output: `
+        declare const varBoolean: boolean;
+        if (!(varBoolean instanceof Event)) {
+        }
+      `,
+    },
+    {
+      code: `
+        declare const varBoolean: boolean;
+        if (!((varBoolean ?? false) !== false)) {
+        }
+      `,
+      errors: [
+        {
+          messageId: 'negated',
+        },
+      ],
+      output: `
+        declare const varBoolean: boolean;
+        if (!(varBoolean ?? false)) {
+        }
+      `,
+    },
+    {
+      code: `
+        declare const varBoolean: boolean;
+        if (!((varBoolean ?? false) === false)) {
+        }
+      `,
+      errors: [
+        {
+          messageId: 'direct',
+        },
+      ],
+      output: `
+        declare const varBoolean: boolean;
+        if (varBoolean ?? false) {
+        }
+      `,
+    },
+    {
+      code: `
+        declare const varBoolean: boolean;
+        if (!((varBoolean ?? true) !== false)) {
+        }
+      `,
+      errors: [
+        {
+          messageId: 'negated',
+        },
+      ],
+      output: `
+        declare const varBoolean: boolean;
+        if (!(varBoolean ?? true)) {
+        }
+      `,
+    },
   ],
 });