New tests for meta class descriptors

Author: sharkdp

crates/red_knot_python_semantic/resources/mdtest/descriptor_protocol.md

@@ -95,7 +95,7 @@ class DataDescriptor:
     def __get__(self, instance: object, owner: type | None = None) -> Literal["data"]:
         return "data"
 
-    def __set__(self, instance: int, value) -> None:
+    def __set__(self, instance: object, value: int) -> None:
         pass
 
 class NonDataDescriptor:
@@ -133,42 +133,6 @@ reveal_type(C.non_data_descriptor)  # revealed: Unknown | Literal["non-data"]
 C.data_descriptor = "something else"  # This is okay
 ```
 
-## Descriptor protocol for class objects
-
-When a descriptor is accessed on a class object, the following precedence chain is used:
-
-- Data descriptor on the metaclass
-- Data or non-data descriptor on the class
-- Class attribute
-- Non-data descriptor on the metaclass
-
-```py
-from typing import Literal
-
-class DataDescriptor:
-    def __get__(self, instance: object, owner: type | None = None) -> Literal["data"]:
-        return "data"
-
-    def __set__(self, instance: int, value) -> None:
-        pass
-
-class NonDataDescriptor:
-    def __get__(self, instance: object, owner: type | None = None) -> Literal["non-data"]:
-        return "non-data"
-
-class Meta(type):
-    meta_data_descriptor: DataDescriptor = DataDescriptor()
-    meta_non_data_descriptor: NonDataDescriptor = NonDataDescriptor()
-
-class DescriptorsOnMetaClass(metaclass=Meta):
-    # meta_data_descriptor = "shadowed?"
-    # meta_non_data_descriptor = "shadowed?"
-    ...
-
-reveal_type(DescriptorsOnMetaClass.meta_data_descriptor)  # revealed: Literal["data"]
-reveal_type(DescriptorsOnMetaClass.meta_non_data_descriptor)  # revealed: Literal["non-data"]
-```
-
 ## Possibly unbound descriptors, unions with other attributes
 
 ```py
