Skip to content

Commit 229e49f

Browse files
authored
[ty] Fix instance-attribute lookup in methods of protocol classes (astral-sh#24213)
## Summary Our existing `@Todo` type for the meta-type of protocols was being applied too broadly. The intent was only ever to use a `@Todo` type for `type[P]` when a user had actually written such an annotation in an annotation expression. This PR makes the situations where we infer this `@Todo` type much narrower, reflecting the original intent and allowing us to fix a bunch of TODOs in our test suite. ## Test Plan mdtests updated
1 parent 466167e commit 229e49f

7 files changed

Lines changed: 43 additions & 18 deletions

File tree

crates/ruff_benchmark/benches/ty_walltime.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ static ALTAIR: Benchmark = Benchmark::new(
109109
max_dep_date: "2025-06-17",
110110
python_version: PythonVersion::PY312,
111111
},
112-
898,
112+
950,
113113
);
114114

115115
static COLOUR_SCIENCE: Benchmark = Benchmark::new(

crates/ty_python_semantic/resources/mdtest/annotations/self.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -698,8 +698,7 @@ class Linkable(Protocol):
698698
return self.next_node
699699

700700
def _(l: Linkable) -> None:
701-
# TODO: Should be `Linkable`
702-
reveal_type(l.next_node) # revealed: @Todo(type[T] for protocols)
701+
reveal_type(l.next_node) # revealed: Linkable
703702

704703
class CopyableImpl:
705704
def copy(self) -> Self:

crates/ty_python_semantic/resources/mdtest/protocols.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1579,6 +1579,29 @@ as something that must be supported by type checkers:
15791579
> To distinguish between protocol class variables and protocol instance variables, the special
15801580
> `ClassVar` annotation should be used.
15811581
1582+
## Declared instance attribute members
1583+
1584+
Declared protocol instance attributes should be available both on protocol-typed values and through
1585+
`self` inside protocol methods, with `Self` rebinding appropriately.
1586+
1587+
```py
1588+
from typing import Protocol
1589+
from typing_extensions import Self
1590+
1591+
class Linked(Protocol):
1592+
value: int
1593+
next: Self
1594+
1595+
def advance(self) -> Self:
1596+
reveal_type(self.value) # revealed: int
1597+
reveal_type(self.next) # revealed: Self@advance
1598+
return self.next
1599+
1600+
def f(x: Linked) -> None:
1601+
reveal_type(x.value) # revealed: int
1602+
reveal_type(x.next) # revealed: Linked
1603+
```
1604+
15821605
## Subtyping of protocols with property members
15831606

15841607
A read-only property on a protocol can be satisfied by a mutable attribute, a read-only property, a

crates/ty_python_semantic/resources/mdtest/type_qualifiers/final.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ class Foo(Protocol):
731731
value: Final[int] = 42
732732

733733
def foo(self, value: int):
734-
# TODO: should emit an invalid-assignment error
734+
# error: [invalid-assignment] "Cannot assign to final attribute `value` on type `Self@foo`: `Final` attributes can only be assigned in the class body or `__init__`"
735735
self.value = value
736736

737737
def bar(x: Foo, value: int):

crates/ty_python_semantic/src/types.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6254,10 +6254,8 @@ fn self_typevar_owner_class_literal<'db>(
62546254
bound_typevar
62556255
.typevar(db)
62566256
.upper_bound(db)
6257-
.and_then(|ty| match ty {
6258-
Type::NominalInstance(instance) => Some(instance.class_literal(db)),
6259-
_ => None,
6260-
})
6257+
.and_then(|ty| ty.nominal_class(db))
6258+
.map(|class| class.class_literal(db))
62616259
}
62626260

62636261
#[salsa::tracked(returns(ref), heap_size=ruff_memory_usage::heap_size)]
@@ -6300,11 +6298,12 @@ impl<'db> SelfBinding<'db> {
63006298
binding_context: Option<BindingContext<'db>>,
63016299
) -> Self {
63026300
let class_literal = match self_type {
6303-
Type::NominalInstance(instance) => Some(instance.class_literal(db)),
63046301
Type::TypeVar(typevar) if typevar.typevar(db).is_self(db) => {
63056302
self_typevar_owner_class_literal(db, typevar)
63066303
}
6307-
_ => None,
6304+
_ => self_type
6305+
.nominal_class(db)
6306+
.map(|class| class.class_literal(db)),
63086307
};
63096308

63106309
Self {

crates/ty_python_semantic/src/types/infer/builder/type_expression.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,12 @@ impl<'db> TypeInferenceBuilder<'db, '_> {
908908

909909
let infer_type_argument = |builder: &mut Self, slice: &ast::Expr| {
910910
let slice_ty = builder.infer_type_expression(slice);
911+
if matches!(slice_ty, Type::ProtocolInstance(_)) {
912+
return SubclassOfType::from(
913+
builder.db(),
914+
todo_type!("type[T] for protocols").expect_dynamic(),
915+
);
916+
}
911917
SubclassOfType::try_from_instance(builder.db(), slice_ty).unwrap_or_else(|| {
912918
match slice_ty {
913919
Type::Callable(_) => invalid_type_argument(builder, slice),

crates/ty_python_semantic/src/types/subclass_of.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,18 @@ impl<'db> SubclassOfType<'db> {
8080
pub(crate) fn try_from_instance(db: &'db dyn Db, ty: Type<'db>) -> Option<Type<'db>> {
8181
// Handle unions by distributing `type[]` over each element:
8282
// `type[A | B]` -> `type[A] | type[B]`
83-
if let Type::Union(union) = ty {
84-
return UnionType::try_from_elements(
83+
match ty {
84+
Type::Union(union) => UnionType::try_from_elements(
8585
db,
8686
union
8787
.elements(db)
8888
.iter()
8989
.map(|element| Self::try_from_instance(db, *element)),
90-
);
90+
),
91+
Type::ProtocolInstance(protocol) => Some(protocol.to_meta_type(db)),
92+
_ => SubclassOfInner::try_from_instance(db, ty)
93+
.map(|subclass_of| Self::from(db, subclass_of)),
9194
}
92-
93-
SubclassOfInner::try_from_instance(db, ty).map(|subclass_of| Self::from(db, subclass_of))
9495
}
9596

9697
/// Return a [`Type`] instance representing the type `type[Unknown]`.
@@ -423,9 +424,6 @@ impl<'db> SubclassOfInner<'db> {
423424
Type::TypeVar(bound_typevar) => SubclassOfInner::TypeVar(bound_typevar),
424425
Type::Dynamic(DynamicType::Any) => SubclassOfInner::Dynamic(DynamicType::Any),
425426
Type::Dynamic(DynamicType::Unknown) => SubclassOfInner::Dynamic(DynamicType::Unknown),
426-
Type::ProtocolInstance(_) => {
427-
SubclassOfInner::Dynamic(todo_type!("type[T] for protocols").expect_dynamic())
428-
}
429427
_ => return None,
430428
})
431429
}

0 commit comments

Comments
 (0)