Fix narrowing in comprehensions when the intersection is impossible#21669
Open
apoorvdarshan wants to merge 1 commit into
Open
Fix narrowing in comprehensions when the intersection is impossible#21669apoorvdarshan wants to merge 1 commit into
apoorvdarshan wants to merge 1 commit into
Conversation
When a comprehension condition narrows the index variable to an impossible type (e.g. isinstance/issubclass where mypy determines a subclass of both classes cannot exist, or the class is @Final), check_for_comp() pushed an unreachable type map, which just marked the binder frame unreachable without recording any narrowing. Unlike statements, the comprehension's left expression is still type checked, so it was checked with the *unnarrowed* type, producing false positives like: error: List comprehension has incompatible type List[type[A]]; expected List[type[M]] Apply the impossible narrowing to the binder instead, so the left expression checks against Never, matching the narrowing an equivalent if statement would produce. Fixes python#21635
Contributor
|
Diff from mypy_primer, showing the effect of this PR on open source code: strawberry (https://github.com/strawberry-graphql/strawberry)
+ strawberry/relay/fields.py:189: error: Need type annotation for "nodes" [var-annotated]
+ strawberry/relay/fields.py:190: error: Unused "type: ignore" comment [unused-ignore]
|
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.
Fixes #21635
Problem
When a comprehension condition narrows the index variable to an impossible type — e.g.
issubclass(cls, M)where mypy determines a subclass of both classes cannot exist (the dataclass case from the issue, classes with incompatible method signatures, or@finalclasses) — the narrowing was silently discarded:Root cause
check_for_comp()pushes the condition'strue_mapviapush_type_map(). For an impossible narrowing the map contains anUninhabitedType, sopush_type_map()takes theis_unreachable_map()branch and only callsbinder.unreachable()— no narrowing is recorded. That's fine for statements, where the checker skips unreachable blocks, but the comprehension's left expression is still type checked, so it was checked with the unnarrowed type (type[A]), producing the false positive.(The equivalent
for/ifstatement version "works" only because theifbody is skipped as unreachable; with--warn-unreachableit reports the impossible subclass instead.)Fix
In
check_for_comp(), when the condition'strue_mapis an unreachable map, apply the impossible narrowing to the binder instead of marking the frame unreachable. The left expression then checks againstNever— matching the narrowing an equivalentifstatement would produce — and the example above typechecks (the item type isNever, which is compatible with any expected item type).This also covers generator expressions and dict comprehensions (all go through
check_for_comp()), and mirrors whatanalyze_cond_branch()already does for conditional expressions (it types the unreachable branch asUninhabitedType).Verification
check-isinstance.test(testComprehensionIsInstanceImpossibleIntersection,testComprehensionIsSubclassImpossibleIntersectionFinal) — both fail on master and pass with this change.testchecksuite: 8189 passed, 33 skipped, 7 xfailed.ruff check/ruff formatclean on the changed file.LLM disclosure
Per the CONTRIBUTING note on LLM-assisted contributions: this fix was developed with the assistance of an AI tool (Claude Code). I've reviewed the change and the investigation behind it, take full responsibility for it, and will respond to review feedback personally.