Repository: python-attrs/attrs
Pull request #718 (2020-11-13 18:27:40)

Fixed slots inheritance
# Pull Request Check List

This is just a friendly reminder about the most common mistakes.  Please make sure that you tick all boxes.  But please read our [contribution guide](https://www.attrs.org/en/latest/contributing.html) at least once, it will save you unnecessary review cycles!

If an item doesn't apply to your pull request, **check it anyway** to make it apparent that there's nothing left to do.  If your pull request is a documentation fix or a trivial typo, feel free to delete the whole thing.

- [x] Added **tests** for changed code.
- [ ] New features have been added to our [Hypothesis testing strategy](https://github.com/python-attrs/attrs/blob/master/tests/strategies.py).
- [x] Changes or additions to public APIs are reflected in our type stubs (files ending in ``.pyi``).
    - [x] ...and used in the stub test file `tests/typing_example.py`.
- [x] Updated **documentation** for changed code.
    - [x] New functions/classes have to be added to `docs/api.rst` by hand.
    - [x] Changes to the signature of `@attr.s()` have to be added by hand too.
    - [x] Changed/added classes/methods/functions have appropriate `versionadded`, `versionchanged`, or `deprecated` [directives](http://www.sphinx-doc.org/en/stable/markup/para.html#directive-versionadded).  Find the appropriate next version in our [``__init__.py``](https://github.com/python-attrs/attrs/blob/master/src/attr/__init__.py) file.
- [x] Documentation in `.rst` files is written using [semantic newlines](https://rhodesmill.org/brandon/2012/one-sentence-per-line/).
- [x] Changes (and possible deprecations) have news fragments in [`changelog.d`](https://github.com/python-attrs/attrs/blob/master/changelog.d).

If you have *any* questions to *any* of the points above, just **submit and ask**!  This checklist is here to *help* you, not to deter you from contributing!



Patch:
diff --git a/changelog.d/718.change.rst b/changelog.d/718.change.rst
new file mode 100644
index 0000000..a5dbfde
--- /dev/null
+++ b/changelog.d/718.change.rst
@@ -0,0 +1,1 @@
+Fixed creation of extra slot for ``attr.ib`` when parent class already has a slot with the same name
diff --git a/src/attr/_make.py b/src/attr/_make.py
index 64af3cc..1ed202f 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -703,7 +703,6 @@ class _ClassBuilder(object):
         """
         Build and return a new class with a `__slots__` attribute.
         """
-        base_names = self._base_names
         cd = {
             k: v
             for k, v in iteritems(self._cls_dict)
@@ -727,12 +726,21 @@ class _ClassBuilder(object):
                         cd["__setattr__"] = object.__setattr__
                         break
 
-        # Traverse the MRO to check for an existing __weakref__.
+        # Traverse the MRO to collect existing slots
+        # and check for an existing __weakref__.
+        existing_slots = dict()
         weakref_inherited = False
         for base_cls in self._cls.__mro__[1:-1]:
             if base_cls.__dict__.get("__weakref__", None) is not None:
                 weakref_inherited = True
-                break
+            existing_slots.update(
+                {
+                    name: getattr(base_cls, name)
+                    for name in getattr(base_cls, "__slots__", [])
+                }
+            )
+
+        base_names = set(self._base_names)
 
         names = self._attr_names
         if (
@@ -746,6 +754,17 @@ class _ClassBuilder(object):
         # We only add the names of attributes that aren't inherited.
         # Setting __slots__ to inherited attributes wastes memory.
         slot_names = [name for name in names if name not in base_names]
+        # There are slots for attributes from current class
+        # that are defined in parent classes.
+        # As their descriptors may be overriden by a child class,
+        # we collect them here and update the class dict
+        reused_slots = {
+            slot: slot_descriptor
+            for slot, slot_descriptor in iteritems(existing_slots)
+            if slot in slot_names
+        }
+        slot_names = [name for name in slot_names if name not in reused_slots]
+        cd.update(reused_slots)
         if self._cache_hash:
             slot_names.append(_hash_cache_field)
         cd["__slots__"] = tuple(slot_names)


Test patch:
diff --git a/tests/test_slots.py b/tests/test_slots.py
index b57fc63..d5106ff 100644
--- a/tests/test_slots.py
+++ b/tests/test_slots.py
@@ -268,6 +268,70 @@ def test_inheritance_from_slots():
     assert {"x": 1, "y": 2, "z": "test"} == attr.asdict(c2)
 
 
+def test_inheritance_from_slots_with_attribute_override():
+    """
+    Inheriting from a slotted class doesn't re-create existing slots
+    """
+
+    class HasXSlot(object):
+        __slots__ = ("x",)
+
+    @attr.s(slots=True, hash=True)
+    class C2Slots(C1Slots):
+        # y re-defined here but it shouldn't get a slot
+        y = attr.ib()
+        z = attr.ib()
+
+    @attr.s(slots=True, hash=True)
+    class NonAttrsChild(HasXSlot):
+        # Parent class has slot for "x" already, so we skip it
+        x = attr.ib()
+        y = attr.ib()
+        z = attr.ib()
+
+    c2 = C2Slots(1, 2, "test")
+    assert 1 == c2.x
+    assert 2 == c2.y
+    assert "test" == c2.z
+
+    assert {"z"} == set(C2Slots.__slots__)
+
+    na = NonAttrsChild(1, 2, "test")
+    assert 1 == na.x
+    assert 2 == na.y
+    assert "test" == na.z
+
+    assert {"__weakref__", "y", "z"} == set(NonAttrsChild.__slots__)
+
+
+def test_inherited_slot_reuses_slot_descriptor():
+    """
+    We reuse slot descriptor for an attr.ib defined in a slotted attr.s
+    """
+
+    class HasXSlot(object):
+        __slots__ = ("x",)
+
+    class OverridesX(HasXSlot):
+        @property
+        def x(self):
+            return None
+
+    @attr.s(slots=True)
+    class Child(OverridesX):
+        x = attr.ib()
+
+    assert Child.x is not OverridesX.x
+    assert Child.x is HasXSlot.x
+
+    c = Child(1)
+    assert 1 == c.x
+    assert set() == set(Child.__slots__)
+
+    ox = OverridesX()
+    assert ox.x is None
+
+
 def test_bare_inheritance_from_slots():
     """
     Inheriting from a bare attrs slotted class works.