[ty] Add decorator check for implicit attribute assignments

crates/ty_python_semantic/resources/mdtest/attributes.md

@@ -522,8 +522,8 @@ class C:
 # error: [unresolved-attribute]
 reveal_type(C.x)  # revealed: Unknown
 
-# TODO: this should raise `unresolved-attribute` as well, and the type should be `Unknown`
-reveal_type(C().x)  # revealed: Unknown | Literal[1]
+# error: [unresolved-attribute]
+reveal_type(C().x)  # revealed: Unknown
 
 # This also works if `staticmethod` is aliased:
 
@@ -537,8 +537,8 @@ class D:
 # error: [unresolved-attribute]
 reveal_type(D.x)  # revealed: Unknown
 
-# TODO: this should raise `unresolved-attribute` as well, and the type should be `Unknown`
-reveal_type(D().x)  # revealed: Unknown | Literal[1]
+# error: [unresolved-attribute]
+reveal_type(D().x)  # revealed: Unknown
 ```
 
 If `staticmethod` is something else, that should not influence the behavior:
@@ -571,8 +571,8 @@ class C:
 # error: [unresolved-attribute]
 reveal_type(C.x)  # revealed: Unknown
 
-# TODO: this should raise `unresolved-attribute` as well, and the type should be `Unknown`
-reveal_type(C().x)  # revealed: Unknown | Literal[1]
+# error: [unresolved-attribute]
+reveal_type(C().x)  # revealed: Unknown
 ```
 
 #### Attributes defined in statically-known-to-be-false branches
@@ -742,17 +742,9 @@ class C:
 # for a more realistic example, let's actually call the method
 C.class_method()
 
-# TODO: We currently plan to support this and show no error here.
-# mypy shows an error here, pyright does not.
-# error: [unresolved-attribute]
-reveal_type(C.pure_class_variable)  # revealed: Unknown
+reveal_type(C.pure_class_variable)  # revealed: Unknown | Literal["value set in class method"]
 
-# TODO: should be no error when descriptor protocol is supported
-# and the assignment is properly attributed to the class method.
-# error: [invalid-attribute-access] "Cannot assign to instance attribute `pure_class_variable` from the class object `<class 'C'>`"
 C.pure_class_variable = "overwritten on class"
-# TODO: should be no error
-# error: [unresolved-attribute] "Attribute `pure_class_variable` can only be accessed on instances, not on the class object `<class 'C'>` itself."
 reveal_type(C.pure_class_variable)  # revealed: Literal["overwritten on class"]
 
 c_instance = C()
