Only short-circuit on declared attributes

Author: sharkdp

crates/ty_python_semantic/resources/mdtest/attributes.md

@@ -891,7 +891,7 @@ reveal_type(Derived().redeclared_with_same_type)  # revealed: str | None
 
 # TODO: It would probably be more consistent if these were `str | None`
 reveal_type(Derived.redeclared_with_narrower_type)  # revealed: str
-reveal_type(Derived().redeclared_with_narrower_type)  # revealed: str
+reveal_type(Derived().redeclared_with_narrower_type)  # revealed: str | None
 
 # TODO: It would probably be more consistent if these were `str | None`
 reveal_type(Derived.redeclared_with_wider_type)  # revealed: str | int | None
@@ -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["base", "intermediate"]
 ```
 
 ## Accessing attributes on class objects

crates/ty_python_semantic/src/member.rs

@@ -3,7 +3,7 @@ use crate::{place::PlaceAndQualifiers, types::Type};
 #[derive(Debug, Clone, PartialEq, Eq, salsa::Update, get_size2::GetSize)]
 pub(crate) struct Member<'db> {
     pub(crate) inner: PlaceAndQualifiers<'db>,
-    pub(crate) declared: bool,
+    pub(crate) is_declared: bool,
 }
 
 impl Default for Member<'_> {
@@ -16,14 +16,14 @@ impl<'db> Member<'db> {
     pub(crate) fn undeclared(inner: PlaceAndQualifiers<'db>) -> Self {
         Self {
             inner,
-            declared: false,
+            is_declared: false,
         }
     }
 
     pub(crate) fn declared(inner: PlaceAndQualifiers<'db>) -> Self {
         Self {
             inner,
-            declared: true,
+            is_declared: true,
         }
     }
 
@@ -39,7 +39,7 @@ impl<'db> Member<'db> {
     pub(crate) fn map_type(self, f: impl FnOnce(Type<'db>) -> Type<'db>) -> Self {
         Self {
             inner: self.inner.map_type(f),
-            declared: self.declared,
+            is_declared: self.is_declared,
         }
     }
 }

crates/ty_python_semantic/src/types/class.rs

@@ -115,7 +115,7 @@ fn implicit_attribute_initial<'db>(
 ) -> Member<'db> {
     Member {
         inner: Place::Unbound.into(),
-        declared: false,
+        is_declared: false,
     }
 }
 
@@ -2871,8 +2871,12 @@ 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) {
+        let mro = self.try_mro(db, specialization);
+        let mro = mro.unwrap_or_else(|e| e.fallback_mro());
+
+        for superclass in mro.iter().rev() {
             match superclass {
                 ClassBase::Generic | ClassBase::Protocol => {
                     // Skip over these very special class bases that aren't really classes.
@@ -2883,22 +2887,30 @@ impl<'db> ClassLiteral<'db> {
                     );
                 }
                 ClassBase::Class(class) => {
-                    if let member @ PlaceAndQualifiers {
-                        place: Place::Type(ty, boundness),
-                        qualifiers,
-                    } = class.own_instance_member(db, name).inner
+                    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
-                                return member;
-                            }
+                            is_definitely_bound = true;
+
+                            if is_declared {
+                                if union.is_empty() {
+                                    // Short-circuit, no need to allocate inside the union builder
+                                    return member;
+                                }
 
-                            return Place::bound(union.add(ty).build())
-                                .with_qualifiers(union_qualifiers);
+                                return Place::bound(union.add(ty).build())
+                                    .with_qualifiers(union_qualifiers);
+                            }
                         }
 
                         // If we see a possibly-unbound symbol, we need to keep looking
@@ -2928,10 +2940,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)
         }
     }