[ty] Prefer declared base class attribute over inferred attribute on subclass

Author: sharkdp

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

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;

crates/ty_python_semantic/src/member.rs

@@ -0,0 +1,66 @@
+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::undeclared(PlaceAndQualifiers::default())
+    }
+}
+
+impl<'db> Member<'db> {
+    pub(crate) fn undeclared(inner: PlaceAndQualifiers<'db>) -> Self {
+        Self {
+            inner,
+            is_declared: false,
+        }
+    }
+
+    pub(crate) fn declared(inner: PlaceAndQualifiers<'db>) -> Self {
+        Self {
+            inner,
+            is_declared: true,
+        }
+    }
+
+    pub(crate) fn declared_and_bound(ty: Type<'db>) -> Self {
+        Self::declared(Place::bound(ty).into())
+    }
+
+    pub(crate) fn is_unbound(&self) -> bool {
+        self.inner.place.is_unbound()
+    }
+
+    /// Represents the absence of a member.
+    pub(crate) fn unbound() -> Self {
+        Self::undeclared(PlaceAndQualifiers::default())
+    }
+
+    pub(crate) fn ignore_possibly_unbound(&self) -> Option<Type<'db>> {
+        self.inner.place.ignore_possibly_unbound()
+    }
+
+    #[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,
+        }
+    }
+}

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::undeclared(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()