fix(no-unnecessary-condition): don't flag required ?. on deferred conditional return types#1042
Open
pelly-ryu wants to merge 3 commits into
Open
Conversation
…onditional return types On optional chains of length >= 3, TypeScript leaves a generic method's conditional return type (`T extends U ? X : Y`) unresolved (`TypeFlagsConditional`) instead of resolving it to a branch. A raw conditional has no null/undefined flag and is not a union, so isNullishType reads it as non-nullish even though its branch is `Box | null`. This is misread at two nullishness checks on the optional-chain path. In isCallExpressionNullableOriginFromCallee the callee return looks non-nullable, so the chain's `| undefined` is attributed to the short-circuit and removeNullishFromType drops the `| null`. The same unresolved conditional also reaches the final nullish check in checkOptionalChain directly when the chain is property access whose type stays unresolved (a type parameter in scope). Fix: add isConstrainedNullishType, which replaces a conditional with its base constraint (the union of its branches) before judging nullishness and recurses through union members. Use it at both checks. The fold is gated on `TypeFlagsConditional`, so other types are unchanged. A deferred conditional whose branches are all non-nullish is still reported (added invalid test). Tests: 6 valid cases (`| null`/`| undefined` branch, explicit `<never>`, named alias, union-member conditional, property access in a generic scope) and 1 invalid precision case. Snapshot diff is additive (+16 / -0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Generated with Claude Code, reviewed and tested by me.
A generic method returning a deferred conditional makes the chain below report a false "Unnecessary optional chain on a non-nullish value", even though
b.get('a')isBox | null(tsc reports TS2531 without the?.).On a chain of 3+ links TypeScript leaves the inner conditional unresolved (
TypeFlagsConditional), which carries no nullish flag, soisNullishTypereads it as non-nullish. InisCallExpressionNullableOriginFromCalleethat makes the callee return look non-nullable, the chain's| undefinedis attributed to the optional-chain short-circuit, andremoveNullishFromTypedrops the real| null. The same unresolved conditional also reaches the final nullish check incheckOptionalChaindirectly when the chain is property access whose type stays unresolved (a type parameter in scope).A new
isConstrainedNullishTypereplaces a conditional with its base constraint (the union of its branches) before the nullish test, recurses into union members, and is gated onTypeFlagsConditionalso other types are unchanged. It is used at both nullishness checks the optional-chain path relies on: the callee-return check and the final check incheckOptionalChain. A deferred conditional whose branches are all non-nullish is still reported.Adds 6 valid regression cases (
| null/| undefinedbranch, explicit<never>, named alias, union-member conditional, property access in a generic scope) and an invalid precision case; the snapshot diff is additive.