fix Not inferring generic type for variable typed as Callable with TypeVar #1555 #2023
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #1555 where generic types weren't being inferred correctly for variables typed as Callable with TypeVar. The fix implements contextual quantification for annotated Callable assignments, ensuring legacy TypeVars are properly converted to quantified forms rather than treated as concrete values.
Key Changes
- Added helper functions to detect and promote legacy TypeVars in Callable signatures to quantified type parameters
- Modified the annotated assignment handling to apply the transformation only when a type annotation is present
- Added regression test demonstrating that
f: Callable[[T], T] = lambda x: xnow correctly infers as[T](T) -> T
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pyrefly/lib/alt/solve.rs | Added wrap_callable_legacy_typevars and promote_callable_legacy_typevars helper functions to convert TypeVars to Quantified types in Callable signatures; updated annotated assignment logic to apply transformation when annotations are present |
| pyrefly/lib/test/callable.rs | Added regression test verifying that Callable with TypeVar annotation correctly produces a quantified type and properly infers argument types during calls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let ty = if annot_key.is_some() { | ||
| self.wrap_callable_legacy_typevars(ty) | ||
| } else { | ||
| ty | ||
| }; | ||
| ty |
There was a problem hiding this comment.
The variable assignment and return can be simplified. The intermediate ty binding on line 3111 is immediately returned on line 3116 without any other use, making this pattern redundant. Consider directly returning the result of the conditional expression.
| let ty = if annot_key.is_some() { | |
| self.wrap_callable_legacy_typevars(ty) | |
| } else { | |
| ty | |
| }; | |
| ty | |
| if annot_key.is_some() { | |
| self.wrap_callable_legacy_typevars(ty) | |
| } else { | |
| ty | |
| } |
| fn promote_callable_legacy_typevars(&self, mut callable: Callable) -> (Callable, Vec<TParam>) { | ||
| let mut seen_type_vars: SmallMap<TypeVar, Quantified> = SmallMap::new(); | ||
| let mut tparams = Vec::new(); | ||
| callable.visit_mut(&mut |ty| { | ||
| if let Type::TypeVar(tv) = ty { | ||
| let q = seen_type_vars | ||
| .entry(tv.dupe()) | ||
| .or_insert_with(|| { | ||
| let q = Quantified::type_var( | ||
| tv.qname().id().clone(), | ||
| self.uniques, | ||
| tv.default().cloned(), | ||
| tv.restriction().clone(), | ||
| ); | ||
| tparams.push(TParam { | ||
| quantified: q.clone(), | ||
| variance: tv.variance(), | ||
| }); | ||
| q | ||
| }) | ||
| .clone(); | ||
| *ty = Type::Quantified(Box::new(q)); | ||
| } | ||
| }); | ||
| (callable, tparams) |
There was a problem hiding this comment.
The logic for promoting TypeVars to Quantified types in promote_callable_legacy_typevars is very similar to the logic in tvars_to_tparams_for_type_alias_type (lines 880-901). Both functions iterate over types, find TypeVars, create Quantified types with the same parameters, and build TParams. Consider extracting this common logic into a shared helper function to reduce code duplication and make maintenance easier.
| testcase!( | ||
| test_callable_variable_typevar_annotation, | ||
| r#" | ||
| from typing import Callable, TypeVar, reveal_type | ||
| T = TypeVar("T") | ||
| f: Callable[[T], T] = lambda x: x | ||
| reveal_type(f) # E: revealed type: [T](T) -> T | ||
| reveal_type(f(1)) # E: revealed type: int | ||
| "#, | ||
| ); |
There was a problem hiding this comment.
Consider adding test cases to verify behavior with multiple TypeVars (e.g., Callable[[T, U], T]), TypeVars with bounds/constraints (e.g., T = TypeVar("T", bound=int)), and calling with different argument types to ensure proper type inference. This would help ensure the implementation handles these common scenarios correctly.
|
Diff from mypy_primer, showing the effect of this PR on open source code: pandas (https://github.com/pandas-dev/pandas)
- ::error file=pandas/core/indexing.py,line=1470,col=9,endLine=1470,endColumn=28,title=Pyrefly bad-override::Class member `_LocIndexer._convert_to_indexer` overrides parent class `_LocationIndexer` in an inconsistent manner%0A `_LocIndexer._convert_to_indexer` has type `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> int | ndarray[tuple[int], dtype[Any]] | ndarray[tuple[Any, ...], dtype[signedinteger[_NBitIntP]]] | ndarray[tuple[Any, ...], dtype[Any]] | slice[int, int, Any] | slice[Any, Any, Any] | dict[str, int | integer[Any]] | dict[str, tuple[Unknown, ...]] | dict[str, Unknown] | Unknown]`, which is not assignable to `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> Never]`, the type of `_LocationIndexer._convert_to_indexer`
+ ::error file=pandas/core/indexing.py,line=1470,col=9,endLine=1470,endColumn=28,title=Pyrefly bad-override::Class member `_LocIndexer._convert_to_indexer` overrides parent class `_LocationIndexer` in an inconsistent manner%0A `_LocIndexer._convert_to_indexer` has type `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> ndarray[tuple[Any, ...], dtype[signedinteger[_NBitIntP]]] | ndarray[tuple[Any, ...], dtype[Any]] | dict[str, int | integer[Any]] | dict[str, tuple[Unknown, ...]] | dict[str, Unknown] | Unknown]`, which is not assignable to `BoundMethod[_LocIndexer, (self: _LocIndexer, key: Unknown, axis: int) -> Never]`, the type of `_LocationIndexer._convert_to_indexer`
|
1 participant
Summary
Fixes #1555
Implemented contextual quantification for annotated Callable assignments so legacy TypeVars are no longer treated as concrete values.
now feeds directly annotated assignments through wrap_callable_legacy_typevars, ensuring only annotated Callables get the extra handling.
introduces the helper that scans a Callable signature for Type::TypeVars, converts them to quantified forms, and builds matching TParams, yielding a Forallable::Callable whenever legacy type variables are present.
Test Plan
adds a regression test showing that
f: Callable[[T], T] = lambda x: xnow reports[T](T) -> Tand calls likef(1)resolve to int instead of the unspecializedTypeVar[T].