Fix type narrowing for in operator with union containers and collection views - #11671
Fix type narrowing for in operator with union containers and collection views#11671Henry Su (hsusul) wants to merge 1 commit into
Conversation
|
🔒 Automated review in progress — Rich Chiodo (@rchiodo) is auto-reviewing this PR. |
| reveal_type(v, expected_text="float") | ||
| else: | ||
| reveal_type(v, expected_text="float | str") | ||
|
|
There was a problem hiding this comment.
Warning · Non-blocking recommendation
📍 packages/pyright-internal/src/tests/samples/typeNarrowingIn1.py:227
The tests cover concrete unions and dictionary views but not the newly supported Sequence, Set, Mapping, KeysView, ValuesView, or Container branches. Add focused positive cases plus a custom ABC implementation with permissive __contains__ to establish both intended behavior and the conservative boundary.
[verified]
| } | ||
|
|
||
| let elementType = containerType.priv.typeArgs[0]; | ||
| if (containerType.shared.name === 'dict_values' || containerType.shared.name === 'ValuesView') { |
There was a problem hiding this comment.
Info · Optional note
📍 packages/pyright-internal/src/analyzer/typeGuards.ts:2258
This logic couples narrowing to specific typeshed names and generic argument layouts, particularly the one-versus-two-argument ValuesView handling. Add focused tests for both layouts so future typeshed alias or signature changes cannot silently select the wrong element type.
[verified]
|
Verification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified. |
Rich Chiodo (rchiodo)
left a comment
Rich Chiodo (rchiodo)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
Rich Chiodo (rchiodo)
added
the
review-auto:approved
|
|
||
| if (!containerType.priv.typeArgs || containerType.priv.typeArgs.length < 1) { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
Issue · Please address or respond
Container[str] (and similarly the newly supported abstract collection interfaces) only promises __contains__(self, x: object) -> bool; a conforming implementation can return True for an int. This path would then narrow that int to str after if x in container. Please restrict narrowing to containers whose membership semantics establish the element-type relationship, and add a custom Container[str] regression case.
| reveal_type(v, expected_text="float") | ||
| else: | ||
| reveal_type(v, expected_text="float | str") | ||
|
|
There was a problem hiding this comment.
Warning · Non-blocking recommendation
Please add representative regression coverage for the newly supported typing and collections.abc collection types, such as Sequence, Set, Mapping, Container, KeysView, and ValuesView. The current additions cover unions and concrete dictionary views only, leaving the new module-prefix and generic-layout paths untested.
|
Verification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified. |
Rich Chiodo (rchiodo)
added
review-auto:changes-requested
2 participants
Description
This PR fixes type narrowing for the
incontainment operator when the container operand is a union of containers (e.g.list[str] | tuple[str, ...],list[str] | set[int]) or a standard collection/mapping view (KeysView,ValuesView,dict_keys,dict_values,Sequence,Set,Mapping, etc.).Previously,
getElementTypeForContainerNarrowingonly checked for a hardcoded list of concrete builtin classes and failed whencontainerTypewas a union or an abstract collection/view fromcollections.abc/typing. This causedx in containerto fail to narrowxwhencontainerwas a union of container types or a dictionary view.Changes
getElementTypeForContainerNarrowingto handleUnionTypecontainers by extracting and combining element types from all constituent subtypes if all subtypes are supported containers.collections.abccollection types and dictionary views (dict_keys,dict_values,KeysView,ValuesView,Sequence,Set,Mapping, etc.).typeNarrowingIn1.py.Validation
typeEvaluator1.test.ts(159/159 passing).pnpm run checkandpnpm run typecheckpassing cleanly).