[ty] Don't confuse multiple occurrences of `typing.Self` when binding bound methods (#21754)

In the following example, there are two occurrences of `typing.Self`,
one for `Foo.foo` and one for `Bar.bar`:

```py
from typing import Self, reveal_type

class Foo[T]:
    def foo(self: Self) -> T:
        raise NotImplementedError

class Bar:
    def bar(self: Self, x: Foo[Self]):
        # SHOULD BE: bound method Foo[Self@bar].foo() -> Self@bar
        # revealed: bound method Foo[Self@bar].foo() -> Foo[Self@bar]
        reveal_type(x.foo)

def f[U: Bar](x: Foo[U]):
    # revealed: bound method Foo[U@f].foo() -> U@f
    reveal_type(x.foo)
```

When accessing a bound method, we replace any occurrences of `Self` with
the bound `self` type.

We were doing this correctly for the second reveal. We would first apply
the specialization, getting `(self: Self@foo) -> U@F` as the signature
of `x.foo`. We would then bind the `self` parameter, substituting
`Self@foo` with `Foo[U@F]` as part of that. The return type was already
specialized to `U@F`, so that substitution had no further affect on the
type that we revealed.

In the first reveal, we would follow the same process, but we confused
the two occurrences of `Self`. We would first apply the specialization,
getting `(self: Self@foo) -> Self@bar` as the method signature. We would
then try to bind the `self` parameter, substituting `Self@foo` with
`Foo[Self@bar]`. However, because we didn't distinguish the two separate
`Self`s, and applied the substitution to the return type as well as to
the `self` parameter.

The fix is to track which particular `Self` we're trying to substitute
when applying the type mapping.

Fixes https://github.com/astral-sh/ty/issues/1713
This commit is contained in:
Douglas Creager 2025-12-02 13:15:09 -05:00 committed by GitHub
parent 0d2792517d
commit 508c0a0861
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 64 additions and 31 deletions

View File

@ -232,6 +232,32 @@ class C:
reveal_type(not_a_method) # revealed: def not_a_method(self) -> Unknown reveal_type(not_a_method) # revealed: def not_a_method(self) -> Unknown
``` ```
## Different occurrences of `Self` represent different types
Here, both `Foo.foo` and `Bar.bar` use `Self`. When accessing a bound method, we replace any
occurrences of `Self` with the bound `self` type. In this example, when we access `x.foo`, we only
want to substitute the occurrences of `Self` in `Foo.foo` — that is, occurrences of `Self@foo`. The
fact that `x` is an instance of `Foo[Self@bar]` (a completely different `Self` type) should not
affect that subtitution. If we blindly substitute all occurrences of `Self`, we would get
`Foo[Self@bar]` as the return type of the bound method.
```py
from typing import Self
class Foo[T]:
def foo(self: Self) -> T:
raise NotImplementedError
class Bar:
def bar(self: Self, x: Foo[Self]):
# revealed: bound method Foo[Self@bar].foo() -> Self@bar
reveal_type(x.foo)
def f[U: Bar](x: Foo[U]):
# revealed: bound method Foo[U@f].foo() -> U@f
reveal_type(x.foo)
```
## typing_extensions ## typing_extensions
```toml ```toml

View File

