Do not treat TypedDict's as descriptors

crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md

@@ -193,6 +193,51 @@ reveal_type(C2().attr)  # revealed: Unknown | Literal["non-data", "normal"]
 C2().attr = 1
 ```
 
+This situation changes if the class attribute is declared. Here, we should error if we see an
+incompatible attribute assignment, but otherwise just follow the declared type:
+
+```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 an integer, which is not compatible with
+        # the `__get__` return type of `NonDataDescriptor` when called on an
+        # instance.
+        self.attr = 1
+
+reveal_type(C3().attr)  # revealed: Literal["non-data"]
+```
+
+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 return type of `__get__` of `C4.f` (a different bound method). Strictly
+        # speaking, we should also emit an error in this case.
+        self.f = self.replacement
+
+reveal_type(C4.f)  # revealed: def f(self) -> None
+
+c4 = C4()
+
+reveal_type(c4.f)  # revealed: bound method C4.f() -> None
+
+# 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/resources/mdtest/protocols.md

@@ -1676,7 +1676,9 @@ def _(r: Recursive):
     reveal_type(r.direct)  # revealed: Recursive
     reveal_type(r.union)  # revealed: None | Recursive
     reveal_type(r.intersection1)  # revealed: C & Recursive
-    reveal_type(r.intersection2)  # revealed: C & ~Recursive
+    # TODO: no error, and a revealed type of `C & ~Recursive`
+    # error: [unresolved-attribute] "Type `Recursive` has no attribute `intersection2`"
+    reveal_type(r.intersection2)  # revealed: Unknown
     reveal_type(r.t)  # revealed: tuple[int, tuple[str, Recursive]]
     reveal_type(r.callable1)  # revealed: (int, /) -> Recursive
     reveal_type(r.callable2)  # revealed: (Recursive, /) -> int

crates/ty_python_semantic/src/types.rs

@@ -5659,6 +5659,7 @@ pub enum DynamicType {
     /// A special Todo-variant for PEP-695 `ParamSpec` types. A temporary variant to detect and special-
     /// case the handling of these types in `Callable` annotations.
     TodoPEP695ParamSpec,
+    TodoTypedDict,
 }
 
 impl DynamicType {
@@ -5683,6 +5684,13 @@ impl std::fmt::Display for DynamicType {
                     f.write_str("@Todo")
                 }
             }
+            DynamicType::TodoTypedDict => {
+                if cfg!(debug_assertions) {
+                    f.write_str("@Todo(TypedDict)")
+                } else {
+                    f.write_str("@Todo")
+                }
+            }
         }
     }
 }

crates/ty_python_semantic/src/types/call/bind.rs

@@ -20,9 +20,9 @@ use crate::types::diagnostic::{
 use crate::types::generics::{Specialization, SpecializationBuilder, SpecializationError};
 use crate::types::signatures::{Parameter, ParameterForm};
 use crate::types::{
-    BoundMethodType, DataclassParams, DataclassTransformerParams, FunctionDecorators, FunctionType,
-    KnownClass, KnownFunction, KnownInstanceType, MethodWrapperKind, PropertyInstanceType,
-    TupleType, TypeMapping, UnionType, WrapperDescriptorKind, todo_type,
+    BoundMethodType, DataclassParams, DataclassTransformerParams, DynamicType, FunctionDecorators,
+    FunctionType, KnownClass, KnownFunction, KnownInstanceType, MethodWrapperKind,
+    PropertyInstanceType, TupleType, TypeMapping, UnionType, WrapperDescriptorKind,
 };
 use ruff_db::diagnostic::{Annotation, Diagnostic, Severity, SubDiagnostic};
 use ruff_python_ast as ast;
@@ -916,7 +916,7 @@ impl<'db> Bindings<'db> {
                     },
 
                     Type::KnownInstance(KnownInstanceType::TypedDict) => {
-                        overload.set_return_type(todo_type!("TypedDict"));
+                        overload.set_return_type(Type::Dynamic(DynamicType::TodoTypedDict));
                     }
 
                     // Not a special case

crates/ty_python_semantic/src/types/class.rs

@@ -1070,6 +1070,10 @@ impl<'db> ClassLiteral<'db> {
                 ClassBase::Generic | ClassBase::Protocol => {
                     // Skip over these very special class bases that aren't really classes.
                 }
+
+                ClassBase::Dynamic(DynamicType::TodoTypedDict) if name == "__get__" => {
+                    return Symbol::Unbound.into();
+                }
                 ClassBase::Dynamic(_) => {
                     // Note: calling `Type::from(superclass).member()` would be incorrect here.
                     // What we'd really want is a `Type::Any.own_class_member()` method,
@@ -1740,13 +1744,42 @@ 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. If they do so in a non-compatible way, we
+                        // should emit an error elsewhere. Here, we simply return `Unbound`,
+                        // to signal that there is no instance attribute of this name.
+                        return Symbol::Unbound.into();
+                    }
 
                     let bindings = use_def.public_bindings(symbol_id);
                     let inferred = symbol_from_bindings(db, bindings);
@@ -1755,10 +1788,7 @@ impl<'db> ClassLiteral<'db> {
                     if has_binding {
                         // The attribute is declared and bound in the class body.
 
-                        if let Some(implicit_ty) =
-                            Self::implicit_instance_attribute(db, body_scope, name)
-                                .ignore_possibly_unbound()
-                        {
+                        if let Some(implicit_ty) = implicit.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 +1820,7 @@ impl<'db> ClassLiteral<'db> {
                         if declaredness == Boundness::Bound {
                             declared.with_qualifiers(qualifiers)
                         } else {
-                            if let Some(implicit_ty) =
-                                Self::implicit_instance_attribute(db, body_scope, name)
-                                    .ignore_possibly_unbound()
-                            {
+                            if let Some(implicit_ty) = implicit.ignore_possibly_unbound() {
                                 Symbol::Type(
                                     UnionType::from_elements(db, [declared_ty, implicit_ty]),
                                     declaredness,

crates/ty_python_semantic/src/types/class_base.rs

@@ -44,7 +44,11 @@ impl<'db> ClassBase<'db> {
             ClassBase::Class(class) => class.name(db),
             ClassBase::Dynamic(DynamicType::Any) => "Any",
             ClassBase::Dynamic(DynamicType::Unknown) => "Unknown",
-            ClassBase::Dynamic(DynamicType::Todo(_) | DynamicType::TodoPEP695ParamSpec) => "@Todo",
+            ClassBase::Dynamic(
+                DynamicType::Todo(_)
+                | DynamicType::TodoPEP695ParamSpec
+                | DynamicType::TodoTypedDict,
+            ) => "@Todo",
             ClassBase::Protocol => "Protocol",
             ClassBase::Generic => "Generic",
         }
@@ -209,7 +213,9 @@ impl<'db> ClassBase<'db> {
                 KnownInstanceType::OrderedDict => {
                     Self::try_from_type(db, KnownClass::OrderedDict.to_class_literal(db))
                 }
-                KnownInstanceType::TypedDict => Self::try_from_type(db, todo_type!("TypedDict")),
+                KnownInstanceType::TypedDict => {
+                    Self::try_from_type(db, Type::Dynamic(DynamicType::TodoTypedDict))
+                }
                 KnownInstanceType::Callable => {
                     Self::try_from_type(db, todo_type!("Support for Callable as a base class"))
                 }

crates/ty_python_semantic/src/types/infer.rs

@@ -6075,13 +6075,21 @@ impl<'db> TypeInferenceBuilder<'db> {
             (unknown @ Type::Dynamic(DynamicType::Unknown), _, _)
             | (_, unknown @ Type::Dynamic(DynamicType::Unknown), _) => Some(unknown),
             (
-                todo @ Type::Dynamic(DynamicType::Todo(_) | DynamicType::TodoPEP695ParamSpec),
+                todo @ Type::Dynamic(
+                    DynamicType::Todo(_)
+                    | DynamicType::TodoPEP695ParamSpec
+                    | DynamicType::TodoTypedDict,
+                ),
                 _,
                 _,
             )
             | (
                 _,
-                todo @ Type::Dynamic(DynamicType::Todo(_) | DynamicType::TodoPEP695ParamSpec),
+                todo @ Type::Dynamic(
+                    DynamicType::Todo(_)
+                    | DynamicType::TodoPEP695ParamSpec
+                    | DynamicType::TodoTypedDict,
+                ),
                 _,
             ) => Some(todo),
             (Type::Never, _, _) | (_, Type::Never, _) => Some(Type::Never),

crates/ty_python_semantic/src/types/type_ordering.rs

@@ -382,5 +382,8 @@ fn dynamic_elements_ordering(left: DynamicType, right: DynamicType) -> Ordering
 
         (DynamicType::TodoPEP695ParamSpec, _) => Ordering::Less,
         (_, DynamicType::TodoPEP695ParamSpec) => Ordering::Greater,
+
+        (DynamicType::TodoTypedDict, _) => Ordering::Less,
+        (_, DynamicType::TodoTypedDict) => Ordering::Greater,
     }
 }