diff --git a/crates/ty_python_semantic/resources/mdtest/attributes.md b/crates/ty_python_semantic/resources/mdtest/attributes.md index 993bf64594..c50d157ed7 100644 --- a/crates/ty_python_semantic/resources/mdtest/attributes.md +++ b/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 ``" 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 `` 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`. diff --git a/crates/ty_python_semantic/src/types.rs b/crates/ty_python_semantic/src/types.rs index 45db3b20ba..a1c9076b9b 100644 --- a/crates/ty_python_semantic/src/types.rs +++ b/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> { + 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) => { diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 67ee634197..3716078370 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/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> 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 { + 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() } } diff --git a/crates/ty_python_semantic/src/types/infer.rs b/crates/ty_python_semantic/src/types/infer.rs index 4cfa98c2a9..997e3a6eed 100644 --- a/crates/ty_python_semantic/src/types/infer.rs +++ b/crates/ty_python_semantic/src/types/infer.rs @@ -10918,4 +10918,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(()) + } }