fix(eslint-plugin): [no-unnecessary-condition] fix false positive whe…

…n checking indexed access types (#6452) * add new tests * allow naked index access type to be a condition
typescript-eslint · Feb 13, 2023 · d569924 · d569924
1 parent a1b3f7b
commit d569924
Show file tree

Hide file tree

Showing 2 changed files with 77 additions and 2 deletions.
diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
@@ -235,13 +235,13 @@ export default createRule<Options, MessageId>({
       const type = getNodeType(node);
 
       // Conditional is always necessary if it involves:
-      //    `any` or `unknown` or a naked type parameter
+      //    `any` or `unknown` or a naked type variable
       if (
         unionTypeParts(type).some(
           part =>
             isTypeAnyType(part) ||
             isTypeUnknownType(part) ||
-            isTypeFlagSet(part, ts.TypeFlags.TypeParameter),
+            isTypeFlagSet(part, ts.TypeFlags.TypeVariable),
         )
       ) {
         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
@@ -549,6 +549,36 @@ type OptionalFoo = Foo | undefined;
 declare const foo: OptionalFoo;
 foo?.[1]?.length;
     `,
+    // https://github.com/typescript-eslint/typescript-eslint/issues/6264
+    `
+function get<Obj, Key extends keyof Obj>(obj: Obj, key: Key) {
+  const value = obj[key];
+  if (value) {
+    return value;
+  }
+  throw new Error('BOOM!');
+}
+
+get({ foo: null }, 'foo');
+    `,
+    {
+      code: `
+function getElem(dict: Record<string, { foo: string }>, key: string) {
+  if (dict[key]) {
+    return dict[key].foo;
+  } else {
+    return '';
+  }
+}
+      `,
+      parserOptions: {
+        tsconfigRootDir: getFixturesRootDir(),
+        project: './tsconfig.noUncheckedIndexedAccess.json',
+      },
+      dependencyConstraints: {
+        typescript: '4.1',
+      },
+    },
   ],
   invalid: [
     // Ensure that it's checking in all the right places
@@ -1601,5 +1631,50 @@ foo?.test.length;
         },
       ],
     },
+    {
+      code: `
+function pick<Obj extends Record<string, 1 | 2 | 3>, Key extends keyof Obj>(
+  obj: Obj,
+  key: Key,
+): Obj[Key] {
+  const k = obj[key];
+  if (obj[key]) {
+    return obj[key];
+  }
+  throw new Error('Boom!');
+}
+
+pick({ foo: 1, bar: 2 }, 'bar');
+      `,
+      errors: [
+        {
+          messageId: 'alwaysTruthy',
+          line: 7,
+          endLine: 7,
+          column: 7,
+          endColumn: 15,
+        },
+      ],
+    },
+    {
+      code: `
+function getElem(dict: Record<string, { foo: string }>, key: string) {
+  if (dict[key]) {
+    return dict[key].foo;
+  } else {
+    return '';
+  }
+}
+      `,
+      errors: [
+        {
+          messageId: 'alwaysTruthy',
+          line: 3,
+          endLine: 3,
+          column: 7,
+          endColumn: 16,
+        },
+      ],
+    },
   ],
 });