Merge branch 'main' into alex/protocol-property-check-2

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

crates/ty_python_semantic/resources/mdtest/annotations/callable.md

@@ -397,10 +397,14 @@ def f_okay(c: Callable[[], None]):
         # error: [invalid-assignment] "Object of type `Literal["my_callable"]` is not assignable to attribute `__qualname__` on type `(() -> None) & <Protocol with members '__qualname__'>`"
         c.__qualname__ = "my_callable"
 
-        result = getattr_static(c, "__qualname__")
-        reveal_type(result)  # revealed: property
-        if isinstance(result, property) and result.fset:
-            c.__qualname__ = "my_callable"  # okay
+        # TODO: should we have some way for users to narrow a read-only attribute
+        # into a writable attribute...? What would that look like? Something like this?
+        if (
+            hasattr(type(c), "__qualname__")
+            and isinstance(type(c).__qualname__, property)
+            and type(c).__qualname__.fset is not None
+        ):
+            c.__qualname__ = "my_callable"  # error: [invalid-assignment]
 ```
 
 [gradual form]: https://typing.python.org/en/latest/spec/glossary.html#term-gradual-form

crates/ty_python_semantic/resources/mdtest/attributes.md

@@ -49,7 +49,7 @@ c_instance.inferred_from_value = "value set on instance"
 # This assignment is also fine:
 c_instance.declared_and_bound = False
 
-# error: [invalid-assignment] "Object of type `Literal["incompatible"]` is not assignable to attribute `declared_and_bound` of type `bool`"
+# error: [invalid-assignment] "Object of type `Literal["incompatible"]` is not assignable to attribute `declared_and_bound` on type `bool`"
 c_instance.declared_and_bound = "incompatible"
 
 # mypy shows no error here, but pyright raises "reportAttributeAccessIssue"
@@ -92,7 +92,7 @@ reveal_type(C.declared_and_bound)  # revealed: str | None
 
 C.declared_and_bound = "overwritten on class"
 
-# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `declared_and_bound` of type `str | None`"
+# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `declared_and_bound` on type `str | None`"
 c_instance.declared_and_bound = 1
 ```
 
@@ -704,7 +704,7 @@ c_instance.pure_class_variable1 = "value set on instance"
 
 C.pure_class_variable1 = "overwritten on class"
 
-# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `pure_class_variable1` of type `str`"
+# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `pure_class_variable1` on type `str`"
 C.pure_class_variable1 = 1
 
 class Subclass(C):
@@ -1118,7 +1118,7 @@ def _(flag: bool):
     reveal_type(C2.y)  # revealed: int | str
 
     C2.y = 100
-    # error: [invalid-assignment] "Object of type `None` is not assignable to attribute `y` of type `int | str`"
+    # error: [invalid-assignment] "Object of type `None` is not assignable to attribute `y` on type `int | str`"
     C2.y = None
     # TODO: should be an error, needs more sophisticated union handling in `validate_attribute_assignment`
     C2.y = "problematic"
@@ -1138,7 +1138,7 @@ def _(flag: bool):
     reveal_type(C3.y)  # revealed: int | str
 
     C3.y = 100
-    # error: [invalid-assignment] "Object of type `None` is not assignable to attribute `y` of type `int | str`"
+    # error: [invalid-assignment] "Object of type `None` is not assignable to attribute `y` on type `int | str`"
     C3.y = None
     # TODO: should be an error, needs more sophisticated union handling in `validate_attribute_assignment`
     C3.y = "problematic"
@@ -1156,7 +1156,7 @@ def _(flag: bool):
     reveal_type(C4.y)  # revealed: int | str
 
     C4.y = 100
-    # error: [invalid-assignment] "Object of type `None` is not assignable to attribute `y` of type `int | str`"
+    # error: [invalid-assignment] "Object of type `None` is not assignable to attribute `y` on type `int | str`"
     C4.y = None
     # TODO: should be an error, needs more sophisticated union handling in `validate_attribute_assignment`
     C4.y = "problematic"
@@ -1253,7 +1253,7 @@ def _(flag: bool):
     # see a type of `int | Any` above because we have the full union handling of possibly-unbound
     # *instance* attributes.
 
-    # error: [invalid-assignment] "Object of type `Literal["a"]` is not assignable to attribute `x` of type `int`"
+    # error: [invalid-assignment] "Object of type `Literal["a"]` is not assignable to attribute `x` on type `int`"
     Derived().x = "a"
 ```
 
@@ -1958,10 +1958,10 @@ import mod
 reveal_type(mod.global_symbol)  # revealed: str
 mod.global_symbol = "b"
 
-# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `global_symbol` of type `str`"
+# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `global_symbol` on type `str`"
 mod.global_symbol = 1
 
-# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `global_symbol` of type `str`"
+# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `global_symbol` on type `str`"
 (_, mod.global_symbol) = (..., 1)
 
 # TODO: this should be an error, but we do not understand list unpackings yet.
@@ -1975,7 +1975,7 @@ class IntIterable:
     def __iter__(self) -> IntIterator:
         return IntIterator()
 
-# error: [invalid-assignment] "Object of type `int` is not assignable to attribute `global_symbol` of type `str`"
+# error: [invalid-assignment] "Object of type `int` is not assignable to attribute `global_symbol` on type `str`"
 for mod.global_symbol in IntIterable():
     pass
 ```

crates/ty_python_semantic/resources/mdtest/dataclasses/dataclasses.md

@@ -898,6 +898,7 @@ class Foo:
 foo = Foo(1)
 
 reveal_type(foo.__dataclass_fields__)  # revealed: dict[str, Field[Any]]
+reveal_type(type(foo).__dataclass_fields__)  # revealed: dict[str, Field[Any]]
 reveal_type(fields(Foo))  # revealed: tuple[Field[Any], ...]
 reveal_type(asdict(foo))  # revealed: dict[str, Any]
 ```
@@ -918,8 +919,7 @@ reveal_type(fields(Foo))  # revealed: tuple[Field[Any], ...]
 But calling `asdict` on the class object is not allowed:
 
 ```py
-# TODO: this should be a invalid-argument-type error, but we don't properly check the
-# types (and more importantly, the `ClassVar` type qualifier) of protocol members yet.
+# error: [invalid-argument-type] "Argument to function `asdict` is incorrect: Expected `DataclassInstance`, found `<class 'Foo'>`"
 asdict(Foo)
 ```
 

crates/ty_python_semantic/resources/mdtest/descriptor_protocol.md

@@ -45,9 +45,9 @@ body, we do not allow these assignments, preventing users from accidentally over
 descriptor, which is what would happen at runtime:
 
 ```py
-# error: [invalid-assignment] "Object of type `Literal[10]` is not assignable to attribute `ten` of type `Ten`"
+# error: [invalid-assignment] "Object of type `Literal[10]` is not assignable to attribute `ten` on type `Ten`"
 C.ten = 10
-# error: [invalid-assignment] "Object of type `Literal[11]` is not assignable to attribute `ten` of type `Ten`"
+# error: [invalid-assignment] "Object of type `Literal[11]` is not assignable to attribute `ten` on type `Ten`"
 C.ten = 11
 ```
 
@@ -213,7 +213,7 @@ reveal_type(C().ten)  # revealed: Ten
 C().ten = Ten()
 
 # The instance attribute is declared as `Ten`, so this is an
-# error: [invalid-assignment] "Object of type `Literal[10]` is not assignable to attribute `ten` of type `Ten`"
+# error: [invalid-assignment] "Object of type `Literal[10]` is not assignable to attribute `ten` on type `Ten`"
 C().ten = 10
 ```
 
@@ -280,7 +280,7 @@ overwrite the data descriptor, but the attribute is declared as `DataDescriptor`
 so we do not allow this:
 
 ```py
-# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `class_data_descriptor` of type `DataDescriptor`"
+# error: [invalid-assignment] "Object of type `Literal[1]` is not assignable to attribute `class_data_descriptor` on type `DataDescriptor`"
 C1.class_data_descriptor = 1
 ```
 
@@ -372,7 +372,7 @@ def _(flag: bool):
     # wrong, but they could be subsumed under a higher-level diagnostic.
 
     # error: [invalid-assignment] "Invalid assignment to data descriptor attribute `meta_data_descriptor1` on type `<class 'C5'>` with custom `__set__` method"
-    # error: [invalid-assignment] "Object of type `None` is not assignable to attribute `meta_data_descriptor1` of type `Literal["value on class"]`"
+    # error: [invalid-assignment] "Object of type `None` is not assignable to attribute `meta_data_descriptor1` on type `Literal["value on class"]`"
     C5.meta_data_descriptor1 = None
 
     # error: [possibly-unbound-attribute]

crates/ty_python_semantic/resources/mdtest/doc/public_type_undeclared_symbols.md

@@ -98,7 +98,7 @@ o = OptionalInt()
 reveal_type(o.value)
 
 # Incompatible assignments are now caught:
-# error: "Object of type `Literal["a"]` is not assignable to attribute `value` of type `int | None`"
+# error: "Object of type `Literal["a"]` is not assignable to attribute `value` on type `int | None`"
 o.value = "a"
 ```
 

crates/ty_python_semantic/resources/mdtest/named_tuple.md

@@ -78,7 +78,7 @@ reveal_type(Person.id)  # revealed: property
 reveal_type(Person.name)  # revealed: property
 reveal_type(Person.age)  # revealed: property
 
-# error: [invalid-assignment] "Cannot assign to read-only property `id` on object of type `Person`"
+# error: [invalid-assignment] "Attribute `id` on object of type `Person` is read-only"
 alice.id = 42
 # error: [invalid-assignment]
 bob.age = None
@@ -218,7 +218,7 @@ james = SuperUser(0, "James", 42, "Jimmy")
 # on the subclass
 james.name = "Robert"
 
-# error: [invalid-assignment] "Cannot assign to read-only property `nickname` on object of type `SuperUser`"
+# error: [invalid-assignment] "Attribute `nickname` on object of type `SuperUser` is read-only"
 james.nickname = "Bob"
 ```
 

crates/ty_python_semantic/resources/mdtest/protocols.md

@@ -413,7 +413,7 @@ To see the kinds and types of the protocol members, you can use the debugging ai
 from ty_extensions import reveal_protocol_interface
 from typing import SupportsIndex, SupportsAbs, ClassVar, Iterator
 
-# error: [revealed-type] "Revealed protocol interface: `{"method_member": MethodMember(`(self) -> bytes`), "x": AttributeMember(`int`), "y": PropertyMember { getter: `def y(self) -> str` }, "z": PropertyMember { getter: `def z(self) -> int`, setter: `def z(self, z: int) -> None` }}`"
+# error: [revealed-type] "Revealed protocol interface: `{"method_member": MethodMember(`(self) -> bytes`), "x": AttributeMember(`int`), "y": PropertyMember { get_type: `str` }, "z": PropertyMember { get_type: `int`, set_type: `int` }}`"
 reveal_protocol_interface(Foo)
 # error: [revealed-type] "Revealed protocol interface: `{"__index__": MethodMember(`(self) -> int`)}`"
 reveal_protocol_interface(SupportsIndex)
@@ -706,12 +706,13 @@ class HasClassVarX(Protocol):
 
 static_assert(is_subtype_of(FooWithZero, HasClassVarX))
 static_assert(is_assignable_to(FooWithZero, HasClassVarX))
+
 # TODO: these should pass
 static_assert(not is_subtype_of(Foo, HasClassVarX))  # error: [static-assert-error]
 static_assert(not is_assignable_to(Foo, HasClassVarX))  # error: [static-assert-error]
-static_assert(not is_subtype_of(Qux, HasClassVarX))  # error: [static-assert-error]
-static_assert(not is_assignable_to(Qux, HasClassVarX))  # error: [static-assert-error]
 
+static_assert(not is_subtype_of(Qux, HasClassVarX))
+static_assert(not is_assignable_to(Qux, HasClassVarX))
 static_assert(is_subtype_of(Sequence[Foo], Sequence[HasX]))
 static_assert(is_assignable_to(Sequence[Foo], Sequence[HasX]))
 static_assert(not is_subtype_of(list[Foo], list[HasX]))
@@ -731,16 +732,14 @@ class A:
     def x(self) -> int:
         return 42
 