@@ -230,6 +194,180 @@ def _(flag: bool):
     reveal_type(UnionWithInstanceAttribute().data)  # revealed: int | str
 ```
 
+## Descriptor protocol for class objects
+
+When attributes are accessed on a class object, the following [precedence chain] is used:
+
+- Data descriptor on the metaclass
+- Data or non-data descriptor on the class
+- Class attribute
+- Non-data descriptor on the metaclass
+- Metaclass attribute
+
+To verify this, we define a data and a non-data descriptor:
+
+```py
+from typing import Literal, Any
+
+class DataDescriptor:
+    def __get__(self, instance: object, owner: type | None = None) -> Literal["data"]:
+        return "data"
+
+    def __set__(self, instance: object, value: str) -> None:
+        pass
+
+class NonDataDescriptor:
+    def __get__(self, instance: object, owner: type | None = None) -> Literal["non-data"]:
+        return "non-data"
+```
+
+First, we make sure that the descriptors are correctly accessed when defined on the metaclass or the
+class:
+
+```py
+class Meta1(type):
+    meta_data_descriptor: DataDescriptor = DataDescriptor()
+    meta_non_data_descriptor: NonDataDescriptor = NonDataDescriptor()
+
+class C1(metaclass=Meta1):
+    class_data_descriptor: DataDescriptor = DataDescriptor()
+    class_non_data_descriptor: NonDataDescriptor = NonDataDescriptor()
+
+reveal_type(C1.meta_data_descriptor)  # revealed: Literal["data"]
+reveal_type(C1.meta_non_data_descriptor)  # revealed: Literal["non-data"]
+
+reveal_type(C1.class_data_descriptor)  # revealed: Literal["data"]
+reveal_type(C1.class_non_data_descriptor)  # revealed: Literal["non-data"]
+```
+
+Next, we demonstrate that a *metaclass data descriptor* takes precedence over all class-level
+attributes:
+
+```py
+class Meta2(type):
+    meta_data_descriptor1: DataDescriptor = DataDescriptor()
+    meta_data_descriptor2: DataDescriptor = DataDescriptor()
+
+class ClassLevelDataDescriptor:
+    def __get__(self, instance: object, owner: type | None = None) -> Literal["class level data descriptor"]:
+        return "class level data descriptor"
+
+    def __set__(self, instance: object, value: str) -> None:
+        pass
+
+class C2(metaclass=Meta2):
+    meta_data_descriptor1: Literal["value on class"] = "value on class"
+    meta_data_descriptor2: ClassLevelDataDescriptor = ClassLevelDataDescriptor()
+
+reveal_type(C2.meta_data_descriptor1)  # revealed: Literal["data"]
+reveal_type(C2.meta_data_descriptor2)  # revealed: Literal["data"]
+```
+
+On the other hand, normal metaclass attributes and metaclass non-data descriptors are shadowed by
+class-level attributes (descriptor or not):
+
+```py
+class Meta3(type):
+    meta_attribute1: Literal["value on metaclass"] = "value on metaclass"
+    meta_attribute2: Literal["value on metaclass"] = "value on metaclass"
+    meta_non_data_descriptor1: NonDataDescriptor = NonDataDescriptor()
+    meta_non_data_descriptor2: NonDataDescriptor = NonDataDescriptor()
+
+class C3(metaclass=Meta3):
+    meta_attribute1: Literal["value on class"] = "value on class"
+    meta_attribute2: ClassLevelDataDescriptor = ClassLevelDataDescriptor()
+    meta_non_data_descriptor1: Literal["value on class"] = "value on class"
+    meta_non_data_descriptor2: ClassLevelDataDescriptor = ClassLevelDataDescriptor()
+
+reveal_type(C3.meta_attribute1)  # revealed: Literal["value on class"]
+reveal_type(C3.meta_attribute2)  # revealed: Literal["class level data descriptor"]
+reveal_type(C3.meta_non_data_descriptor1)  # revealed: Literal["value on class"]
+reveal_type(C3.meta_non_data_descriptor2)  # revealed: Literal["class level data descriptor"]
+```
+
+Finally, metaclass attributes and metaclass non-data descriptors are only accessible when they are
+not shadowed by class-level attributes:
+
+```py
+class Meta4(type):
+    meta_attribute: Literal["value on metaclass"] = "value on metaclass"
+    meta_non_data_descriptor: NonDataDescriptor = NonDataDescriptor()
+
+class C4(metaclass=Meta4): ...
+
+reveal_type(C4.meta_attribute)  # revealed: Literal["value on metaclass"]
+reveal_type(C4.meta_non_data_descriptor)  # revealed: Literal["non-data"]
+```
+
+When a metaclass data descriptor is possibly unbound, we union the result type of its `__get__`
+method with an underlying class level attribute, if present:
+
+```py
+def _(flag: bool):
+    class Meta5(type):
+        if flag:
+            meta_data_descriptor1: DataDescriptor = DataDescriptor()
+            meta_data_descriptor2: DataDescriptor = DataDescriptor()
+
+    class C5(metaclass=Meta5):
+        meta_data_descriptor1: Literal["value on class"] = "value on class"
+
+    reveal_type(C5.meta_data_descriptor1)  # revealed: Literal["value on class", "data"]
+    # error: [possibly-unbound-attribute]
+    reveal_type(C5.meta_data_descriptor2)  # revealed: Literal["data"]
+```
+
+When a class-level attribute is possibly unbound, we union its (descriptor protocol) type with the
+metaclass attribute (unless it's a data descriptor, which always takes precedence):
+
+```py
+def _(flag: bool):
+    class Meta6(type):
+        attribute1: DataDescriptor = DataDescriptor()
+        attribute2: NonDataDescriptor = NonDataDescriptor()
+        attribute3: Literal["value on metaclass"] = "value on metaclass"
+
+    class C6(metaclass=Meta6):
+        if flag:
+            attribute1: Literal["value on class"] = "value on class"
+            attribute2: Literal["value on class"] = "value on class"
+            attribute3: Literal["value on class"] = "value on class"
+            attribute4: Literal["value on class"] = "value on class"
+
+    reveal_type(C6.attribute1)  # revealed: Literal["data"]
+    reveal_type(C6.attribute2)  # revealed: Literal["value on class", "non-data"]
+    reveal_type(C6.attribute3)  # revealed: Literal["value on class", "value on metaclass"]
+    # error: [possibly-unbound-attribute]
+    reveal_type(C6.attribute4)  # revealed: Literal["value on class"]
+```
+
+Finally, we can also have unions of various types of attributes:
+
+```py
+def _(flag: bool):
+    class Meta7(type):
+        if flag:
+            union_of_metaclass_attributes: Literal[1] = 1
+            union_of_metaclass_data_descriptor_and_attribute: DataDescriptor = DataDescriptor()
+        else:
+            union_of_metaclass_attributes: Literal[2] = 2
+            union_of_metaclass_data_descriptor_and_attribute: Literal[2] = 2
+
+    class C7(metaclass=Meta7):
+        if flag:
+            union_of_class_attributes: Literal[1] = 1
+            union_of_class_data_descriptor_and_attribute: DataDescriptor = DataDescriptor()
+        else:
+            union_of_class_attributes: Literal[2] = 2
+            union_of_class_data_descriptor_and_attribute: Literal[2] = 2
+
+    reveal_type(C7.union_of_metaclass_attributes)  # revealed: Literal[1, 2]
+    # TODO: should be `Literal["data", 2]`
+    reveal_type(C7.union_of_metaclass_data_descriptor_and_attribute)  # revealed: Literal["data"]
+    reveal_type(C7.union_of_class_attributes)  # revealed: Literal[1, 2]
+    reveal_type(C7.union_of_class_data_descriptor_and_attribute)  # revealed: Literal["data", 2]
+```
+
 ## Built-in `property` descriptor
 
 The built-in `property` decorator creates a descriptor. The names for attribute reads/writes are
@@ -533,4 +671,5 @@ wrapper_descriptor(f, None, type(f), "one too many")
 ```
 
 [descriptors]: https://docs.python.org/3/howto/descriptor.html
