[ty] Preserve descriptor validation after frozen setter delegation

Author: charliermarsh

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

@@ -833,14 +833,12 @@ class ChildWithRejectingAssignmentBase(Frozen, RejectsAssignment): ...
 ChildWithRejectingAssignmentBase().y = 2
 ```
 
-A later base class can instead allow assignment to an otherwise read-only property. Its setter still
-determines which values are accepted:
+A later base class can customize assignment to an ordinary attribute. The value must satisfy both
+the later `__setattr__` and the attribute declaration:
 
 ```py
 class AllowsAssignment:
-    @property
-    def y(self) -> int:
-        return 1
+    y: object = 1
 
     def __setattr__(self, name: str, value: int) -> None: ...
 
@@ -853,6 +851,82 @@ allowed.y = 2
 allowed.y = "invalid"
 ```
 
+A later `__setattr__` can forward to `object.__setattr__`, which still invokes data descriptors:
+
+```py
+class ForwardsAssignment:
+    def __setattr__(self, name: str, value: object) -> None:
+        super().__setattr__(name, value)
+```
+
+A read-only property therefore remains read-only:
+
+```py
+class ReadOnlyPropertyBase(ForwardsAssignment):
+    @property
+    def y(self) -> int:
+        return 1
+
+class ChildWithReadOnlyProperty(Frozen, ReadOnlyPropertyBase): ...
+
+# error: [invalid-assignment] "Cannot assign to read-only property `y` on object of type `ChildWithReadOnlyProperty`"
+ChildWithReadOnlyProperty().y = 2
+```
+
+The property's setter still determines which values it accepts:
+
+```py
+class TypedPropertyBase(ForwardsAssignment):
+    @property
+    def y(self) -> int:
+        return 1
+
+    @y.setter
+    def y(self, value: int) -> None: ...
+
+class ChildWithTypedProperty(Frozen, TypedPropertyBase): ...
+
+# error: [invalid-assignment] "Expected `int`, found `Literal["invalid"]`"
+ChildWithTypedProperty().y = "invalid"
+```
+
+A property setter that never returns prevents assignment:
+
+```py
+class TerminalPropertyBase(ForwardsAssignment):
+    @property
+    def y(self) -> int:
+        return 1
+
+    @y.setter
+    def y(self, value: int) -> NoReturn:
+        raise AttributeError
+
+class ChildWithTerminalProperty(Frozen, TerminalPropertyBase): ...
+
+# error: [invalid-assignment] "Cannot assign to attribute `y` on type `ChildWithTerminalProperty` whose `__set__` method returns `Never`/`NoReturn`"
+ChildWithTerminalProperty().y = 2
+```
+
+The same rule applies to a custom descriptor whose setter never returns:
+
+```py
+class TerminalDescriptor:
+    def __get__(self, instance: object, owner: type | None = None) -> int:
+        return 1
+
+    def __set__(self, instance: object, value: int) -> NoReturn:
+        raise AttributeError
+
+class TerminalDescriptorBase(ForwardsAssignment):
+    y: TerminalDescriptor = TerminalDescriptor()
+
+class ChildWithTerminalDescriptor(Frozen, TerminalDescriptorBase): ...
+
+# error: [invalid-assignment] "Cannot assign to attribute `y` on type `ChildWithTerminalDescriptor` whose `__set__` method returns `Never`/`NoReturn`"
+ChildWithTerminalDescriptor().y = 2
+```
+
 A later `__setattr__` does not make the declared type of an ordinary attribute disappear:
 
 ```py

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

@@ -120,17 +120,26 @@ pub struct StaticClassLiteral<'db> {
 // The Salsa heap is tracked separately.
 impl get_size2::GetSize for StaticClassLiteral<'_> {}
 
-/// The outcome of dispatching a frozen-dataclass method on a subclass instance.
+/// The result of [`StaticClassLiteral::inherited_frozen_dataclass_dispatch`].
+///
+/// See that method for details on how generated frozen-dataclass methods handle fields and
+/// non-fields on subclass instances.
 #[derive(Clone, Copy)]
 pub(crate) enum FrozenDataclassDispatch<'db> {
     /// A reachable frozen dataclass rejects modification of one of its fields.
     FrozenField,
-    /// Every reachable frozen method delegates past this base.
+    /// Every reachable frozen method delegates, with lookup resuming after this base.
     Delegate(StaticClassLiteral<'db>),
 }
 
 impl<'db> FrozenDataclassDispatch<'db> {
-    /// Returns `object` for a frozen field or `super(frozen_base, object)` for a non-field.
+    /// Returns the receiver for the next step of assignment or deletion validation.
+    ///
+    /// Validation stays on `object_ty` for a frozen field because the generated method rejects the
+    /// mutation. For a non-field, the generated method calls `super(frozen_base, object_ty)`, so
+    /// lookup must resume after the last frozen base. For example, assigning `Child().y` for
+    /// `class Child(Frozen, Later)` uses `super(Frozen, child)` when `y` is not a field of `Frozen`;
+    /// this preserves a later `__setattr__` or a descriptor for `y`.
     pub(crate) fn receiver(self, db: &'db dyn Db, object_ty: Type<'db>) -> Type<'db> {
         match self {
             Self::FrozenField => object_ty,
@@ -147,6 +156,9 @@ impl<'db> FrozenDataclassDispatch<'db> {
 /// Fields protected by reachable frozen-dataclass methods.
 struct InheritedFrozenDataclassFields<'db> {
     names: Box<[Name]>,
+    /// The final frozen dataclass whose generated method participates in dispatch.
+    ///
+    /// For a non-field, mutation validation resumes after this class in the MRO.
     last_frozen_base: StaticClassLiteral<'db>,
 }
 
@@ -1915,11 +1927,33 @@ impl<'db> StaticClassLiteral<'db> {
         )))
     }
 
