Handle attrib redefs thru multiple inheritance (#287)

changelog.d/285.change.rst

@@ -0,0 +1,2 @@
+The attribute redefinition feature introduced in 17.3.0 now takes into account if an attribute is redefined via multiple inheritance.
+In that case, the definition that is closer to the base of the class hierarchy wins.

changelog.d/287.change.rst

@@ -0,0 +1,2 @@
+The attribute redefinition feature introduced in 17.3.0 now takes into account if an attribute is redefined via multiple inheritance.
+In that case, the definition that is closer to the base of the class hierarchy wins.

src/attr/_make.py

@@ -270,17 +270,24 @@ def _transform_attrs(cls, these, auto_attribs):
     # of attributes we've seen in `take_attr_names` and ignore their
     # redefinitions deeper in the hierarchy.
     super_attrs = []
-    taken_attr_names = set(a.name for a in non_super_attrs)
+    taken_attr_names = {a.name: a for a in non_super_attrs}
     for super_cls in cls.__mro__[1:-1]:
         sub_attrs = getattr(super_cls, "__attrs_attrs__", None)
         if sub_attrs is not None:
             # We iterate over sub_attrs backwards so we can reverse the whole
             # list in the end and get all attributes in the order they have
             # been defined.
             for a in reversed(sub_attrs):
-                if a.name not in taken_attr_names:
+                prev_a = taken_attr_names.get(a.name)
+                if prev_a is None:
+                    super_attrs.append(a)
+                    taken_attr_names[a.name] = a
+                elif prev_a == a:
+                    # This happens thru multiple inheritance.  We don't want
+                    # to favor attributes that are further down in the tree
+                    # so we move them to the back.
+                    super_attrs.remove(a)
                     super_attrs.append(a)
-                    taken_attr_names.add(a.name)
 
     # Now reverse the list, such that the attributes are sorted by *descending*
     # age.  IOW: the oldest attribute definition is at the head of the list.

tests/test_make.py

@@ -214,6 +214,42 @@ class C(Base):
             simple_attr("x"),
         ) == attrs
 
+    def test_multiple_inheritance(self):
+        """
+        Order of attributes doesn't get mixed up by multiple inheritance.
+
+        See #285
+        """
+        @attr.s
+        class A(object):
+            a1 = attr.ib(default="a1")
+            a2 = attr.ib(default="a2")
+
+        @attr.s
+        class B(A):
+            b1 = attr.ib(default="b1")
+            b2 = attr.ib(default="b2")
+
+        @attr.s
+        class C(B, A):
+            c1 = attr.ib(default="c1")
+            c2 = attr.ib(default="c2")
+
+        @attr.s
+        class D(A):
+            d1 = attr.ib(default="d1")
+            d2 = attr.ib(default="d2")
+
+        @attr.s
+        class E(D, C):
+            e1 = attr.ib(default="e1")
+            e2 = attr.ib(default="e2")
+
+        assert (
+            "E(a1='a1', a2='a2', b1='b1', b2='b2', c1='c1', c2='c2', d1='d1', "
+            "d2='d2', e1='e1', e2='e2')"
+        ) == repr(E())
+
 
 class TestAttributes(object):
     """