Don't reset custom __setattr__ in slotted classes

Fixes #680

Signed-off-by: Hynek Schlawack <hs@ox.cx>

changelog.d/681.change.rst

@@ -0,0 +1 @@
+It's now possible to define custom ``__setattr__`` methods on slotted classes again.

src/attr/_make.py

@@ -669,24 +669,29 @@ def _create_slots_class(self):
             if k not in tuple(self._attr_names) + ("__dict__", "__weakref__")
         }
 
-        # Traverse the MRO to check for an existing __weakref__ and
-        # __setattr__.
-        custom_setattr_inherited = False
+        # Check the bases if one of them has an attrs-made __setattr__ that
+        # needs to be reset.
+        # XXX: This can be confused by subclassing a slotted attrs class with
+        # XXX: a non-attrs class and subclass the resulting class with an attrs
+        # XXX: class.  See `test_slotted_confused for details.  For now that's
+        # XXX: OK with us.
+        for base_cls in self._cls.__bases__:
+            if not self._has_own_setattr and getattr(
+                base_cls, "__dict__", {}
+            ).get("__attrs_own_setattr__", False):
+                cd["__setattr__"] = object.__setattr__
+                break
+
+        # Traverse the MRO to check for an existing __weakref__.
         weakref_inherited = False
         for base_cls in self._cls.__mro__[1:-1]:
-            d = getattr(base_cls, "__dict__", {})
-
-            weakref_inherited = weakref_inherited or "__weakref__" in d
-            custom_setattr_inherited = custom_setattr_inherited or not (
-                d.get("__attrs_own_setattr__", False)
-            )
-
-            if weakref_inherited and custom_setattr_inherited:
+            if (
+                getattr(base_cls, "__dict__", {}).get("__weakref__", None)
+                is not None
+            ):
+                weakref_inherited = True
                 break
 
-        if not self._has_own_setattr and not custom_setattr_inherited:
-            cd["__setattr__"] = object.__setattr__
-
         names = self._attr_names
         if (
             self._weakref_slot
@@ -697,7 +702,7 @@ def _create_slots_class(self):
             names += ("__weakref__",)
 
         # We only add the names of attributes that aren't inherited.
-        # Settings __slots__ to inherited attributes wastes memory.
+        # Setting __slots__ to inherited attributes wastes memory.
         slot_names = [name for name in names if name not in base_names]
         if self._cache_hash:
             slot_names.append(_hash_cache_field)

tests/test_setattr.py

@@ -274,6 +274,45 @@ def test_pickling_retains_attrs_own(self, slots):
 
         assert True is WOSAH.__attrs_own_setattr__
 
+    def test_slotted_class_can_have_custom_setattr(self):
+        """
+        A slotted class can define a custom setattr and it doesn't get
+        overwritten.
+
+        Regression test for #680.
+        """
+
+        @attr.s(slots=True)
+        class A(object):
+            def __setattr__(self, key, value):
+                raise SystemError
+
+        with pytest.raises(SystemError):
+            A().x = 1
+
+    @pytest.mark.xfail
+    def test_slotted_confused(self):
+        """
+        If we have a in-between non-attrs class, setattr reset detection
+        should still work, but currently doesn't.
+
+        It works with dict classes because we can look the finished class and
+        patch it.  With slotted classes we have to deduce it ourselves.
+        """
+
+        @attr.s(slots=True)
+        class A(object):
+            x = attr.ib(on_setattr=setters.frozen)
+
+        class B(A):
+            pass
+
+        @attr.s(slots=True)
+        class C(B):
+            x = attr.ib()
+
+        C(1).x = 2
+
 
 @pytest.mark.skipif(PY2, reason="Python 3-only.")
 class TestSetAttrNoPy2(object):

tox.ini

@@ -1,6 +1,7 @@
 [pytest]
 addopts = -ra
 testpaths = tests
+xfail_strict = true
 filterwarnings =
     once::Warning
     ignore:::pympler[.*]