@ -7550,10 +7550,9 @@ impl<'db> Type<'db> {
// If we are binding `typing.Self`, and this type is what we are binding `Self` to, return // If we are binding `typing.Self`, and this type is what we are binding `Self` to, return
// early. This is not just an optimization, it also prevents us from infinitely expanding // early. This is not just an optimization, it also prevents us from infinitely expanding
// the type, if it's something that can contain a `Self` reference. // the type, if it's something that can contain a `Self` reference.
if let TypeMapping::BindSelf(self_type) = type_mapping match type_mapping {
&& self == *self_type TypeMapping::BindSelf { self_type, .. } if self == *self_type => return self,
{ _ => {}
return self;
} }
match self { match self {
@ -7568,7 +7567,7 @@ impl<'db> Type<'db> {
TypeMapping::Specialization(_) | TypeMapping::Specialization(_) |
TypeMapping::PartialSpecialization(_) | TypeMapping::PartialSpecialization(_) |
TypeMapping::PromoteLiterals(_) | TypeMapping::PromoteLiterals(_) |
TypeMapping::BindSelf(_) | TypeMapping::BindSelf { .. } |
TypeMapping::ReplaceSelf { .. } | TypeMapping::ReplaceSelf { .. } |
TypeMapping::Materialize(_) | TypeMapping::Materialize(_) |
TypeMapping::ReplaceParameterDefaults | TypeMapping::ReplaceParameterDefaults |
@ -7744,7 +7743,7 @@ impl<'db> Type<'db> {
TypeMapping::Specialization(_) | TypeMapping::Specialization(_) |
TypeMapping::PartialSpecialization(_) | TypeMapping::PartialSpecialization(_) |
TypeMapping::BindLegacyTypevars(_) | TypeMapping::BindLegacyTypevars(_) |
TypeMapping::BindSelf(_) | TypeMapping::BindSelf { .. } |
TypeMapping::ReplaceSelf { .. } | TypeMapping::ReplaceSelf { .. } |
TypeMapping::Materialize(_) | TypeMapping::Materialize(_) |
TypeMapping::ReplaceParameterDefaults | TypeMapping::ReplaceParameterDefaults |
@ -7757,7 +7756,7 @@ impl<'db> Type<'db> {
TypeMapping::Specialization(_) | TypeMapping::Specialization(_) |
TypeMapping::PartialSpecialization(_) | TypeMapping::PartialSpecialization(_) |
TypeMapping::BindLegacyTypevars(_) | TypeMapping::BindLegacyTypevars(_) |
TypeMapping::BindSelf(_) | TypeMapping::BindSelf { .. } |
TypeMapping::ReplaceSelf { .. } | TypeMapping::ReplaceSelf { .. } |
TypeMapping::PromoteLiterals(_) | TypeMapping::PromoteLiterals(_) |
TypeMapping::ReplaceParameterDefaults | TypeMapping::ReplaceParameterDefaults |
@ -8421,7 +8420,10 @@ pub enum TypeMapping<'a, 'db> {
/// being used in. /// being used in.
BindLegacyTypevars(BindingContext<'db>), BindLegacyTypevars(BindingContext<'db>),
/// Binds any `typing.Self` typevar with a particular `self` class. /// Binds any `typing.Self` typevar with a particular `self` class.
BindSelf(Type<'db>), BindSelf {
self_type: Type<'db>,
binding_context: Option<BindingContext<'db>>,
},
/// Replaces occurrences of `typing.Self` with a new `Self` type variable with the given upper bound. /// Replaces occurrences of `typing.Self` with a new `Self` type variable with the given upper bound.
ReplaceSelf { new_upper_bound: Type<'db> }, ReplaceSelf { new_upper_bound: Type<'db> },
/// Create the top or bottom materialization of a type. /// Create the top or bottom materialization of a type.
@ -8449,7 +8451,7 @@ impl<'db> TypeMapping<'_, 'db> {
| TypeMapping::Materialize(_) | TypeMapping::Materialize(_)
| TypeMapping::ReplaceParameterDefaults | TypeMapping::ReplaceParameterDefaults
| TypeMapping::EagerExpansion => context, | TypeMapping::EagerExpansion => context,
TypeMapping::BindSelf(_) => GenericContext::from_typevar_instances( TypeMapping::BindSelf { .. } => GenericContext::from_typevar_instances(
db, db,
context context
.variables(db) .variables(db)
@ -8482,7 +8484,7 @@ impl<'db> TypeMapping<'_, 'db> {
TypeMapping::Specialization(_) TypeMapping::Specialization(_)
| TypeMapping::PartialSpecialization(_) | TypeMapping::PartialSpecialization(_)
| TypeMapping::BindLegacyTypevars(_) | TypeMapping::BindLegacyTypevars(_)
| TypeMapping::BindSelf(_) | TypeMapping::BindSelf { .. }
| TypeMapping::ReplaceSelf { .. } | TypeMapping::ReplaceSelf { .. }
| TypeMapping::ReplaceParameterDefaults | TypeMapping::ReplaceParameterDefaults
| TypeMapping::EagerExpansion => self.clone(), | TypeMapping::EagerExpansion => self.clone(),
@ -9837,8 +9839,13 @@ impl<'db> BoundTypeVarInstance<'db> {
TypeMapping::PartialSpecialization(partial) => { TypeMapping::PartialSpecialization(partial) => {
partial.get(db, self).unwrap_or(Type::TypeVar(self)) partial.get(db, self).unwrap_or(Type::TypeVar(self))
} }
TypeMapping::BindSelf(self_type) => { TypeMapping::BindSelf {
if self.typevar(db).is_self(db) { self_type,
binding_context,
} => {
if self.typevar(db).is_self(db)
&& binding_context.is_none_or(|context| self.binding_context(db) == context)
{
*self_type *self_type
} else { } else {
Type::TypeVar(self) Type::TypeVar(self)

View File

@ -29,9 +29,10 @@ use crate::types::generics::{
}; };
use crate::types::infer::nearest_enclosing_class; use crate::types::infer::nearest_enclosing_class;
use crate::types::{ use crate::types::{
ApplyTypeMappingVisitor, BoundTypeVarInstance, ClassLiteral, FindLegacyTypeVarsVisitor, ApplyTypeMappingVisitor, BindingContext, BoundTypeVarInstance, ClassLiteral,
HasRelationToVisitor, IsDisjointVisitor, IsEquivalentVisitor, KnownClass, MaterializationKind, FindLegacyTypeVarsVisitor, HasRelationToVisitor, IsDisjointVisitor, IsEquivalentVisitor,
NormalizedVisitor, TypeContext, TypeMapping, TypeRelation, VarianceInferable, todo_type, KnownClass, MaterializationKind, NormalizedVisitor, TypeContext, TypeMapping, TypeRelation,
VarianceInferable, todo_type,
}; };
use crate::{Db, FxOrderSet}; use crate::{Db, FxOrderSet};
use ruff_python_ast::{self as ast, name::Name}; use ruff_python_ast::{self as ast, name::Name};
@ -667,19 +668,18 @@ impl<'db> Signature<'db> {
let mut parameters = Parameters::new(db, parameters); let mut parameters = Parameters::new(db, parameters);
let mut return_ty = self.return_ty; let mut return_ty = self.return_ty;
if let Some(self_type) = self_type { if let Some(self_type) = self_type {
let self_mapping = TypeMapping::BindSelf {
self_type,
binding_context: self.definition.map(BindingContext::Definition),
};
parameters = parameters.apply_type_mapping_impl( parameters = parameters.apply_type_mapping_impl(
db, db,
&TypeMapping::BindSelf(self_type), &self_mapping,
TypeContext::default(), TypeContext::default(),
&ApplyTypeMappingVisitor::default(), &ApplyTypeMappingVisitor::default(),
); );
return_ty = return_ty.map(|ty| { return_ty = return_ty
ty.apply_type_mapping( .map(|ty| ty.apply_type_mapping(db, &self_mapping, TypeContext::default()));
db,
&TypeMapping::BindSelf(self_type),
TypeContext::default(),
)
});
} }
Self { Self {
generic_context: self.generic_context, generic_context: self.generic_context,
@ -690,19 +690,19 @@ impl<'db> Signature<'db> {
} }
pub(crate) fn apply_self(&self, db: &'db dyn Db, self_type: Type<'db>) -> Self { pub(crate) fn apply_self(&self, db: &'db dyn Db, self_type: Type<'db>) -> Self {
let self_mapping = TypeMapping::BindSelf {
self_type,
binding_context: self.definition.map(BindingContext::Definition),
};
let parameters = self.parameters.apply_type_mapping_impl( let parameters = self.parameters.apply_type_mapping_impl(
db, db,
&TypeMapping::BindSelf(self_type), &self_mapping,
TypeContext::default(), TypeContext::default(),
&ApplyTypeMappingVisitor::default(), &ApplyTypeMappingVisitor::default(),
); );
let return_ty = self.return_ty.map(|ty| { let return_ty = self
ty.apply_type_mapping( .return_ty
db, .map(|ty| ty.apply_type_mapping(db, &self_mapping, TypeContext::default()));
&TypeMapping::BindSelf(self_type),
TypeContext::default(),
)
});
Self { Self {
generic_context: self.generic_context, generic_context: self.generic_context,
definition: self.definition, definition: self.definition,