fix map is confused by string literals #2854#2856
fix map is confused by string literals #2854#2856asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
Conversation
|
Diff from mypy_primer, showing the effect of this PR on open source code: ibis (https://github.com/ibis-project/ibis)
+ ERROR ibis/backends/sql/compilers/snowflake.py:220:35-81: Argument `str` is not assignable to parameter `object` with type `Literal['COMMENT = \'{comment}\'', 'CREATE OR REPLACE TEMPORARY FUNCTION {name}({signature})', 'IMMUTABLE', 'LANGUAGE PYTHON', 'RETURNS {return_type}', 'RUNTIME_VERSION = \'{version}\'']` in function `list.append` [bad-argument-type]
+ ERROR ibis/backends/sql/compilers/snowflake.py:225:31-55: Argument `str` is not assignable to parameter `object` with type `Literal['COMMENT = \'{comment}\'', 'CREATE OR REPLACE TEMPORARY FUNCTION {name}({signature})', 'IMMUTABLE', 'LANGUAGE PYTHON', 'RETURNS {return_type}', 'RUNTIME_VERSION = \'{version}\'']` in function `list.append` [bad-argument-type]
openlibrary (https://github.com/internetarchive/openlibrary)
- ERROR openlibrary/plugins/worksearch/code.py:320:13-19: Argument `list[tuple[str, SolrRequestLabel] | tuple[str, int | Unknown] | tuple[str, str] | tuple[str, Unknown]]` is not assignable to parameter `cur_solr_params` with type `list[tuple[str, str]]` in function `openlibrary.plugins.worksearch.schemes.SearchScheme.q_to_solr_params` [bad-argument-type]
+ ERROR openlibrary/plugins/worksearch/code.py:320:13-19: Argument `list[tuple[Literal['fq'], str] | tuple[str, SolrRequestLabel] | tuple[str, int | Unknown] | tuple[str, Unknown]]` is not assignable to parameter `cur_solr_params` with type `list[tuple[str, str]]` in function `openlibrary.plugins.worksearch.schemes.SearchScheme.q_to_solr_params` [bad-argument-type]
strawberry (https://github.com/strawberry-graphql/strawberry)
+ ERROR strawberry/codegen/query_codegen.py:887:26-44: `list[GraphQLField]` is not assignable to variable `fields` with type `list[GraphQLField | GraphQLFragmentSpread]` [bad-assignment]
pandas (https://github.com/pandas-dev/pandas)
- ERROR pandas/tests/indexes/multi/test_indexing.py:959:36-41: Argument `range` is not assignable to parameter `iterable` with type `Iterable[_NestedSequence[_SupportsArray[dtype]] | _SupportsArray[dtype]]` in function `list.__init__` [bad-argument-type]
core (https://github.com/home-assistant/core)
+ ERROR homeassistant/components/devolo_home_control/siren.py:54:38-59:10: `list[int]` is not assignable to attribute `_attr_available_tones` with type `dict[int, str] | list[int | str] | None` [bad-assignment]
+ ERROR homeassistant/components/fritz/switch.py:203:20-89: Returned type `list[FritzBoxWifiSwitch]` is not assignable to declared return type `list[Entity]` [bad-return]
+ ERROR homeassistant/components/fritz/switch.py:206:12-211:6: Returned type `list[FritzBoxDeflectionSwitch | FritzBoxPortSwitch | FritzBoxProfileSwitch | FritzBoxWifiSwitch]` is not assignable to declared return type `list[Entity]` [bad-return]
+ ERROR homeassistant/components/home_connect/sensor.py:514:12-532:6: Returned type `list[HomeConnectEventSensor | HomeConnectProgramSensor | HomeConnectSensor]` is not assignable to declared return type `list[HomeConnectEntity]` [bad-return]
+ ERROR homeassistant/components/number/const.py:499:80-614:2: `dict[NumberDeviceClass, set[UnitOfSpeed | UnitOfVolumetricFlux] | set[str | type[StrEnum] | None]]` is not assignable to `dict[NumberDeviceClass, set[str | type[StrEnum] | None]]` [bad-assignment]
+ ERROR homeassistant/components/sensor/const.py:594:80-709:2: `dict[SensorDeviceClass, set[UnitOfSpeed | UnitOfVolumetricFlux] | set[str | type[StrEnum] | None]]` is not assignable to `dict[SensorDeviceClass, set[str | type[StrEnum] | None]]` [bad-assignment]
sphinx (https://github.com/sphinx-doc/sphinx)
- ERROR sphinx/ext/autosummary/__init__.py:607:28-43: Argument `list[str]` is not assignable to parameter `iterable` with type `Iterable[LiteralString]` in function `list.__init__` [bad-argument-type]
django-modern-rest (https://github.com/wemake-services/django-modern-rest)
+ ERROR dmr/metadata.py:365:16-370:10: Returned type `list[type[AsyncAuth] | type[Parser] | type[Renderer] | type[SyncAuth] | type[ComponentParser]]` is not assignable to declared return type `list[type[ResponseSpecProvider]]` [bad-return]
jax (https://github.com/google/jax)
+ ERROR jax/experimental/key_reuse/_core.py:389:33-66: `list[Var]` is not assignable to `list[Atom]` [bad-assignment]
- ERROR jax/experimental/mosaic/gpu/fragmented_array.py:4107:50-53: Argument `type[str]` is not assignable to parameter `func` with type `(object) -> LiteralString` in function `map.__new__` [bad-argument-type]
materialize (https://github.com/MaterializeInc/materialize)
+ ERROR misc/python/materialize/parallel_workload/action.py:2191:44-75: `list[Table | View]` is not assignable to `list[DBObject]` [bad-assignment]
+ ERROR misc/python/materialize/parallel_workload/action.py:2220:44-75: `list[Table | View]` is not assignable to `list[DBObject]` [bad-assignment]
|
There was a problem hiding this comment.
Pull request overview
Fixes an inference bug where LiteralString context from str.join(...) could incorrectly over-constrain nested starred iterables (e.g., *map(str, ...)) during list/set element inference.
Changes:
- Update list/set starred-element inference to infer without a contextual hint first, and only retry with the hint when the unhinted result contains partial (placeholder) types.
- Add a regression test reproducing issue #2854 involving
",".join([*genexpr, *map(str, range(n))]).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
pyrefly/lib/alt/expr.rs |
Adjusts starred element inference in list/set literals to avoid premature contextual-hint over-constraint while preserving empty-container inference via a retry. |
pyrefly/lib/test/literal.rs |
Adds a regression testcase ensuring the join + starred + map(str, ...) pattern no longer produces overload/type errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let retry_with_hint = star_hint.as_ref().is_some() | ||
| && unpacked_ty.any(|ty| self.solver().is_partial(ty)); | ||
| if retry_with_hint { | ||
| unpacked_ty = self.expr_infer_with_hint_promote( | ||
| value, | ||
| star_hint.as_ref().map(|hint| hint.as_ref()), |
There was a problem hiding this comment.
star_hint is wrapped in a LazyCell, but star_hint.as_ref().is_some() forces it to be computed even when you don't retry with the hint. Consider checking elt_hint.is_some() (or similar) to decide whether a hint exists, and only evaluating star_hint when the retry path is actually taken, so the derived Iterable[...] hint construction stays lazy.
| let retry_with_hint = star_hint.as_ref().is_some() | |
| && unpacked_ty.any(|ty| self.solver().is_partial(ty)); | |
| if retry_with_hint { | |
| unpacked_ty = self.expr_infer_with_hint_promote( | |
| value, | |
| star_hint.as_ref().map(|hint| hint.as_ref()), | |
| let retry_with_hint = | |
| elt_hint.is_some() && unpacked_ty.any(|ty| self.solver().is_partial(ty)); | |
| if retry_with_hint { | |
| let star_hint_ref = star_hint.force(); | |
| unpacked_ty = self.expr_infer_with_hint_promote( | |
| value, | |
| star_hint_ref.as_ref().map(|hint| hint.as_ref()), |
Primer Diff Classification❌ 4 regression(s) | ✅ 3 improvement(s) | ➖ 2 neutral | 9 project(s) total | +14, -4 errors 4 regression(s) across strawberry, core, django-modern-rest, materialize. error kinds:
Detailed analysis❌ Regression (4)strawberry (+1)
core (+6)
django-modern-rest (+1)
Looking at the code, the types come from the field declarations on
Whether these classes actually inherit from Even if they are subclasses, the key issue is list invariance: Previously, the contextual return type hint would guide the list literal to be typed as This is a regression introduced by the PR's change to starred element inference. The fix for the
materialize (+2)
✅ Improvement (3)ibis (+2)
pandas (-1)
sphinx (-1)
➖ Neutral (2)openlibrary (+1, -1)
jax (+1, -1)
Suggested fixesSummary: The PR's change to starred element inference fixes LiteralString over-constraining but breaks contextual typing for starred unpacking in list literals, causing 10+ false positives across 6 projects where list[SubClass] is not assignable to list[SuperClass] due to lost type widening. **1. In the starred element inference logic in rust This preserves the fix for the
**2. Alternative simpler fix: In the starred element inference logic in rust This narrowly targets the original problem (LiteralString hints over-constraining starred elements) while preserving contextual typing for all other cases.**
Was this helpful? React with 👍 or 👎 Classification by primer-classifier (1 heuristic, 8 LLM) |
Summary
Fixes #2854
starred list/set elements now infer their iterable type without a contextual hint first, and only retry with the hint when the unhinted result still contains partial placeholders.
That stops Iterable[LiteralString] from over-constraining nested calls like map(str, range(n)) while preserving empty-container inference.
Test Plan
add test