@@ -2150,6 +2142,25 @@ class C:
 reveal_type(C().x)  # revealed: int
 ```
 
+### Attributes defined in methods with unknown decorators
+
+When an attribute is defined in a method that is decorated with an unknown decorator, we consider it
+to be accessible on both the class itself and instances of that class. This is consistent with the
+gradual guarantee, because the unknown decorator *could* be an alias for `builtins.classmethod`.
+
+```py
+# error: [unresolved-import]
+from unknown_library import unknown_decorator
+
+class C:
+    @unknown_decorator
+    def f(self):
+        self.x: int = 1
+
+reveal_type(C.x)  # revealed: int
+reveal_type(C().x)  # revealed: int
+```
+
 ## Enum classes
 
 Enums are not supported yet; attribute access on an enum class is inferred as `Todo`.

crates/ty_python_semantic/src/types.rs

@@ -2503,6 +2503,18 @@ impl<'db> Type<'db> {
         }
     }
 
+    #[salsa::tracked]
+    #[allow(unused_variables)]
+    // If we choose name `_unit`, the macro will generate code that uses `_unit`, causing clippy to fail.
+    fn lookup_dunder_new(self, db: &'db dyn Db, unit: ()) -> Option<PlaceAndQualifiers<'db>> {
+        self.find_name_in_mro_with_policy(
+            db,
+            "__new__",
+            MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK
+                | MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK,
+        )
+    }
+
     /// Look up an attribute in the MRO of the meta-type of `self`. This returns class-level attributes
     /// when called on an instance-like type, and metaclass attributes when called on a class-like type.
     ///
@@ -4662,12 +4674,7 @@ impl<'db> Type<'db> {
         // An alternative might be to not skip `object.__new__` but instead mark it such that it's
         // easy to check if that's the one we found?
         // Note that `__new__` is a static method, so we must inject the `cls` argument.
-        let new_method = self_type.find_name_in_mro_with_policy(
-            db,
-            "__new__",
-            MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK
-                | MemberLookupPolicy::META_CLASS_NO_TYPE_FALLBACK,
-        );
+        let new_method = self_type.lookup_dunder_new(db, ());
         let new_call_outcome = new_method.and_then(|new_method| {
             match new_method.place.try_call_dunder_get(db, self_type) {
                 Place::Type(new_method, boundness) => {

crates/ty_python_semantic/src/types/class.rs

@@ -4,8 +4,10 @@ use std::sync::{LazyLock, Mutex};
 use super::TypeVarVariance;
 use super::{
     IntersectionBuilder, MemberLookupPolicy, Mro, MroError, MroIterator, SpecialFormType,
-    SubclassOfType, Truthiness, Type, TypeQualifiers, class_base::ClassBase, infer_expression_type,
-    infer_unpack_types,
+    SubclassOfType, Truthiness, Type, TypeQualifiers,
+    class_base::ClassBase,
+    function::{FunctionDecorators, FunctionType},
+    infer_expression_type, infer_unpack_types,
 };
 use crate::semantic_index::DeclarationWithConstraint;
 use crate::semantic_index::definition::{Definition, DefinitionState};
@@ -639,6 +641,29 @@ impl<'db> From<ClassType<'db>> for Type<'db> {
     }
 }
 
+/// A filter that describes which methods are considered when looking for implicit attribute assignments
+/// in [`ClassLiteral::implicit_attribute`].
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub(super) enum MethodDecorator {
+    None,
+    ClassMethod,
+    StaticMethod,
+}
+
+impl MethodDecorator {
+    fn try_from_fn_type(db: &dyn Db, fn_type: FunctionType) -> Result<Self, ()> {
+        match (
+            fn_type.has_known_decorator(db, FunctionDecorators::CLASSMETHOD),
+            fn_type.has_known_decorator(db, FunctionDecorators::STATICMETHOD),
+        ) {
+            (true, true) => Err(()), // A method can't be static and class method at the same time.
+            (true, false) => Ok(Self::ClassMethod),
+            (false, true) => Ok(Self::StaticMethod),
+            (false, false) => Ok(Self::None),
+        }
+    }
+}
+
 /// Representation of a class definition statement in the AST: either a non-generic class, or a
 /// generic class that has not been specialized.
 ///
@@ -1284,8 +1309,10 @@ impl<'db> ClassLiteral<'db> {
             {
                 return Place::bound(synthesized_member).into();
             }
+            // The symbol was not found in the class scope. It might still be implicitly defined in `@classmethod`s.
+            return Self::implicit_attribute(db, body_scope, name, MethodDecorator::ClassMethod)
+                .into();
         }
-
         symbol
     }
 
@@ -1601,12 +1628,15 @@ impl<'db> ClassLiteral<'db> {
         }
     }
 
-    /// Tries to find declarations/bindings of an instance attribute named `name` that are only
-    /// "implicitly" defined in a method of the class that corresponds to `class_body_scope`.
-    fn implicit_instance_attribute(
+    /// Tries to find declarations/bindings of an attribute named `name` that are only
+    /// "implicitly" defined (`self.x = …`, `cls.x = …`) in a method of the class that
+    /// corresponds to `class_body_scope`. The `target_method_decorator` parameter is
+    /// used to skip methods that do not have the expected decorator.
+    fn implicit_attribute(
         db: &'db dyn Db,
         class_body_scope: ScopeId<'db>,
         name: &str,
+        target_method_decorator: MethodDecorator,
     ) -> Place<'db> {
         // If we do not see any declarations of an attribute, neither in the class body nor in
         // any method, we build a union of `Unknown` with the inferred types of all bindings of
@@ -1626,6 +1656,17 @@ impl<'db> ClassLiteral<'db> {
             attribute_assignments(db, class_body_scope, name)
         {
             let method_scope = method_scope_id.to_scope_id(db, file);
+            if let Some(method_def) = method_scope.node(db).as_function(&module) {
+                let method_name = method_def.name.as_str();
+                if let Place::Type(Type::FunctionLiteral(method_type), _) =
+                    class_symbol(db, class_body_scope, method_name).place
+                {
+                    let method_decorator = MethodDecorator::try_from_fn_type(db, method_type);
+                    if method_decorator != Ok(target_method_decorator) {
+                        continue;
+                    }
+                }
+            }
             let method_map = use_def_map(db, method_scope);
 
             // The attribute assignment inherits the reachability of the method which contains it
@@ -1901,7 +1942,7 @@ impl<'db> ClassLiteral<'db> {
                         // The attribute is declared and bound in the class body.
 
                         if let Some(implicit_ty) =
-                            Self::implicit_instance_attribute(db, body_scope, name)
+                            Self::implicit_attribute(db, body_scope, name, MethodDecorator::None)
                                 .ignore_possibly_unbound()
                         {
                             if declaredness == Boundness::Bound {
@@ -1935,9 +1976,13 @@ 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) = Self::implicit_attribute(
+                                db,
+                                body_scope,
+                                name,
+                                MethodDecorator::None,
+                            )
+                            .ignore_possibly_unbound()
                             {
                                 Place::Type(
                                     UnionType::from_elements(db, [declared_ty, implicit_ty]),
@@ -1958,7 +2003,7 @@ impl<'db> ClassLiteral<'db> {
                     // The attribute is not *declared* in the class body. It could still be declared/bound
                     // in a method.
 
-                    Self::implicit_instance_attribute(db, body_scope, name).into()
+                    Self::implicit_attribute(db, body_scope, name, MethodDecorator::None).into()
                 }
                 Err((declared, _conflicting_declarations)) => {
                     // There are conflicting declarations for this attribute in the class body.
@@ -1969,7 +2014,7 @@ impl<'db> ClassLiteral<'db> {
             // This attribute is neither declared nor bound in the class body.
             // It could still be implicitly defined in a method.
 
-            Self::implicit_instance_attribute(db, body_scope, name).into()
+            Self::implicit_attribute(db, body_scope, name, MethodDecorator::None).into()
         }
     }
 

crates/ty_python_semantic/src/types/infer.rs

@@ -10905,4 +10905,98 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn dependency_implicit_class_member() -> anyhow::Result<()> {
+        fn x_rhs_expression(db: &TestDb) -> Expression<'_> {
+            let file_main = system_path_to_file(db, "/src/main.py").unwrap();
+            let ast = parsed_module(db, file_main).load(db);
+            // Get the third statement in `main.py` (x = …) and extract the expression
+            // node on the right-hand side:
+            let x_rhs_node = &ast.syntax().body[2].as_assign_stmt().unwrap().value;
+
+            let index = semantic_index(db, file_main);
+            index.expression(x_rhs_node.as_ref())
+        }
+
+        let mut db = setup_db();
+
+        db.write_dedented(
+            "/src/mod.py",
+            r#"
+            class C:
+                def __init__(self):
+                    self.instance_attr: str = "24"
+
+                @classmethod
+                def method(cls):
+                    cls.class_attr: int = 42
+            "#,
+        )?;
+        db.write_dedented(
+            "/src/main.py",
+            r#"
+            from mod import C
+            C.method()
+            x = C().class_attr
+            "#,
+        )?;
+
+        let file_main = system_path_to_file(&db, "/src/main.py").unwrap();
+        let attr_ty = global_symbol(&db, file_main, "x").place.expect_type();
+        assert_eq!(attr_ty.display(&db).to_string(), "Unknown | int");
+
+        // Change the type of `class_attr` to `str`; this should trigger the type of `x` to be re-inferred
+        db.write_dedented(
+            "/src/mod.py",
+            r#"
+            class C:
+                def __init__(self):
+                    self.instance_attr: str = "24"
+
+                @classmethod
+                def method(cls):
+                    cls.class_attr: str = "42"
+            "#,
+        )?;
+
+        let events = {
+            db.clear_salsa_events();
+            let attr_ty = global_symbol(&db, file_main, "x").place.expect_type();
+            assert_eq!(attr_ty.display(&db).to_string(), "Unknown | str");
+            db.take_salsa_events()
+        };
+        assert_function_query_was_run(&db, infer_expression_types, x_rhs_expression(&db), &events);
+
+        // Add a comment; this should not trigger the type of `x` to be re-inferred
+        db.write_dedented(
+            "/src/mod.py",
+            r#"
+            class C:
+                def __init__(self):
+                    self.instance_attr: str = "24"
+
+                @classmethod
+                def method(cls):
+                    # comment
+                    cls.class_attr: str = "42"
+            "#,
+        )?;
+
+        let events = {
+            db.clear_salsa_events();
+            let attr_ty = global_symbol(&db, file_main, "x").place.expect_type();
+            assert_eq!(attr_ty.display(&db).to_string(), "Unknown | str");
+            db.take_salsa_events()
+        };
+
+        assert_function_query_was_not_run(
+            &db,
+            infer_expression_types,
+            x_rhs_expression(&db),
+            &events,
+        );
+
+        Ok(())
+    }
 }