fix(eslint-plugin): [no-unnecessary-condition] correct regression wit…

…h unary negations (#2422) Fixes #2421
typescript-eslint · Aug 25, 2020 · d1f0887 · d1f0887
1 parent 797a133
commit d1f0887
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 66 deletions.
diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
@@ -165,13 +165,16 @@ export default createRule<Options, MessageId>({
      * Checks if a conditional node is necessary:
      * if the type of the node is always true or always false, it's not necessary.
      */
-    function checkNode(node: TSESTree.Expression): void {
+    function checkNode(
+      node: TSESTree.Expression,
+      isUnaryNotArgument = false,
+    ): void {
       // Check if the node is Unary Negation expression and handle it
       if (
         node.type === AST_NODE_TYPES.UnaryExpression &&
         node.operator === '!'
       ) {
-        return checkIfUnaryNegationExpressionIsNecessaryConditional(node);
+        return checkNode(node.argument, true);
       }
 
       // Since typescript array index signature types don't represent the
@@ -208,41 +211,19 @@ export default createRule<Options, MessageId>({
       ) {
         return;
       }
-      const messageId = isTypeFlagSet(type, ts.TypeFlags.Never)
-        ? 'never'
-        : !isPossiblyTruthy(type)
-        ? 'alwaysFalsy'
-        : !isPossiblyFalsy(type)
-        ? 'alwaysTruthy'
-        : undefined;
-
-      if (messageId) {
-        context.report({ node, messageId });
+      let messageId: MessageId | null = null;
+
+      if (isTypeFlagSet(type, ts.TypeFlags.Never)) {
+        messageId = 'never';
+      } else if (!isPossiblyTruthy(type)) {
+        messageId = !isUnaryNotArgument ? 'alwaysFalsy' : 'alwaysTruthy';
+      } else if (!isPossiblyFalsy(type)) {
+        messageId = !isUnaryNotArgument ? 'alwaysTruthy' : 'alwaysFalsy';
       }
-    }
-
-    /**
-     * If it is Unary Negation Expression, check the argument for alwaysTruthy / alwaysFalsy
-     */
-    function checkIfUnaryNegationExpressionIsNecessaryConditional(
-      node: TSESTree.UnaryExpression,
-    ): void {
-      if (isArrayIndexExpression(node.argument)) {
-        return;
-      }
-      const type = getNodeType(node.argument);
-      const messageId = isTypeFlagSet(type, ts.TypeFlags.Never)
-        ? 'never'
-        : isPossiblyTruthy(type)
-        ? 'alwaysFalsy'
-        : isPossiblyFalsy(type)
-        ? 'alwaysTruthy'
-        : undefined;
 
       if (messageId) {
         context.report({ node, messageId });
       }
-      return;
     }
 
     function checkNodeForNullish(node: TSESTree.Expression): void {
@@ -257,13 +238,15 @@ export default createRule<Options, MessageId>({
       if (isTypeAnyType(type) || isTypeUnknownType(type)) {
         return;
       }
-      const messageId = isTypeFlagSet(type, ts.TypeFlags.Never)
-        ? 'never'
-        : !isPossiblyNullish(type)
-        ? 'neverNullish'
-        : isAlwaysNullish(type)
-        ? 'alwaysNullish'
-        : undefined;
+
+      let messageId: MessageId | null = null;
+      if (isTypeFlagSet(type, ts.TypeFlags.Never)) {
+        messageId = 'never';
+      } else if (!isPossiblyNullish(type)) {
+        messageId = 'neverNullish';
+      } else if (isAlwaysNullish(type)) {
+        messageId = 'alwaysNullish';
+      }
 
       if (messageId) {
         context.report({ node, messageId });

diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts
@@ -472,37 +472,27 @@ function test(testVal?: boolean) {
   }
 }
     `,
-  ],
-  invalid: [
-    // Ensure that it's checking in all the right places
-    {
-      code: `
-const a = null;
-if (!a) {
-}
-      `,
-      errors: [ruleError(3, 5, 'alwaysTruthy')],
-    },
-    {
-      code: `
-const a = true;
-if (!a) {
+    `
+declare const x: string[];
+if (!x[0]) {
 }
-      `,
-      errors: [ruleError(3, 5, 'alwaysFalsy')],
-    },
-    {
-      code: `
-function sayHi(): void {
-  console.log('Hi!');
+    `,
+    // https://github.com/typescript-eslint/typescript-eslint/issues/2421
+    `
+const isEven = (val: number) => val % 2 === 0;
+if (!isEven(1)) {
 }
+    `,
+    `
+declare const booleanTyped: boolean;
+declare const unknownTyped: unknown;
 
-let speech: never = sayHi();
-if (!speech) {
+if (!(booleanTyped || unknownTyped)) {
 }
-      `,
-      errors: [ruleError(7, 5, 'never')],
-    },
+    `,
+  ],
+  invalid: [
+    // Ensure that it's checking in all the right places
     {
       code: `
 const b1 = true;
@@ -1442,5 +1432,34 @@ function test(testVal?: true) {
         },
       ],
     },
+    // https://github.com/typescript-eslint/typescript-eslint/issues/2255
+    {
+      code: `
+const a = null;
+if (!a) {
+}
+      `,
+      errors: [ruleError(3, 6, 'alwaysTruthy')],
+    },
+    {
+      code: `
+const a = true;
+if (!a) {
+}
+      `,
+      errors: [ruleError(3, 6, 'alwaysFalsy')],
+    },
+    {
+      code: `
+function sayHi(): void {
+  console.log('Hi!');
+}
+
+let speech: never = sayHi();
+if (!speech) {
+}
+      `,
+      errors: [ruleError(7, 6, 'never')],
+    },
   ],
 });