Skip to content

Commit 8ca9cba

Browse files
authored
fix(eslint-plugin): [no-unnecessary-condition] don't flag values of an 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
1 parent ba39dde commit 8ca9cba

File tree

2 files changed

+63
-8
lines changed

2 files changed

+63
-8
lines changed

packages/eslint-plugin/src/rules/no-unnecessary-condition.ts

+39-8
Original file line numberDiff line numberDiff line change
@@ -649,22 +649,53 @@ export default createRule<Options, MessageId>({
649649
.getCallSignaturesOfType(
650650
getConstrainedTypeAtLocation(services, callback),
651651
)
652-
.map(sig => sig.getReturnType());
653-
/* istanbul ignore if */ if (returnTypes.length === 0) {
654-
// Not a callable function
652+
.map(sig => sig.getReturnType())
653+
.map(t => {
654+
// TODO: use `getConstraintTypeInfoAtLocation` once it's merged
655+
// https://github.com/typescript-eslint/typescript-eslint/pull/10496
656+
if (tsutils.isTypeParameter(t)) {
657+
return checker.getBaseConstraintOfType(t);
658+
}
659+
660+
return t;
661+
});
662+
663+
if (returnTypes.length === 0) {
664+
// Not a callable function, e.g. `any`
655665
return;
656666
}
657-
// Predicate is always necessary if it involves `any` or `unknown`
658-
if (returnTypes.some(t => isTypeAnyType(t) || isTypeUnknownType(t))) {
659-
return;
667+
668+
let hasFalsyReturnTypes = false;
669+
let hasTruthyReturnTypes = false;
670+
671+
for (const type of returnTypes) {
672+
// Predicate is always necessary if it involves `any` or `unknown`
673+
if (!type || isTypeAnyType(type) || isTypeUnknownType(type)) {
674+
return;
675+
}
676+
677+
if (isPossiblyFalsy(type)) {
678+
hasFalsyReturnTypes = true;
679+
}
680+
681+
if (isPossiblyTruthy(type)) {
682+
hasTruthyReturnTypes = true;
683+
}
684+
685+
// bail early if both a possibly-truthy and a possibly-falsy have been detected
686+
if (hasFalsyReturnTypes && hasTruthyReturnTypes) {
687+
return;
688+
}
660689
}
661-
if (!returnTypes.some(isPossiblyFalsy)) {
690+
691+
if (!hasFalsyReturnTypes) {
662692
return context.report({
663693
node: callback,
664694
messageId: 'alwaysTruthyFunc',
665695
});
666696
}
667-
if (!returnTypes.some(isPossiblyTruthy)) {
697+
698+
if (!hasTruthyReturnTypes) {
668699
return context.report({
669700
node: callback,
670701
messageId: 'alwaysFalsyFunc',

packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts

+24
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,22 @@ function count(
286286
) {
287287
return list.filter(predicate).length;
288288
}
289+
`,
290+
`
291+
declare const test: <T>() => T;
292+
293+
[1, null].filter(test);
294+
`,
295+
`
296+
declare const test: <T extends boolean>() => T;
297+
298+
[1, null].filter(test);
299+
`,
300+
`
301+
[1, null].filter(1 as any);
302+
`,
303+
`
304+
[1, null].filter(1 as never);
289305
`,
290306
// Ignores non-array methods of the same name
291307
`
@@ -1745,6 +1761,14 @@ function nothing3(x: [string, string]) {
17451761
{ column: 25, line: 17, messageId: 'alwaysFalsy' },
17461762
],
17471763
},
1764+
{
1765+
code: `
1766+
declare const test: <T extends true>() => T;
1767+
1768+
[1, null].filter(test);
1769+
`,
1770+
errors: [{ column: 18, line: 4, messageId: 'alwaysTruthyFunc' }],
1771+
},
17481772
// Indexing cases
17491773
{
17501774
// This is an error because 'dict' doesn't represent

0 commit comments

Comments
 (0)