fix(eslint-plugin): [no-unnecessary-condition] improve checking optio…

…nal callee
typescript-eslint · Jan 6, 2024 · 3dd8119 · 3dd8119
1 parent b0d3d75
commit 3dd8119
Show file tree

Hide file tree

Showing 2 changed files with 160 additions and 3 deletions.
diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
@@ -547,7 +547,7 @@ export default createRule<Options, MessageId>({
     //  declare const foo: { bar : { baz: string } } | null
     //  foo?.bar;
     //  ```
-    function isNullableOriginFromPrev(
+    function isMemberExpressionNullableOriginFromObject(
       node: TSESTree.MemberExpression,
     ): boolean {
       const prevType = getConstrainedTypeAtLocation(services, node.object);
@@ -580,12 +580,35 @@ export default createRule<Options, MessageId>({
       return false;
     }
 
+    function isCallExpressionNullableOriginFromCallee(
+      node: TSESTree.CallExpression,
+    ): boolean {
+      const prevType = getConstrainedTypeAtLocation(services, node.callee);
+
+      if (prevType.isUnion()) {
+        const isOwnNullable = prevType.types.some(type => {
+          const signatures = type.getCallSignatures();
+          return signatures.some(sig =>
+            isNullableType(sig.getReturnType(), { allowUndefined: true }),
+          );
+        });
+        return (
+          !isOwnNullable && isNullableType(prevType, { allowUndefined: true })
+        );
+      }
+
+      return false;
+    }
+
     function isOptionableExpression(node: TSESTree.Expression): boolean {
       const type = getConstrainedTypeAtLocation(services, node);
       const isOwnNullable =
         node.type === AST_NODE_TYPES.MemberExpression
-          ? !isNullableOriginFromPrev(node)
-          : true;
+          ? !isMemberExpressionNullableOriginFromObject(node)
+          : node.type === AST_NODE_TYPES.CallExpression
+            ? !isCallExpressionNullableOriginFromCallee(node)
+            : true;
+
       const possiblyVoid = isTypeFlagSet(type, ts.TypeFlags.Void);
       return (
         isTypeFlagSet(type, ts.TypeFlags.Any | ts.TypeFlags.Unknown) ||

diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts
@@ -774,6 +774,28 @@ function getElem(dict: Record<string, { foo: string }>, key: string) {
         typescript: '4.1',
       },
     },
+    `
+type Foo = { bar: () => number | undefined } | null;
+declare const foo: Foo;
+foo?.bar()?.toExponential();
+    `,
+    `
+type Foo = (() => number | undefined) | null;
+declare const foo: Foo;
+foo?.()?.toExponential();
+    `,
+    `
+type FooUndef = () => undefined;
+type FooNum = () => number;
+type Foo = FooUndef | FooNum | null;
+declare const foo: Foo;
+foo?.()?.toExponential();
+    `,
+    `
+type Foo = { [key: string]: () => number | undefined } | null;
+declare const foo: Foo;
+foo?.['bar']()?.toExponential();
+    `,
   ],
   invalid: [
     // Ensure that it's checking in all the right places
@@ -1993,6 +2015,118 @@ foo &&= null;
         },
       ],
     },
+    {
+      code: noFormat`
+type Foo = { bar: () => number } | null;
+declare const foo: Foo;
+foo?.bar()?.toExponential();
+      `,
+      output: noFormat`
+type Foo = { bar: () => number } | null;
+declare const foo: Foo;
+foo?.bar().toExponential();
+      `,
+      errors: [
+        {
+          messageId: 'neverOptionalChain',
+          line: 4,
+          column: 11,
+          endLine: 4,
+          endColumn: 13,
+        },
+      ],
+    },
+    {
+      code: noFormat`
+type Foo = { bar: null | { baz: () => { qux: number } } } | null;
+declare const foo: Foo;
+foo?.bar?.baz()?.qux?.toExponential();
+      `,
+      output: noFormat`
+type Foo = { bar: null | { baz: () => { qux: number } } } | null;
+declare const foo: Foo;
+foo?.bar?.baz().qux.toExponential();
+      `,
+      errors: [
+        {
+          messageId: 'neverOptionalChain',
+          line: 4,
+          column: 16,
+          endLine: 4,
+          endColumn: 18,
+        },
+        {
+          messageId: 'neverOptionalChain',
+          line: 4,
+          column: 21,
+          endLine: 4,
+          endColumn: 23,
+        },
+      ],
+    },
+    {
+      code: noFormat`
+type Foo = (() => number) | null;
+declare const foo: Foo;
+foo?.()?.toExponential();
+      `,
+      output: noFormat`
+type Foo = (() => number) | null;
+declare const foo: Foo;
+foo?.().toExponential();
+      `,
+      errors: [
+        {
+          messageId: 'neverOptionalChain',
+          line: 4,
+          column: 8,
+          endLine: 4,
+          endColumn: 10,
+        },
+      ],
+    },
+    {
+      code: noFormat`
+type Foo = { [key: string]: () => number } | null;
+declare const foo: Foo;
+foo?.['bar']()?.toExponential();
+      `,
+      output: noFormat`
+type Foo = { [key: string]: () => number } | null;
+declare const foo: Foo;
+foo?.['bar']().toExponential();
+      `,
+      errors: [
+        {
+          messageId: 'neverOptionalChain',
+          line: 4,
+          column: 15,
+          endLine: 4,
+          endColumn: 17,
+        },
+      ],
+    },
+    {
+      code: noFormat`
+type Foo = { [key: string]: () => number } | null;
+declare const foo: Foo;
+foo?.['bar']?.()?.toExponential();
+      `,
+      output: noFormat`
+type Foo = { [key: string]: () => number } | null;
+declare const foo: Foo;
+foo?.['bar']?.().toExponential();
+      `,
+      errors: [
+        {
+          messageId: 'neverOptionalChain',
+          line: 4,
+          column: 17,
+          endLine: 4,
+          endColumn: 19,
+        },
+      ],
+    },
 
     // "branded" types
     unnecessaryConditionTest('"" & {}', 'alwaysFalsy'),