-    /// Returns the outcome of an inherited frozen-dataclass method for `name`.
+    /// Determines how an inherited generated frozen-dataclass `method` handles `name`.
+    ///
+    /// CPython's generated `__setattr__` and `__delattr__` reject every mutation when called on an
+    /// instance of the exact frozen class. On an ordinary subclass instance, they reject only
+    /// dataclass fields and delegate other names with `super(frozen_class, instance)`.
+    ///
+    /// For example:
+    ///
+    /// ```python
+    /// @dataclass(frozen=True)
+    /// class Frozen:
+    ///     x: int
+    ///
+    /// class Later:
+    ///     y: int
+    ///
+    /// class Child(Frozen, Later): ...
+    /// ```
+    ///
+    /// Assigning to `Child().x` is rejected because `x` is a field of `Frozen`. Assigning to
+    /// `Child().y` instead delegates to `super(Frozen, child).__setattr__`, where a later
+    /// `__setattr__` or the descriptor for `y` can still reject the assignment.
     ///
-    /// For a non-field, CPython delegates past each generated frozen method. Preserving the final
-    /// frozen base lets assignment validation perform the same `super()` lookup without hiding a
-    /// later method or an attribute's descriptor.
+    /// If multiple frozen dataclasses are reachable before an explicit implementation of
+    /// `method`, a non-field delegates past each generated method. [`FrozenDataclassDispatch::Delegate`]
+    /// stores the last frozen base so the caller can perform the equivalent lookup once, after all
+    /// of them.
     pub(crate) fn inherited_frozen_dataclass_dispatch(
         self,
         db: &'db dyn Db,

crates/ty_python_semantic/src/types/infer/builder/attribute_assignment.rs

@@ -433,27 +433,15 @@ impl<'db> AssignmentAttributeWriteEvaluator<'_, 'db, '_, '_> {
                 if matches!(
                     frozen_dataclass_dispatch,
                     Some(FrozenDataclassDispatch::Delegate(_))
-                ) {
-                    match setattr_result {
-                        Ok(_) | Err(CallDunderError::PossiblyUnbound { .. })
-                            if matches!(
-                                member,
-                                ExplicitAttributeWriteRequirement::Descriptor { .. }
-                            ) =>
-                        {
-                            return true;
-                        }
-                        Err(CallDunderError::CallError(kind, bindings, _)) => {
-                            if emit_diagnostics {
-                                self.report(AssignmentAttributeWriteDiagnostic::BadSetAttr {
-                                    value_ty,
-                                    failure: CallError(kind, bindings),
-                                });
-                            }
-                            return false;
-                        }
-                        _ => {}
+                ) && let Err(CallDunderError::CallError(kind, bindings, _)) = setattr_result
+                {
+                    if emit_diagnostics {
+                        self.report(AssignmentAttributeWriteDiagnostic::BadSetAttr {
+                            value_ty,
+                            failure: CallError(kind, bindings),
+                        });
                     }
+                    return false;
                 }
                 let member_valid =
                     self.evaluate_explicit_member(object_ty, member, value_ty, emit_diagnostics);
@@ -679,17 +667,27 @@ impl<'db> AssignmentAttributeWriteEvaluator<'_, 'db, '_, '_> {
         emit_diagnostics: bool,
     ) -> bool {
         let db = self.builder.db();
-        if property_setter_returns_never(db, descriptor_ty, object_ty, value_ty) {
+        let setter_result = setter_ty.try_call(
+            db,
+            &CallArguments::positional([descriptor_ty, object_ty, value_ty]),
+        );
+        // `Never` supports arbitrary operations only because there can be no runtime value to
+        // mutate; it is not a concrete descriptor with a terminal setter.
+        let setter_returns_never = !descriptor_ty.is_never()
+            && match &setter_result {
+                Ok(bindings) => bindings.return_type(db).is_never(),
+                Err(error) => error.return_type(db).is_never(),
+            };
+        if setter_returns_never
+            || property_setter_returns_never(db, descriptor_ty, object_ty, value_ty)
+        {
             if emit_diagnostics {
                 self.report(AssignmentAttributeWriteDiagnostic::TerminalDescriptor);
             }
             return false;
         }
 
-        match setter_ty.try_call(
-            db,
-            &CallArguments::positional([descriptor_ty, object_ty, value_ty]),
-        ) {
+        match setter_result {
             Ok(_) => true,
             Err(error) => {
                 if emit_diagnostics {