-# TODO: these should pass
-static_assert(not is_subtype_of(A, HasX))  # error: [static-assert-error]
-static_assert(not is_assignable_to(A, HasX))  # error: [static-assert-error]
+static_assert(not is_subtype_of(A, HasX))
+static_assert(not is_assignable_to(A, HasX))
 
 class B:
     x: Final = 42
 
-# TODO: these should pass
-static_assert(not is_subtype_of(A, HasX))  # error: [static-assert-error]
-static_assert(not is_assignable_to(A, HasX))  # error: [static-assert-error]
+static_assert(not is_subtype_of(A, HasX))
+static_assert(not is_assignable_to(A, HasX))
 
 class IntSub(int): ...
 
@@ -772,16 +771,14 @@ static_assert(is_assignable_to(MutableDataclass, HasX))
 class ImmutableDataclass:
     x: int
 
-# TODO: these should pass
-static_assert(not is_subtype_of(ImmutableDataclass, HasX))  # error: [static-assert-error]
-static_assert(not is_assignable_to(ImmutableDataclass, HasX))  # error: [static-assert-error]
+static_assert(not is_subtype_of(ImmutableDataclass, HasX))
+static_assert(not is_assignable_to(ImmutableDataclass, HasX))
 
 class NamedTupleWithX(NamedTuple):
     x: int
 
-# TODO: these should pass
-static_assert(not is_subtype_of(NamedTupleWithX, HasX))  # error: [static-assert-error]
-static_assert(not is_assignable_to(NamedTupleWithX, HasX))  # error: [static-assert-error]
+static_assert(not is_subtype_of(NamedTupleWithX, HasX))
+static_assert(not is_assignable_to(NamedTupleWithX, HasX))
 ```
 
 However, a type with a read-write property `x` *does* satisfy the `HasX` protocol. The `HasX`
@@ -1401,9 +1398,8 @@ class PropertyX:
     def x(self) -> int:
         return 42
 
-# TODO: these should pass
-static_assert(not is_assignable_to(PropertyX, ClassVarXProto))  # error: [static-assert-error]
-static_assert(not is_subtype_of(PropertyX, ClassVarXProto))  # error: [static-assert-error]
+static_assert(not is_assignable_to(PropertyX, ClassVarXProto))
+static_assert(not is_subtype_of(PropertyX, ClassVarXProto))
 
 class ClassVarX:
     x: ClassVar[int] = 42
@@ -1512,9 +1508,8 @@ class XReadProperty:
     def x(self) -> int:
         return 42
 
-# TODO: these should pass
-static_assert(not is_subtype_of(XReadProperty, HasXProperty))  # error: [static-assert-error]
-static_assert(not is_assignable_to(XReadProperty, HasXProperty))  # error: [static-assert-error]
+static_assert(is_subtype_of(XReadProperty, HasXProperty))
+static_assert(is_assignable_to(XReadProperty, HasXProperty))
 
 class XReadWriteProperty:
     @property
@@ -1598,9 +1593,8 @@ class MyIntSub(MyInt):
 class XAttrSubSub:
     x: MyIntSub
 
-# TODO: should pass
-static_assert(not is_subtype_of(XAttrSubSub, HasAsymmetricXProperty))  # error: [static-assert-error]
-static_assert(not is_assignable_to(XAttrSubSub, HasAsymmetricXProperty))  # error: [static-assert-error]
+static_assert(not is_subtype_of(XAttrSubSub, HasAsymmetricXProperty))
+static_assert(not is_assignable_to(XAttrSubSub, HasAsymmetricXProperty))
 ```
 
 An asymmetric property on a protocol can also be satisfied by an asymmetric property on a nominal
@@ -1649,17 +1643,15 @@ class HasGetAttr:
 static_assert(is_subtype_of(HasGetAttr, HasXProperty))
 static_assert(is_assignable_to(HasGetAttr, HasXProperty))
 
-# TODO: these should pass
-static_assert(not is_subtype_of(HasGetAttr, HasMutableXAttr))  # error: [static-assert-error]
-static_assert(not is_subtype_of(HasGetAttr, HasMutableXAttr))  # error: [static-assert-error]
+static_assert(not is_subtype_of(HasGetAttr, HasMutableXAttr))
+static_assert(not is_subtype_of(HasGetAttr, HasMutableXAttr))
 
 class HasGetAttrWithUnsuitableReturn:
     def __getattr__(self, attr: str) -> tuple[int, int]:
         return (1, 2)
 
-# TODO: these should pass
-static_assert(not is_subtype_of(HasGetAttrWithUnsuitableReturn, HasXProperty))  # error: [static-assert-error]
-static_assert(not is_assignable_to(HasGetAttrWithUnsuitableReturn, HasXProperty))  # error: [static-assert-error]
+static_assert(not is_subtype_of(HasGetAttrWithUnsuitableReturn, HasXProperty))
+static_assert(not is_assignable_to(HasGetAttrWithUnsuitableReturn, HasXProperty))
 
 class HasGetAttrAndSetAttr:
     def __getattr__(self, attr: str) -> MyInt:
@@ -1670,9 +1662,8 @@ class HasGetAttrAndSetAttr:
 static_assert(is_subtype_of(HasGetAttrAndSetAttr, HasXProperty))
 static_assert(is_assignable_to(HasGetAttrAndSetAttr, HasXProperty))
 
-# TODO: these should pass
-static_assert(is_subtype_of(HasGetAttrAndSetAttr, XAsymmetricProperty))  # error: [static-assert-error]
-static_assert(is_assignable_to(HasGetAttrAndSetAttr, XAsymmetricProperty))  # error: [static-assert-error]
+static_assert(is_subtype_of(HasGetAttrAndSetAttr, HasAsymmetricXProperty))
+static_assert(is_assignable_to(HasGetAttrAndSetAttr, HasAsymmetricXProperty))
 
 class HasSetAttrWithUnsuitableInput:
     def __getattr__(self, attr: str) -> int:
@@ -1680,9 +1671,8 @@ class HasSetAttrWithUnsuitableInput:
 
     def __setattr__(self, attr: str, value: str) -> None: ...
 
-# TODO: these should pass
-static_assert(not is_subtype_of(HasSetAttrWithUnsuitableInput, HasMutableXProperty))  # error: [static-assert-error]
-static_assert(not is_assignable_to(HasSetAttrWithUnsuitableInput, HasMutableXProperty))  # error: [static-assert-error]
+static_assert(not is_subtype_of(HasSetAttrWithUnsuitableInput, HasMutableXProperty))
+static_assert(not is_assignable_to(HasSetAttrWithUnsuitableInput, HasMutableXProperty))
 ```
 
 ## Subtyping of protocols with method members
@@ -1789,9 +1779,21 @@ from ty_extensions import is_equivalent_to, static_assert
 
 class P1(Protocol):
     def x(self, y: int) -> None: ...
+    @property
+    def y(self) -> str: ...
+    @property
+    def z(self) -> bytes: ...
+    @z.setter
+    def z(self, value: int) -> None: ...
 
 class P2(Protocol):
     def x(self, y: int) -> None: ...
+    @property
+    def y(self) -> str: ...
+    @property
+    def z(self) -> bytes: ...
+    @z.setter
+    def z(self, value: int) -> None: ...
 
 class P3(Protocol):
     @property
@@ -1810,9 +1812,7 @@ class P4(Protocol):
     def z(self, value: int) -> None: ...
 
 static_assert(is_equivalent_to(P1, P2))
-
-# TODO: should pass
-static_assert(is_equivalent_to(P3, P4))  # error: [static-assert-error]
+static_assert(is_equivalent_to(P3, P4))
 ```
 
 As with protocols that only have non-method members, this also holds true when they appear in
@@ -1823,9 +1823,7 @@ class A: ...
 class B: ...
 
 static_assert(is_equivalent_to(A | B | P1, P2 | B | A))
-
-# TODO: should pass
-static_assert(is_equivalent_to(A | B | P3, P4 | B | A))  # error: [static-assert-error]
+static_assert(is_equivalent_to(A | B | P3, P4 | B | A))
 ```
 
 ## Narrowing of protocols
@@ -2106,8 +2104,6 @@ class Bar(Protocol):
     @property
     def x(self) -> "Bar": ...
 
-# TODO: this should pass
-# error: [static-assert-error]
 static_assert(is_equivalent_to(Foo, Bar))
 
 T = TypeVar("T", bound="TypeVarRecursive")

crates/ty_python_semantic/resources/mdtest/snapshots/attribute_assignment…_-_Attribute_assignment_-_Instance_attributes_…_(ebfb3de6d1b96b23).snap

@@ -26,7 +26,7 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/diagnostics/attribute_as
 # Diagnostics
 
 ```
-error[invalid-assignment]: Object of type `Literal["wrong"]` is not assignable to attribute `attr` of type `int`
+error[invalid-assignment]: Object of type `Literal["wrong"]` is not assignable to attribute `attr` on type `int`
  --> src/mdtest_snippet.py:6:1
   |
 4 | instance = C()
@@ -41,7 +41,7 @@ info: rule `invalid-assignment` is enabled by default
 ```
 
 ```
-error[invalid-assignment]: Object of type `Literal["wrong"]` is not assignable to attribute `attr` of type `int`
+error[invalid-assignment]: Object of type `Literal["wrong"]` is not assignable to attribute `attr` on type `int`
  --> src/mdtest_snippet.py:9:1
   |
 8 | C.attr = 1  # fine

crates/ty_python_semantic/resources/mdtest/snapshots/attribute_assignment…_-_Attribute_assignment_-_Pure_instance_attrib…_(d13d57d3cc36face).snap

@@ -26,7 +26,7 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/diagnostics/attribute_as
 # Diagnostics
 
 ```
-error[invalid-assignment]: Object of type `Literal["wrong"]` is not assignable to attribute `attr` of type `int`
+error[invalid-assignment]: Object of type `Literal["wrong"]` is not assignable to attribute `attr` on type `int`
  --> src/mdtest_snippet.py:7:1
   |
 5 | instance = C()

crates/ty_python_semantic/resources/mdtest/snapshots/attribute_assignment…_-_Attribute_assignment_-_`ClassVar`s_(8d7cca27987b099d).snap

@@ -27,7 +27,7 @@ mdtest path: crates/ty_python_semantic/resources/mdtest/diagnostics/attribute_as
 # Diagnostics
 
 ```
-error[invalid-assignment]: Object of type `Literal["wrong"]` is not assignable to attribute `attr` of type `int`
+error[invalid-assignment]: Object of type `Literal["wrong"]` is not assignable to attribute `attr` on type `int`
  --> src/mdtest_snippet.py:7:1
   |
 6 | C.attr = 1  # fine

crates/ty_python_semantic/resources/mdtest/type_properties/is_disjoint_from.md

@@ -489,7 +489,7 @@ static_assert(is_disjoint_from(type[UsesMeta1], type[UsesMeta2]))
 
 ```py
 from ty_extensions import is_disjoint_from, static_assert, TypeOf
-from typing import final
+from typing import final, Protocol, Literal
 
 class C:
     @property
@@ -508,6 +508,29 @@ static_assert(not is_disjoint_from(Whatever, TypeOf[C.prop]))
 static_assert(not is_disjoint_from(TypeOf[C.prop], Whatever))
 static_assert(is_disjoint_from(TypeOf[C.prop], D))
 static_assert(is_disjoint_from(D, TypeOf[C.prop]))
+
+@final
+class E:
+    @property
+    def prop(self) -> int:
+        return 1
+
+class F:
+    prop: Literal["a"]
+
+class HasIntProp(Protocol):
+    @property
+    def prop(self) -> int: ...
+
+class HasReadWriteIntProp(Protocol):
+    @property
+    def prop(self) -> int: ...
+    @prop.setter
+    def prop(self, value: int) -> None: ...
+
+static_assert(not is_disjoint_from(HasIntProp, E))
+static_assert(is_disjoint_from(HasIntProp, F))
+static_assert(is_disjoint_from(HasReadWriteIntProp, E))
 ```
 
 ### `TypeGuard` and `TypeIs`

crates/ty_python_semantic/src/types.rs

@@ -243,6 +243,74 @@ impl AttributeKind {
     }
 }
 
