[ty] Ignore descriptor class-level declarations for purposes of finding instance attributes

crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md

@@ -193,6 +193,54 @@ reveal_type(C2().attr)  # revealed: Unknown | Literal["non-data", "normal"]
 C2().attr = 1
 ```
 
+This situation does not change if the attribute is declared on the class body:
+
+```py
+class C3:
+    attr: NonDataDescriptor = NonDataDescriptor()
+
+    def f(self):
+        # TODO: we should ideally emit an error here. We are overwriting the
+        # non-data descriptor with a string, which is not compatible with the
+        # declared type.
+        self.attr = "normal"
+
+reveal_type(C3().attr)  # revealed: Literal["non-data", "normal"] | Unknown
+```
+
+The scenario above is similar to a use case where a method on a class is dynamically replaced.
+
+```py
+class C4:
+    def f(self) -> None:
+        print("original f")
+
+    def replacement(self) -> None:
+        print("a replacement")
+
+    def switch(self):
+        # Similar to the `C3` example, we are overwriting a non-data descriptor (the
+        # function `C4.f`) with something (a bound method) that is not compatible with
+        # the (implicitly) declared type of `C4.f`, which is a function literal type:
+        # `def f(self) -> None`. Strictly speaking, this we should also emit an error
+        # here.. or we should not consider the function definition to be a declaration.
+        self.f = self.replacement
+
+reveal_type(C4.f)  # revealed: def f(self) -> None
+
+c4 = C4()
+
+# call c4.switch() or not
+
+# TODO: This should reveal the following type, as soon as we understand the type of self:
+# `(bound method C4.f() -> None) | (bound method C4.replacement() -> None) | Unknown`
+reveal_type(c4.f)  # revealed: (bound method C4.f() -> None) | Unknown
+
+# As a regression test for https://github.com/astral-sh/ty/issues/350, make sure that no
+# error is emitted when calling `c4.f()`:
+c4.f()
+```
+
 ### Descriptors only work when used as class variables
 
 Descriptors only work when used as class variables. When put in instances, they have no effect.

crates/ty_python_semantic/src/types/class.rs

@@ -1740,13 +1740,47 @@ impl<'db> ClassLiteral<'db> {
                     symbol: mut declared @ Symbol::Type(declared_ty, declaredness),
                     qualifiers,
                 }) => {
+                    let implicit = Self::implicit_instance_attribute(db, body_scope, name);
+
                     // For the purpose of finding instance attributes, ignore `ClassVar`
                     // declarations:
                     if qualifiers.contains(TypeQualifiers::CLASS_VAR) {
                         declared = Symbol::Unbound;
                     }
 
-                    // The attribute is declared in the class body.
+                    // Invoke the descriptor protocol on the declared type, to check
+                    // if it is a descriptor attribute.
+                    let declared_resolved = Type::try_call_dunder_get_on_attribute(
+                        db,
+                        SymbolAndQualifiers {
+                            symbol: declared.clone(),
+                            qualifiers,
+                        },
+                        Type::instance(db, self.apply_optional_specialization(db, None)),
+                        Type::ClassLiteral(self),
+                    )
+                    .0
+                    .symbol;
+
+                    if declared != declared_resolved {
+                        // If we end up here, it means that the class-level attribute is a
+                        // non-data descriptor (a data descriptor would have taken precedence
+                        // over the instance attribute). In this method, we look at declared
+                        // types on the class body because they might indicate the declared
+                        // type of implicit instance attributes. However, if the class-level
+                        // attribute is a non-data descriptor, it can not possibly be the
+                        // correct type of the implicit instance attribute. If there are any
+                        // attribute assignments in methods of this class, they would overwrite
+                        // the non-data descriptor. In this case, we just return the type
+                        // inferred from attribute assignments in methods. The descriptor
+                        // protocol implementation in `Type::invoke_descriptor_protocol` will
+                        // take care of unioning with the non-data descriptor type (because we
+                        // account for the fact that the methods containing these assignments
+                        // might never be called).
+                        if !implicit.is_unbound() {
+                            return implicit.into();
+                        }
+                    }
 
                     let bindings = use_def.public_bindings(symbol_id);
                     let inferred = symbol_from_bindings(db, bindings);
@@ -1755,10 +1789,7 @@ impl<'db> ClassLiteral<'db> {
                     if has_binding {
                         // The attribute is declared and bound in the class body.
 
-                        if let Some(implicit_ty) =
+                        if let Some(implicit_ty) = implicit.ignore_possibly_unbound() {
-                            Self::implicit_instance_attribute(db, body_scope, name)
-                                .ignore_possibly_unbound()
-                        {
                             if declaredness == Boundness::Bound {
                                 // If a symbol is definitely declared, and we see
                                 // attribute assignments in methods of the class,
@@ -1790,10 +1821,7 @@ impl<'db> ClassLiteral<'db> {
                         if declaredness == Boundness::Bound {
                             declared.with_qualifiers(qualifiers)
                         } else {
-                            if let Some(implicit_ty) =
+                            if let Some(implicit_ty) = implicit.ignore_possibly_unbound() {
-                                Self::implicit_instance_attribute(db, body_scope, name)
-                                    .ignore_possibly_unbound()
-                            {
                                 Symbol::Type(
                                     UnionType::from_elements(db, [declared_ty, implicit_ty]),
                                     declaredness,