diff --git a/changelog.d/1637.change.md b/changelog.d/1637.change.md new file mode 100644 index 000000000..70fd2eb6c --- /dev/null +++ b/changelog.d/1637.change.md @@ -0,0 +1 @@ +Slots that a slotted class declares on itself are now preserved instead of being dropped while the class is rebuilt. This allows for instance attributes that `attrs` is entirely unaware of -- they don't show up in `attrs.fields()`, `attrs.asdict()`, or the generated `__init__`/`__eq__`/`__hash__`. diff --git a/docs/examples.md b/docs/examples.md index f48b848bd..75c289562 100644 --- a/docs/examples.md +++ b/docs/examples.md @@ -653,6 +653,25 @@ Defining `__slots__` by hand is tedious, in *attrs* it's just a matter of using {func}`~attrs.define` sets `slots=True` by default. +If you need instance attributes that *attrs* should be entirely unaware of -- they don't appear in {func}`attrs.fields`, {func}`attrs.asdict`, or the generated `__init__`, `__eq__`, and `__hash__` -- you can declare them as slots on the class yourself: + +```{doctest} +>>> from attrs import asdict + +>>> @define +... class C: +... __slots__ = ("_internal",) +... +... x: int + +>>> c = C(1) +>>> c._internal = "not part of the data model" +>>> c._internal +'not part of the data model' +>>> asdict(c) +{'x': 1} +``` + ## Immutability diff --git a/src/attr/_make.py b/src/attr/_make.py index e5b3760c1..d6cec006a 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -644,6 +644,25 @@ def evolve(*args, **changes): return cls(**changes) +def _mangle_slot_name(cls, name): + """ + Return the name *name* is stored under in *cls*'s class dict. + + Python mangles identifiers that start with two underscores and don't end + with two -- `__slots__` entries included. Since we re-create classes + from a plain dict, type() applies the mangling to us, but we have to + look up (and deduplicate against) the mangled names ourselves. + """ + if not name.startswith("__") or name.endswith("__"): + return name + + class_name = cls.__name__.lstrip("_") + if not class_name: + return name + + return f"_{class_name}{name}" + + class _ClassBuilder: """ Iteratively build *one* class. @@ -947,6 +966,25 @@ def _create_slots_class(self): # Setting __slots__ to inherited attributes wastes memory. slot_names = [name for name in names if name not in base_names] + # Slots the class declares itself are not attrs fields, but they still + # have to be carried over -- otherwise `__slots__` below overwrites + # them and the descriptors created for the original class are left + # behind, raising on any access to instances of the new class. + # See . + for declared_name in self._declared_slot_names(): + name = _mangle_slot_name(self._cls, declared_name) + + # The descriptor belongs to the original class and is useless for + # the new one, no matter whether the slot is laid out again. + cd.pop(name, None) + + # Slots that are inherited (be it from attrs or from a hand-written + # base class) are already laid out and the descriptors of the base + # classes stay reachable through the MRO. + if name not in slot_names and name not in existing_slots: + # type() mangles the name of the new class for us again. + slot_names.append(declared_name) + # There are slots for attributes from current class # that are defined in parent classes. # As their descriptors may be overridden by a child class, @@ -1022,6 +1060,20 @@ def __str__(self): self._cls_dict["__str__"] = self._add_method_dunders(__str__) return self + def _declared_slot_names(self): + """ + Return the names the class itself declares in its `__slots__`, as a + tuple. + + Python accepts any iterable -- strings and mappings included -- as + `__slots__`, so this can't be used as-is. + """ + declared_slots = self._cls_dict.get("__slots__", ()) + if isinstance(declared_slots, str): + return (declared_slots,) + + return tuple(declared_slots) + def _make_getstate_setstate(self): """ Create custom __setstate__ and __getstate__ methods. @@ -1031,11 +1083,30 @@ def _make_getstate_setstate(self): an for an in self._attr_names if an != "__weakref__" ) + # Slots declared by the class itself are not attrs fields, but they're + # still part of the instance state and have to survive pickling and + # copying. + extra_state_names = tuple( + mangled + for mangled in ( + _mangle_slot_name(self._cls, name) + for name in self._declared_slot_names() + ) + if mangled not in state_attr_names + and mangled not in ("__dict__", "__weakref__", "__slots__") + ) + def slots_getstate(self): """ Automatically created by attrs. """ - return {name: getattr(self, name) for name in state_attr_names} + state = {name: getattr(self, name) for name in state_attr_names} + for name in extra_state_names: + value = getattr(self, name, _SENTINEL) + if value is not _SENTINEL: + state[name] = value + + return state hash_caching_enabled = self._cache_hash @@ -1054,6 +1125,10 @@ def slots_setstate(self, state): if name in state: __bound_setattr(name, state[name]) + for name in extra_state_names: + if name in state: + __bound_setattr(name, state[name]) + # The hash code cache is not included when the object is # serialized, but it still needs to be initialized to None to # indicate that the first call to __hash__ should be a cache diff --git a/tests/test_slots.py b/tests/test_slots.py index a74c32b03..1382c7d28 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -4,6 +4,7 @@ Unit tests for slots-related functionality. """ +import copy import functools import pickle import weakref @@ -1199,6 +1200,296 @@ class NEW_A: assert not hasattr(new_a, "d") +def test_slots_declared_on_class_are_kept(): + """ + Slots that a slotted class declares itself -- and that aren't attrs + fields -- are kept when attrs rebuilds the class. + + Regression test for https://github.com/python-attrs/attrs/issues/1637. + """ + + @attrs.define + class A: + __slots__ = ("private",) + + public: str = "foo" + + a = A() + a.private = 1 + + assert 1 == a.private + assert {"private"} <= set(A.__slots__) + # The descriptor must belong to the rebuilt class; the one of the original + # class is useless and raises on assignment. + assert A.__dict__["private"].__objclass__ is A + # attrs stays unaware of the extra slot. + assert ["public"] == [f.name for f in attrs.fields(A)] + assert {"public": "foo"} == attrs.asdict(a) + assert A() == a + + +def test_slots_declared_as_string_are_kept(): + """ + A plain string is a legal __slots__ value and is kept, too. + """ + + @attr.s(slots=True) + class A: + __slots__ = "private" # noqa: PLC0205 -- strings are legal + + x = attr.ib() + + a = A(1) + a.private = 42 + + assert 42 == a.private + assert {"private"} <= set(A.__slots__) + + +def test_slots_declared_as_mapping_are_kept(): + """ + Any iterable is accepted for __slots__ by Python, mappings included. + """ + + @attrs.define + class A: + __slots__ = {"private": "Docstrings are allowed, too."} + + public: str = "foo" + + a = A() + a.private = 2 + + assert 2 == a.private + assert "private" in A.__slots__ + + +def test_slots_declared_on_class_do_not_duplicate_inherited_slots(): + """ + A declared slot that a hand-written base class already lays out is not + laid out a second time. + + ``__weakref__`` can't be declared again once a base class laid it out + (plain Python raises), so it's omitted here. + """ + + class Base: + __slots__ = ("shared",) + + @attrs.define + class A(Base): + __slots__ = ("private", "shared") + + public: str = "foo" + + a = A() + a.shared = "s" + a.private = "p" + + assert "s" == a.shared + assert "p" == a.private + assert weakref.ref(a)() is a + assert "shared" not in A.__slots__ + assert "private" in A.__slots__ + + +def test_slots_declared_that_are_base_attrs_fields_are_not_duplicated(): + """ + A declared slot whose name an inherited attrs field already occupies is not + laid out a second time on the subclass. + """ + + @attrs.define + class A: + x: int + + @attrs.define + class B(A): + __slots__ = ("private", "x") + + b = B(1) + b.private = 2 + + assert 1 == b.x + assert 2 == b.private + assert "x" not in B.__slots__ + assert ["x"] == [f.name for f in attrs.fields(B)] + + +def test_slots_declared_with_private_name_are_mangled(): + """ + Privately named slots are mangled like Python does it, even though attrs + re-creates the class from a plain dict. + """ + + @attrs.define + class A: + __slots__ = ("__private",) + + public: str = "foo" + + def set_private(self, value): + self.__private = value + + def get_private(self): + return self.__private + + a = A() + a.set_private(1) + + assert 1 == a.get_private() + assert "__private" in A.__slots__ + assert "_A__private" in A.__dict__ + assert A.__dict__["_A__private"].__objclass__ is A + assert ["public"] == [f.name for f in attrs.fields(A)] + + +def test_slots_declared_on_class_are_inherited(): + """ + Declared slots of a slotted attrs class are usable by subclasses and are + not laid out a second time. + """ + + @attrs.define + class A: + __slots__ = ("private",) + + public: str = "foo" + + @attrs.define + class B(A): + extra: int = 0 + + b = B() + b.private = 3 + + assert 3 == b.private + assert "private" not in B.__slots__ + + +def test_slots_declared_with_dict_and_weakref_are_kept(): + """ + Declaring __dict__ and __weakref__ in __slots__ works like it does on a + plain class. + """ + + @attrs.define + class A: + __slots__ = ("__dict__", "__weakref__", "private") + + public: str = "foo" + + a = A() + a.private = 1 + a.dynamic = 2 + + assert 1 == a.private + assert 2 == a.dynamic + assert weakref.ref(a)() is a + + +def test_slots_declared_for_attrs_field_are_not_duplicated(): + """ + A slot that is also an attrs field is laid out exactly once. + """ + + @attrs.define + class A: + __slots__ = ("public",) + + public: str + + a = A("foo") + + assert "foo" == a.public + assert ["public"] == [s for s in A.__slots__ if s == "public"] + assert ["public"] == [f.name for f in attrs.fields(A)] + + +def test_slots_declared_on_frozen_class_are_kept(): + """ + Frozen classes keep declared slots as well -- the frozen __setattr__ has to + be bypassed to fill them. + """ + + @attrs.frozen + class A: + __slots__ = ("private",) + + public: str = "foo" + + a = A() + object.__setattr__(a, "private", 1) + + assert 1 == a.private + + +@attrs.define +class _DeclaredSlotsPickleable: + """Module-level so that pickle can find it.""" + + __slots__ = ("private",) + + public: str = "foo" + + +@attrs.frozen +class _DeclaredSlotsFrozenPickleable: + """Module-level so that pickle can find it.""" + + __slots__ = ("private",) + + public: str = "foo" + + +@pytest.mark.parametrize( + "cls", [_DeclaredSlotsPickleable, _DeclaredSlotsFrozenPickleable] +) +def test_slots_declared_on_class_survive_pickle(cls): + """ + Declared slots are part of the instance state and survive pickling. + """ + a = cls() + object.__setattr__(a, "private", 1) + + a_unpickled = pickle.loads(pickle.dumps(a)) + + assert cls is type(a_unpickled) + assert "foo" == a_unpickled.public + assert 1 == a_unpickled.private + + +def test_slots_declared_on_class_unset_survive_pickle(): + """ + Declared slots that were never set neither break pickling nor get set. + """ + a_unpickled = pickle.loads(pickle.dumps(_DeclaredSlotsPickleable())) + + assert "foo" == a_unpickled.public + with pytest.raises(AttributeError): + a_unpickled.private + + +def test_slots_declared_on_class_survive_copy(): + """ + Declared slots are copied by copy.copy() and copy.deepcopy(). + """ + + @attrs.define + class A: + __slots__ = ("private",) + + public: str = "foo" + + a = A() + a.private = [1] + + assert [1] == copy.copy(a).private + deep = copy.deepcopy(a) + assert [1] == deep.private + assert deep.private is not a.private + + def test_slots_unpickle_is_backward_compatible(frozen): """ Ensure object pickled before v22.2.0 can still be unpickled.