From 40a0961003f15e7633915d054756660fff3b8d91 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 7 Dec 2024 20:11:57 +0200 Subject: [PATCH 1/4] [no-unnecessary-condition] don't flag values of an unconstrained or valid type parameter --- .../src/rules/no-unnecessary-condition.ts | 18 ++++++++++++++---- .../rules/no-unnecessary-condition.test.ts | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index c4bed147eb88..60a9619a8661 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -646,22 +646,32 @@ export default createRule({ .getCallSignaturesOfType( getConstrainedTypeAtLocation(services, callback), ) - .map(sig => sig.getReturnType()); + .map(sig => sig.getReturnType()) + .map(t => { + if (tsutils.isTypeParameter(t)) { + return checker.getBaseConstraintOfType(t); + } + + return t; + }); /* istanbul ignore if */ if (returnTypes.length === 0) { // Not a callable function return; } // Predicate is always necessary if it involves `any` or `unknown` - if (returnTypes.some(t => isTypeAnyType(t) || isTypeUnknownType(t))) { + if ( + // https://github.com/typescript-eslint/typescript-eslint/issues/10438 + returnTypes.some(t => !t || isTypeAnyType(t) || isTypeUnknownType(t)) + ) { return; } - if (!returnTypes.some(isPossiblyFalsy)) { + if (!returnTypes.some(t => t && isPossiblyFalsy(t))) { return context.report({ node: callback, messageId: 'alwaysTruthyFunc', }); } - if (!returnTypes.some(isPossiblyTruthy)) { + if (!returnTypes.some(t => t && isPossiblyTruthy(t))) { 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 index 7aaed5539e21..8b2ad2ea50f2 100644 --- 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,16 @@ function count( ) { return list.filter(predicate).length; } + `, + ` +declare const test: () => T; + +[1, null].filter(test); + `, + ` +declare const test: () => T; + +[1, null].filter(test); `, // Ignores non-array methods of the same name ` @@ -1598,6 +1608,14 @@ function nothing3(x: [string, string]) { { column: 25, line: 17, messageId: 'alwaysFalsy' }, ], }, + { + code: ` +declare const test: () => T; + +[1, null].filter(test); + `, + errors: [{ column: 18, line: 4, messageId: 'alwaysTruthyFunc' }], + }, // Indexing cases { // This is an error because 'dict' doesn't represent From 991ff23818e893f19c3b1c7f11a786cce3657f0d Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Sat, 14 Dec 2024 21:35:39 +0200 Subject: [PATCH 2/4] use a much simpelr for loop instead of several confusing .some() checks --- .../src/rules/no-unnecessary-condition.ts | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index 60a9619a8661..d9c16491f6c2 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -648,30 +648,41 @@ export default createRule({ ) .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; }); - /* istanbul ignore if */ if (returnTypes.length === 0) { - // Not a callable function - return; - } - // Predicate is always necessary if it involves `any` or `unknown` - if ( - // https://github.com/typescript-eslint/typescript-eslint/issues/10438 - returnTypes.some(t => !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; + } } - if (!returnTypes.some(t => t && isPossiblyFalsy(t))) { + + if (!hasFalsyReturnTypes) { return context.report({ node: callback, messageId: 'alwaysTruthyFunc', }); } - if (!returnTypes.some(t => t && isPossiblyTruthy(t))) { + + if (!hasTruthyReturnTypes) { return context.report({ node: callback, messageId: 'alwaysFalsyFunc', From d33a7d4d34c1a2c8a1d4405a4e56d33b808fb924 Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 16 Dec 2024 18:47:19 +0200 Subject: [PATCH 3/4] bail early if both a possibly-truthy and a possibly-falsy have been detected --- packages/eslint-plugin/src/rules/no-unnecessary-condition.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index d9c16491f6c2..3c4585fe93ac 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -673,6 +673,11 @@ export default createRule({ if (isPossiblyTruthy(type)) { hasTruthyReturnTypes = true; } + + // bail early if both a possibly-truthy and a possibly-falsy have been detected + if (hasFalsyReturnTypes && hasTruthyReturnTypes) { + return; + } } if (!hasFalsyReturnTypes) { From ff59a13e23f4511079e964bd3c7ff7956e13583c Mon Sep 17 00:00:00 2001 From: Ronen Amiel Date: Mon, 16 Dec 2024 21:07:04 +0200 Subject: [PATCH 4/4] put back code that checks non-callables --- .../eslint-plugin/src/rules/no-unnecessary-condition.ts | 5 +++++ .../tests/rules/no-unnecessary-condition.test.ts | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index 3c4585fe93ac..892d3a9201ce 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -657,6 +657,11 @@ export default createRule({ return t; }); + if (returnTypes.length === 0) { + // Not a callable function, e.g. `any` + return; + } + let hasFalsyReturnTypes = false; let hasTruthyReturnTypes = false; diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index 8b2ad2ea50f2..e69e5c969dab 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -296,6 +296,12 @@ declare const test: () => T; declare const test: () => 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 `