[ty] Check for instance storage without inferring method assignments (#29709)

## Summary

Checking Home Assistant’s [HomeKit light
module](https://github.com/home-assistant/core/blob/a0162d2ff0ed061501405a617961be149b15bd1f/homeassistant/components/homekit_controller/light.py#L53)
opened 38 Python files instead of 45 and used 10% fewer instructions.

When a class declares `value: int = 0`, ty may still need to know
whether a method writes `self.value`. It used to infer `make()` in
`self.value = make()` just to answer that question. Ty now stops once it
finds a reachable assignment. The annotation supplies the type; checking
the method still validates the assigned value.

crates/ty_python_semantic/resources/mdtest/attributes.md

@@ -1303,6 +1303,57 @@ reveal_type(C.variable_with_class_default1)  # revealed: Literal["overwritten on
 reveal_type(c_instance.variable_with_class_default1)  # revealed: Literal["value set on instance"]
 ```
 
+#### Instance storage for annotated descriptor defaults
+
+A non-data descriptor can be shadowed by instance storage. An ordinary receiver assignment or
+annotation establishes that storage; assignments in static or class methods do not. An augmented
+assignment also reads the descriptor before writing its result.
+
+```py
+def flag() -> bool:
+    return True
+
+class Descriptor:
+    def __get__(self, instance: object, owner: type) -> str:
+        return ""
+
+    def __iadd__(self, other: str):
+        return self
+
+class Model:
+    ordinary: Descriptor = Descriptor()
+    annotated: Descriptor = Descriptor()
+    static_only: Descriptor = Descriptor()
+    class_only: Descriptor = Descriptor()
+    augmented: Descriptor = Descriptor()
+    if flag():
+        conditional: Descriptor = Descriptor()
+
+    def assign(self, value: Descriptor):
+        self.ordinary = value
+        self.annotated: Descriptor
+        self.augmented += "!"  # error: [invalid-assignment]
+        # error: [possibly-missing-attribute]
+        self.conditional = b"instance"  # error: [invalid-assignment]
+
+    @staticmethod
+    def assign_static(receiver):
+        receiver.static_only = Descriptor()
+
+    @classmethod
+    def assign_class(cls):
+        cls.class_only = Descriptor()
+
+m = Model()
+reveal_type(m.ordinary)  # revealed: str | Descriptor
+reveal_type(m.annotated)  # revealed: str | Descriptor
+reveal_type(m.static_only)  # revealed: str
+reveal_type(m.class_only)  # revealed: str
+reveal_type(m.augmented)  # revealed: str | Descriptor
+# error: [possibly-missing-attribute]
+reveal_type(m.conditional)  # revealed: str | Descriptor | bytes
+```
+
 #### Augmented assignments to overriding class-level defaults
 
 An unannotated class-level default retains the inherited declaration, including through a diamond.

crates/ty_python_semantic/src/types/class/implicit_attributes.rs

@@ -1,5 +1,7 @@
 //! Implicit instance and class attributes inferred from method assignments.
 
+use std::{convert::Infallible, ops::ControlFlow};
+
 use super::{MethodDecorator, static_literal::StaticClassLiteral};
 use crate::{
     Db, ProgramEnvironment, TypeQualifiers, attribute_assignments, attribute_declarations,
@@ -55,27 +57,41 @@ impl<'db> StaticClassLiteral<'db> {
         name: &str,
         target_method_decorator: MethodDecorator,
     ) -> ImplicitAttribute<'db> {
+        self.implicit_attribute_name(db, name, target_method_decorator)
+            .map_or_else(
+                || ImplicitAttribute {
+                    member: Member::unbound(),
+                    augmented_bindings: None,
+                },
+                |attribute| Self::implicit_attribute_inner(db, attribute),
+            )
+    }
+
+    /// Does a method establish instance storage for an attribute already typed by the class?
+    pub(super) fn has_implicit_instance_attribute(self, db: &'db dyn Db, name: &str) -> bool {
+        self.implicit_attribute_name(db, name, MethodDecorator::None)
+            .is_some_and(|attribute| Self::has_implicit_attribute_inner(db, attribute))
+    }
+
+    fn implicit_attribute_name(
+        self,
+        db: &'db dyn Db,
+        name: &str,
+        target_method_decorator: MethodDecorator,
+    ) -> Option<ImplicitAttributeName<'db>> {
         let class_body_scope = self.body_scope(db);
         // Collect names in a tracked query so unrelated edits can preserve dependent member
         // lookups, and avoid retaining query entries for names that no method can define.
         let names = implicit_attribute_definition_names(db, class_body_scope);
-        let Ok(name_index) = names.binary_search_by(|candidate| candidate.as_str().cmp(name))
-        else {
-            return ImplicitAttribute {
-                member: Member::unbound(),
-                augmented_bindings: None,
-            };
-        };
-
-        Self::implicit_attribute_inner(
+        let name_index = names
+            .binary_search_by(|candidate| candidate.as_str().cmp(name))
+            .ok()?;
+        Some(ImplicitAttributeName::new(
             db,
-            ImplicitAttributeName::new(
-                db,
-                class_body_scope,
-                &names[name_index],
-                target_method_decorator,
-            ),
-        )
+            class_body_scope,
+            &names[name_index],
+            target_method_decorator,
+        ))
     }
 
     #[salsa::tracked(
@@ -93,13 +109,31 @@ impl<'db> StaticClassLiteral<'db> {
         db: &'db dyn Db,
         attribute: ImplicitAttributeName<'db>,
     ) -> ImplicitAttribute<'db> {
-        Self::implicit_attribute_impl(db, attribute)
+        let ControlFlow::Continue(result) =
+            Self::implicit_attribute_impl(db, attribute, |binding| {
+                ControlFlow::<Infallible, _>::Continue(implicit_attribute_binding_type(db, binding))
+            });
+        result
     }
 
-    fn implicit_attribute_impl(
+    #[salsa::tracked(returns(copy), cycle_initial=|_, _, _| true, heap_size=ruff_memory_usage::heap_size)]
+    fn has_implicit_attribute_inner(
         db: &'db dyn Db,
         attribute: ImplicitAttributeName<'db>,
-    ) -> ImplicitAttribute<'db> {
+    ) -> bool {
+        match Self::implicit_attribute_impl(db, attribute, |_| ControlFlow::Break(())) {
+            ControlFlow::Break(()) => true,
+            ControlFlow::Continue(result) => !result.member.is_undefined(),
+        }
+    }
+
+    /// An ordinary reachable assignment establishes the attribute even when its value's type
+    /// is unknown. Callers that only need presence can stop before inferring that value.
+    fn implicit_attribute_impl<B>(
+        db: &'db dyn Db,
+        attribute: ImplicitAttributeName<'db>,
+        mut binding_type: impl FnMut(Definition<'db>) -> ControlFlow<B, Option<Type<'db>>>,
+    ) -> ControlFlow<B, ImplicitAttribute<'db>> {
         let class_body_scope = attribute.class_body_scope(db);
         let name = attribute.name(db).as_str();
         let target_method_decorator = attribute.target_method_decorator(db);
@@ -208,14 +242,14 @@ impl<'db> StaticClassLiteral<'db> {
                             index.expression(value),
                             TypeContext::default(),
                         );
-                        return ImplicitAttribute {
+                        return ControlFlow::Continue(ImplicitAttribute {
                             member: Member {
                                 inner: Place::bound(inferred_ty)
                                     .with_definition(declaration)
                                     .with_qualifiers(all_qualifiers),
                             },
                             augmented_bindings: None,
-                        };
+                        });
                     }
 
                     // If there is no right-hand side, just record that we saw a `Final` qualifier
@@ -223,10 +257,10 @@ impl<'db> StaticClassLiteral<'db> {
                     continue;
                 }
 
-                return ImplicitAttribute {
+                return ControlFlow::Continue(ImplicitAttribute {
                     member: Member { inner: annotation },
                     augmented_bindings: None,
-                };
+                });
             }
         }
 
@@ -293,7 +327,7 @@ impl<'db> StaticClassLiteral<'db> {
                     is_attribute_bound = true;
                 }
 
-                let inferred_ty = implicit_attribute_binding_type(db, binding);
+                let inferred_ty = binding_type(binding)?;
 
                 if let Some(inferred_ty) = inferred_ty {
                     provenance = provenance.or(Provenance::SingleDefinition(binding));
@@ -317,11 +351,11 @@ impl<'db> StaticClassLiteral<'db> {
             Member::unbound()
         };
 
-        ImplicitAttribute {
+        ControlFlow::Continue(ImplicitAttribute {
             member,
             augmented_bindings: (!augmented_bindings.is_empty())
                 .then(|| AugmentedBindings::new(db, augmented_bindings.into_boxed_slice())),
-        }
+        })
     }
 }
 

crates/ty_python_semantic/src/types/class/static_literal.rs

@@ -3045,36 +3045,41 @@ impl<'db> StaticClassLiteral<'db> {
                     if has_binding {
                         // The attribute is declared and bound in the class body.
 
-                        let implicit = self.implicit_attribute(db, name, MethodDecorator::None);
+                        let implicit = match declaredness {
+                            Definedness::AlwaysDefined
+                                if self.has_implicit_instance_attribute(db, name) =>
+                            {
+                                // A method assigns to this attribute, but its declared type
+                                // already determines what an instance can read.
+                                return Member {
+                                    inner: declared.with_qualifiers(qualifiers),
+                                };
+                            }
+                            Definedness::AlwaysDefined => Member::unbound(),
+                            Definedness::PossiblyUndefined => {
+                                self.implicit_attribute(db, name, MethodDecorator::None)
+                            }
+                        };
                         if let Place::Defined(DefinedPlace {
                             ty: implicit_ty,
                             provenance: implicit_provenance,
                             ..
                         }) = implicit.inner.place
                         {
-                            if declaredness == Definedness::AlwaysDefined {
-                                // If a symbol is definitely declared, and we see
-                                // attribute assignments in methods of the class,
-                                // we trust the declared type.
-                                Member {
-                                    inner: declared.with_qualifiers(qualifiers),
-                                }
-                            } else {
-                                Member {
-                                    inner: Place::Defined(DefinedPlace {
-                                        ty: UnionType::from_two_elements(
-                                            db,
-                                            env,
-                                            declared_ty,
-                                            implicit_ty,
-                                        ),
-                                        origin: TypeOrigin::Declared,
-                                        definedness: declaredness,
-                                        public_type_policy: PublicTypePolicy::Raw,
-                                        provenance: implicit_provenance.or(declared_provenance),
-                                    })
-                                    .with_qualifiers(qualifiers),
-                                }
+                            Member {
+                                inner: Place::Defined(DefinedPlace {
+                                    ty: UnionType::from_two_elements(
+                                        db,
+                                        env,
+                                        declared_ty,
+                                        implicit_ty,
+                                    ),
+                                    origin: TypeOrigin::Declared,
+                                    definedness: declaredness,
+                                    public_type_policy: PublicTypePolicy::Raw,
+                                    provenance: implicit_provenance.or(declared_provenance),
+                                })
+                                .with_qualifiers(qualifiers),
                             }
                         } else if self.is_own_dataclass_instance_field(db, name)
                             && declared_ty

crates/ty_python_semantic/src/types/infer/tests.rs

@@ -1644,6 +1644,45 @@ fn declared_instance_attribute_avoids_inferring_method_assignments() -> anyhow::
     Ok(())
 }
 
+#[test]
+fn inherited_class_defaults_avoid_inferring_instance_assignments() -> anyhow::Result<()> {
+    let mut db = setup_db();
+    db.write_file("/src/dependency.py", "value = 'updated'")?;
+    db.write_dedented(
+        "/src/bases.py",
+        r#"
+        class First:
+            value: int = 0
+
+        class Second:
+            value: str = ""
+
+            def reset(self):
+                from dependency import value
+                self.value = value
+        "#,
+    )?;
+    db.write_dedented(
+        "/src/main.py",
+        r#"
+        from bases import First, Second
+
+        class Child(First, Second): ...
+        "#,
+    )?;
+
+    let dependency = system_path_to_file(&db, "/src/dependency.py")?;
+    db.clear_salsa_events();
+    assert_file_diagnostics(
+        &db,
+        "/src/main.py",
+        &["Incompatible inherited attribute `value`"],
+    );
+    let events = db.take_salsa_events();
+    assert_function_query_was_not_run(&db, semantic_index, program_file(&db, dependency), &events);
+    Ok(())
+}
+
 #[test]
 fn dependency_implicit_instance_attribute() -> anyhow::Result<()> {
     fn x_rhs_expression(db: &TestDb) -> Expression<'_> {