+#[derive(Debug, Default)]
+pub(crate) struct AttributeAssignmentErrors<'db>(FxOrderSet<AttributeAssignmentError<'db>>);
+
+impl<'db> IntoIterator for AttributeAssignmentErrors<'db> {
+    type Item = AttributeAssignmentError<'db>;
+    type IntoIter = ordermap::set::IntoIter<AttributeAssignmentError<'db>>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.0.into_iter()
+    }
+}
+
+impl<'db> AttributeAssignmentErrors<'db> {
+    pub(crate) fn is_possibly_unbound(&self) -> bool {
+        self.0
+            .iter()
+            .any(AttributeAssignmentError::is_possibly_unbound)
+    }
+
+    fn insert(&mut self, result: AttributeAssignmentError<'db>) {
+        self.0.insert(result);
+    }
+
+    fn insert_if_error<T>(&mut self, result: Result<T, AttributeAssignmentError<'db>>) {
+        if let Err(error) = result {
+            self.insert(error);
+        }
+    }
+
+    fn and<T>(mut self, result: Result<T, AttributeAssignmentError<'db>>) -> Result<T, Self> {
+        match result {
+            Ok(value) => {
+                if self.0.is_empty() {
+                    Ok(value)
+                } else {
+                    Err(self)
+                }
+            }
+            Err(error) => {
+                self.0.insert(error);
+                Err(self)
+            }
+        }
+    }
+}
+
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+pub(crate) enum AttributeAssignmentError<'db> {
+    PossiblyUnbound,
+    TypeMismatch(Type<'db>),
+    CannotAssign,
+    CannotAssignToClassVar,
+    CannotAssignToInstanceAttr,
+    CannotAssignToFinal,
+    CannotAssignToUnresolved,
+    ReadOnlyProperty(Option<PropertyInstanceType<'db>>),
+    FailToSet,
+    FailToSetAttr,
+    SetAttrReturnsNeverOrNoReturn,
+    Unresolved,
+}
+
+impl AttributeAssignmentError<'_> {
+    pub(crate) const fn is_possibly_unbound(&self) -> bool {
+        matches!(self, Self::PossiblyUnbound)
+    }
+}
+
 /// This enum is used to control the behavior of the descriptor protocol implementation.
 /// When invoked on a class object, the fallback type (a class attribute) can shadow a
 /// non-data descriptor of the meta-type (the class's metaclass). However, this is not
@@ -1633,9 +1701,9 @@ impl<'db> Type<'db> {
             }
             // A protocol instance can never be a subtype of a nominal type, with the *sole* exception of `object`.
             (Type::ProtocolInstance(_), _) => C::unsatisfiable(db),
-            (_, Type::ProtocolInstance(protocol)) => {
+            (_, Type::ProtocolInstance(protocol)) => visitor.visit((self, target), || {
                 self.satisfies_protocol(db, protocol, relation, visitor)
-            }
+            }),
 
             // All `StringLiteral` types are a subtype of `LiteralString`.
             (Type::StringLiteral(_), Type::LiteralString) => C::always_satisfiable(db),
@@ -1973,12 +2041,28 @@ impl<'db> Type<'db> {
             visitor: &IsDisjointVisitor<'db, C>,
         ) -> C {
             protocol.interface(db).members(db).when_any(db, |member| {
-                other
-                    .member(db, member.name())
-                    .place
-                    .ignore_possibly_unbound()
-                    .when_none_or(db, |attribute_type| {
-                        member.has_disjoint_type_from(db, attribute_type, visitor)
+                let attribute = member.name();
+
+                member
+                    .instance_get_type(db)
+                    .when_some_and(db, |get_type| {
+                        other
+                            .member(db, attribute)
+                            .place
+                            .ignore_possibly_unbound()
+                            .when_none_or(db, |attribute_type| {
+                                get_type.is_disjoint_from_impl(db, attribute_type, visitor)
+                            })
+                    })
+                    .or(db, || {
+                        C::from_bool(
+                            db,
+                            member.instance_set_type().is_ok_and(|set_type| {
+                                other
+                                    .validate_attribute_assignment(db, attribute, set_type)
+                                    .is_err()
+                            }),
+                        )
                     })
             })
         }
@@ -2248,15 +2332,17 @@ impl<'db> Type<'db> {
                 })
             }
 
-            (Type::ProtocolInstance(protocol), other)
-            | (other, Type::ProtocolInstance(protocol)) => visitor.visit((self, other), || {
+            (Type::ProtocolInstance(protocol), other_ty)
+            | (other_ty, Type::ProtocolInstance(protocol)) => visitor.visit((self, other), || {
                 protocol.interface(db).members(db).when_any(db, |member| {
-                    match other.member(db, member.name()).place {
-                        Place::Type(attribute_type, _) => {
-                            member.has_disjoint_type_from(db, attribute_type, visitor)
-                        }
-                        Place::Unbound => C::unsatisfiable(db),
-                    }
+                    member.instance_get_type(db).when_some_and(db, |get_type| {
+                        let Place::Type(attribute_type, _) =
+                            other_ty.member(db, member.name()).place
+                        else {
+                            return C::unsatisfiable(db);
+                        };
+                        get_type.is_disjoint_from_impl(db, attribute_type, visitor)
+                    })
                 })
             }),
 
@@ -2878,9 +2964,14 @@ impl<'db> Type<'db> {
             }),
             // TODO: Once `to_meta_type` for the synthesized protocol is fully implemented, this handling should be removed.
             Type::ProtocolInstance(ProtocolInstanceType {
-                inner: Protocol::Synthesized(_),
+                inner: Protocol::Synthesized(synthesized),
                 ..
-            }) => self.instance_member(db, &name),
+            }) => synthesized
+                .interface()
+                .member_by_name(db, &name)
+                .and_then(|member| member.meta_get_type())
+                .map(|ty| Place::bound(ty).into())
+                .unwrap_or_default(),
             _ => self
                 .to_meta_type(db)
                 .find_name_in_mro_with_policy(db, name.as_str(), policy)
@@ -4831,6 +4922,364 @@ impl<'db> Type<'db> {
         }
     }
 
