From 948f3f856cb47c29a556f1fff9ca23b0dd5a1106 Mon Sep 17 00:00:00 2001 From: David Peter Date: Tue, 5 Aug 2025 13:59:10 +0200 Subject: [PATCH] [ty] Fix attribute access on `TypedDict`s (#19758) ## Summary This PR fixes a few inaccuracies in attribute access on `TypedDict`s. It also changes the return type of `type(person)` to `type[dict[str, object]]` if `person: Person` is an inhabitant of a `TypedDict` `Person`. We still use `type[Person]` as the *meta type* of Person, however (see reasoning [here](https://github.com/astral-sh/ruff/pull/19733#discussion_r2253297926)). ## Test Plan Updated Markdown tests. --- crates/ty_ide/src/completion.rs | 72 +++++++++---------- .../mdtest/ide_support/all_members.md | 19 +++-- .../resources/mdtest/typed_dict.md | 44 +++++++++--- crates/ty_python_semantic/src/types.rs | 32 ++++++++- .../ty_python_semantic/src/types/call/bind.rs | 2 +- .../src/types/ide_support.rs | 24 +++++-- .../src/types/subclass_of.rs | 6 ++ 7 files changed, 139 insertions(+), 60 deletions(-) diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 830ee2c2c0..0c7453a892 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -1233,28 +1233,28 @@ quux. baz :: Unknown | Literal[3] foo :: Unknown | Literal[1] __annotations__ :: dict[str, Any] - __class__ :: type - __delattr__ :: bound method object.__delattr__(name: str, /) -> None + __class__ :: type[Quux] + __delattr__ :: bound method Quux.__delattr__(name: str, /) -> None __dict__ :: dict[str, Any] - __dir__ :: bound method object.__dir__() -> Iterable[str] + __dir__ :: bound method Quux.__dir__() -> Iterable[str] __doc__ :: str | None - __eq__ :: bound method object.__eq__(value: object, /) -> bool - __format__ :: bound method object.__format__(format_spec: str, /) -> str - __getattribute__ :: bound method object.__getattribute__(name: str, /) -> Any - __getstate__ :: bound method object.__getstate__() -> object - __hash__ :: bound method object.__hash__() -> int + __eq__ :: bound method Quux.__eq__(value: object, /) -> bool + __format__ :: bound method Quux.__format__(format_spec: str, /) -> str + __getattribute__ :: bound method Quux.__getattribute__(name: str, /) -> Any + __getstate__ :: bound method Quux.__getstate__() -> object + __hash__ :: bound method Quux.__hash__() -> int __init__ :: bound method Quux.__init__() -> Unknown - __init_subclass__ :: bound method object.__init_subclass__() -> None + __init_subclass__ :: bound method Quux.__init_subclass__() -> None __module__ :: str - __ne__ :: bound method object.__ne__(value: object, /) -> bool - __new__ :: bound method object.__new__() -> Self@object - __reduce__ :: bound method object.__reduce__() -> str | tuple[Any, ...] - __reduce_ex__ :: bound method object.__reduce_ex__(protocol: SupportsIndex, /) -> str | tuple[Any, ...] - __repr__ :: bound method object.__repr__() -> str - __setattr__ :: bound method object.__setattr__(name: str, value: Any, /) -> None - __sizeof__ :: bound method object.__sizeof__() -> int - __str__ :: bound method object.__str__() -> str - __subclasshook__ :: bound method type.__subclasshook__(subclass: type, /) -> bool + __ne__ :: bound method Quux.__ne__(value: object, /) -> bool + __new__ :: bound method Quux.__new__() -> Self@object + __reduce__ :: bound method Quux.__reduce__() -> str | tuple[Any, ...] + __reduce_ex__ :: bound method Quux.__reduce_ex__(protocol: SupportsIndex, /) -> str | tuple[Any, ...] + __repr__ :: bound method Quux.__repr__() -> str + __setattr__ :: bound method Quux.__setattr__(name: str, value: Any, /) -> None + __sizeof__ :: bound method Quux.__sizeof__() -> int + __str__ :: bound method Quux.__str__() -> str + __subclasshook__ :: bound method type[Quux].__subclasshook__(subclass: type, /) -> bool "); } @@ -1278,28 +1278,28 @@ quux.b baz :: Unknown | Literal[3] foo :: Unknown | Literal[1] __annotations__ :: dict[str, Any] - __class__ :: type - __delattr__ :: bound method object.__delattr__(name: str, /) -> None + __class__ :: type[Quux] + __delattr__ :: bound method Quux.__delattr__(name: str, /) -> None __dict__ :: dict[str, Any] - __dir__ :: bound method object.__dir__() -> Iterable[str] + __dir__ :: bound method Quux.__dir__() -> Iterable[str] __doc__ :: str | None - __eq__ :: bound method object.__eq__(value: object, /) -> bool - __format__ :: bound method object.__format__(format_spec: str, /) -> str - __getattribute__ :: bound method object.__getattribute__(name: str, /) -> Any - __getstate__ :: bound method object.__getstate__() -> object - __hash__ :: bound method object.__hash__() -> int + __eq__ :: bound method Quux.__eq__(value: object, /) -> bool + __format__ :: bound method Quux.__format__(format_spec: str, /) -> str + __getattribute__ :: bound method Quux.__getattribute__(name: str, /) -> Any + __getstate__ :: bound method Quux.__getstate__() -> object + __hash__ :: bound method Quux.__hash__() -> int __init__ :: bound method Quux.__init__() -> Unknown - __init_subclass__ :: bound method object.__init_subclass__() -> None + __init_subclass__ :: bound method Quux.__init_subclass__() -> None __module__ :: str - __ne__ :: bound method object.__ne__(value: object, /) -> bool - __new__ :: bound method object.__new__() -> Self@object - __reduce__ :: bound method object.__reduce__() -> str | tuple[Any, ...] - __reduce_ex__ :: bound method object.__reduce_ex__(protocol: SupportsIndex, /) -> str | tuple[Any, ...] - __repr__ :: bound method object.__repr__() -> str - __setattr__ :: bound method object.__setattr__(name: str, value: Any, /) -> None - __sizeof__ :: bound method object.__sizeof__() -> int - __str__ :: bound method object.__str__() -> str - __subclasshook__ :: bound method type.__subclasshook__(subclass: type, /) -> bool + __ne__ :: bound method Quux.__ne__(value: object, /) -> bool + __new__ :: bound method Quux.__new__() -> Self@object + __reduce__ :: bound method Quux.__reduce__() -> str | tuple[Any, ...] + __reduce_ex__ :: bound method Quux.__reduce_ex__(protocol: SupportsIndex, /) -> str | tuple[Any, ...] + __repr__ :: bound method Quux.__repr__() -> str + __setattr__ :: bound method Quux.__setattr__(name: str, value: Any, /) -> None + __sizeof__ :: bound method Quux.__sizeof__() -> int + __str__ :: bound method Quux.__str__() -> str + __subclasshook__ :: bound method type[Quux].__subclasshook__(subclass: type, /) -> bool "); } diff --git a/crates/ty_python_semantic/resources/mdtest/ide_support/all_members.md b/crates/ty_python_semantic/resources/mdtest/ide_support/all_members.md index d02317fc15..c7ec7a5fa6 100644 --- a/crates/ty_python_semantic/resources/mdtest/ide_support/all_members.md +++ b/crates/ty_python_semantic/resources/mdtest/ide_support/all_members.md @@ -219,18 +219,27 @@ class Person(TypedDict): age: int | None static_assert(not has_member(Person, "name")) -static_assert(not has_member(Person, "age")) - +static_assert(has_member(Person, "keys")) static_assert(has_member(Person, "__total__")) -static_assert(has_member(Person, "__required_keys__")) def _(person: Person): static_assert(not has_member(person, "name")) - static_assert(not has_member(person, "age")) - + static_assert(not has_member(person, "__total__")) static_assert(has_member(person, "keys")) + + # type(person) is `dict` at runtime, so `__total__` is not available: + static_assert(not has_member(type(person), "name")) + static_assert(not has_member(type(person), "__total__")) + static_assert(has_member(type(person), "keys")) + +def _(t_person: type[Person]): + static_assert(not has_member(t_person, "name")) + static_assert(has_member(t_person, "__total__")) + static_assert(has_member(t_person, "keys")) ``` +### Unions + For unions, `ide_support::all_members` only returns members that are available on all elements of the union. diff --git a/crates/ty_python_semantic/resources/mdtest/typed_dict.md b/crates/ty_python_semantic/resources/mdtest/typed_dict.md index dff4124977..2882397404 100644 --- a/crates/ty_python_semantic/resources/mdtest/typed_dict.md +++ b/crates/ty_python_semantic/resources/mdtest/typed_dict.md @@ -148,8 +148,8 @@ def _(p: Person) -> None: ## Unlike normal classes -`TypedDict` types are not like normal classes. The "attributes" can not be accessed. Neither on the -class itself, nor on inhabitants of the type defined by the class: +`TypedDict` types do not act like normal classes. For example, calling `type(..)` on an inhabitant +of a `TypedDict` type will return `dict`: ```py from typing import TypedDict @@ -158,6 +158,16 @@ class Person(TypedDict): name: str age: int | None +def _(p: Person) -> None: + reveal_type(type(p)) # revealed: + + reveal_type(p.__class__) # revealed: +``` + +Also, the "attributes" on the class definition can not be accessed. Neither on the class itself, nor +on inhabitants of the type defined by the class: + +```py # error: [unresolved-attribute] "Type `` has no attribute `name`" Person.name @@ -168,6 +178,8 @@ def _(P: type[Person]): def _(p: Person) -> None: # error: [unresolved-attribute] "Type `Person` has no attribute `name`" p.name + + type(p).name # error: [unresolved-attribute] "Type `` has no attribute `name`" ``` ## Special properties @@ -190,20 +202,30 @@ These attributes can not be accessed on inhabitants: ```py def _(person: Person) -> None: - # TODO: these should be errors - person.__total__ - person.__required_keys__ - person.__optional_keys__ + person.__total__ # error: [unresolved-attribute] + person.__required_keys__ # error: [unresolved-attribute] + person.__optional_keys__ # error: [unresolved-attribute] ``` Also, they can not be accessed on `type(person)`, as that would be `dict` at runtime: ```py -def _(t_person: type[Person]) -> None: - # TODO: these should be errors - t_person.__total__ - t_person.__required_keys__ - t_person.__optional_keys__ +def _(person: Person) -> None: + type(person).__total__ # error: [unresolved-attribute] + type(person).__required_keys__ # error: [unresolved-attribute] + type(person).__optional_keys__ # error: [unresolved-attribute] +``` + +But they *can* be accessed on `type[Person]`, because this function would accept the class object +`Person` as an argument: + +```py +def accepts_typed_dict_class(t_person: type[Person]) -> None: + reveal_type(t_person.__total__) # revealed: bool + reveal_type(t_person.__required_keys__) # revealed: frozenset[str] + reveal_type(t_person.__optional_keys__) # revealed: frozenset[str] + +accepts_typed_dict_class(Person) ``` ## Subclassing diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 86c14e0827..c171013e20 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/crates/ty_python_semantic/src/types.rs @@ -669,6 +669,10 @@ impl<'db> Type<'db> { matches!(self, Type::Dynamic(_)) } + pub(crate) const fn is_typed_dict(&self) -> bool { + matches!(self, Type::TypedDict(..)) + } + /// Returns the top materialization (or upper bound materialization) of this type, which is the /// most general form of the type that is fully static. #[must_use] @@ -3108,7 +3112,7 @@ impl<'db> Type<'db> { ) -> PlaceAndQualifiers<'db> { tracing::trace!("member_lookup_with_policy: {}.{}", self.display(db), name); if name == "__class__" { - return Place::bound(self.to_meta_type(db)).into(); + return Place::bound(self.dunder_class(db)).into(); } let name_str = name.as_str(); @@ -3325,6 +3329,12 @@ impl<'db> Type<'db> { .into() }; + if result.is_class_var() && self.is_typed_dict() { + // `ClassVar`s on `TypedDictFallback` can not be accessed on inhabitants of `SomeTypedDict`. + // They can only be accessed on `SomeTypedDict` directly. + return Place::Unbound.into(); + } + match result { member @ PlaceAndQualifiers { place: Place::Type(_, Boundness::Bound), @@ -5533,6 +5543,9 @@ impl<'db> Type<'db> { /// Given a type that is assumed to represent an instance of a class, /// return a type that represents that class itself. + /// + /// Note: the return type of `type(obj)` is subtly different from this. + /// See `Self::dunder_class` for more details. #[must_use] pub fn to_meta_type(&self, db: &'db dyn Db) -> Type<'db> { match self { @@ -5595,6 +5608,23 @@ impl<'db> Type<'db> { } } + /// Get the type of the `__class__` attribute of this type. + /// + /// For most types, this is equivalent to the meta type of this type. For `TypedDict` types, + /// this returns `type[dict[str, object]]` instead, because inhabitants of a `TypedDict` are + /// instances of `dict` at runtime. + #[must_use] + pub fn dunder_class(self, db: &'db dyn Db) -> Type<'db> { + if self.is_typed_dict() { + return KnownClass::Dict + .to_specialized_class_type(db, [KnownClass::Str.to_instance(db), Type::object(db)]) + .map(Type::from) + .unwrap_or_else(Type::unknown); + } + + self.to_meta_type(db) + } + #[must_use] pub fn apply_optional_specialization( self, diff --git a/crates/ty_python_semantic/src/types/call/bind.rs b/crates/ty_python_semantic/src/types/call/bind.rs index 003d06d7ae..035c9593a4 100644 --- a/crates/ty_python_semantic/src/types/call/bind.rs +++ b/crates/ty_python_semantic/src/types/call/bind.rs @@ -1010,7 +1010,7 @@ impl<'db> Bindings<'db> { Some(KnownClass::Type) if overload_index == 0 => { if let [Some(arg)] = overload.parameter_types() { - overload.set_return_type(arg.to_meta_type(db)); + overload.set_return_type(arg.dunder_class(db)); } } diff --git a/crates/ty_python_semantic/src/types/ide_support.rs b/crates/ty_python_semantic/src/types/ide_support.rs index ba6d458c60..06d30244f6 100644 --- a/crates/ty_python_semantic/src/types/ide_support.rs +++ b/crates/ty_python_semantic/src/types/ide_support.rs @@ -95,7 +95,7 @@ impl<'db> AllMembers<'db> { Type::NominalInstance(instance) => { let (class_literal, _specialization) = instance.class.class_literal(db); - self.extend_with_instance_members(db, class_literal); + self.extend_with_instance_members(db, ty, class_literal); } Type::ClassLiteral(class_literal) if class_literal.is_typed_dict(db) => { @@ -106,6 +106,10 @@ impl<'db> AllMembers<'db> { self.extend_with_type(db, KnownClass::TypedDictFallback.to_class_literal(db)); } + Type::SubclassOf(subclass_of_type) if subclass_of_type.is_typed_dict(db) => { + self.extend_with_type(db, KnownClass::TypedDictFallback.to_class_literal(db)); + } + Type::ClassLiteral(class_literal) => { self.extend_with_class_members(db, ty, class_literal); @@ -168,7 +172,11 @@ impl<'db> AllMembers<'db> { self.extend_with_class_members(db, ty, class_literal); } - self.extend_with_type(db, KnownClass::TypedDictFallback.to_instance(db)); + if let Type::ClassLiteral(class) = + KnownClass::TypedDictFallback.to_class_literal(db) + { + self.extend_with_instance_members(db, ty, class); + } } Type::ModuleLiteral(literal) => { @@ -281,13 +289,17 @@ impl<'db> AllMembers<'db> { } } - fn extend_with_instance_members(&mut self, db: &'db dyn Db, class_literal: ClassLiteral<'db>) { + fn extend_with_instance_members( + &mut self, + db: &'db dyn Db, + ty: Type<'db>, + class_literal: ClassLiteral<'db>, + ) { for parent in class_literal .iter_mro(db, None) .filter_map(ClassBase::into_class) .map(|class| class.class_literal(db).0) { - let parent_instance = Type::instance(db, parent.default_specialization(db)); let class_body_scope = parent.body_scope(db); let file = class_body_scope.file(db); let index = semantic_index(db, file); @@ -297,7 +309,7 @@ impl<'db> AllMembers<'db> { let Some(name) = place_expr.as_instance_attribute() else { continue; }; - let result = parent_instance.member(db, name.as_str()); + let result = ty.member(db, name.as_str()); let Some(ty) = result.place.ignore_possibly_unbound() else { continue; }; @@ -314,7 +326,7 @@ impl<'db> AllMembers<'db> { // member, e.g., `SomeClass.__delattr__` is not a bound // method, but `instance_of_SomeClass.__delattr__` is. for Member { name, .. } in all_declarations_and_bindings(db, class_body_scope) { - let result = parent_instance.member(db, name.as_str()); + let result = ty.member(db, name.as_str()); let Some(ty) = result.place.ignore_possibly_unbound() else { continue; }; diff --git a/crates/ty_python_semantic/src/types/subclass_of.rs b/crates/ty_python_semantic/src/types/subclass_of.rs index 88eedf2139..e69e5f9500 100644 --- a/crates/ty_python_semantic/src/types/subclass_of.rs +++ b/crates/ty_python_semantic/src/types/subclass_of.rs @@ -196,6 +196,12 @@ impl<'db> SubclassOfType<'db> { SubclassOfInner::Dynamic(dynamic_type) => Type::Dynamic(dynamic_type), } } + + pub(crate) fn is_typed_dict(self, db: &'db dyn Db) -> bool { + self.subclass_of + .into_class() + .is_some_and(|class| class.class_literal(db).0.is_typed_dict(db)) + } } /// An enumeration of the different kinds of `type[]` types that a [`SubclassOfType`] can represent: