fix(eslint-plugin): false-positive/negative with array index in no-un…

…necessary-condition (#3805) * fix: false negative with always nullish after array index access * fix: false positive with nullish + optional array index * fix: false negative with optional chain after array index
typescript-eslint · Sep 20, 2021 · bdb8f0b · bdb8f0b
1 parent 4e99961
commit bdb8f0b
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 11 deletions.
diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
@@ -261,12 +261,6 @@ export default createRule<Options, MessageId>({
     }
 
     function checkNodeForNullish(node: TSESTree.Expression): void {
-      // Since typescript array index signature types don't represent the
-      //  possibility of out-of-bounds access, if we're indexing into an array
-      //  just skip the check, to avoid false positives
-      if (isArrayIndexExpression(node)) {
-        return;
-      }
       const type = getNodeType(node);
       // Conditional is always necessary if it involves `any` or `unknown`
       if (isTypeAnyType(type) || isTypeUnknownType(type)) {
@@ -277,7 +271,19 @@ export default createRule<Options, MessageId>({
       if (isTypeFlagSet(type, ts.TypeFlags.Never)) {
         messageId = 'never';
       } else if (!isPossiblyNullish(type)) {
-        messageId = 'neverNullish';
+        // Since typescript array index signature types don't represent the
+        //  possibility of out-of-bounds access, if we're indexing into an array
+        //  just skip the check, to avoid false positives
+        if (
+          !isArrayIndexExpression(node) &&
+          !(
+            node.type === AST_NODE_TYPES.ChainExpression &&
+            node.expression.type !== AST_NODE_TYPES.TSNonNullExpression &&
+            optionChainContainsOptionArrayIndex(node.expression)
+          )
+        ) {
+          messageId = 'neverNullish';
+        }
       } else if (isAlwaysNullish(type)) {
         messageId = 'alwaysNullish';
       }
@@ -477,19 +483,19 @@ export default createRule<Options, MessageId>({
     //    ?.x // type is {y: "z"}
     //    ?.y // This access is considered "unnecessary" according to the types
     //  ```
-    function optionChainContainsArrayIndex(
+    function optionChainContainsOptionArrayIndex(
       node: TSESTree.MemberExpression | TSESTree.CallExpression,
     ): boolean {
       const lhsNode =
         node.type === AST_NODE_TYPES.CallExpression ? node.callee : node.object;
-      if (isArrayIndexExpression(lhsNode)) {
+      if (node.optional && isArrayIndexExpression(lhsNode)) {
         return true;
       }
       if (
         lhsNode.type === AST_NODE_TYPES.MemberExpression ||
         lhsNode.type === AST_NODE_TYPES.CallExpression
       ) {
-        return optionChainContainsArrayIndex(lhsNode);
+        return optionChainContainsOptionArrayIndex(lhsNode);
       }
       return false;
     }
@@ -584,7 +590,7 @@ export default createRule<Options, MessageId>({
       // Since typescript array index signature types don't represent the
       //  possibility of out-of-bounds access, if we're indexing into an array
       //  just skip the check, to avoid false positives
-      if (optionChainContainsArrayIndex(node)) {
+      if (optionChainContainsOptionArrayIndex(node)) {
         return;
       }
 

diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts
@@ -315,6 +315,11 @@ returnsArr?.()[42]?.toUpperCase();
     `
 declare const arr: string[][];
 arr[x] ?? [];
+    `,
+    // nullish + optional array index
+    `
+declare const arr: { foo: number }[];
+const bar = arr[42]?.foo ?? 0;
     `,
     // Doesn't check the right-hand side of a logical expression
     //  in a non-conditional context
@@ -751,6 +756,15 @@ function test(a: string) {
       code: `
 function test(a: string | false) {
   return a ?? 'default';
+}
+      `,
+      errors: [ruleError(3, 10, 'neverNullish')],
+    },
+    // nullish + array index without optional chaining
+    {
+      code: `
+function test(a: { foo: string }[]) {
+  return a[0].foo ?? 'default';
 }
       `,
       errors: [ruleError(3, 10, 'neverNullish')],
@@ -765,6 +779,14 @@ function test(a: null) {
     },
     {
       code: `
+function test(a: null[]) {
+  return a[0] ?? 'default';
+}
+      `,
+      errors: [ruleError(3, 10, 'alwaysNullish')],
+    },
+    {
+      code: `
 function test(a: never) {
   return a ?? 'default';
 }
@@ -1315,6 +1337,17 @@ foo?.fooOrBar.baz?.qux;
     },
     {
       code: `
+declare const x: { a: { b: number } }[];
+x[0].a?.b;
+      `,
+      output: `
+declare const x: { a: { b: number } }[];
+x[0].a.b;
+      `,
+      errors: [ruleError(3, 7, 'neverOptionalChain')],
+    },
+    {
+      code: `
 type Foo = { [key: string]: string; foo: 'foo'; bar: 'bar' } | null;
 type Key = 'bar' | 'foo';
 declare const foo: Foo;