+[precedence chain]: https://github.com/python/cpython/blob/3.13/Objects/typeobject.c#L5393-L5481
 [simple example]: https://docs.python.org/3/howto/descriptor.html#simple-example-a-descriptor-that-returns-a-constant

crates/red_knot_python_semantic/src/types.rs

@@ -1836,7 +1836,17 @@ impl<'db> Type<'db> {
                             {
                                 let meta_descr_get =
                                     meta_attribute_ty.class_member(db, "__get__").0;
+
                                 if let Symbol::Type(meta_descr_get, _) = meta_descr_get {
+                                    let is_data = !meta_attribute_ty
+                                        .class_member(db, "__set__")
+                                        .0
+                                        .is_unbound()
+                                        || !meta_attribute_ty
+                                            .class_member(db, "__delete__")
+                                            .0
+                                            .is_unbound();
+
                                     let return_ty = Symbol::Type(
                                         meta_descr_get
                                             .try_call(
@@ -1851,8 +1861,7 @@ impl<'db> Type<'db> {
                                             .unwrap_or(Type::unknown()),
                                         meta_attribute_boundness,
                                     );
-                                    let is_data = !self.class_member(db, "__set__").0.is_unbound()
-                                        || !self.class_member(db, "__delete__").0.is_unbound();
+
                                     (return_ty, is_data)
                                 } else {
                                     (meta_attribute, false)
@@ -1879,7 +1888,7 @@ impl<'db> Type<'db> {
                                 class_attr_boundness,
                             )
                         } else {
-                            class_attr.0.clone() // TODO
+                            class_attr.0
                         };
 
                         match (
@@ -1898,28 +1907,22 @@ impl<'db> Type<'db> {
                                 Symbol::Type(_, _),
                             ) => SymbolAndQualifiers(
                                 Symbol::Type(
-                                    UnionType::from_elements(
-                                        db,
-                                        [class_attr_ty.clone(), meta_attr_ty.clone()],
-                                    ),
+                                    UnionType::from_elements(db, [class_attr_ty, *meta_attr_ty]),
                                     class_attr_boundness,
                                 ),
                                 class_attr_qualifiers,
                             ),
                             (Symbol::Type(_, _), false, Symbol::Type(_, Boundness::Bound)) => {
-                                class_attr
+                                SymbolAndQualifiers(class_attr_resolved, class_attr_qualifiers)
                             }
                             (
                                 Symbol::Type(meta_attr_ty, meta_attr_boundness),
                                 false,
                                 Symbol::Type(_, Boundness::PossiblyUnbound),
                             ) => SymbolAndQualifiers(
                                 Symbol::Type(
-                                    UnionType::from_elements(
-                                        db,
-                                        [class_attr_ty.clone(), meta_attr_ty.clone()],
-                                    ),
-                                    dbg!(*meta_attr_boundness),
+                                    UnionType::from_elements(db, [class_attr_ty, *meta_attr_ty]),
+                                    *meta_attr_boundness,
                                 ),
                                 class_attr_qualifiers,
                             ),
@@ -1934,7 +1937,9 @@ impl<'db> Type<'db> {
 
                         let meta_attribute = self.class_member(db, name);
 
-                        if let Symbol::Type(meta_attribute_ty, _) = meta_attribute.0 {
+                        if let Symbol::Type(meta_attribute_ty, meta_attribute_boundness) =
+                            meta_attribute.0
+                        {
                             let meta_descr_get = meta_attribute_ty.class_member(db, "__get__").0;
                             if let Symbol::Type(meta_descr_get, _) = meta_descr_get {
                                 Symbol::Type(
@@ -1949,7 +1954,7 @@ impl<'db> Type<'db> {
                                         )
                                         .map(|outcome| outcome.return_type(db))
                                         .unwrap_or(Type::unknown()),
-                                    Boundness::Bound,
+                                    meta_attribute_boundness,
                                 )
                                 .into()
                             } else {

crates/red_knot_python_semantic/src/types/class.rs

@@ -479,12 +479,9 @@ impl<'db> Class<'db> {
 
         let attribute_assignments = attribute_assignments(db, class_body_scope);
 
-        let Some(attribute_assignments) = attribute_assignments
+        let attribute_assignments = attribute_assignments
             .as_deref()
-            .and_then(|assignments| assignments.get(name))
-        else {
-            return None;
-        };
+            .and_then(|assignments| assignments.get(name))?;
 
         for attribute_assignment in attribute_assignments {
             match attribute_assignment {
@@ -575,11 +572,10 @@ impl<'db> Class<'db> {
                     if !inferred.is_unbound() {
                         if declaredness == Boundness::Bound {
                             return Symbol::Unbound.into();
-                        } else {
-                            return Self::implicit_instance_attribute(db, body_scope, name)
-                                .map_or(Symbol::Unbound, Symbol::bound)
-                                .into();
                         }
+                        return Self::implicit_instance_attribute(db, body_scope, name)
+                            .map_or(Symbol::Unbound, Symbol::bound)
+                            .into();
                     }
 
                     if declaredness == Boundness::Bound {