Skip to content

fix in operation on tuple of Literal doesn't narrow type #3474#3504

Open
asukaminato0721 wants to merge 1 commit into
facebook:mainfrom
asukaminato0721:3474
Open

fix in operation on tuple of Literal doesn't narrow type #3474#3504
asukaminato0721 wants to merge 1 commit into
facebook:mainfrom
asukaminato0721:3474

Conversation

@asukaminato0721
Copy link
Copy Markdown
Contributor

@asukaminato0721 asukaminato0721 commented May 20, 2026

Summary

Fixes #3474

x in some_tuple now can narrow x from the tuple’s element type, including tuple[Literal[...], ...], while skipping tuple shapes with Any or unresolved variadics.

Test Plan

add test

@meta-cla meta-cla Bot added the cla signed label May 20, 2026
@asukaminato0721 asukaminato0721 marked this pull request as ready for review May 20, 2026 13:58
Copilot AI review requested due to automatic review settings May 20, 2026 13:58
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a type-narrowing gap for membership checks (x in some_tuple) by narrowing the left-hand side to the tuple’s element type (including tuple[Literal[...], ...]), while intentionally skipping tuple shapes involving Any-like element types or unresolved variadic/unpack forms.

Changes:

  • Add tuple_membership_type helper to derive a safe “membership element type” from Type::Tuple, with bail-outs for Any-like elements and unresolved variadics.
  • Extend AtomicNarrowOp::In narrowing to intersect the LHS with the RHS tuple’s derived membership type.
  • Add a regression test covering str in tuple[Literal[...], ...] narrowing via a Literal alias + get_args pattern.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pyrefly/lib/alt/narrow.rs Implements tuple-type-based narrowing for in checks via a new helper and integrates it into AtomicNarrowOp::In.
pyrefly/lib/test/narrow.rs Adds a regression test asserting in over a tuple[Literal[...], ...] narrows a str to the Literal union alias.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Copy Markdown

Diff from mypy_primer, showing the effect of this PR on open source code:

