diff --git a/crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md b/crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md index cc9661b54f..f3617fff74 100644 --- a/crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md +++ b/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. diff --git a/crates/ty_python_semantic/src/types/class.rs b/crates/ty_python_semantic/src/types/class.rs index 83870384ee..d0db73c75a 100644 --- a/crates/ty_python_semantic/src/types/class.rs +++ b/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) = - 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 +1821,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,