diff --git a/Lib/dataclasses.py b/Lib/dataclasses.py index a567a33d646f44..a9c4d433ba2a8e 100644 --- a/Lib/dataclasses.py +++ b/Lib/dataclasses.py @@ -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, @@ -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), @@ -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): + 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 diff --git a/Lib/test/test_dataclasses.py b/Lib/test/test_dataclasses.py index e2eab695789de7..b120fbb2e36285 100644 --- a/Lib/test/test_dataclasses.py +++ b/Lib/test/test_dataclasses.py @@ -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: diff --git a/Misc/NEWS.d/next/Library/2021-12-01-21-45-43.bpo-45897.lAZPRy.rst b/Misc/NEWS.d/next/Library/2021-12-01-21-45-43.bpo-45897.lAZPRy.rst new file mode 100644 index 00000000000000..7f873b7f8899ab --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-12-01-21-45-43.bpo-45897.lAZPRy.rst @@ -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.