fix(eslint-plugin): [no-unnecessary-condition] don't flag values of a…

…n unconstrained or valid type parameter (#10473) * [no-unnecessary-condition] don't flag values of an unconstrained or valid type parameter * use a much simpelr for loop instead of several confusing .some() checks * bail early if both a possibly-truthy and a possibly-falsy have been detected * put back code that checks non-callables
typescript-eslint · Dec 21, 2024 · 8ca9cba · 8ca9cba
1 parent ba39dde
commit 8ca9cba
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 8 deletions.
diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
@@ -649,22 +649,53 @@ export default createRule<Options, MessageId>({
           .getCallSignaturesOfType(
             getConstrainedTypeAtLocation(services, callback),
           )
-          .map(sig => sig.getReturnType());
-        /* istanbul ignore if */ if (returnTypes.length === 0) {
-          // Not a callable function
+          .map(sig => sig.getReturnType())
+          .map(t => {
+            // TODO: use `getConstraintTypeInfoAtLocation` once it's merged
+            // https://github.com/typescript-eslint/typescript-eslint/pull/10496
+            if (tsutils.isTypeParameter(t)) {
+              return checker.getBaseConstraintOfType(t);
+            }
+
+            return t;
+          });
+
+        if (returnTypes.length === 0) {
+          // Not a callable function, e.g. `any`
           return;
         }
-        // Predicate is always necessary if it involves `any` or `unknown`
-        if (returnTypes.some(t => isTypeAnyType(t) || isTypeUnknownType(t))) {
-          return;
+
+        let hasFalsyReturnTypes = false;
+        let hasTruthyReturnTypes = false;
+
+        for (const type of returnTypes) {
+          // Predicate is always necessary if it involves `any` or `unknown`
+          if (!type || isTypeAnyType(type) || isTypeUnknownType(type)) {
+            return;
+          }
+
+          if (isPossiblyFalsy(type)) {
+            hasFalsyReturnTypes = true;
+          }
+
+          if (isPossiblyTruthy(type)) {
+            hasTruthyReturnTypes = true;
+          }
+
+          // bail early if both a possibly-truthy and a possibly-falsy have been detected
+          if (hasFalsyReturnTypes && hasTruthyReturnTypes) {
+            return;
+          }
         }
-        if (!returnTypes.some(isPossiblyFalsy)) {
+
+        if (!hasFalsyReturnTypes) {
           return context.report({
             node: callback,
             messageId: 'alwaysTruthyFunc',
           });
         }
-        if (!returnTypes.some(isPossiblyTruthy)) {
+
+        if (!hasTruthyReturnTypes) {
           return context.report({
             node: callback,
             messageId: 'alwaysFalsyFunc',

diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts
@@ -286,6 +286,22 @@ function count(
 ) {
   return list.filter(predicate).length;
 }
+    `,
+    `
+declare const test: <T>() => T;
+
+[1, null].filter(test);
+    `,
+    `
+declare const test: <T extends boolean>() => T;
+
+[1, null].filter(test);
+    `,
+    `
+[1, null].filter(1 as any);
+    `,
+    `
+[1, null].filter(1 as never);
     `,
     // Ignores non-array methods of the same name
     `
@@ -1745,6 +1761,14 @@ function nothing3(x: [string, string]) {
         { column: 25, line: 17, messageId: 'alwaysFalsy' },
       ],
     },
+    {
+      code: `
+declare const test: <T extends true>() => T;
+
+[1, null].filter(test);
+      `,
+      errors: [{ column: 18, line: 4, messageId: 'alwaysTruthyFunc' }],
+    },
     // Indexing cases
     {
       // This is an error because 'dict' doesn't represent