8000 bpo-45897: Fix frozen-slotted dataclass bug by AlexWaygood · Pull Request #29895 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

bpo-45897: Fix frozen-slotted dataclass bug #29895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 16 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions Lib/dataclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,16 +605,25 @@ def _frozen_get_del_attr(cls, fields, globals):
else:
# Special case for the zero-length tuple.
fields_str = '()'

# bpo-45897: Do not raise FrozenInstanceError
# if the dataclass has slots & the attribute is not specified in __slots__.
# Instead, let the standard __slots__ machinery raise AttributeError naturally
# in the super(cls, self).__setattr__ call
return (_create_fn('__setattr__',
('self', 'name', 'value'),
(f'if type(self) is cls or name in {fields_str}:',
(f'if "__slots__" in cls.__dict__ and name not in cls.__slots__:',
' pass',
f'elif type(self) is cls or name in {fields_str}:',
' raise FrozenInstanceError(f"cannot assign to field {name!r}")',
f'super(cls, self).__setattr__(name, value)'),
locals=locals,
globals=globals),
_create_fn('__delattr__',
('self', 'name'),
(f'if type(self) is cls or name in {fields_str}:',
(f'if "__slots__" in cls.__dict__ and name not in cls.__slots__:',
' pass',
f'elif type(self) is cls or name in {fields_str}:',
' raise FrozenInstanceError(f"cannot delete field {name!r}")',
f'super(cls, self).__delattr__(name)'),
locals=locals,
Expand Down Expand Up @@ -1075,12 +1084,6 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen,
f'in class {cls.__name__}. Consider using '
'functools.total_ordering')

if frozen:
for fn in _frozen_get_del_attr(cls, field_list, globals):
if _set_new_attribute(cls, fn.__name__, fn):
raise TypeError(f'Cannot overwrite attribute {fn.__name__} '
f'in class {cls.__name__}')

# Decide if/how we're going to create a hash function.
hash_action = _hash_action[bool(unsafe_hash),
bool(eq),
Expand All @@ -1107,6 +1110,14 @@ def _process_class(cls, init, repr, eq, order, unsafe_hash, frozen,
if slots:
cls = _add_slots(cls, frozen, weakref_slot)

# Frozen __setattr__/__delattr__ must be added after __slots__
# bpo-45897
if frozen:
for fn in _frozen_get_del_attr(cls, field_list, globals):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this function ought to be named _frozen_set_del_attr. That's outside the scope of this PR though.

if _set_new_attribute(cls, fn.__name__, fn):
raise TypeError(f'Cannot overwrite attribute {fn.__name__} '
f'in class {cls.__name__}')

abc.update_abstractmethods(cls)

return cls
Expand Down
57 changes: 57 additions & 0 deletions Lib/test/test_dataclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -3039,6 +3039,63 @@ class A:
self.assertEqual(obj.a, 'a')
self.assertEqual(obj.b, 'b')

def test_frozen_slots_raises_correct_error(self):
# bpo-45897: for a frozen-slotted dataclass instance, attempting to set
# an attribute not specified in __slots__ should raise AttributeError,
# not TypeError or FrozenInstanceError
@dataclass(slots=True, frozen=True)
class Point:
x: int
y: int

p = Point(1, 2)
self.assertEqual(p.x, 1)
self.assertEqual(p.y, 2)
with self.assertRaises(FrozenInstanceError):
p.x = 2

regex = 'object has no attribute'
with self.assertRaisesRegex(AttributeError, regex) as ex:
p.z = 5
type_of_caught_exception = type(ex.exception)

# Verify that the caught exception is `AttributeError` exactly,
# and *not* a subclass of `AttributeError` such as `FrozenInstanceError`.
#
# We need to ensure that this error is being caused by the standard machinery that
# prevents new attributes being added that aren't specified in __slots__.
#
# We *do not* want the error to be caused by anything related to dataclasses.
self.assertIs(
type_of_caught_exception,
AttributeError,
msg=f"expected 'AttributeError', got {type_of_caught_exception.__name__!r}"
)

@dataclass(frozen=True)
class DataclassSubclassOfPointWithNoSlots(Point): pass

d = DataclassSubclassOfPointWithNoSlots(1, 2)
with self.assertRaises(FrozenInstanceError):
d.x = 2

# The subclass is frozen, so any attempt to set an attribute should
# raise an error. The subclass does not define slots, however, so the
# error should be `FrozenInstanceError` here rather than
# `AttributeError`.
with self.assertRaises(FrozenInstanceError):
d.z = 5

class NonDataclassSubclassOfPointWithNoSlots(Point): pass

n = NonDataclassSubclassOfPointWithNoSlots(1, 2)
with self.assertRaises(FrozenInstanceError):
n.x = 2

# This should pass without any exception being raised, since the
# subclass does not define __slots__ and is not frozen.
n.z = 5

def test_slots_no_weakref(self):
@dataclass(slots=True)
class A:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fix bug in dataclasses where a frozen dataclass with slots would raise
``TypeError`` rather than ``AttributeError`` if a user attempted to
set/delete an attribute that was not specified in ``__slots__``. Patch by
Alex Waygood.
0