+    /// Make sure that the attribute assignment `obj.attribute = value` is valid.
+    ///
+    /// `attribute` is the name of the attribute being assigned, and `value_ty` is the type of the right-hand side of
+    /// the assignment.
+    fn validate_attribute_assignment(
+        self,
+        db: &'db dyn Db,
+        attribute: &str,
+        value_ty: Type<'db>,
+    ) -> Result<(), AttributeAssignmentErrors<'db>> {
+        let ensure_assignable_to = |attr_ty| -> Result<(), AttributeAssignmentError> {
+            if value_ty.is_assignable_to(db, attr_ty) {
+                Ok(())
+            } else {
+                Err(AttributeAssignmentError::TypeMismatch(attr_ty))
+            }
+        };
+
+        // Return true if this is an invalid assignment to a `Final` attribute.
+        let invalid_assignment_to_final =
+            |qualifiers: TypeQualifiers| -> bool { qualifiers.contains(TypeQualifiers::FINAL) };
+
+        let mut results = AttributeAssignmentErrors::default();
+
+        match self {
+            Type::Union(union) => {
+                if union.elements(db).iter().all(|elem| {
+                    let res = elem.validate_attribute_assignment(db, attribute, value_ty);
+                    match res {
+                        Ok(()) => true,
+                        Err(errors) if errors.is_possibly_unbound() => {
+                            results.insert(AttributeAssignmentError::PossiblyUnbound);
+                            true
+                        }
+                        _ => false,
+                    }
+                }) {
+                    results.and(Ok(()))
+                } else {
+                    results.and(Err(AttributeAssignmentError::TypeMismatch(self)))
+                }
+            }
+
+            Type::Intersection(intersection) => {
+                // TODO: Handle negative intersection elements
+                if intersection.positive(db).iter().any(|elem| {
+                    let res = elem.validate_attribute_assignment(db, attribute, value_ty);
+                    match res {
+                        Ok(()) => true,
+                        Err(errors) if errors.is_possibly_unbound() => {
+                            results.insert(AttributeAssignmentError::PossiblyUnbound);
+                            true
+                        }
+                        _ => false,
+                    }
+                }) {
+                    results.and(Ok(()))
+                } else {
+                    results.and(Err(AttributeAssignmentError::TypeMismatch(self)))
+                }
+            }
+
+            Type::TypeAlias(alias) => {
+                self.validate_attribute_assignment(db, attribute, alias.value_type(db))
+            }
+
+            // Super instances do not allow attribute assignment
+            Type::NominalInstance(instance)
+                if instance.class(db).is_known(db, KnownClass::Super) =>
+            {
+                results.and(Err(AttributeAssignmentError::CannotAssign))
+            }
+            Type::BoundSuper(_) => results.and(Err(AttributeAssignmentError::CannotAssign)),
+
+            Type::Dynamic(..) | Type::Never => results.and(Ok(())),
+
+            Type::NominalInstance(..)
+            | Type::ProtocolInstance(_)
+            | Type::BooleanLiteral(..)
+            | Type::IntLiteral(..)
+            | Type::StringLiteral(..)
+            | Type::BytesLiteral(..)
+            | Type::EnumLiteral(_)
+            | Type::LiteralString
+            | Type::SpecialForm(..)
+            | Type::KnownInstance(..)
+            | Type::PropertyInstance(..)
+            | Type::FunctionLiteral(..)
+            | Type::Callable(..)
+            | Type::BoundMethod(_)
+            | Type::MethodWrapper(_)
+            | Type::WrapperDescriptor(_)
+            | Type::DataclassDecorator(_)
+            | Type::DataclassTransformer(_)
+            | Type::NonInferableTypeVar(_)
+            | Type::TypeVar(..)
+            | Type::AlwaysTruthy
+            | Type::AlwaysFalsy
+            | Type::TypeIs(_)
+            | Type::TypedDict(_) => {
+                if let Type::ProtocolInstance(protocol) = self {
+                    if let Some(member) = protocol.interface(db).member_by_name(db, attribute) {
+                        if let Err(err) = member.instance_set_type() {
+                            return results.and(Err(err));
+                        }
+                    }
+                }
+
+                // First, try to call the `__setattr__` dunder method. If this is present/defined, overrides
+                // assigning the attributed by the normal mechanism.
+                let setattr_dunder_call_result = self.try_call_dunder_with_policy(
+                    db,
+                    "__setattr__",
+                    &mut CallArguments::positional([
+                        Type::StringLiteral(StringLiteralType::new(db, Box::from(attribute))),
+                        value_ty,
+                    ]),
+                    MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK,
+                );
+
+                let check_setattr_return_type = |result: Bindings<'db>| match result.return_type(db)
+                {
+                    Type::Never => {
+                        let is_setattr_synthesized = match self.class_member_with_policy(
+                            db,
+                            "__setattr__".into(),
+                            MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK,
+                        ) {
+                            PlaceAndQualifiers {
+                                place: Place::Type(attr_ty, _),
+                                qualifiers: _,
+                            } => attr_ty.is_callable_type(),
+                            _ => false,
+                        };
+
+                        let member_exists = !self.member(db, attribute).place.is_unbound();
+
+                        Err(if !member_exists {
+                            AttributeAssignmentError::CannotAssignToUnresolved
+                        } else if is_setattr_synthesized {
+                            AttributeAssignmentError::ReadOnlyProperty(None)
+                        } else {
+                            AttributeAssignmentError::SetAttrReturnsNeverOrNoReturn
+                        })
+                    }
+                    _ => Ok(()),
+                };
+
+                match setattr_dunder_call_result {
+                    Ok(bindings) => results.and(check_setattr_return_type(bindings)),
+                    Err(CallDunderError::PossiblyUnbound(bindings)) => {
+                        results.and(check_setattr_return_type(*bindings))
+                    }
+                    Err(CallDunderError::CallError(..)) => {
+                        results.and(Err(AttributeAssignmentError::FailToSetAttr))
+                    }
+                    Err(CallDunderError::MethodNotAvailable) => {
+                        match self.class_member(db, attribute.into()) {
+                            meta_attr @ PlaceAndQualifiers { .. } if meta_attr.is_class_var() => {
+                                results.and(Err(AttributeAssignmentError::CannotAssignToClassVar))
+                            }
+                            PlaceAndQualifiers {
+                                place: Place::Type(meta_attr_ty, meta_attr_boundness),
+                                qualifiers,
+                            } => {
+                                if invalid_assignment_to_final(qualifiers) {
+                                    return results
+                                        .and(Err(AttributeAssignmentError::CannotAssignToFinal));
+                                }
+
+                                // Check if it is assignable to the meta attribute type.
+                                if let Place::Type(meta_dunder_set, _) =
+                                    meta_attr_ty.class_member(db, "__set__".into()).place
+                                {
+                                    let dunder_set_result = meta_dunder_set.try_call(
+                                        db,
+                                        &CallArguments::positional([meta_attr_ty, self, value_ty]),
+                                    );
+
+                                    if let Err(dunder_set_error) = dunder_set_result {
+                                        results.insert(
+                                            if let Some(property) = dunder_set_error
+                                                .as_attempt_to_set_property_with_no_setter()
+                                            {
+                                                AttributeAssignmentError::ReadOnlyProperty(Some(
+                                                    property,
+                                                ))
+                                            } else {
+                                                AttributeAssignmentError::FailToSet
+                                            },
+                                        );
+                                    }
+                                } else {
+                                    results.insert_if_error(ensure_assignable_to(meta_attr_ty));
+                                }
+
+                                // Check if it is assignable to the instance attribute type.
+                                if meta_attr_boundness == Boundness::PossiblyUnbound {
+                                    let (assignable, boundness) = if let Place::Type(
+                                        instance_attr_ty,
+                                        instance_attr_boundness,
+                                    ) =
+                                        self.instance_member(db, attribute).place
+                                    {
+                                        (
+                                            ensure_assignable_to(instance_attr_ty),
+                                            instance_attr_boundness,
+                                        )
+                                    } else {
+                                        (Ok(()), Boundness::PossiblyUnbound)
+                                    };
+
+                                    results.insert_if_error(assignable);
+
+                                    if boundness == Boundness::PossiblyUnbound {
+                                        results.insert(AttributeAssignmentError::PossiblyUnbound);
+                                    }
+                                }
+
+                                results.and(Ok(()))
+                            }
+
+                            PlaceAndQualifiers {
+                                place: Place::Unbound,
+                                ..
+                            } => {
+                                if let PlaceAndQualifiers {
+                                    place: Place::Type(instance_attr_ty, instance_attr_boundness),
+                                    qualifiers,
+                                } = self.instance_member(db, attribute)
+                                {
+                                    if invalid_assignment_to_final(qualifiers) {
+                                        return results.and(Err(
+                                            AttributeAssignmentError::CannotAssignToFinal,
+                                        ));
+                                    }
+
+                                    if instance_attr_boundness == Boundness::PossiblyUnbound {
+                                        results.insert(AttributeAssignmentError::PossiblyUnbound);
+                                    }
+                                    results.and(ensure_assignable_to(instance_attr_ty))
+                                } else {
+                                    results.and(Err(AttributeAssignmentError::Unresolved))
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+
+            Type::ClassLiteral(..) | Type::GenericAlias(..) | Type::SubclassOf(..) => {
+                match self.class_member(db, attribute.into()) {
+                    PlaceAndQualifiers {
+                        place: Place::Type(meta_attr_ty, meta_attr_boundness),
+                        qualifiers,
+                    } => {
+                        if invalid_assignment_to_final(qualifiers) {
+                            return results.and(Err(AttributeAssignmentError::CannotAssignToFinal));
+                        }
+
+                        // Check if it is assignable to the meta attribute type.
+                        if let Place::Type(meta_dunder_set, _) =
+                            meta_attr_ty.class_member(db, "__set__".into()).place
+                        {
+                            let dunder_set_result = meta_dunder_set.try_call(
+                                db,
+                                &CallArguments::positional([meta_attr_ty, self, value_ty]),
+                            );
+
+                            if let Err(dunder_set_error) = dunder_set_result {
+                                results.insert(
+                                    if let Some(property) =
+                                        dunder_set_error.as_attempt_to_set_property_with_no_setter()
+                                    {
+                                        AttributeAssignmentError::ReadOnlyProperty(Some(property))
+                                    } else {
+                                        AttributeAssignmentError::FailToSet
+                                    },
+                                );
+                            }
+                        } else {
+                            results.insert_if_error(ensure_assignable_to(meta_attr_ty));
+                        }
+
+                        // Check if it is assignable to the class attribute type.
+                        if meta_attr_boundness == Boundness::PossiblyUnbound {
+                            let (assignable, boundness) =
+                                if let Place::Type(class_attr_ty, class_attr_boundness) = self
+                                    .find_name_in_mro(db, attribute)
+                                    .expect("called on Type::ClassLiteral or Type::SubclassOf")
+                                    .place
+                                {
+                                    (ensure_assignable_to(class_attr_ty), class_attr_boundness)
+                                } else {
+                                    (Ok(()), Boundness::PossiblyUnbound)
+                                };
+
+                            if boundness == Boundness::PossiblyUnbound {
+                                results.insert(AttributeAssignmentError::PossiblyUnbound);
+                            }
+
+                            results.insert_if_error(assignable);
+                        }
+
+                        results.and(Ok(()))
+                    }
+                    PlaceAndQualifiers {
+                        place: Place::Unbound,
+                        ..
+                    } => {
+                        if let PlaceAndQualifiers {
+                            place: Place::Type(class_attr_ty, class_attr_boundness),
+                            qualifiers,
+                        } = self
+                            .find_name_in_mro(db, attribute)
+                            .expect("called on Type::ClassLiteral or Type::SubclassOf")
+                        {
+                            if invalid_assignment_to_final(qualifiers) {
+                                return results
+                                    .and(Err(AttributeAssignmentError::CannotAssignToFinal));
+                            }
+
+                            if class_attr_boundness == Boundness::PossiblyUnbound {
+                                results.insert(AttributeAssignmentError::PossiblyUnbound);
+                            }
+                            results.and(ensure_assignable_to(class_attr_ty))
+                        } else {
+                            let attribute_is_bound_on_instance =
+                                self.to_instance(db).is_some_and(|instance| {
+                                    !instance.instance_member(db, attribute).place.is_unbound()
+                                });
+
+                            // Attribute is declared or bound on instance. Forbid access from the class object
+                            if attribute_is_bound_on_instance {
+                                results
+                                    .and(Err(AttributeAssignmentError::CannotAssignToInstanceAttr))
+                            } else {
+                                results.and(Err(AttributeAssignmentError::Unresolved))
+                            }
+                        }
+                    }
+                }
+            }
+
+            Type::ModuleLiteral(module) => {
+                if let Place::Type(attr_ty, _) = module.static_member(db, attribute).place {
+                    if value_ty.is_assignable_to(db, attr_ty) {
+                        results.and(Ok(()))
+                    } else {
+                        results.and(Err(AttributeAssignmentError::TypeMismatch(attr_ty)))
+                    }
+                } else {
+                    results.and(Err(AttributeAssignmentError::Unresolved))
+                }
+            }
+        }
+    }
+
     /// Calls `self`. Returns a [`CallError`] if `self` is (always or possibly) not callable, or if
     /// the arguments are not compatible with the formal parameters.
     ///

crates/ty_python_semantic/src/types/constraints.rs

@@ -167,6 +167,21 @@ impl<T> OptionConstraintsExtension<T> for Option<T> {
     }
 }
 
+pub(crate) trait ResultConstraintsExtension<T> {
+    /// Returns [`always_satisfiable`][Constraints::always_satisfiable] if the result is `Err(_)`; otherwise
+    /// applies a function to determine under what constraints the value inside of it holds.
+    fn when_err_or<'db, C: Constraints<'db>>(self, db: &'db dyn Db, f: impl FnOnce(T) -> C) -> C;
+}
+
+impl<T, E> ResultConstraintsExtension<T> for Result<T, E> {
+    fn when_err_or<'db, C: Constraints<'db>>(self, db: &'db dyn Db, f: impl FnOnce(T) -> C) -> C {
+        match self {
+            Ok(value) => f(value),
+            Err(_) => C::always_satisfiable(db),
+        }
+    }
+}
+
 /// An extension trait for building constraint sets from an [`Iterator`].
 pub(crate) trait IteratorConstraintsExtension<T> {
     /// Returns the constraints under which any element of the iterator holds.

crates/ty_python_semantic/src/types/diagnostic.rs

@@ -10,7 +10,6 @@ use crate::semantic_index::SemanticIndex;
 use crate::semantic_index::definition::Definition;
 use crate::semantic_index::place::{PlaceTable, ScopedPlaceId};
 use crate::suppression::FileSuppressionId;
-use crate::types::call::CallError;
 use crate::types::class::{DisjointBase, DisjointBaseKind, Field};
 use crate::types::function::KnownFunction;
 use crate::types::string_annotation::{
@@ -19,7 +18,8 @@ use crate::types::string_annotation::{
     RAW_STRING_TYPE_ANNOTATION,
 };
 use crate::types::{
-    DynamicType, LintDiagnosticGuard, Protocol, ProtocolInstanceType, SubclassOfInner, binding_type,
+    DynamicType, LintDiagnosticGuard, PropertyInstanceType, Protocol, ProtocolInstanceType,
+    SubclassOfInner, binding_type,
 };
 use crate::types::{SpecialFormType, Type, protocol_class::ProtocolClass};
 use crate::util::diagnostics::format_enumeration;
@@ -1973,16 +1973,16 @@ pub(super) fn report_invalid_attribute_assignment(
         node,
         target_ty,
         format_args!(
-            "Object of type `{}` is not assignable to attribute `{attribute_name}` of type `{}`",
+            "Object of type `{}` is not assignable to attribute `{attribute_name}` on type `{}`",
             source_ty.display(context.db()),
             target_ty.display(context.db()),
         ),
     );
 }
 
-pub(super) fn report_bad_dunder_set_call<'db>(
+pub(super) fn report_attempted_write_to_read_only_property<'db>(
     context: &InferContext<'db, '_>,
-    dunder_set_failure: &CallError<'db>,
+    property: Option<PropertyInstanceType<'db>>,
     attribute: &str,
     object_type: Type<'db>,
     target: &ast::ExprAttribute,
@@ -1991,30 +1991,27 @@ pub(super) fn report_bad_dunder_set_call<'db>(
         return;
     };
     let db = context.db();
-    if let Some(property) = dunder_set_failure.as_attempt_to_set_property_with_no_setter() {
-        let object_type = object_type.display(db);
+    let object_type = object_type.display(db);
+
+    if let Some(file_range) = property
+        .and_then(|property| property.getter(db))
+        .and_then(|getter| getter.definition(db))
+        .and_then(|definition| definition.focus_range(db))
+    {
         let mut diagnostic = builder.into_diagnostic(format_args!(
             "Cannot assign to read-only property `{attribute}` on object of type `{object_type}`",
         ));
-        if let Some(file_range) = property
-            .getter(db)
-            .and_then(|getter| getter.definition(db))
-            .and_then(|definition| definition.focus_range(db))
-        {
-            diagnostic.annotate(Annotation::secondary(Span::from(file_range)).message(
-                format_args!("Property `{object_type}.{attribute}` defined here with no setter"),
-            ));
-            diagnostic.set_primary_message(format_args!(
-                "Attempted assignment to `{object_type}.{attribute}` here"
-            ));
-        }
+        diagnostic.annotate(
+            Annotation::secondary(Span::from(file_range)).message(format_args!(
+                "Property `{object_type}.{attribute}` defined here with no setter"
+            )),
+        );
+        diagnostic.set_primary_message(format_args!(
+            "Attempted assignment to `{object_type}.{attribute}` here"
+        ));
     } else {
-        // TODO: Here, it would be nice to emit an additional diagnostic
-        // that explains why the call failed
         builder.into_diagnostic(format_args!(
-            "Invalid assignment to data descriptor attribute \
-            `{attribute}` on type `{}` with custom `__set__` method",
-            object_type.display(db)
+            "Attribute `{attribute}` on object of type `{object_type}` is read-only",
         ));
     }
 }

crates/ty_python_semantic/src/types/infer.rs

@@ -102,12 +102,12 @@ use crate::types::diagnostic::{
     INVALID_TYPE_VARIABLE_CONSTRAINTS, IncompatibleBases, POSSIBLY_UNBOUND_IMPLICIT_CALL,
     POSSIBLY_UNBOUND_IMPORT, TypeCheckDiagnostics, UNDEFINED_REVEAL, UNRESOLVED_ATTRIBUTE,
     UNRESOLVED_GLOBAL, UNRESOLVED_IMPORT, UNRESOLVED_REFERENCE, UNSUPPORTED_OPERATOR,
-    report_bad_dunder_set_call, report_implicit_return_type, report_instance_layout_conflict,
-    report_invalid_argument_number_to_special_form, report_invalid_arguments_to_annotated,
-    report_invalid_arguments_to_callable, report_invalid_assignment,
-    report_invalid_attribute_assignment, report_invalid_generator_function_return_type,
-    report_invalid_key_on_typed_dict, report_invalid_return_type,
-    report_namedtuple_field_without_default_after_field_with_default,
+    report_attempted_write_to_read_only_property, report_implicit_return_type,
+    report_instance_layout_conflict, report_invalid_argument_number_to_special_form,
+    report_invalid_arguments_to_annotated, report_invalid_arguments_to_callable,
+    report_invalid_assignment, report_invalid_attribute_assignment,
+    report_invalid_generator_function_return_type, report_invalid_key_on_typed_dict,
+    report_invalid_return_type, report_namedtuple_field_without_default_after_field_with_default,
     report_possibly_unbound_attribute,
 };
 use crate::types::enums::is_enum_class;
@@ -125,13 +125,13 @@ use crate::types::typed_dict::{
 };
 use crate::types::unpacker::{UnpackResult, Unpacker};
 use crate::types::{
-    CallDunderError, CallableType, ClassLiteral, ClassType, DataclassParams, DynamicType,
-    IntersectionBuilder, IntersectionType, KnownClass, KnownInstanceType, LintDiagnosticGuard,
-    MemberLookupPolicy, MetaclassCandidate, PEP695TypeAliasType, Parameter, ParameterForm,
-    Parameters, SpecialFormType, SubclassOfType, Truthiness, Type, TypeAliasType,
-    TypeAndQualifiers, TypeIsType, TypeQualifiers, TypeVarBoundOrConstraintsEvaluation,
-    TypeVarDefaultEvaluation, TypeVarInstance, TypeVarKind, UnionBuilder, UnionType, binding_type,
-    todo_type,
+    AttributeAssignmentError, CallDunderError, CallableType, ClassLiteral, ClassType,
+    DataclassParams, DynamicType, IntersectionBuilder, IntersectionType, KnownClass,
+    KnownInstanceType, LintDiagnosticGuard, MemberLookupPolicy, MetaclassCandidate,
+    PEP695TypeAliasType, Parameter, ParameterForm, Parameters, SpecialFormType, SubclassOfType,
+    Truthiness, Type, TypeAliasType, TypeAndQualifiers, TypeIsType, TypeQualifiers,
+    TypeVarBoundOrConstraintsEvaluation, TypeVarDefaultEvaluation, TypeVarInstance, TypeVarKind,
+    UnionBuilder, UnionType, binding_type, todo_type,
 };
 use crate::unpack::{EvaluationMode, Unpack, UnpackPosition};
 use crate::util::diagnostics::format_enumeration;
@@ -3965,531 +3965,122 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         object_ty: Type<'db>,
         attribute: &str,
         value_ty: Type<'db>,
-        emit_diagnostics: bool,
-    ) -> bool {
-        let db = self.db();
-
-        let ensure_assignable_to = |attr_ty| -> bool {
-            let assignable = value_ty.is_assignable_to(db, attr_ty);
-            if !assignable && emit_diagnostics {
-                report_invalid_attribute_assignment(
-                    &self.context,
-                    target.into(),
-                    attr_ty,
-                    value_ty,
-                    attribute,
-                );
-            }
-            assignable
+    ) {
+        let Err(errors) = object_ty.validate_attribute_assignment(self.db(), attribute, value_ty)
+        else {
+            return;
         };
-
-        // Return true (and emit a diagnostic) if this is an invalid assignment to a `Final` attribute.
-        let invalid_assignment_to_final = |qualifiers: TypeQualifiers| -> bool {
-            if qualifiers.contains(TypeQualifiers::FINAL) {
-                if emit_diagnostics {
+        for result in errors {
+            match result {
+                AttributeAssignmentError::PossiblyUnbound => {
+                    report_possibly_unbound_attribute(&self.context, target, attribute, object_ty);
+                }
+                AttributeAssignmentError::TypeMismatch(target_ty) => {
+                    // TODO: This is not a very helpful error message for union/intersection, as it does not include the underlying reason
+                    // why the assignment is invalid. This would be a good use case for sub-diagnostics.
+                    report_invalid_attribute_assignment(
+                        &self.context,
+                        target.into(),
+                        target_ty,
+                        value_ty,
+                        attribute,
+                    );
+                }
+                AttributeAssignmentError::CannotAssign => {
+                    if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, target) {
+                        builder.into_diagnostic(format_args!(
+                            "Cannot assign to attribute `{attribute}` on type `{}`",
+                            object_ty.display(self.db()),
+                        ));
+                    }
+                }
+                AttributeAssignmentError::CannotAssignToClassVar => {
+                    if let Some(builder) =
+                        self.context.report_lint(&INVALID_ATTRIBUTE_ACCESS, target)
+                    {
+                        builder.into_diagnostic(format_args!(
+                            "Cannot assign to ClassVar `{attribute}` \
+                                 from an instance of type `{ty}`",
+                            ty = object_ty.display(self.db()),
+                        ));
+                    }
+                }
+                AttributeAssignmentError::CannotAssignToInstanceAttr => {
+                    if let Some(builder) =
+                        self.context.report_lint(&INVALID_ATTRIBUTE_ACCESS, target)
+                    {
+                        builder.into_diagnostic(format_args!(
+                            "Cannot assign to instance attribute \
+                                    `{attribute}` from the class object `{ty}`",
+                            ty = object_ty.display(self.db()),
+                        ));
+                    }
+                }
+                AttributeAssignmentError::CannotAssignToFinal => {
                     if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, target) {
                         builder.into_diagnostic(format_args!(
                             "Cannot assign to final attribute `{attribute}` \
-                                         on type `{}`",
-                            object_ty.display(db)
+                                         on type `{ty}`",
+                            ty = object_ty.display(self.db()),
                         ));
                     }
                 }
-                true
-            } else {
-                false
-            }
-        };
-
-        match object_ty {
-            Type::Union(union) => {
-                if union.elements(self.db()).iter().all(|elem| {
-                    self.validate_attribute_assignment(target, *elem, attribute, value_ty, false)
-                }) {
-                    true
-                } else {
-                    // TODO: This is not a very helpful error message, as it does not include the underlying reason
-                    // why the assignment is invalid. This would be a good use case for sub-diagnostics.
-                    if emit_diagnostics {
-                        if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, target)
-                        {
-                            builder.into_diagnostic(format_args!(
-                                "Object of type `{}` is not assignable \
-                                 to attribute `{attribute}` on type `{}`",
-                                value_ty.display(self.db()),
-                                object_ty.display(self.db()),
-                            ));
-                        }
+                AttributeAssignmentError::CannotAssignToUnresolved => {
+                    if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, target) {
+                        builder.into_diagnostic(format!(
+                            "Can not assign to unresolved attribute `{attribute}` on type `{ty}`",
+                            ty = object_ty.display(self.db()),
+                        ));
                     }
-
-                    false
                 }
-            }
-
-            Type::Intersection(intersection) => {
-                // TODO: Handle negative intersection elements
-                if intersection.positive(db).iter().any(|elem| {
-                    self.validate_attribute_assignment(target, *elem, attribute, value_ty, false)
-                }) {
-                    true
-                } else {
-                    if emit_diagnostics {
-                        if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, target)
-                        {
-                            // TODO: same here, see above
-                            builder.into_diagnostic(format_args!(
-                                "Object of type `{}` is not assignable \
-                                 to attribute `{attribute}` on type `{}`",
-                                value_ty.display(self.db()),
-                                object_ty.display(self.db()),
-                            ));
-                        }
+                AttributeAssignmentError::ReadOnlyProperty(property) => {
+                    report_attempted_write_to_read_only_property(
+                        &self.context,
+                        property,
+                        attribute,
+                        object_ty,
+                        target,
+                    );
+                }
+                AttributeAssignmentError::FailToSet => {
+                    if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, target) {
+                        // TODO: Here, it would be nice to emit an additional diagnostic that explains why the call failed
+                        builder.into_diagnostic(format_args!(
+                            "Invalid assignment to data descriptor attribute \
+                            `{attribute}` on type `{}` with custom `__set__` method",
+                            object_ty.display(self.db())
+                        ));
                     }
-                    false
                 }
-            }
-
-            Type::TypeAlias(alias) => self.validate_attribute_assignment(
-                target,
-                alias.value_type(self.db()),
-                attribute,
-                value_ty,
-                emit_diagnostics,
-            ),
-
-            // Super instances do not allow attribute assignment
-            Type::NominalInstance(instance)
-                if instance.class(db).is_known(db, KnownClass::Super) =>
-            {
-                if emit_diagnostics {
+                AttributeAssignmentError::FailToSetAttr => {
+                    if let Some(builder) = self.context.report_lint(&UNRESOLVED_ATTRIBUTE, target) {
+                        builder.into_diagnostic(format_args!(
+                            "Can not assign object of type `{}` to attribute \
+                                    `{attribute}` on type `{}` with \
+                                    custom `__setattr__` method.",
+                            value_ty.display(self.db()),
+                            object_ty.display(self.db())
+                        ));
+                    }
+                }
+                AttributeAssignmentError::SetAttrReturnsNeverOrNoReturn => {
                     if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, target) {
                         builder.into_diagnostic(format_args!(
-                            "Cannot assign to attribute `{attribute}` on type `{}`",
-                            object_ty.display(self.db()),
+                            "Cannot assign to attribute `{attribute}` on type `{}` \
+                            whose `__setattr__` method returns `Never`/`NoReturn`",
+                            object_ty.display(self.db())
                         ));
                     }
                 }
-                false
-            }
-            Type::BoundSuper(_) => {
-                if emit_diagnostics {
-                    if let Some(builder) = self.context.report_lint(&INVALID_ASSIGNMENT, target) {
+                AttributeAssignmentError::Unresolved => {
+                    if let Some(builder) = self.context.report_lint(&UNRESOLVED_ATTRIBUTE, target) {
                         builder.into_diagnostic(format_args!(
-                            "Cannot assign to attribute `{attribute}` on type `{}`",
-                            object_ty.display(self.db()),
+                            "Unresolved attribute `{}` on type `{}`.",
+                            attribute,
+                            object_ty.display(self.db())
                         ));
                     }
                 }
-                false
-            }
-
-            Type::Dynamic(..) | Type::Never => true,
-
-            Type::NominalInstance(..)
-            | Type::ProtocolInstance(_)
-            | Type::BooleanLiteral(..)
-            | Type::IntLiteral(..)
-            | Type::StringLiteral(..)
-            | Type::BytesLiteral(..)
-            | Type::EnumLiteral(..)
-            | Type::LiteralString
-            | Type::SpecialForm(..)
-            | Type::KnownInstance(..)
-            | Type::PropertyInstance(..)
-            | Type::FunctionLiteral(..)
-            | Type::Callable(..)
-            | Type::BoundMethod(_)
-            | Type::MethodWrapper(_)
-            | Type::WrapperDescriptor(_)
-            | Type::DataclassDecorator(_)
-            | Type::DataclassTransformer(_)
-            | Type::NonInferableTypeVar(..)
-            | Type::TypeVar(..)
-            | Type::AlwaysTruthy
-            | Type::AlwaysFalsy
-            | Type::TypeIs(_)
-            | Type::TypedDict(_) => {
-                // First, try to call the `__setattr__` dunder method. If this is present/defined, overrides
-                // assigning the attributed by the normal mechanism.
-                let setattr_dunder_call_result = object_ty.try_call_dunder_with_policy(
-                    db,
-                    "__setattr__",
-                    &mut CallArguments::positional([Type::string_literal(db, attribute), value_ty]),
-                    MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK,
-                );
-
-                let check_setattr_return_type = |result: Bindings<'db>| -> bool {
-                    match result.return_type(db) {
-                        Type::Never => {
-                            if emit_diagnostics {
-                                if let Some(builder) =
-                                    self.context.report_lint(&INVALID_ASSIGNMENT, target)
-                                {
-                                    let is_setattr_synthesized = match object_ty
-                                        .class_member_with_policy(
-                                            db,
-                                            "__setattr__".into(),
-                                            MemberLookupPolicy::MRO_NO_OBJECT_FALLBACK,
-                                        ) {
-                                        PlaceAndQualifiers {
-                                            place: Place::Type(attr_ty, _),
-                                            qualifiers: _,
-                                        } => attr_ty.is_callable_type(),
-                                        _ => false,
-                                    };
-
-                                    let member_exists =
-                                        !object_ty.member(db, attribute).place.is_unbound();
-
-                                    let msg = if !member_exists {
-                                        format!(
-                                            "Can not assign to unresolved attribute `{attribute}` on type `{}`",
-                                            object_ty.display(db)
-                                        )
-                                    } else if is_setattr_synthesized {
-                                        format!(
-                                            "Property `{attribute}` defined in `{}` is read-only",
-                                            object_ty.display(db)
-                                        )
-                                    } else {
-                                        format!(
-                                            "Cannot assign to attribute `{attribute}` on type `{}` \
-                                                 whose `__setattr__` method returns `Never`/`NoReturn`",
-                                            object_ty.display(db)
-                                        )
-                                    };
-
-                                    builder.into_diagnostic(msg);
-                                }
-                            }
-                            false
-                        }
-                        _ => true,
-                    }
-                };
-
-                match setattr_dunder_call_result {
-                    Ok(result) => check_setattr_return_type(result),
-                    Err(CallDunderError::PossiblyUnbound(result)) => {
-                        check_setattr_return_type(*result)
-                    }
-                    Err(CallDunderError::CallError(..)) => {
-                        if emit_diagnostics {
-                            if let Some(builder) =
-                                self.context.report_lint(&UNRESOLVED_ATTRIBUTE, target)
-                            {
-                                builder.into_diagnostic(format_args!(
-                                    "Can not assign object of type `{}` to attribute \
-                                     `{attribute}` on type `{}` with \
-                                     custom `__setattr__` method.",
-                                    value_ty.display(db),
-                                    object_ty.display(db)
-                                ));
-                            }
-                        }
-                        false
-                    }
-                    Err(CallDunderError::MethodNotAvailable) => {
-                        match object_ty.class_member(db, attribute.into()) {
-                            meta_attr @ PlaceAndQualifiers { .. } if meta_attr.is_class_var() => {
-                                if emit_diagnostics {
-                                    if let Some(builder) =
-                                        self.context.report_lint(&INVALID_ATTRIBUTE_ACCESS, target)
-                                    {
-                                        builder.into_diagnostic(format_args!(
-                                            "Cannot assign to ClassVar `{attribute}` \
-                                     from an instance of type `{ty}`",
-                                            ty = object_ty.display(self.db()),
-                                        ));
-                                    }
-                                }
-                                false
-                            }
-                            PlaceAndQualifiers {
-                                place: Place::Type(meta_attr_ty, meta_attr_boundness),
-                                qualifiers,
-                            } => {
-                                if invalid_assignment_to_final(qualifiers) {
-                                    return false;
-                                }
-
-                                let assignable_to_meta_attr =
-                                    if let Place::Type(meta_dunder_set, _) =
-                                        meta_attr_ty.class_member(db, "__set__".into()).place
-                                    {
-                                        let dunder_set_result = meta_dunder_set.try_call(
-                                            db,
-                                            &CallArguments::positional([
-                                                meta_attr_ty,
-                                                object_ty,
-                                                value_ty,
-                                            ]),
-                                        );
-
-                                        if emit_diagnostics {
-                                            if let Err(dunder_set_failure) =
-                                                dunder_set_result.as_ref()
-                                            {
-                                                report_bad_dunder_set_call(
-                                                    &self.context,
-                                                    dunder_set_failure,
-                                                    attribute,
-                                                    object_ty,
-                                                    target,
-                                                );
-                                            }
-                                        }
-
-                                        dunder_set_result.is_ok()
-                                    } else {
-                                        ensure_assignable_to(meta_attr_ty)
-                                    };
-
-                                let assignable_to_instance_attribute = if meta_attr_boundness
-                                    == Boundness::PossiblyUnbound
-                                {
-                                    let (assignable, boundness) = if let PlaceAndQualifiers {
-                                        place:
-                                            Place::Type(instance_attr_ty, instance_attr_boundness),
-                                        qualifiers,
-                                    } =
-                                        object_ty.instance_member(db, attribute)
-                                    {
-                                        if invalid_assignment_to_final(qualifiers) {
-                                            return false;
-                                        }
-
-                                        (
-                                            ensure_assignable_to(instance_attr_ty),
-                                            instance_attr_boundness,
-                                        )
-                                    } else {
-                                        (true, Boundness::PossiblyUnbound)
-                                    };
-
-                                    if boundness == Boundness::PossiblyUnbound {
-                                        report_possibly_unbound_attribute(
-                                            &self.context,
-                                            target,
-                                            attribute,
-                                            object_ty,
-                                        );
-                                    }
-
-                                    assignable
-                                } else {
-                                    true
-                                };
-
-                                assignable_to_meta_attr && assignable_to_instance_attribute
-                            }
-
-                            PlaceAndQualifiers {
-                                place: Place::Unbound,
-                                ..
-                            } => {
-                                if let PlaceAndQualifiers {
-                                    place: Place::Type(instance_attr_ty, instance_attr_boundness),
-                                    qualifiers,
-                                } = object_ty.instance_member(db, attribute)
-                                {
-                                    if invalid_assignment_to_final(qualifiers) {
-                                        return false;
-                                    }
-
-                                    if instance_attr_boundness == Boundness::PossiblyUnbound {
-                                        report_possibly_unbound_attribute(
-                                            &self.context,
-                                            target,
-                                            attribute,
-                                            object_ty,
-                                        );
-                                    }
-
-                                    ensure_assignable_to(instance_attr_ty)
-                                } else {
-                                    if emit_diagnostics {
-                                        if let Some(builder) =
-                                            self.context.report_lint(&UNRESOLVED_ATTRIBUTE, target)
-                                        {
-                                            builder.into_diagnostic(format_args!(
-                                                "Unresolved attribute `{}` on type `{}`.",
-                                                attribute,
-                                                object_ty.display(db)
-                                            ));
-                                        }
-                                    }
-
-                                    false
-                                }
-                            }
-                        }
-                    }
-                }
-            }
-
-            Type::ClassLiteral(..) | Type::GenericAlias(..) | Type::SubclassOf(..) => {
-                match object_ty.class_member(db, attribute.into()) {
-                    PlaceAndQualifiers {
-                        place: Place::Type(meta_attr_ty, meta_attr_boundness),
-                        qualifiers,
-                    } => {
-                        if invalid_assignment_to_final(qualifiers) {
-                            return false;
-                        }
-
-                        let assignable_to_meta_attr = if let Place::Type(meta_dunder_set, _) =
-                            meta_attr_ty.class_member(db, "__set__".into()).place
-                        {
-                            let dunder_set_result = meta_dunder_set.try_call(
-                                db,
-                                &CallArguments::positional([meta_attr_ty, object_ty, value_ty]),
-                            );
-
-                            if emit_diagnostics {
-                                if let Err(dunder_set_failure) = dunder_set_result.as_ref() {
-                                    report_bad_dunder_set_call(
-                                        &self.context,
-                                        dunder_set_failure,
-                                        attribute,
-                                        object_ty,
-                                        target,
-                                    );
-                                }
-                            }
-
-                            dunder_set_result.is_ok()
-                        } else {
-                            ensure_assignable_to(meta_attr_ty)
-                        };
-
-                        let assignable_to_class_attr = if meta_attr_boundness
-                            == Boundness::PossiblyUnbound
-                        {
-                            let (assignable, boundness) =
-                                if let Place::Type(class_attr_ty, class_attr_boundness) = object_ty
-                                    .find_name_in_mro(db, attribute)
-                                    .expect("called on Type::ClassLiteral or Type::SubclassOf")
-                                    .place
-                                {
-                                    (ensure_assignable_to(class_attr_ty), class_attr_boundness)
-                                } else {
-                                    (true, Boundness::PossiblyUnbound)
-                                };
-
-                            if boundness == Boundness::PossiblyUnbound {
-                                report_possibly_unbound_attribute(
-                                    &self.context,
-                                    target,
-                                    attribute,
-                                    object_ty,
-                                );
-                            }
-
-                            assignable
-                        } else {
-                            true
-                        };
-
-                        assignable_to_meta_attr && assignable_to_class_attr
-                    }
-                    PlaceAndQualifiers {
-                        place: Place::Unbound,
-                        ..
-                    } => {
-                        if let PlaceAndQualifiers {
-                            place: Place::Type(class_attr_ty, class_attr_boundness),
-                            qualifiers,
-                        } = object_ty
-                            .find_name_in_mro(db, attribute)
-                            .expect("called on Type::ClassLiteral or Type::SubclassOf")
-                        {
-                            if invalid_assignment_to_final(qualifiers) {
-                                return false;
-                            }
-
-                            if class_attr_boundness == Boundness::PossiblyUnbound {
-                                report_possibly_unbound_attribute(
-                                    &self.context,
-                                    target,
-                                    attribute,
-                                    object_ty,
-                                );
-                            }
-
-                            ensure_assignable_to(class_attr_ty)
-                        } else {
-                            let attribute_is_bound_on_instance =
-                                object_ty.to_instance(self.db()).is_some_and(|instance| {
-                                    !instance
-                                        .instance_member(self.db(), attribute)
-                                        .place
-                                        .is_unbound()
-                                });
-
-                            // Attribute is declared or bound on instance. Forbid access from the class object
-                            if emit_diagnostics {
-                                if attribute_is_bound_on_instance {
-                                    if let Some(builder) =
-                                        self.context.report_lint(&INVALID_ATTRIBUTE_ACCESS, target)
-                                    {
-                                        builder.into_diagnostic(format_args!(
-                                            "Cannot assign to instance attribute \
-                                             `{attribute}` from the class object `{ty}`",
-                                            ty = object_ty.display(self.db()),
-                                        ));
-                                    }
-                                } else {
-                                    if let Some(builder) =
-                                        self.context.report_lint(&UNRESOLVED_ATTRIBUTE, target)
-                                    {
-                                        builder.into_diagnostic(format_args!(
-                                            "Unresolved attribute `{}` on type `{}`.",
-                                            attribute,
-                                            object_ty.display(db)
-                                        ));
-                                    }
-                                }
-                            }
-
-                            false
-                        }
-                    }
-                }
-            }
-
-            Type::ModuleLiteral(module) => {
-                if let Place::Type(attr_ty, _) = module.static_member(db, attribute).place {
-                    let assignable = value_ty.is_assignable_to(db, attr_ty);
-                    if assignable {
-                        true
-                    } else {
-                        if emit_diagnostics {
-                            report_invalid_attribute_assignment(
-                                &self.context,
-                                target.into(),
-                                attr_ty,
-                                value_ty,
-                                attribute,
-                            );
-                        }
-                        false
-                    }
-                } else {
-                    if emit_diagnostics {
-                        if let Some(builder) =
-                            self.context.report_lint(&UNRESOLVED_ATTRIBUTE, target)
-                        {
-                            builder.into_diagnostic(format_args!(
-                                "Unresolved attribute `{}` on type `{}`.",
-                                attribute,
-                                object_ty.display(db)
-                            ));
-                        }
-                    }
-
-                    false
-                }
             }
         }
     }
@@ -4535,7 +4126,6 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                         object_ty,
                         attr.id(),
                         assigned_ty,
-                        true,
                     );
                 }
             }

crates/ty_python_semantic/src/types/protocol_class.rs

@@ -1,28 +1,21 @@
-use std::fmt::Write;
-use std::{collections::BTreeMap, ops::Deref};
+use std::{collections::BTreeMap, fmt::Write, ops::Deref};
 
 use itertools::Itertools;
-
 use ruff_python_ast::name::Name;
 use rustc_hash::FxHashMap;
 
-use super::TypeVarVariance;
-use crate::semantic_index::place::ScopedPlaceId;
-use crate::semantic_index::{SemanticIndex, place_table};
-use crate::types::ClassType;
-use crate::types::context::InferContext;
-use crate::types::diagnostic::report_undeclared_protocol_member;
 use crate::{
     Db, FxOrderSet,
     place::{Boundness, Place, PlaceAndQualifiers, place_from_bindings, place_from_declarations},
-    semantic_index::{definition::Definition, use_def_map},
+    semantic_index::{SemanticIndex, definition::Definition, place_table, use_def_map},
     types::{
-        BoundTypeVarInstance, CallableType, ClassBase, ClassLiteral, FindLegacyTypeVarsVisitor,
-        HasRelationToVisitor, IsDisjointVisitor, KnownFunction, MaterializationKind,
-        NormalizedVisitor, PropertyInstanceType, Signature, Type, TypeMapping, TypeQualifiers,
-        TypeRelation, VarianceInferable,
-        constraints::{Constraints, IteratorConstraintsExtension},
-        signatures::{Parameter, Parameters},
+        AttributeAssignmentError, BoundTypeVarInstance, CallArguments, CallableType, ClassBase,
+        ClassLiteral, ClassType, Constraints, FindLegacyTypeVarsVisitor, HasRelationToVisitor,
+        InferContext, KnownFunction, MaterializationKind, NormalizedVisitor,
+        OptionConstraintsExtension, PropertyInstanceType, ScopedPlaceId, Signature, Type,
+        TypeMapping, TypeQualifiers, TypeRelation, TypeVarVariance, UnionType, VarianceInferable,
+        constraints::{IteratorConstraintsExtension, ResultConstraintsExtension},
+        diagnostic::report_undeclared_protocol_member,
     },
 };
 
@@ -173,17 +166,14 @@ impl<'db> ProtocolInterface<'db> {
             .map(|(name, ty)| {
                 // Synthesize a read-only property (one that has a getter but no setter)
                 // which returns the specified type from its getter.
-                let property_getter_signature = Signature::new(
-                    Parameters::new([Parameter::positional_only(Some(Name::new_static("self")))]),
-                    Some(ty.normalized(db)),
-                );
-                let property_getter = CallableType::single(db, property_getter_signature);
-                let property = PropertyInstanceType::new(db, Some(property_getter), None);
                 (
                     Name::new(name),
                     ProtocolMemberData {
                         qualifiers: TypeQualifiers::default(),
-                        kind: ProtocolMemberKind::Property(property),
+                        kind: ProtocolMemberKind::Property {
+                            get_type: Some(ty),
+                            set_type: None,
+                        },
                     },
                 )
             })
@@ -204,15 +194,19 @@ impl<'db> ProtocolInterface<'db> {
     {
         self.inner(db).iter().map(|(name, data)| ProtocolMember {
             name,
-            kind: data.kind,
+            kind: &data.kind,
             qualifiers: data.qualifiers,
         })
     }
 
-    fn member_by_name<'a>(self, db: &'db dyn Db, name: &'a str) -> Option<ProtocolMember<'a, 'db>> {
+    pub(super) fn member_by_name<'a>(
+        self,
+        db: &'db dyn Db,
+        name: &'a str,
+    ) -> Option<ProtocolMember<'a, 'db>> {
         self.inner(db).get(name).map(|data| ProtocolMember {
             name,
-            kind: data.kind,
+            kind: &data.kind,
             qualifiers: data.qualifiers,
         })
     }
@@ -223,9 +217,14 @@ impl<'db> ProtocolInterface<'db> {
 
     pub(super) fn instance_member(self, db: &'db dyn Db, name: &str) -> PlaceAndQualifiers<'db> {
         self.member_by_name(db, name)
-            .map(|member| PlaceAndQualifiers {
-                place: Place::bound(member.ty()),
-                qualifiers: member.qualifiers(),
+            .map(|member| {
+                member
+                    .instance_get_type(db)
+                    .map(|get_type| PlaceAndQualifiers {
+                        place: Place::bound(get_type),
+                        qualifiers: member.qualifiers(),
+                    })
+                    .unwrap_or(Place::Unbound.into())
             })
             .unwrap_or_else(|| Type::object(db).instance_member(db, name))
     }
@@ -330,8 +329,20 @@ impl<'db> ProtocolInterface<'db> {
 impl<'db> VarianceInferable<'db> for ProtocolInterface<'db> {
     fn variance_of(self, db: &'db dyn Db, typevar: BoundTypeVarInstance<'db>) -> TypeVarVariance {
         self.members(db)
-            // TODO do we need to switch on member kind?
-            .map(|member| member.ty().variance_of(db, typevar))
+            .flat_map(|member| {
+                member
+                    .instance_get_type(db)
+                    .into_iter()
+                    .chain(member.meta_get_type())
+                    .map(|get_type| get_type.variance_of(db, typevar))
+                    .chain(
+                        member
+                            .instance_set_type()
+                            .into_iter()
+                            .chain(member.meta_set_type())
+                            .map(|set_type| set_type.variance_of(db, typevar).flip()),
+                    )
+            })
             .collect()
     }
 }
@@ -379,30 +390,30 @@ impl<'db> ProtocolMemberData<'db> {
         }
     }
 
-    fn display(&self, db: &'db dyn Db) -> impl std::fmt::Display {
-        struct ProtocolMemberDataDisplay<'db> {
+    fn display(&self, db: &'db dyn Db) -> impl std::fmt::Display + '_ {
+        struct ProtocolMemberDataDisplay<'a, 'db> {
             db: &'db dyn Db,
-            data: ProtocolMemberKind<'db>,
+            data: &'a ProtocolMemberKind<'db>,
             qualifiers: TypeQualifiers,
         }
 
-        impl std::fmt::Display for ProtocolMemberDataDisplay<'_> {
+        impl std::fmt::Display for ProtocolMemberDataDisplay<'_, '_> {
             fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-                match self.data {
+                match &self.data {
                     ProtocolMemberKind::Method(callable) => {
                         write!(f, "MethodMember(`{}`)", callable.display(self.db))
                     }
-                    ProtocolMemberKind::Property(property) => {
+                    ProtocolMemberKind::Property { get_type, set_type } => {
                         let mut d = f.debug_struct("PropertyMember");
-                        if let Some(getter) = property.getter(self.db) {
-                            d.field("getter", &format_args!("`{}`", &getter.display(self.db)));
+                        if let Some(getter) = get_type {
+                            d.field("get_type", &format_args!("`{}`", &getter.display(self.db)));
                         }
-                        if let Some(setter) = property.setter(self.db) {
-                            d.field("setter", &format_args!("`{}`", &setter.display(self.db)));
+                        if let Some(setter) = set_type {
+                            d.field("set_type", &format_args!("`{}`", &setter.display(self.db)));
                         }
                         d.finish()
                     }
-                    ProtocolMemberKind::Other(ty) => {
+                    ProtocolMemberKind::Attribute(ty) => {
                         f.write_str("AttributeMember(")?;
                         write!(f, "`{}`", ty.display(self.db))?;
                         if self.qualifiers.contains(TypeQualifiers::CLASS_VAR) {
@@ -416,30 +427,85 @@ impl<'db> ProtocolMemberData<'db> {
 
         ProtocolMemberDataDisplay {
             db,
-            data: self.kind,
+            data: &self.kind,
             qualifiers: self.qualifiers,
         }
     }
 }
 
-#[derive(Debug, Copy, Clone, PartialEq, Eq, salsa::Update, Hash, get_size2::GetSize)]
+#[derive(Debug, Clone, PartialEq, Eq, salsa::Update, Hash, get_size2::GetSize)]
 enum ProtocolMemberKind<'db> {
     Method(CallableType<'db>),
-    Property(PropertyInstanceType<'db>),
-    Other(Type<'db>),
+    Property {
+        get_type: Option<Type<'db>>,
+        set_type: Option<Type<'db>>,
+    },
+    Attribute(Type<'db>),
 }
 
 impl<'db> ProtocolMemberKind<'db> {
+    fn from_property_instance(property: PropertyInstanceType<'db>, db: &'db dyn Db) -> Self {
+        fn inner<'db>(
+            db: &'db dyn Db,
+            property: PropertyInstanceType<'db>,
+        ) -> Option<(Option<Type<'db>>, Option<Type<'db>>)> {
+            let get_type = match property.getter(db) {
+                None => None,
+                Some(getter) => Some(
+                    getter
+                        .try_call(db, &CallArguments::positional([Type::any()]))
+                        .ok()?
+                        .return_type(db),
+                ),
+            };
+
+            let setter_signature = match property.setter(db) {
+                None => None,
+                Some(Type::Callable(callable)) => Some(callable.signatures(db)),
+                Some(Type::FunctionLiteral(function)) => Some(function.signature(db)),
+                _ => return None,
+            };
+
+            let set_type_from_signature = |sig: &Signature<'db>| match sig.parameters().as_slice() {
+                [_, parameter] if parameter.is_positional() && parameter.form.is_value() => {
+                    Some(parameter.annotated_type().unwrap_or_else(Type::unknown))
+                }
+                _ => None,
+            };
+
+            let set_type = if let Some(signature) = setter_signature {
+                if let Some(ty) =
+                    UnionType::try_from_elements(db, signature.iter().map(set_type_from_signature))
+                {
+                    Some(ty)
+                } else {
+                    return None;
+                }
+            } else {
+                None
+            };
+
+            Some((get_type, set_type))
+        }
+
+        inner(db, property)
+            .map(|(get_type, set_type)| ProtocolMemberKind::Property { get_type, set_type })
+            .unwrap_or(ProtocolMemberKind::Attribute(Type::PropertyInstance(
+                property,
+            )))
+    }
+
     fn normalized_impl(&self, db: &'db dyn Db, visitor: &NormalizedVisitor<'db>) -> Self {
         match self {
             ProtocolMemberKind::Method(callable) => {
                 ProtocolMemberKind::Method(callable.normalized_impl(db, visitor))
             }
-            ProtocolMemberKind::Property(property) => {
-                ProtocolMemberKind::Property(property.normalized_impl(db, visitor))
-            }
-            ProtocolMemberKind::Other(ty) => {
-                ProtocolMemberKind::Other(ty.normalized_impl(db, visitor))
+            ProtocolMemberKind::Property { get_type, set_type } => ProtocolMemberKind::Property {
+                get_type: get_type.map(|ty| ty.normalized_impl(db, visitor)),
+                set_type: set_type.map(|ty| ty.normalized_impl(db, visitor)),
+            },
+            ProtocolMemberKind::Attribute(attribute) => {
+                ProtocolMemberKind::Attribute(attribute.normalized_impl(db, visitor))
             }
         }
     }
@@ -449,11 +515,12 @@ impl<'db> ProtocolMemberKind<'db> {
             ProtocolMemberKind::Method(callable) => {
                 ProtocolMemberKind::Method(callable.apply_type_mapping(db, type_mapping))
             }
-            ProtocolMemberKind::Property(property) => {
-                ProtocolMemberKind::Property(property.apply_type_mapping(db, type_mapping))
-            }
-            ProtocolMemberKind::Other(ty) => {
-                ProtocolMemberKind::Other(ty.apply_type_mapping(db, type_mapping))
+            ProtocolMemberKind::Property { get_type, set_type } => ProtocolMemberKind::Property {
+                get_type: get_type.map(|ty| ty.apply_type_mapping(db, type_mapping)),
+                set_type: set_type.map(|ty| ty.apply_type_mapping(db, type_mapping)),
+            },
+            ProtocolMemberKind::Attribute(attribute) => {
+                ProtocolMemberKind::Attribute(attribute.apply_type_mapping(db, type_mapping))
             }
         }
     }
@@ -469,25 +536,31 @@ impl<'db> ProtocolMemberKind<'db> {
             ProtocolMemberKind::Method(callable) => {
                 callable.find_legacy_typevars_impl(db, binding_context, typevars, visitor);
             }
-            ProtocolMemberKind::Property(property) => {
-                property.find_legacy_typevars_impl(db, binding_context, typevars, visitor);
+            ProtocolMemberKind::Property { get_type, set_type } => {
+                if let Some(getter) = get_type {
+                    getter.find_legacy_typevars_impl(db, binding_context, typevars, visitor);
+                }
+                if let Some(setter) = set_type {
+                    setter.find_legacy_typevars_impl(db, binding_context, typevars, visitor);
+                }
             }
-            ProtocolMemberKind::Other(ty) => {
-                ty.find_legacy_typevars(db, binding_context, typevars);
+            ProtocolMemberKind::Attribute(attribute) => {
+                attribute.find_legacy_typevars_impl(db, binding_context, typevars, visitor);
             }
         }
     }
 
-    fn materialize(self, db: &'db dyn Db, materialization_kind: MaterializationKind) -> Self {
+    fn materialize(&self, db: &'db dyn Db, materialization_kind: MaterializationKind) -> Self {
         match self {
             ProtocolMemberKind::Method(callable) => {
                 ProtocolMemberKind::Method(callable.materialize(db, materialization_kind))
             }
-            ProtocolMemberKind::Property(property) => {
-                ProtocolMemberKind::Property(property.materialize(db, materialization_kind))
-            }
-            ProtocolMemberKind::Other(ty) => {
-                ProtocolMemberKind::Other(ty.materialize(db, materialization_kind))
+            ProtocolMemberKind::Property { get_type, set_type } => ProtocolMemberKind::Property {
+                get_type: get_type.map(|ty| ty.materialize(db, materialization_kind)),
+                set_type: set_type.map(|ty| ty.materialize(db, materialization_kind)),
+            },
+            ProtocolMemberKind::Attribute(attribute) => {
+                ProtocolMemberKind::Attribute(attribute.materialize(db, materialization_kind))
             }
         }
     }
@@ -497,7 +570,7 @@ impl<'db> ProtocolMemberKind<'db> {
 #[derive(Debug, PartialEq, Eq)]
 pub(super) struct ProtocolMember<'a, 'db> {
     name: &'a str,
-    kind: ProtocolMemberKind<'db>,
+    kind: &'a ProtocolMemberKind<'db>,
     qualifiers: TypeQualifiers,
 }
 
@@ -507,11 +580,16 @@ fn walk_protocol_member<'db, V: super::visitor::TypeVisitor<'db> + ?Sized>(
     visitor: &V,
 ) {
     match member.kind {
-        ProtocolMemberKind::Method(method) => visitor.visit_callable_type(db, method),
-        ProtocolMemberKind::Property(property) => {
-            visitor.visit_property_instance_type(db, property);
+        ProtocolMemberKind::Method(method) => visitor.visit_callable_type(db, *method),
+        ProtocolMemberKind::Property { get_type, set_type } => {
+            if let Some(get_type) = get_type {
+                visitor.visit_type(db, *get_type);
+            }
+            if let Some(set_type) = set_type {
+                visitor.visit_type(db, *set_type);
+            }
         }
-        ProtocolMemberKind::Other(ty) => visitor.visit_type(db, ty),
+        ProtocolMemberKind::Attribute(ty) => visitor.visit_type(db, *ty),
     }
 }
 
@@ -524,24 +602,74 @@ impl<'a, 'db> ProtocolMember<'a, 'db> {
         self.qualifiers
     }
 
-    fn ty(&self) -> Type<'db> {
-        match &self.kind {
-            ProtocolMemberKind::Method(callable) => Type::Callable(*callable),
-            ProtocolMemberKind::Property(property) => Type::PropertyInstance(*property),
-            ProtocolMemberKind::Other(ty) => *ty,
+    /// Must this member be present on an instance of a class `X`
+    /// for `X` to be considered a subtype of the protocol?
+    /// If so, what type must that member have?
+    pub(super) fn instance_get_type(&self, db: &'db dyn Db) -> Option<Type<'db>> {
+        match self.kind {
+            ProtocolMemberKind::Method(callable) => Some(callable.bind_self(db)),
+            ProtocolMemberKind::Property { get_type, .. } => *get_type,
+            ProtocolMemberKind::Attribute(ty) => Some(*ty),
         }
     }
 
-    pub(super) fn has_disjoint_type_from<C: Constraints<'db>>(
-        &self,
-        db: &'db dyn Db,
-        other: Type<'db>,
-        visitor: &IsDisjointVisitor<'db, C>,
-    ) -> C {
-        match &self.kind {
-            // TODO: implement disjointness for property/method members as well as attribute members
-            ProtocolMemberKind::Property(_) | ProtocolMemberKind::Method(_) => C::unsatisfiable(db),
-            ProtocolMemberKind::Other(ty) => ty.is_disjoint_from_impl(db, other, visitor),
+    /// Must this member be present on the class object `X` itself
+    /// for `X` to be considered a subtype of the protocol?
+    /// If so, what type must that member have when read from the class object itself?
+    pub(super) fn meta_get_type(&self) -> Option<Type<'db>> {
+        match self.kind {
+            ProtocolMemberKind::Method(callable) => Some(Type::Callable(*callable)),
+            ProtocolMemberKind::Property { .. } => None,
+            ProtocolMemberKind::Attribute(ty) => {
+                if self.qualifiers.contains(TypeQualifiers::CLASS_VAR) {
+                    Some(*ty)
+                } else {
+                    None
+                }
+            }
+        }
+    }
+
+    /// Must this member be writable on an instance of a class `X`
+    /// for `X` to be considered a subtype of the protocol?
+    /// If so, what types must it be permissible to write to that member?
+    /// If not, what error should be returned when a user tries to write
+    /// to this member on an instance?
+    pub(super) fn instance_set_type(&self) -> Result<Type<'db>, AttributeAssignmentError<'db>> {
+        match self.kind {
+            ProtocolMemberKind::Property { set_type, .. } => {
+                set_type.ok_or(AttributeAssignmentError::ReadOnlyProperty(None))
+            }
+            ProtocolMemberKind::Method(_) => Err(AttributeAssignmentError::CannotAssign),
+            ProtocolMemberKind::Attribute(ty) => {
+                if self.qualifiers.contains(TypeQualifiers::CLASS_VAR) {
+                    Err(AttributeAssignmentError::CannotAssignToClassVar)
+                } else {
+                    Ok(*ty)
+                }
+            }
+        }
+    }
+
+    /// Must this member be writable on the class object `X` itself
+    /// for `X` to be considered a subtype of the protocol?
+    /// If so, what types must it be permissible to write to that
+    /// member on the class object `X`? If not, what error should be
+    /// returned when a user tries to write to this member on the
+    /// class object itself?
+    pub(super) fn meta_set_type(&self) -> Result<Type<'db>, AttributeAssignmentError<'db>> {
+        match self.kind {
+            ProtocolMemberKind::Property { .. } => {
+                Err(AttributeAssignmentError::CannotAssignToInstanceAttr)
+            }
+            ProtocolMemberKind::Method(_) => Err(AttributeAssignmentError::CannotAssign),
+            ProtocolMemberKind::Attribute(ty) => {
+                if self.qualifiers.contains(TypeQualifiers::CLASS_VAR) {
+                    Ok(*ty)
+                } else {
+                    Err(AttributeAssignmentError::CannotAssignToInstanceAttr)
+                }
+            }
         }
     }
 
@@ -554,37 +682,59 @@ impl<'a, 'db> ProtocolMember<'a, 'db> {
         relation: TypeRelation,
         visitor: &HasRelationToVisitor<'db, C>,
     ) -> C {
-        match &self.kind {
-            // TODO: consider the types of the attribute on `other` for method members
-            ProtocolMemberKind::Method(_) => C::from_bool(
+        if let ProtocolMemberKind::Method(_) = &self.kind {
+            // TODO: use the same generalised logic for method members
+            // that we do for attribute/protocol members below.
+            return C::from_bool(
                 db,
                 matches!(
                     other.to_meta_type(db).member(db, self.name).place,
                     Place::Type(ty, Boundness::Bound)
                     if ty.is_assignable_to(db, CallableType::single(db, Signature::dynamic(Type::any())))
                 ),
-            ),
-            // TODO: consider the types of the attribute on `other` for property members
-            ProtocolMemberKind::Property(_) => C::from_bool(
-                db,
-                matches!(
-                    other.member(db, self.name).place,
-                    Place::Type(_, Boundness::Bound)
-                ),
-            ),
-            ProtocolMemberKind::Other(member_type) => {
+            );
+        }
+
+        self.instance_get_type(db)
+            .when_none_or(db, |get_type| {
                 let Place::Type(attribute_type, Boundness::Bound) =
                     other.member(db, self.name).place
                 else {
                     return C::unsatisfiable(db);
                 };
-                member_type
-                    .has_relation_to_impl(db, attribute_type, relation, visitor)
-                    .and(db, || {
-                        attribute_type.has_relation_to_impl(db, *member_type, relation, visitor)
-                    })
-            }
-        }
+                attribute_type.has_relation_to_impl(db, get_type, relation, visitor)
+            })
+            .and(db, || {
+                self.instance_set_type().when_err_or(db, |set_type| {
+                    C::from_bool(
+                        db,
+                        other
+                            .validate_attribute_assignment(db, self.name, set_type)
+                            .is_ok(),
+                    )
+                })
+            })
+            .and(db, || {
+                self.meta_get_type().when_none_or(db, |get_type| {
+                    let Place::Type(attribute_type, Boundness::Bound) =
+                        other.class_member(db, Name::from(self.name)).place
+                    else {
+                        return C::unsatisfiable(db);
+                    };
+                    attribute_type.has_relation_to_impl(db, get_type, relation, visitor)
+                })
+            })
+            .and(db, || {
+                self.meta_set_type().when_err_or(db, |set_type| {
+                    C::from_bool(
+                        db,
+                        other
+                            .to_meta_type(db)
+                            .validate_attribute_assignment(db, self.name, set_type)
+                            .is_ok(),
+                    )
+                })
+            })
     }
 }
 
@@ -627,13 +777,21 @@ fn excluded_from_proto_members(member: &str) -> bool {
     ) || member.starts_with("_abc_")
 }
 
-#[derive(Debug, Copy, Clone, PartialEq, Eq)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, get_size2::GetSize, Hash)]
 enum BoundOnClass {
     Yes,
     No,
 }
 
 impl BoundOnClass {
+    const fn from_qualifiers(qualifiers: TypeQualifiers) -> Self {
+        if qualifiers.contains(TypeQualifiers::CLASS_VAR) {
+            BoundOnClass::Yes
+        } else {
+            BoundOnClass::No
+        }
+    }
+
     const fn is_yes(self) -> bool {
         matches!(self, BoundOnClass::Yes)
     }
@@ -687,7 +845,13 @@ fn cached_protocol_interface<'db>(
                         *ty = new_type;
                         *quals = place.qualifiers;
                     })
-                    .or_insert((new_type, place.qualifiers, BoundOnClass::No));
+                    .or_insert_with(|| {
+                        (
+                            new_type,
+                            place.qualifiers,
+                            BoundOnClass::from_qualifiers(place.qualifiers),
+                        )
+                    });
             }
         }
 
@@ -703,7 +867,9 @@ fn cached_protocol_interface<'db>(
             let ty = ty.apply_optional_specialization(db, specialization);
 
             let member = match ty {
-                Type::PropertyInstance(property) => ProtocolMemberKind::Property(property),
+                Type::PropertyInstance(property) => {
+                    ProtocolMemberKind::from_property_instance(property, db)
+                }
                 Type::Callable(callable)
                     if bound_on_class.is_yes() && callable.is_function_like(db) =>
                 {
@@ -712,7 +878,7 @@ fn cached_protocol_interface<'db>(
                 Type::FunctionLiteral(function) if bound_on_class.is_yes() => {
                     ProtocolMemberKind::Method(function.into_callable_type(db))
                 }
-                _ => ProtocolMemberKind::Other(ty),
+                _ => ProtocolMemberKind::Attribute(ty),
             };
 
             members.insert(

crates/ty_python_semantic/src/types/signatures.rs

@@ -1657,6 +1657,13 @@ pub(crate) enum ParameterForm {
     Type,
 }
 
+impl ParameterForm {
+    /// Returns `true` if this is a value form.
+    pub(crate) const fn is_value(self) -> bool {
+        matches!(self, Self::Value)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;