rotki (https://github.com/rotki/rotki)
+ ERROR rotkehlchen/api/services/transactions.py:144:24-45: Object of class `ChainManagerWithTransactions` has no attribute `node_inquirer` [missing-attribute]
- ERROR rotkehlchen/api/services/transactions.py:733:40-737:18: Argument `list[SupportedBlockchain]` is not assignable to parameter `iterable` with type `Iterable[CHAINS_WITH_TRANSACTION_DECODERS_TYPE]` in function `list.extend` [bad-argument-type]

mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ ERROR pymongo/common.py:371:12-17: Returned type `Literal['nearest', 'primary', 'primaryPreferred', 'secondary', 'secondaryPreferred']` is not assignable to declared return type `_ServerMode` [bad-return]

sphinx (https://github.com/sphinx-doc/sphinx)
- ERROR sphinx/ext/imgmath.py:257:32-37: `depth` may be uninitialized [unbound-name]
- ERROR sphinx/ext/imgmath.py:282:28-33: `depth` may be uninitialized [unbound-name]

xarray (https://github.com/pydata/xarray)
- ERROR xarray/core/dataset.py:9175:37-61: Argument `Literal[0] | Mapping[Any, Mapping[Any, float | tuple[float, float]] | float | tuple[float, float]] | Mapping[Any, float | tuple[float, float]] | dict[Any, Any] | float | tuple[float, float] | Any | None` is not assignable to parameter `constant_values` with type `Mapping[Any, float | tuple[float, float]] | float | tuple[float, float] | None` in function `xarray.core.variable.Variable.pad` [bad-argument-type]
+ ERROR xarray/core/dataset.py:9175:37-61: Argument `Literal[0] | Mapping[Any, Mapping[Any, float | tuple[float, float]] | float | tuple[float, float]] | Mapping[Any, float | tuple[float, float]] | dict[Hashable, Any] | float | tuple[float, float] | Any | None` is not assignable to parameter `constant_values` with type `Mapping[Any, float | tuple[float, float]] | float | tuple[float, float] | None` in function `xarray.core.variable.Variable.pad` [bad-argument-type]

parso (https://github.com/davidhalter/parso)
+ ERROR parso/python/pep8.py:603:54-64: Object of class `str` has no attribute `value` [missing-attribute]
- ERROR parso/python/pep8.py:621:38-49: Object of class `NoneType` has no attribute `parent` [missing-attribute]
+ ERROR parso/python/pep8.py:621:38-49: Object of class `str` has no attribute `parent` [missing-attribute]
+ ERROR parso/python/pep8.py:666:62-72: Object of class `str` has no attribute `value` [missing-attribute]

pylint (https://github.com/pycqa/pylint)
- ERROR pylint/checkers/stdlib.py:703:71-85: Argument `str | Unknown | None` is not assignable to parameter `func_name` with type `str` in function `StdlibChecker._check_open_call` [bad-argument-type]

scrapy (https://github.com/scrapy/scrapy)
- ERROR scrapy/utils/deprecate.py:83:34-37: Argument `type[Any] | None` is not assignable to parameter `cls` with type `type[Any]` in function `_clspath` [bad-argument-type]

schema_salad (https://github.com/common-workflow-language/schema_salad)
- ERROR src/schema_salad/avro/schema.py:712:36-50: Argument `Any | None` is not assignable to parameter `atype` with type `str` in function `PrimitiveSchema.__init__` [bad-argument-type]

@github-actions
Copy link
Copy Markdown

Primer Diff Classification

❌ 1 regression(s) | ✅ 5 improvement(s) | ➖ 2 neutral | 8 project(s) total | +6, -8 errors

1 regression(s) across parso. error kinds: missing-attribute. caused by tuple_membership_type(). 5 improvement(s) across mongo-python-driver, sphinx, pylint, scrapy, schema_salad.

Project Verdict Changes Error Kinds Root Cause
rotki ➖ Neutral +1, -1 bad-argument-type, missing-attribute tuple_membership_type()
mongo-python-driver ✅ Improvement +1 bad-return tuple_membership_type()
sphinx ✅ Improvement -2 unbound-name tuple_membership_type()
xarray ➖ Neutral +1, -1 bad-argument-type
parso ❌ Regression +3, -1 missing-attribute tuple_membership_type()
pylint ✅ Improvement -1 bad-argument-type tuple_membership_type()
scrapy ✅ Improvement -1 bad-argument-type tuple_membership_type()
schema_salad ✅ Improvement -1 bad-argument-type tuple_membership_type()
Detailed analysis

❌ Regression (1)

parso (+3, -1)

The new in-based narrowing is a reasonable feature, but it produces false positives in code that uses custom __eq__ to compare non-string objects with strings. The parso tree nodes compare equal to strings via __eq__ but are not strings — they have .value, .parent, etc. Pyright also flags these (3/3 co-reported), suggesting this is a known limitation of in-based narrowing. The removed error was also a false positive (different root cause). Net effect: 3 new false positives added, 1 false positive removed — a slight regression in error quality for this project.
Attribution: The change to AnswersSolver::[tuple_membership_type()](https://github.com/facebook/pyrefly/blob/main/pyrefly/lib/alt/narrow.rs) and the new narrowing logic in pyrefly/lib/alt/narrow.rs (around line 1111-1116) causes x in tuple_of_str_literals to narrow x to str. This produces the new false positives where parso tree node objects (which have .value and .parent) are narrowed to str after an in check against string literal tuples.

✅ Improvement (5)

mongo-python-driver (+1)

The function validate_read_preference_mode declares return type _ServerMode but actually returns the raw value parameter after checking membership in _MONGOS_MODES. After the PR's new tuple-literal narrowing, pyrefly correctly identifies that value is narrowed to Literal['nearest', 'primary', 'primaryPreferred', 'secondary', 'secondaryPreferred'] after the membership check, which is not assignable to _ServerMode (a class type). Pyright also flags this (pyright: yes in the metadata). This is a genuine type annotation bug in pymongo — the function's docstring explicitly states 'Returns the original value instead of the validated read preference mode,' confirming it returns the string, not a _ServerMode instance. The return type should either be str or the appropriate literal type rather than _ServerMode.
Attribution: The change to tuple_membership_type() and the new narrowing logic in pyrefly/lib/alt/narrow.rs (around line 1114) now narrows value from Any to the union of literal types in _MONGOS_MODES after the value not in _MONGOS_MODES guard. This causes pyrefly to see the return type as Literal['nearest', ...] instead of Any, revealing the mismatch with the declared _ServerMode return type.

sphinx (-2)

The PR adds tuple-of-literal narrowing for in/not in operations. This fixes two false positive unbound-name errors in sphinx where depth was incorrectly flagged as potentially uninitialized. The variable image_format is checked against SUPPORT_FORMAT = ('png', 'svg') at line 241 with if image_format not in SUPPORT_FORMAT: raise .... With the improved narrowing, pyrefly can now narrow image_format from str to Literal['png', 'svg'] after this guard. This makes the subsequent if image_format == 'png' / elif image_format == 'svg' chains at lines 253-256 and 274-277 exhaustive, guaranteeing that depth is always assigned before use at lines 257 and 282. Without this narrowing, pyrefly saw image_format as a general str, meaning neither if nor elif branch was guaranteed to execute, leaving depth potentially uninitialized.
Attribution: The new tuple_membership_type() function in pyrefly/lib/alt/narrow.rs and its integration into the in operator narrowing logic (around line 1111) enables pyrefly to narrow types based on x in tuple_of_literals. This allows pyrefly to understand that after image_format not in SUPPORT_FORMAT raises, image_format is narrowed to the tuple's element types ('png' | 'svg'), making the subsequent if/elif chains exhaustive and eliminating the false unbound-name errors.

pylint (-1)

This is a clear improvement. The PR adds support for narrowing types through in checks against tuples of literals. In the pylint code, open_func_name is typed as str | None (line 697), but pyrefly infers it as str | Unknown | None due to the conditional assignment paths (lines 698-701). It is then checked with if open_func_name in OPEN_FILES_FUNCS (a tuple of string literals) on line 702, and passed to _check_open_call which expects str on line 703. The in check against a tuple of string literals guarantees open_func_name is a str at that point, so the previous bad-argument-type error was a false positive caused by pyrefly's inability to narrow through tuple membership tests. The PR correctly removes this false positive.
Attribution: The change to tuple_membership_type() in pyrefly/lib/alt/narrow.rs and the new narrowing logic at line 1111-1116 in the same file enable narrowing of x in some_tuple expressions. This directly fixes the false positive: after open_func_name in OPEN_FILES_FUNCS (a tuple of string literals), open_func_name is now correctly narrowed from str | Unknown | None to str, eliminating the bad-argument-type error.

scrapy (-1)

The analysis is factually correct. On line 78, old is assigned from meta.deprecated_class which has type type | None (from the class variable declaration on line 65). On line 79, the condition old in bases is checked, where bases: tuple[type, ...]. If old in bases is true, then old must be a value that exists in a tuple[type, ...], meaning it must be of type type. The PR adds type narrowing for x in tuple_value expressions, which correctly narrows old from type | None to type within the if block. On line 83, _clspath(old, old_class_path) is called where _clspath expects cls: type as its first parameter (line 138). Without the narrowing, old is still type | None, causing the bad-argument-type error. With the narrowing, old is type, and the error is correctly eliminated as a false positive.
Attribution: The change to tuple_membership_type() in pyrefly/lib/alt/narrow.rs and the new narrowing logic at line 1114-1116 that calls self.tuple_membership_type(tuple, range, errors) when the right operand of in is a tuple type. This enables narrowing old from type | None to type after old in bases where bases: tuple[type, ...].

schema_salad (-1)

This is a correct improvement. The PR adds tuple membership narrowing, which allows pyrefly to understand that after atype in PRIMITIVE_TYPES (where PRIMITIVE_TYPES is a tuple of string literals), atype can be narrowed. On line 708, atype has type Any | None because it comes from json_data.get("type") where json_data is typed as JsonDataType (which is Any). The .get() method on a dict returns Any | None in this context. The check atype in PRIMITIVE_TYPES on line 710 allows the type checker to narrow atype — since None cannot be a member of a tuple of strings, and the tuple elements are all string literals, the type can be narrowed to str (or the union of those specific string literal types). On line 711, primative_type = atype inherits this narrowed type, and on line 712 it is passed to PrimitiveSchema.__init__ which expects str. The previous error claiming Any | None is not assignable to parameter atype with type str was a false positive that is now correctly eliminated by the new tuple membership narrowing capability.
Attribution: The new tuple_membership_type() method in pyrefly/lib/alt/narrow.rs and its integration at line 1114 of the same file enable narrowing x when x in some_tuple is checked. For PRIMITIVE_TYPES = ("null", "boolean", ...), the tuple elements are all str, so atype gets narrowed from Any | None to str after the in check.

➖ Neutral (2)

rotki (+1, -1)

Mixed result: The removal of the bad-argument-type error is a genuine improvement — the type checker now correctly recognizes that the list comprehension filtered through in CHAINS_WITH_TRANSACTION_DECODERS produces elements of the right type. However, the new missing-attribute error is a false positive. At line 142, blockchain in CHAINS_WITH_NODES narrows the type, and get_chain_manager is called with the narrowed type. The return type from the overload resolution doesn't include node_inquirer in its type signature, but at runtime the returned object does have this attribute (the concrete chain managers inherit from both the transaction management class and the nodes mixin). The code already has # type: ignore[call-overload] on line 143 acknowledging the type system limitation, but pyrefly still flags the attribute access on line 144. Note that similar patterns elsewhere in the same file (lines 184, 258-262) use # type: ignore[attr-defined] or # type: ignore to suppress the same kind of error, confirming this is a known type system limitation. Net: 1 false positive removed, 1 false positive added — roughly neutral but the new FP is arguably worse since it's on a different line and the code was already working around the type system limitation.
Attribution: The change to tuple_membership_type() and its integration in pyrefly/lib/alt/narrow.rs (around line 1111-1116) enables narrowing x when x in some_tuple where the tuple has Literal element types. This correctly fixes the removed error (line 733) but introduces a false positive at line 144 where the narrowed type resolves to a return type from get_chain_manager that lacks node_inquirer.

xarray (+1, -1)

Same errors at same locations with same error kinds — message wording changed, no behavioral impact.


Was this helpful? React with 👍 or 👎

Classification by primer-classifier (1 heuristic, 7 LLM)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

in operation on tuple of Literal doesn't narrow type

2 participants