[ty] Preserve frozen-dataclass setter delegation - #27217
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 96.84%. The percentage of expected errors that received a diagnostic held steady at 92.11%. The number of fully passing files held steady at 99/133. |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
sphinx
trio
flake8
|
|
| ExplicitAttributeWriteRequirement::Descriptor { .. } | ||
| ) => | ||
| { | ||
| return true; |
There was a problem hiding this comment.
A successful delegated __setattr__ does not establish that a descriptor was bypassed: the later setter can call super().__setattr__, which still invokes property.__set__ or the target data descriptor.
from dataclasses import dataclass
@dataclass(frozen=True)
class Frozen:
x: int = 1
class ReadOnlyBase:
@property
def y(self) -> int:
return 1
def __setattr__(self, name: str, value: object) -> None:
super().__setattr__(name, value)
class Child(Frozen, ReadOnlyBase):
pass
Child().y = 1 # raises AttributeError; main and all other type checkers reject, this PR accepts itI also reproduced the same false negative with a mistyped property setter, a property setter returning Never, and a custom descriptor whose __set__ returns Never. I think we need to retain descriptor validation whenever the delegated setter can forward through super(), and add mdtests for read-only, typed, and terminal property/descriptor setters.
| } | ||
|
|
||
| impl<'db> FrozenDataclassDispatch<'db> { | ||
| /// Returns `object` for a frozen field or `super(frozen_base, object)` for a non-field. |
There was a problem hiding this comment.
Could we have a little more clarification here on the purpose of this method -- why it behaves the way it does? Maybe with an example?
| /// Fields protected by reachable frozen-dataclass methods. | ||
| struct InheritedFrozenDataclassFields<'db> { | ||
| names: Box<[Name]>, | ||
| last_frozen_base: StaticClassLiteral<'db>, |
There was a problem hiding this comment.
I think we need some documentation on what this field means
| // The Salsa heap is tracked separately. | ||
| impl get_size2::GetSize for StaticClassLiteral<'_> {} | ||
|
|
||
| /// The outcome of dispatching a frozen-dataclass method on a subclass instance. |
There was a problem hiding this comment.
maybe we could refer to the doc comment on inherited_frozen_dataclass_dispatch for more clarity on what this is all about
| /// 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. |
There was a problem hiding this comment.
I think we could expand this a bit, with examples -- what does "for a non-field" mean, what does "delegates past each generated frozen method" mean.
0b2c74a to
1ca0a23
Compare
## Summary Stacked on #27217. Prior to this change, we rejected assignments to fields inherited from a frozen dataclass but allowed the equivalent deletion through an ordinary subclass, even though it raises `FrozenInstanceError` at runtime: ```python from dataclasses import dataclass @DataClass(frozen=True) class Frozen: x: int = 1 class Child(Frozen): ... del Child().x ``` We now synthesize frozen `__delattr__` for both frozen dataclasses and their ordinary subclasses, reuse the frozen-method dispatch from #27217, and continue through `super()` for non-field deletions. This preserves descriptor and later-MRO checks, supports multiple frozen bases and generic specializations, excludes `InitVar`, and retains the existing Python 3.12 behavior for slotted frozen dataclasses. The generated signatures also make direct and intermediate method overrides consistent with frozen `__setattr__`.
Summary
Prior to this change, we modeled a frozen dataclass's inherited
__setattr__as an overload that rejects known frozen fields and returnsNonefor every other attribute. That catch-all loses the actual setter that Python calls after the frozen base.For example, this assignment must be rejected because the next base's setter never returns:
Previously, we accepted it. We also rejected valid assignments when a later setter explicitly accepts a value for an otherwise read-only property.
We now collect fields from every reachable frozen base, distinguish frozen fields from attributes that are delegated through
super(), and validate delegated assignments against the actual next setter. This preserves setter argument checking and declared attribute types, handles both generic syntaxes, excludesInitVararguments, and keeps the existing behavior for slotted frozen dataclasses.This is a prerequisite for #27001: it fixes the existing assignment behavior and establishes the shared frozen-method dispatch that the deletion change can reuse without expanding its scope.