Skip to content

Commit 5da3b41

Browse files
authored
[ty] Give Self type variables distinct class identities (astral-sh#29191)
Currently, all `Self` type variables share a single `TypeVarIdentity`. This isn't really semantically right, but it's mostly fine in practice, because they get bound to different methods and so have distinct `BoundTypeVarIdentity`. It does cause bugs in some cases with synthesized methods: unions of `_replace` methods from different named tuple classes can collapse to a single callable because their synthetic `Self` variables share an identity. This can incorrectly accept a callable promised to return only one of the classes, depending on the union order. Also, we currently have to recover the owning class of a `Self` type variable via its upper bound (which should be set to the owning class), but this is a bit implicit and can be awkward when we transform upper bounds (e.g. for `type[T]` or due to specialization or materialization). Instead, give class-owned `Self` variables explicitly distinct identities using `SelfTypeVarOrigin::Class`. Keep that origin stable when specializing or materializing their bounds, and represent receiver placeholders and synthesized `TypedDict` schemas with explicit unit variants.
1 parent acba99d commit 5da3b41

12 files changed

Lines changed: 194 additions & 105 deletions

File tree

‎crates/ty_ide/src/semantic_tokens.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,10 @@ impl<'db> SemanticTokenVisitor<'db> {
441441
};
442442

443443
if let Some((type_var, is_cls)) = type_var
444-
&& matches!(type_var.typevar(db).kind(db), TypeVarKind::TypingSelf)
444+
&& matches!(
445+
type_var.typevar(db).kind(db),
446+
TypeVarKind::TypingSelf { .. }
447+
)
445448
{
446449
let kind = if is_cls {
447450
SemanticTokenType::ClsParameter

‎crates/ty_python_semantic/resources/mdtest/named_tuple.md‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1370,6 +1370,44 @@ class IntBox(Box[int]):
13701370
reveal_type(IntBox(1)._replace(content=42)) # revealed: IntBox
13711371
```
13721372

1373+
### `_replace` methods from different classes
1374+
1375+
The `_replace` methods of different named tuples retain different return types even when their
1376+
fields match. A union of these methods cannot promise to return only one of the classes, regardless
1377+
of branch order:
1378+
1379+
```py
1380+
from typing import Callable, NamedTuple
1381+
1382+
class A(NamedTuple):
1383+
x: int
1384+
1385+
class B(NamedTuple):
1386+
x: int
1387+
1388+
def choose(flag: bool) -> Callable[..., A]:
1389+
# error: [invalid-return-type]
1390+
return A._replace if flag else B._replace
1391+
1392+
def reversed_order(flag: bool) -> Callable[..., A]:
1393+
# error: [invalid-return-type]
1394+
return B._replace if flag else A._replace
1395+
1396+
def either_class(flag: bool) -> Callable[..., A | B]:
1397+
return A._replace if flag else B._replace # no diagnostic
1398+
```
1399+
1400+
The same distinction applies to named tuples created with the functional syntax:
1401+
1402+
```py
1403+
DynamicA = NamedTuple("DynamicA", [("x", int)])
1404+
DynamicB = NamedTuple("DynamicB", [("x", int)])
1405+
1406+
def choose_dynamic(flag: bool) -> Callable[..., DynamicA]:
1407+
# error: [invalid-return-type]
1408+
return DynamicA._replace if flag else DynamicB._replace
1409+
```
1410+
13731411
## `collections.namedtuple`
13741412

13751413
```py

‎crates/ty_python_semantic/src/types.rs‎

Lines changed: 50 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ pub use crate::types::type_alias::TypeAliasType;
106106
pub use crate::types::type_form::TypeFormType;
107107
pub(crate) use crate::types::typed_dict::TypedDictType;
108108
pub(crate) use crate::types::typevar::{
109-
BindingContext, BoundTypeVarIdentity, ParamSpecAttrKind, TypeVarBoundOrConstraints,
110-
TypeVarNonce,
109+
BindingContext, BoundTypeVarIdentity, ParamSpecAttrKind, SelfTypeVarOrigin,
110+
TypeVarBoundOrConstraints, TypeVarNonce,
111111
};
112112
pub use crate::types::typevar::{BoundTypeVarInstance, TypeVarKind};
113113
use crate::types::typevar::{TypeVarInstance, TypeVarSet};
@@ -10000,7 +10000,9 @@ impl<'db> Type<'db> {
1000010000
) {
1000110001
let matching_typevar = |bound_typevar: &BoundTypeVarInstance<'db>| {
1000210002
match bound_typevar.typevar(db).kind(db) {
10003-
TypeVarKind::LegacyTypeVar | TypeVarKind::Pep613Alias | TypeVarKind::TypingSelf
10003+
TypeVarKind::LegacyTypeVar
10004+
| TypeVarKind::Pep613Alias
10005+
| TypeVarKind::TypingSelf { .. }
1000410006
if binding_context.is_none_or(|binding_context| {
1000510007
bound_typevar.binding_context(db)
1000610008
== BindingContext::Definition(binding_context)
@@ -11146,17 +11148,17 @@ pub enum PromotionKind {
1114611148
SingletonsOnly,
1114711149
}
1114811150

11149-
/// Returns the [`ClassLiteral`] that "owns" a `Self` typevar (i.e., the class from its upper bound).
11150-
fn self_typevar_owner_class_literal<'db>(
11151+
/// Returns the class recorded in a `Self` typevar's origin.
11152+
fn self_typevar_origin_class_literal<'db>(
1115111153
db: &'db dyn Db,
11152-
env: &ProgramEnvironment<'db>,
1115311154
bound_typevar: BoundTypeVarInstance<'db>,
1115411155
) -> Option<ClassLiteral<'db>> {
11155-
bound_typevar
11156-
.typevar(db)
11157-
.upper_bound(db, env)
11158-
.and_then(|ty| ty.nominal_class(db, env))
11159-
.map(|class| class.class_literal(db))
11156+
match bound_typevar.kind(db) {
11157+
TypeVarKind::TypingSelf {
11158+
origin: SelfTypeVarOrigin::Class(class),
11159+
} => Some(class),
11160+
_ => None,
11161+
}
1116011162
}
1116111163

1116211164
#[salsa::tracked(returns(ref), heap_size=ruff_memory_usage::heap_size)]
@@ -11173,8 +11175,8 @@ fn class_mro_literals<'db>(
1117311175

1117411176
/// Information needed to bind `Self` typevars to a concrete type.
1117511177
///
11176-
/// Uses MRO-based matching: a `Self` typevar is bound only if its owner class
11177-
/// is in the MRO of the self type's class.
11178+
/// Uses the binding context and MRO-based matching against the class recorded in a `Self`
11179+
/// typevar's origin.
1117811180
#[derive(Clone, Debug, Eq, PartialEq, get_size2::GetSize)]
1117911181
pub struct SelfBinding<'db> {
1118011182
ty: Type<'db>,
@@ -11201,7 +11203,7 @@ impl<'db> SelfBinding<'db> {
1120111203
) -> Self {
1120211204
let class_literal = match self_type {
1120311205
Type::TypeVar(typevar) if typevar.typevar(db).is_self(db) => {
11204-
self_typevar_owner_class_literal(db, env, typevar)
11206+
self_typevar_origin_class_literal(db, typevar)
1120511207
}
1120611208
_ => self_type
1120711209
.nominal_class(db, env)
@@ -11216,12 +11218,7 @@ impl<'db> SelfBinding<'db> {
1121611218
}
1121711219

1121811220
/// Returns whether `bound_typevar` should be replaced by this binding's concrete self type.
11219-
fn should_bind(
11220-
&self,
11221-
db: &'db dyn Db,
11222-
env: &ProgramEnvironment<'db>,
11223-
bound_typevar: BoundTypeVarInstance<'db>,
11224-
) -> bool {
11221+
fn should_bind(&self, db: &'db dyn Db, bound_typevar: BoundTypeVarInstance<'db>) -> bool {
1122511222
if !bound_typevar.typevar(db).is_self(db) {
1122611223
return false;
1122711224
}
@@ -11232,12 +11229,12 @@ impl<'db> SelfBinding<'db> {
1123211229
return true;
1123311230
}
1123411231

11235-
// Check that the Self typevar's owner class is in the MRO of the self type's class.
11236-
// If we can't determine either class, conservatively don't bind.
11232+
// Check that Self's originating class is in the receiver's MRO. Receiver placeholders
11233+
// and synthesized TypedDict schemas have no class to restrict matching.
1123711234
self.class_literal.is_some_and(|class_literal| {
1123811235
let class_mro = class_mro_literals(db, class_literal);
11239-
self_typevar_owner_class_literal(db, env, bound_typevar)
11240-
.is_none_or(|owner_class| class_mro.contains(&owner_class))
11236+
self_typevar_origin_class_literal(db, bound_typevar)
11237+
.is_none_or(|origin_class| class_mro.contains(&origin_class))
1124111238
})
1124211239
}
1124311240
}
@@ -11274,8 +11271,11 @@ pub enum TypeMapping<'a, 'db> {
1127411271
},
1127511272
/// Binds any `typing.Self` typevar with a particular `self` class.
1127611273
BindSelf(SelfBinding<'db>),
11277-
/// Replaces occurrences of `typing.Self` with a new `Self` type variable with the given upper bound.
11278-
ReplaceSelf { new_upper_bound: Type<'db> },
11274+
/// Replaces `typing.Self` when adapting a synthesized method to another class or schema.
11275+
ReplaceSelf {
11276+
new_origin: SelfTypeVarOrigin<'db>,
11277+
new_upper_bound: Type<'db>,
11278+
},
1127911279
/// Create the top or bottom materialization of a type.
1128011280
Materialize(MaterializationKind),
1128111281
/// Replace default types in parameters of callables with `Unknown`. This is used to avoid infinite
@@ -11358,23 +11358,25 @@ impl<'db> TypeMapping<'_, 'db> {
1135811358
Some(context)
1135911359
}
1136011360
}
11361-
TypeMapping::ReplaceSelf { new_upper_bound } => {
11362-
Some(GenericContext::from_typevar_instances(
11363-
db,
11364-
env,
11365-
context.variables(db).map(|typevar| {
11366-
if typevar.typevar(db).is_self(db) {
11367-
BoundTypeVarInstance::synthetic_self(
11368-
db,
11369-
*new_upper_bound,
11370-
typevar.binding_context(db),
11371-
)
11372-
} else {
11373-
typevar
11374-
}
11375-
}),
11376-
))
11377-
}
11361+
TypeMapping::ReplaceSelf {
11362+
new_origin,
11363+
new_upper_bound,
11364+
} => Some(GenericContext::from_typevar_instances(
11365+
db,
11366+
env,
11367+
context.variables(db).map(|typevar| {
11368+
if typevar.typevar(db).is_self(db) {
11369+
BoundTypeVarInstance::synthetic_self(
11370+
db,
11371+
*new_origin,
11372+
*new_upper_bound,
11373+
typevar.binding_context(db),
11374+
)
11375+
} else {
11376+
typevar
11377+
}
11378+
}),
11379+
)),
1137811380
}
1137911381
}
1138011382

@@ -12652,24 +12654,21 @@ impl<'db> TypeGuardLike<'db> for TypeGuardType<'db> {
1265212654
}
1265312655

1265412656
/// Walk the MRO of this class and return the last class just before the specified known base.
12655-
/// This can be used to determine upper bounds for `Self` type variables on methods that are
12656-
/// being added to the given class.
12657+
/// This determines the owner and upper bound of `Self` on synthesized fallback methods.
1265712658
///
1265812659
/// Preserve the class's specialization so that a method on `Child[int]` has a bound such as
1265912660
/// `Base[int]`, rather than retaining the type variable in `Base[T@Child]`.
12660-
pub(super) fn determine_upper_bound<'db>(
12661+
pub(super) fn determine_self_class<'db>(
1266112662
db: &'db dyn Db,
12662-
env: &ProgramEnvironment<'db>,
1266312663
class: ClassType<'db>,
1266412664
is_known_base: impl Fn(ClassBase<'db>) -> bool,
12665-
) -> Type<'db> {
12666-
let upper_bound = class
12665+
) -> ClassType<'db> {
12666+
class
1266712667
.iter_mro(db)
1266812668
.take_while(|base| !is_known_base(*base))
1266912669
.filter_map(ClassBase::into_class)
1267012670
.last()
12671-
.unwrap_or(class);
12672-
Type::instance(db, env, upper_bound)
12671+
.unwrap_or(class)
1267312672
}
1267412673

1267512674
// Make sure that the `Type` enum does not grow unexpectedly.

‎crates/ty_python_semantic/src/types/class/named_tuple.rs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ use crate::{
99
types::{
1010
BindingContext, BoundTypeVarInstance, ClassBase, ClassLiteral, ClassType, GenericContext,
1111
KnownClass, KnownInstanceType, MemberLookupPolicy, Parameter, Parameters,
12-
PropertyInstanceType, Signature, SubclassOfType, Type, TypeContext, TypeMapping,
12+
PropertyInstanceType, SelfTypeVarOrigin, Signature, SubclassOfType, Type, TypeContext,
13+
TypeMapping,
1314
class::{DynamicClassHeaderAnchor, DynamicClassScopeOffset, dynamic_class_header_range},
1415
definition_expression_type,
1516
member::Member,
@@ -30,15 +31,18 @@ pub(super) fn synthesize_namedtuple_class_member<'db>(
3031
db: &'db dyn Db,
3132
env: &ProgramEnvironment<'db>,
3233
name: &str,
33-
instance_ty: Type<'db>,
34+
class: ClassType<'db>,
3435
fields: impl Iterator<Item = NamedTupleField<'db>>,
3536
inherited_generic_context: Option<GenericContext<'db>>,
3637
) -> Option<Type<'db>> {
38+
let instance_ty = Type::instance(db, env, class);
39+
let origin = SelfTypeVarOrigin::Class(class.class_literal(db));
3740
match name {
3841
"__new__" => {
3942
// __new__(cls, field1, field2, ...) -> Self
4043
let self_typevar = BoundTypeVarInstance::synthetic_self(
4144
db,
45+
origin,
4246
instance_ty,
4347
BindingContext::Synthetic(env.program(db)),
4448
);
@@ -96,6 +100,7 @@ pub(super) fn synthesize_namedtuple_class_member<'db>(
96100
// _replace(self, *, field1=..., field2=...) -> Self
97101
let self_ty = Type::TypeVar(BoundTypeVarInstance::synthetic_self(
98102
db,
103+
origin,
99104
instance_ty,
100105
BindingContext::Synthetic(env.program(db)),
101106
));
@@ -398,6 +403,7 @@ impl<'db> DynamicNamedTupleLiteral<'db> {
398403
db,
399404
env,
400405
&TypeMapping::ReplaceSelf {
406+
new_origin: SelfTypeVarOrigin::Class(self.into()),
401407
new_upper_bound: instance_ty,
402408
},
403409
TypeContext::default(),
@@ -412,7 +418,7 @@ impl<'db> DynamicNamedTupleLiteral<'db> {
412418
db,
413419
env,
414420
name,
415-
instance_ty,
421+
ClassType::NonGeneric(self.into()),
416422
self.fields(db).iter().cloned(),
417423
None,
418424
);
@@ -430,6 +436,7 @@ impl<'db> DynamicNamedTupleLiteral<'db> {
430436
db,
431437
env,
432438
&TypeMapping::ReplaceSelf {
439+
new_origin: SelfTypeVarOrigin::Class(self.into()),
433440
new_upper_bound: instance_ty,
434441
},
435442
TypeContext::default(),

‎crates/ty_python_semantic/src/types/class/static_literal.rs‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ use crate::{
2525
CallableType, ClassBase, ClassLiteral, ClassType, DATACLASS_FLAGS, DataclassFlags,
2626
DataclassParams, GenericAlias, GenericContext, KnownClass, KnownInstanceType,
2727
MaterializationKind, MemberLookupPolicy, MetaclassCandidate, MetaclassTransformInfo,
28-
Parameter, Parameters, PropertyInstanceType, Signature, SpecialFormType, StaticMroError,
29-
SubclassOfType, Type, TypeContext, TypeMapping, TypeVarVariance, TypingModule,
30-
UnionBuilder, UnionType,
28+
Parameter, Parameters, PropertyInstanceType, SelfTypeVarOrigin, Signature, SpecialFormType,
29+
StaticMroError, SubclassOfType, Type, TypeContext, TypeMapping, TypeVarVariance,
30+
TypingModule, UnionBuilder, UnionType,
3131
attribute_write::DescriptorSetterDomain,
3232
bound_super::BoundSuperType,
3333
call::{CallError, CallErrorKind},
@@ -41,7 +41,7 @@ use crate::{
4141
},
4242
context::InferContext,
4343
dedicated::pydantic,
44-
definition_expression_type, determine_upper_bound,
44+
definition_expression_type, determine_self_class,
4545
diagnostic::INVALID_DATACLASS_OVERRIDE,
4646
enums::{enum_metadata, is_enum_class_by_inheritance, try_unwrap_nonmember_value},
4747
function::{DataclassTransformerParams, KnownFunction},
@@ -2031,6 +2031,7 @@ impl<'db> StaticClassLiteral<'db> {
20312031
db,
20322032
env,
20332033
&TypeMapping::ReplaceSelf {
2034+
new_origin: SelfTypeVarOrigin::Class(self.into()),
20342035
new_upper_bound: instance_ty,
20352036
},
20362037
TypeContext::default(),
@@ -2058,7 +2059,7 @@ impl<'db> StaticClassLiteral<'db> {
20582059
db,
20592060
env,
20602061
name,
2061-
instance_ty,
2062+
self.apply_optional_specialization(db, specialization),
20622063
fields_iter,
20632064
specialization.map(|s| s.generic_context(db)),
20642065
)
@@ -2139,19 +2140,20 @@ impl<'db> StaticClassLiteral<'db> {
21392140
.own_class_member(db, env, self.inherited_generic_context(db), None, name)
21402141
.ignore_possibly_undefined()
21412142
.map(|ty| {
2143+
let self_class = determine_self_class(
2144+
db,
2145+
self.apply_optional_specialization(db, specialization),
2146+
|base| {
2147+
base.into_class()
2148+
.is_some_and(|c| c.is_known(db, KnownClass::Tuple))
2149+
},
2150+
);
21422151
ty.apply_type_mapping(
21432152
db,
21442153
env,
21452154
&TypeMapping::ReplaceSelf {
2146-
new_upper_bound: determine_upper_bound(
2147-
db,
2148-
env,
2149-
self.apply_optional_specialization(db, specialization),
2150-
|base| {
2151-
base.into_class()
2152-
.is_some_and(|c| c.is_known(db, KnownClass::Tuple))
2153-
},
2154-
),
2155+
new_origin: SelfTypeVarOrigin::Class(self_class.class_literal(db)),
2156+
new_upper_bound: Type::instance(db, env, self_class),
21552157
},
21562158
TypeContext::default(),
21572159
)
@@ -2940,6 +2942,7 @@ impl<'db> StaticClassLiteral<'db> {
29402942
db,
29412943
env,
29422944
&TypeMapping::ReplaceSelf {
2945+
new_origin: SelfTypeVarOrigin::Class(self.into()),
29432946
new_upper_bound: Type::instance(
29442947
db,
29452948
env,

0 commit comments

Comments
 (0)