diff --git a/crates/ty_python_semantic/resources/mdtest/attributes.md b/crates/ty_python_semantic/resources/mdtest/attributes.md index 60aaa6830606c..bff72953e2d82 100644 --- a/crates/ty_python_semantic/resources/mdtest/attributes.md +++ b/crates/ty_python_semantic/resources/mdtest/attributes.md @@ -901,24 +901,21 @@ reveal_type(Derived().redeclared_with_wider_type) # revealed: str | int | None reveal_type(Derived.overwritten_in_subclass_body) # revealed: Unknown | None reveal_type(Derived().overwritten_in_subclass_body) # revealed: Unknown | None | str -# TODO: Both of these should be `str` reveal_type(Derived.overwritten_in_subclass_method) # revealed: str -reveal_type(Derived().overwritten_in_subclass_method) # revealed: str | Unknown | None +reveal_type(Derived().overwritten_in_subclass_method) # revealed: str reveal_type(Derived().pure_attribute) # revealed: str | None # TODO: This should be `str` reveal_type(Derived().pure_overwritten_in_subclass_body) # revealed: Unknown | None | str -# TODO: This should be `str` -reveal_type(Derived().pure_overwritten_in_subclass_method) # revealed: Unknown | None +reveal_type(Derived().pure_overwritten_in_subclass_method) # revealed: str # TODO: Both of these should be `Unknown | Literal["intermediate", "base"]` reveal_type(Derived.undeclared) # revealed: Unknown | Literal["intermediate"] reveal_type(Derived().undeclared) # revealed: Unknown | Literal["intermediate"] -# TODO: This should be `Unknown | Literal["intermediate", "base"]` -reveal_type(Derived().pure_undeclared) # revealed: Unknown | Literal["intermediate"] +reveal_type(Derived().pure_undeclared) # revealed: Unknown | Literal["intermediate", "base"] ``` ## Accessing attributes on class objects diff --git a/crates/ty_python_semantic/src/lib.rs b/crates/ty_python_semantic/src/lib.rs index dbe07aa600ddc..e0619d9f2049a 100644 --- a/crates/ty_python_semantic/src/lib.rs +++ b/crates/ty_python_semantic/src/lib.rs @@ -34,6 +34,7 @@ mod db; mod dunder_all; pub mod lint; pub(crate) mod list; +mod member; mod module_name; mod module_resolver; mod node_key; diff --git a/crates/ty_python_semantic/src/member.rs b/crates/ty_python_semantic/src/member.rs new file mode 100644 index 0000000000000..0c078d1fb1029 --- /dev/null +++ b/crates/ty_python_semantic/src/member.rs @@ -0,0 +1,73 @@ +use crate::{ + place::{Place, PlaceAndQualifiers}, + types::Type, +}; + +/// The return type of certain member-lookup operations. Contains information +/// about the type, type qualifiers, boundness/declaredness, and additional +/// metadata (e.g. whether or not the member was declared) +#[derive(Debug, Clone, PartialEq, Eq, salsa::Update, get_size2::GetSize)] +pub(crate) struct Member<'db> { + /// Type, qualifiers, and boundness information of this member + pub(crate) inner: PlaceAndQualifiers<'db>, + + /// Whether or not this member was explicitly declared (e.g. `attr: int = 1` + /// on the class body or `self.attr: int = 1` in a class method), or if the + /// type was inferred (e.g. `attr = 1` on the class body or `self.attr = 1` + /// in a class method). + pub(crate) is_declared: bool, +} + +impl Default for Member<'_> { + fn default() -> Self { + Member::inferred(PlaceAndQualifiers::default()) + } +} + +impl<'db> Member<'db> { + /// Create a new [`Member`] whose type was inferred (rather than explicitly declared). + pub(crate) fn inferred(inner: PlaceAndQualifiers<'db>) -> Self { + Self { + inner, + is_declared: false, + } + } + + /// Create a new [`Member`] whose type was explicitly declared (rather than inferred). + pub(crate) fn declared(inner: PlaceAndQualifiers<'db>) -> Self { + Self { + inner, + is_declared: true, + } + } + + /// Create a new [`Member`] whose type was explicitly and definitively declared, i.e. + /// there is no control flow path in which it might be possibly undeclared. + pub(crate) fn definitely_declared(ty: Type<'db>) -> Self { + Self::declared(Place::bound(ty).into()) + } + + /// Represents the absence of a member. + pub(crate) fn unbound() -> Self { + Self::inferred(PlaceAndQualifiers::default()) + } + + /// Returns `true` if the inner place is unbound (i.e. there is no such member). + pub(crate) fn is_unbound(&self) -> bool { + self.inner.place.is_unbound() + } + + /// Returns the inner type, unless it is definitely unbound. + pub(crate) fn ignore_possibly_unbound(&self) -> Option> { + self.inner.place.ignore_possibly_unbound() + } + + /// Map a type transformation function over the type of this member. + #[must_use] + pub(crate) fn map_type(self, f: impl FnOnce(Type<'db>) -> Type<'db>) -> Self { + Self { + inner: self.inner.map_type(f), + is_declared: self.is_declared, + } + } +} diff --git a/crates/ty_python_semantic/src/place.rs b/crates/ty_python_semantic/src/place.rs index bf3388de61b7b..7495373b09572 100644 --- a/crates/ty_python_semantic/src/place.rs +++ b/crates/ty_python_semantic/src/place.rs @@ -1,6 +1,7 @@ use ruff_db::files::File; use crate::dunder_all::dunder_all_names; +use crate::member::Member; use crate::module_resolver::{KnownModule, file_to_module}; use crate::semantic_index::definition::{Definition, DefinitionState}; use crate::semantic_index::place::{PlaceExprRef, ScopedPlaceId}; @@ -232,13 +233,9 @@ pub(crate) fn place<'db>( ) } -/// Infer the public type of a class symbol (its type as seen from outside its scope) in the given +/// Infer the public type of a class member/symbol (its type as seen from outside its scope) in the given /// `scope`. -pub(crate) fn class_symbol<'db>( - db: &'db dyn Db, - scope: ScopeId<'db>, - name: &str, -) -> PlaceAndQualifiers<'db> { +pub(crate) fn class_member<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Member<'db> { place_table(db, scope) .symbol_id(name) .map(|symbol_id| { @@ -252,7 +249,7 @@ pub(crate) fn class_symbol<'db>( if !place_and_quals.place.is_unbound() && !place_and_quals.is_init_var() { // Trust the declared type if we see a class-level declaration - return place_and_quals; + return Member::declared(place_and_quals); } if let PlaceAndQualifiers { @@ -267,14 +264,14 @@ pub(crate) fn class_symbol<'db>( // TODO: we should not need to calculate inferred type second time. This is a temporary // solution until the notion of Boundness and Declaredness is split. See #16036, #16264 - match inferred { + Member::inferred(match inferred { Place::Unbound => Place::Unbound.with_qualifiers(qualifiers), Place::Type(_, boundness) => { Place::Type(ty, boundness).with_qualifiers(qualifiers) } - } + }) } else { - Place::Unbound.into() + Member::unbound() } }) .unwrap_or_default() diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 896c2730de0b1..718357cd63a54 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/crates/ty_python_semantic/src/types/class.rs @@ -7,6 +7,7 @@ use super::{ function::FunctionType, infer_expression_type, infer_unpack_types, }; use crate::FxOrderMap; +use crate::member::Member; use crate::module_resolver::KnownModule; use crate::semantic_index::definition::{Definition, DefinitionState}; use crate::semantic_index::scope::{NodeWithScopeKind, Scope}; @@ -37,7 +38,7 @@ use crate::{ Db, FxIndexMap, FxOrderSet, Program, module_resolver::file_to_module, place::{ - Boundness, LookupError, LookupResult, Place, PlaceAndQualifiers, class_symbol, + Boundness, LookupError, LookupResult, Place, PlaceAndQualifiers, class_member, known_module_symbol, place_from_bindings, place_from_declarations, }, semantic_index::{ @@ -97,12 +98,12 @@ fn inheritance_cycle_initial<'db>( fn implicit_attribute_recover<'db>( _db: &'db dyn Db, - _value: &PlaceAndQualifiers<'db>, + _value: &Member<'db>, _count: u32, _class_body_scope: ScopeId<'db>, _name: String, _target_method_decorator: MethodDecorator, -) -> salsa::CycleRecoveryAction> { +) -> salsa::CycleRecoveryAction> { salsa::CycleRecoveryAction::Iterate } @@ -111,8 +112,8 @@ fn implicit_attribute_initial<'db>( _class_body_scope: ScopeId<'db>, _name: String, _target_method_decorator: MethodDecorator, -) -> PlaceAndQualifiers<'db> { - Place::Unbound.into() +) -> Member<'db> { + Member::unbound() } fn try_mro_cycle_recover<'db>( @@ -731,7 +732,7 @@ impl<'db> ClassType<'db> { db: &'db dyn Db, inherited_generic_context: Option>, name: &str, - ) -> PlaceAndQualifiers<'db> { + ) -> Member<'db> { fn synthesize_getitem_overload_signature<'db>( index_annotation: Type<'db>, return_annotation: Type<'db>, @@ -769,7 +770,7 @@ impl<'db> ClassType<'db> { let synthesized_dunder_method = CallableType::function_like(db, Signature::new(parameters, Some(return_type))); - Place::bound(synthesized_dunder_method).into() + Member::definitely_declared(synthesized_dunder_method) }; match name { @@ -943,7 +944,7 @@ impl<'db> ClassType<'db> { CallableSignature::from_overloads(overload_signatures); let getitem_type = Type::Callable(CallableType::new(db, getitem_signature, true)); - Place::bound(getitem_type).into() + Member::definitely_declared(getitem_type) }) .unwrap_or_else(fallback_member_lookup) } @@ -1015,7 +1016,7 @@ impl<'db> ClassType<'db> { Signature::new_generic(inherited_generic_context, parameters, None), ); - Place::bound(synthesized_dunder).into() + Member::definitely_declared(synthesized_dunder) } _ => fallback_member_lookup(), @@ -1039,7 +1040,7 @@ impl<'db> ClassType<'db> { /// A helper function for `instance_member` that looks up the `name` attribute only on /// this class, not on its superclasses. - fn own_instance_member(self, db: &'db dyn Db, name: &str) -> PlaceAndQualifiers<'db> { + fn own_instance_member(self, db: &'db dyn Db, name: &str) -> Member<'db> { let (class_literal, specialization) = self.class_literal(db); class_literal .own_instance_member(db, name) @@ -2004,7 +2005,9 @@ impl<'db> ClassLiteral<'db> { lookup_result = lookup_result.or_else(|lookup_error| { lookup_error.or_fall_back_to( db, - class.own_class_member(db, self.inherited_generic_context(db), name), + class + .own_class_member(db, self.inherited_generic_context(db), name) + .inner, ) }); } @@ -2075,17 +2078,19 @@ impl<'db> ClassLiteral<'db> { inherited_generic_context: Option>, specialization: Option>, name: &str, - ) -> PlaceAndQualifiers<'db> { + ) -> Member<'db> { if name == "__dataclass_fields__" && self.dataclass_params(db).is_some() { // Make this class look like a subclass of the `DataClassInstance` protocol - return Place::bound(KnownClass::Dict.to_specialized_instance( - db, - [ - KnownClass::Str.to_instance(db), - KnownClass::Field.to_specialized_instance(db, [Type::any()]), - ], - )) - .with_qualifiers(TypeQualifiers::CLASS_VAR); + return Member::declared( + Place::bound(KnownClass::Dict.to_specialized_instance( + db, + [ + KnownClass::Str.to_instance(db), + KnownClass::Field.to_specialized_instance(db, [Type::any()]), + ], + )) + .with_qualifiers(TypeQualifiers::CLASS_VAR), + ); } if CodeGeneratorKind::NamedTuple.matches(db, self) { @@ -2099,12 +2104,12 @@ impl<'db> ClassLiteral<'db> { ); let property_getter = CallableType::single(db, property_getter_signature); let property = PropertyInstanceType::new(db, Some(property_getter), None); - return Place::bound(Type::PropertyInstance(property)).into(); + return Member::definitely_declared(Type::PropertyInstance(property)); } } let body_scope = self.body_scope(db); - let symbol = class_symbol(db, body_scope, name).map_type(|ty| { + let member = class_member(db, body_scope, name).map_type(|ty| { // The `__new__` and `__init__` members of a non-specialized generic class are handled // specially: they inherit the generic context of their class. That lets us treat them // as generic functions when constructing the class, and infer the specialization of @@ -2129,15 +2134,15 @@ impl<'db> ClassLiteral<'db> { } }); - if symbol.place.is_unbound() { + if member.is_unbound() { if let Some(synthesized_member) = self.own_synthesized_member(db, specialization, name) { - return Place::bound(synthesized_member).into(); + return Member::definitely_declared(synthesized_member); } // The symbol was not found in the class scope. It might still be implicitly defined in `@classmethod`s. return Self::implicit_attribute(db, body_scope, name, MethodDecorator::ClassMethod); } - symbol + member } /// Returns the type of a synthesized dataclass member like `__init__` or `__lt__`, or @@ -2310,7 +2315,6 @@ impl<'db> ClassLiteral<'db> { .to_class_literal(db) .into_class_literal()? .own_class_member(db, self.inherited_generic_context(db), None, name) - .place .ignore_possibly_unbound() .map(|ty| { ty.apply_type_mapping( @@ -2862,6 +2866,7 @@ impl<'db> ClassLiteral<'db> { let mut union = UnionBuilder::new(db); let mut union_qualifiers = TypeQualifiers::empty(); + let mut is_definitely_bound = false; for superclass in self.iter_mro(db, specialization) { match superclass { @@ -2874,27 +2879,32 @@ impl<'db> ClassLiteral<'db> { ); } ClassBase::Class(class) => { - if let member @ PlaceAndQualifiers { - place: Place::Type(ty, boundness), - qualifiers, + if let Member { + inner: + member @ PlaceAndQualifiers { + place: Place::Type(ty, boundness), + qualifiers, + }, + is_declared, } = class.own_instance_member(db, name) { - // TODO: We could raise a diagnostic here if there are conflicting type qualifiers - union_qualifiers |= qualifiers; - if boundness == Boundness::Bound { - if union.is_empty() { - // Short-circuit, no need to allocate inside the union builder + if is_declared { + // We found a definitely-declared attribute. Discard possibly collected + // inferred types from subclasses and return the declared type. return member; } - return Place::bound(union.add(ty).build()) - .with_qualifiers(union_qualifiers); + is_definitely_bound = true; } - // If we see a possibly-unbound symbol, we need to keep looking - // higher up in the MRO. + // If the attribute is not definitely declared on this class, keep looking higher + // up in the MRO, and build a union of all inferred types (and possibly-declared + // types): union = union.add(ty); + + // TODO: We could raise a diagnostic here if there are conflicting type qualifiers + union_qualifiers |= qualifiers; } } ClassBase::TypedDict => { @@ -2919,10 +2929,13 @@ impl<'db> ClassLiteral<'db> { if union.is_empty() { Place::Unbound.with_qualifiers(TypeQualifiers::empty()) } else { - // If we have reached this point, we know that we have only seen possibly-unbound places. - // This means that the final result is still possibly-unbound. + let boundness = if is_definitely_bound { + Boundness::Bound + } else { + Boundness::PossiblyUnbound + }; - Place::Type(union.build(), Boundness::PossiblyUnbound).with_qualifiers(union_qualifiers) + Place::Type(union.build(), boundness).with_qualifiers(union_qualifiers) } } @@ -2935,7 +2948,7 @@ impl<'db> ClassLiteral<'db> { class_body_scope: ScopeId<'db>, name: &str, target_method_decorator: MethodDecorator, - ) -> PlaceAndQualifiers<'db> { + ) -> Member<'db> { Self::implicit_attribute_inner( db, class_body_scope, @@ -2954,7 +2967,7 @@ impl<'db> ClassLiteral<'db> { class_body_scope: ScopeId<'db>, name: String, target_method_decorator: MethodDecorator, - ) -> PlaceAndQualifiers<'db> { + ) -> Member<'db> { // If we do not see any declarations of an attribute, neither in the class body nor in // any method, we build a union of `Unknown` with the inferred types of all bindings of // that attribute. We include `Unknown` in that union to account for the fact that the @@ -2973,8 +2986,8 @@ impl<'db> ClassLiteral<'db> { let is_valid_scope = |method_scope: &Scope| { if let Some(method_def) = method_scope.node().as_function() { let method_name = method_def.node(&module).name.as_str(); - if let Place::Type(Type::FunctionLiteral(method_type), _) = - class_symbol(db, class_body_scope, method_name).place + if let Some(Type::FunctionLiteral(method_type)) = + class_member(db, class_body_scope, method_name).ignore_possibly_unbound() { let method_decorator = MethodDecorator::try_from_fn_type(db, method_type); if method_decorator != Ok(target_method_decorator) { @@ -3026,7 +3039,9 @@ impl<'db> ClassLiteral<'db> { index.expression(value), TypeContext::default(), ); - return Place::bound(inferred_ty).with_qualifiers(all_qualifiers); + return Member::inferred( + Place::bound(inferred_ty).with_qualifiers(all_qualifiers), + ); } // If there is no right-hand side, just record that we saw a `Final` qualifier @@ -3034,7 +3049,7 @@ impl<'db> ClassLiteral<'db> { continue; } - return annotation; + return Member::declared(annotation); } } @@ -3226,20 +3241,16 @@ impl<'db> ClassLiteral<'db> { } } - if is_attribute_bound { + Member::inferred(if is_attribute_bound { Place::bound(union_of_inferred_types.build()).with_qualifiers(qualifiers) } else { Place::Unbound.with_qualifiers(qualifiers) - } + }) } /// A helper function for `instance_member` that looks up the `name` attribute only on /// this class, not on its superclasses. - pub(crate) fn own_instance_member( - self, - db: &'db dyn Db, - name: &str, - ) -> PlaceAndQualifiers<'db> { + pub(crate) fn own_instance_member(self, db: &'db dyn Db, name: &str) -> Member<'db> { // TODO: There are many things that are not yet implemented here: // - `typing.Final` // - Proper diagnostics @@ -3269,10 +3280,9 @@ impl<'db> ClassLiteral<'db> { // We ignore `InitVar` declarations on the class body, unless that attribute is overwritten // by an implicit assignment in a method if Self::implicit_attribute(db, body_scope, name, MethodDecorator::None) - .place .is_unbound() { - return Place::Unbound.into(); + return Member::unbound(); } } @@ -3287,20 +3297,21 @@ impl<'db> ClassLiteral<'db> { if let Some(implicit_ty) = Self::implicit_attribute(db, body_scope, name, MethodDecorator::None) - .place .ignore_possibly_unbound() { if declaredness == Boundness::Bound { // If a symbol is definitely declared, and we see // attribute assignments in methods of the class, // we trust the declared type. - declared.with_qualifiers(qualifiers) + Member::declared(declared.with_qualifiers(qualifiers)) } else { - Place::Type( - UnionType::from_elements(db, [declared_ty, implicit_ty]), - declaredness, + Member::declared( + Place::Type( + UnionType::from_elements(db, [declared_ty, implicit_ty]), + declaredness, + ) + .with_qualifiers(qualifiers), ) - .with_qualifiers(qualifiers) } } else { // The symbol is declared and bound in the class body, @@ -3309,7 +3320,7 @@ impl<'db> ClassLiteral<'db> { // has a class-level default value, but it would not be // found in a `__dict__` lookup. - Place::Unbound.into() + Member::unbound() } } else { // The attribute is declared but not bound in the class body. @@ -3319,7 +3330,7 @@ impl<'db> ClassLiteral<'db> { // union with the inferred type from attribute assignments. if declaredness == Boundness::Bound { - declared.with_qualifiers(qualifiers) + Member::declared(declared.with_qualifiers(qualifiers)) } else { if let Some(implicit_ty) = Self::implicit_attribute( db, @@ -3327,16 +3338,19 @@ impl<'db> ClassLiteral<'db> { name, MethodDecorator::None, ) + .inner .place .ignore_possibly_unbound() { - Place::Type( - UnionType::from_elements(db, [declared_ty, implicit_ty]), - declaredness, + Member::declared( + Place::Type( + UnionType::from_elements(db, [declared_ty, implicit_ty]), + declaredness, + ) + .with_qualifiers(qualifiers), ) - .with_qualifiers(qualifiers) } else { - declared.with_qualifiers(qualifiers) + Member::declared(declared.with_qualifiers(qualifiers)) } } } @@ -3541,7 +3555,7 @@ impl<'db> VarianceInferable<'db> for ClassLiteral<'db> { let attribute_variances = attribute_names .map(|name| { - let place_and_quals = self.own_instance_member(db, &name); + let place_and_quals = self.own_instance_member(db, &name).inner; (name, place_and_quals) }) .chain(attribute_places_and_qualifiers) @@ -5436,6 +5450,7 @@ impl SlotsKind { fn from(db: &dyn Db, base: ClassLiteral) -> Self { let Place::Type(slots_ty, bound) = base .own_class_member(db, base.inherited_generic_context(db), None, "__slots__") + .inner .place else { return Self::NotSpecified;