Skip to content

Commit e867830

Browse files
dcreagercarljmAlexWaygood
authored
[ty] Don't include already-bound legacy typevars in function generic context (#19558)
We now correctly exclude legacy typevars from enclosing scopes when constructing the generic context for a generic function. more detail: A function is generic if it refers to legacy typevars in its signature: ```py from typing import TypeVar T = TypeVar("T") def f(t: T) -> T: return t ``` Generic functions are allowed to appear inside of other generic contexts. When they do, they can refer to the typevars of those enclosing generic contexts, and that should not rebind the typevar: ```py from typing import TypeVar, Generic T = TypeVar("T") U = TypeVar("U") class C(Generic[T]): @staticmethod def method(t: T, u: U) -> None: ... # revealed: def method(t: int, u: U) -> None reveal_type(C[int].method) ``` This substitution was already being performed correctly, but we were also still including the enclosing legacy typevars in the method's own generic context, which can be seen via `ty_extensions.generic_context` (which has been updated to work on generic functions and methods): ```py from ty_extensions import generic_context # before: tuple[T, U] # after: tuple[U] reveal_type(generic_context(C[int].method)) ``` --------- Co-authored-by: Carl Meyer <[email protected]> Co-authored-by: Alex Waygood <[email protected]>
1 parent 72fdb7d commit e867830

File tree

7 files changed

+145
-18
lines changed

7 files changed

+145
-18
lines changed

crates/ty_python_semantic/resources/mdtest/generics/legacy/classes.md

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -433,17 +433,31 @@ the typevars of the enclosing generic class, and introduce new (distinct) typeva
433433
scope for the method.
434434

435435
```py
436+
from ty_extensions import generic_context
436437
from typing import Generic, TypeVar
437438

438439
T = TypeVar("T")
439440
U = TypeVar("U")
440441

441442
class C(Generic[T]):
442-
def method(self, u: U) -> U:
443+
def method(self, u: int) -> int:
444+
return u
445+
446+
def generic_method(self, t: T, u: U) -> U:
443447
return u
444448

449+
reveal_type(generic_context(C)) # revealed: tuple[T]
450+
reveal_type(generic_context(C.method)) # revealed: None
451+
reveal_type(generic_context(C.generic_method)) # revealed: tuple[U]
452+
reveal_type(generic_context(C[int])) # revealed: None
453+
reveal_type(generic_context(C[int].method)) # revealed: None
454+
reveal_type(generic_context(C[int].generic_method)) # revealed: tuple[U]
455+
445456
c: C[int] = C[int]()
446-
reveal_type(c.method("string")) # revealed: Literal["string"]
457+
reveal_type(c.generic_method(1, "string")) # revealed: Literal["string"]
458+
reveal_type(generic_context(c)) # revealed: None
459+
reveal_type(generic_context(c.method)) # revealed: None
460+
reveal_type(generic_context(c.generic_method)) # revealed: tuple[U]
447461
```
448462

449463
## Specializations propagate

crates/ty_python_semantic/resources/mdtest/generics/pep695/classes.md

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,17 +392,32 @@ the typevars of the enclosing generic class, and introduce new (distinct) typeva
392392
scope for the method.
393393

394394
```py
395+
from ty_extensions import generic_context
396+
395397
class C[T]:
396-
def method[U](self, u: U) -> U:
398+
def method(self, u: int) -> int:
399+
return u
400+
401+
def generic_method[U](self, t: T, u: U) -> U:
397402
return u
398403
# error: [unresolved-reference]
399404
def cannot_use_outside_of_method(self, u: U): ...
400405

401406
# TODO: error
402407
def cannot_shadow_class_typevar[T](self, t: T): ...
403408

409+
reveal_type(generic_context(C)) # revealed: tuple[T]
410+
reveal_type(generic_context(C.method)) # revealed: None
411+
reveal_type(generic_context(C.generic_method)) # revealed: tuple[U]
412+
reveal_type(generic_context(C[int])) # revealed: None
413+
reveal_type(generic_context(C[int].method)) # revealed: None
414+
reveal_type(generic_context(C[int].generic_method)) # revealed: tuple[U]
415+
404416
c: C[int] = C[int]()
405-
reveal_type(c.method("string")) # revealed: Literal["string"]
417+
reveal_type(c.generic_method(1, "string")) # revealed: Literal["string"]
418+
reveal_type(generic_context(c)) # revealed: None
419+
reveal_type(generic_context(c.method)) # revealed: None
420+
reveal_type(generic_context(c.generic_method)) # revealed: tuple[U]
406421
```
407422

408423
## Specializations propagate

crates/ty_python_semantic/resources/mdtest/generics/scoping.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,22 @@ class Legacy(Generic[T]):
141141
def m(self, x: T, y: S) -> S:
142142
return y
143143

144-
legacy: Legacy[int] = Legacy()
144+
legacy: Legacy[int] = Legacy[int]()
145145
reveal_type(legacy.m(1, "string")) # revealed: Literal["string"]
146146
```
147147

148+
The class typevar in the method signature does not bind a _new_ instance of the typevar; it was
149+
already solved and specialized when the class was specialized:
150+
151+
```py
152+
from ty_extensions import generic_context
153+
154+
legacy.m("string", None) # error: [invalid-argument-type]
155+
reveal_type(legacy.m) # revealed: bound method Legacy[int].m(x: int, y: S) -> S
156+
reveal_type(generic_context(Legacy)) # revealed: tuple[T]
157+
reveal_type(generic_context(legacy.m)) # revealed: tuple[S]
158+
```
159+
148160
With PEP 695 syntax, it is clearer that the method uses a separate typevar:
149161

150162
```py

crates/ty_python_semantic/src/types/call/bind.rs

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -623,19 +623,38 @@ impl<'db> Bindings<'db> {
623623

624624
Some(KnownFunction::GenericContext) => {
625625
if let [Some(ty)] = overload.parameter_types() {
626+
let function_generic_context = |function: FunctionType<'db>| {
627+
let union = UnionType::from_elements(
628+
db,
629+
function
630+
.signature(db)
631+
.overloads
632+
.iter()
633+
.filter_map(|signature| signature.generic_context)
634+
.map(|generic_context| generic_context.as_tuple(db)),
635+
);
636+
if union.is_never() {
637+
Type::none(db)
638+
} else {
639+
union
640+
}
641+
};
642+
626643
// TODO: Handle generic functions, and unions/intersections of
627644
// generic types
628645
overload.set_return_type(match ty {
629-
Type::ClassLiteral(class) => match class.generic_context(db) {
630-
Some(generic_context) => TupleType::from_elements(
631-
db,
632-
generic_context
633-
.variables(db)
634-
.iter()
635-
.map(|typevar| Type::TypeVar(*typevar)),
636-
),
637-
None => Type::none(db),
638-
},
646+
Type::ClassLiteral(class) => class
647+
.generic_context(db)
648+
.map(|generic_context| generic_context.as_tuple(db))
649+
.unwrap_or_else(|| Type::none(db)),
650+
651+
Type::FunctionLiteral(function) => {
652+
function_generic_context(*function)
653+
}
654+
655+
Type::BoundMethod(bound_method) => {
656+
function_generic_context(bound_method.function(db))
657+
}
639658

640659
_ => Type::none(db),
641660
});

crates/ty_python_semantic/src/types/function.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ impl<'db> OverloadLiteral<'db> {
340340
let index = semantic_index(db, scope.file(db));
341341
GenericContext::from_type_params(db, index, type_params)
342342
});
343+
343344
Signature::from_function(
344345
db,
345346
generic_context,

crates/ty_python_semantic/src/types/generics.rs

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,64 @@
11
use std::borrow::Cow;
22

3+
use ruff_db::parsed::{ParsedModuleRef, parsed_module};
34
use ruff_python_ast as ast;
45
use rustc_hash::FxHashMap;
56

6-
use crate::semantic_index::SemanticIndex;
7+
use crate::semantic_index::definition::Definition;
8+
use crate::semantic_index::scope::{FileScopeId, NodeWithScopeKind};
9+
use crate::semantic_index::{SemanticIndex, semantic_index};
710
use crate::types::class::ClassType;
811
use crate::types::class_base::ClassBase;
912
use crate::types::instance::{NominalInstanceType, Protocol, ProtocolInstanceType};
1013
use crate::types::signatures::{Parameter, Parameters, Signature};
1114
use crate::types::tuple::{TupleSpec, TupleType};
1215
use crate::types::{
1316
KnownInstanceType, Type, TypeMapping, TypeRelation, TypeTransformer, TypeVarBoundOrConstraints,
14-
TypeVarInstance, TypeVarVariance, UnionType, declaration_type,
17+
TypeVarInstance, TypeVarVariance, UnionType, binding_type, declaration_type,
1518
};
1619
use crate::{Db, FxOrderSet};
1720

21+
/// Returns an iterator of any generic context introduced by the given scope or any enclosing
22+
/// scope.
23+
fn enclosing_generic_contexts<'db>(
24+
db: &'db dyn Db,
25+
module: &ParsedModuleRef,
26+
index: &SemanticIndex<'db>,
27+
scope: FileScopeId,
28+
) -> impl Iterator<Item = GenericContext<'db>> {
29+
index
30+
.ancestor_scopes(scope)
31+
.filter_map(|(_, ancestor_scope)| match ancestor_scope.node() {
32+
NodeWithScopeKind::Class(class) => {
33+
binding_type(db, index.expect_single_definition(class.node(module)))
34+
.into_class_literal()?
35+
.generic_context(db)
36+
}
37+
NodeWithScopeKind::Function(function) => {
38+
binding_type(db, index.expect_single_definition(function.node(module)))
39+
.into_function_literal()?
40+
.signature(db)
41+
.iter()
42+
.last()
43+
.expect("function should have at least one overload")
44+
.generic_context
45+
}
46+
_ => None,
47+
})
48+
}
49+
50+
/// Returns the legacy typevars that have been bound in the given scope or any enclosing scope.
51+
fn bound_legacy_typevars<'db>(
52+
db: &'db dyn Db,
53+
module: &ParsedModuleRef,
54+
index: &'db SemanticIndex<'db>,
55+
scope: FileScopeId,
56+
) -> impl Iterator<Item = TypeVarInstance<'db>> {
57+
enclosing_generic_contexts(db, module, index, scope)
58+
.flat_map(|generic_context| generic_context.variables(db).iter().copied())
59+
.filter(|typevar| typevar.is_legacy(db))
60+
}
61+
1862
/// A list of formal type variables for a generic function, class, or type alias.
1963
///
2064
/// TODO: Handle nested generic contexts better, with actual parent links to the lexically
@@ -82,9 +126,11 @@ impl<'db> GenericContext<'db> {
82126
/// list.
83127
pub(crate) fn from_function_params(
84128
db: &'db dyn Db,
129+
definition: Definition<'db>,
85130
parameters: &Parameters<'db>,
86131
return_type: Option<Type<'db>>,
87132
) -> Option<Self> {
133+
// Find all of the legacy typevars mentioned in the function signature.
88134
let mut variables = FxOrderSet::default();
89135
for param in parameters {
90136
if let Some(ty) = param.annotated_type() {
@@ -97,6 +143,16 @@ impl<'db> GenericContext<'db> {
97143
if let Some(ty) = return_type {
98144
ty.find_legacy_typevars(db, &mut variables);
99145
}
146+
147+
// Then remove any that were bound in enclosing scopes.
148+
let file = definition.file(db);
149+
let module = parsed_module(db, file).load(db);
150+
let index = semantic_index(db, file);
151+
let containing_scope = definition.file_scope(db);
152+
for typevar in bound_legacy_typevars(db, &module, index, containing_scope) {
153+
variables.remove(&typevar);
154+
}
155+
100156
if variables.is_empty() {
101157
return None;
102158
}
@@ -171,6 +227,16 @@ impl<'db> GenericContext<'db> {
171227
self.specialize(db, types.into())
172228
}
173229

230+
/// Returns a tuple type of the typevars introduced by this generic context.
231+
pub(crate) fn as_tuple(self, db: &'db dyn Db) -> Type<'db> {
232+
TupleType::from_elements(
233+
db,
234+
self.variables(db)
235+
.iter()
236+
.map(|typevar| Type::TypeVar(*typevar)),
237+
)
238+
}
239+
174240
pub(crate) fn is_subset_of(self, db: &'db dyn Db, other: GenericContext<'db>) -> bool {
175241
self.variables(db).is_subset(other.variables(db))
176242
}

crates/ty_python_semantic/src/types/signatures.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ impl<'db> Signature<'db> {
331331
}
332332
});
333333
let legacy_generic_context =
334-
GenericContext::from_function_params(db, &parameters, return_ty);
334+
GenericContext::from_function_params(db, definition, &parameters, return_ty);
335335

336336
if generic_context.is_some() && legacy_generic_context.is_some() {
337337
// TODO: Raise a diagnostic!

0 commit comments

Comments
 (0)