From c5b690ff93904bfdc716e656d84bd7007faf32ee Mon Sep 17 00:00:00 2001
From: Hynek Schlawack <hs@ox.cx>
Date: Fri, 4 Sep 2020 08:27:10 +0200
Subject: [PATCH] Differentiate between own (= attrs) and custom (= user)
 __setattrs__

Signed-off-by: Hynek Schlawack <hs@ox.cx>
---
 src/attr/_make.py     | 35 ++++++++++++++++----------------
 tests/test_setattr.py | 46 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 17 deletions(-)

diff --git a/src/attr/_make.py b/src/attr/_make.py
index 5edcc0d54..7e8ace39f 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -556,6 +556,7 @@ class _ClassBuilder(object):
         "_slots",
         "_weakref_slot",
         "_has_own_setattr",
+        "_has_custom_setattr",
     )
 
     def __init__(
@@ -593,7 +594,8 @@ def __init__(
         self._is_exc = is_exc
         self._on_setattr = on_setattr
 
-        self._has_own_setattr = has_custom_setattr
+        self._has_custom_setattr = has_custom_setattr
+        self._has_own_setattr = False
 
         self._cls_dict["__attrs_attrs__"] = self._attrs
 
@@ -654,9 +656,11 @@ def _patch_original_class(self):
         if not self._has_own_setattr and getattr(
             cls, "__attrs_own_setattr__", False
         ):
-            cls.__setattr__ = object.__setattr__
             cls.__attrs_own_setattr__ = False
 
+            if not self._has_custom_setattr:
+                cls.__setattr__ = object.__setattr__
+
         return cls
 
     def _create_slots_class(self):
@@ -679,13 +683,16 @@ def _create_slots_class(self):
         # XXX: class.  See `test_slotted_confused` for details.  For now that's
         # XXX: OK with us.
         if not self._has_own_setattr:
-            # There's metaclass magic that may result in a baseclass without
-            # __bases__ which results in _us_ not having one.  cf. #681
-            for base_cls in getattr(self._cls, "__bases__", ()):
-                if base_cls.__dict__.get("__attrs_own_setattr__", False):
-                    cd["__setattr__"] = object.__setattr__
-                    cd["__attrs_own_setattr__"] = False
-                    break
+            cd["__attrs_own_setattr__"] = False
+
+            if not self._has_custom_setattr:
+                # There's metaclass magic that may result in a baseclassa
+                # without __bases__ which results in _us_ not having one. cf.
+                # #681
+                for base_cls in getattr(self._cls, "__bases__", ()):
+                    if 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
@@ -864,20 +871,14 @@ def add_setattr(self):
         if not sa_attrs:
             return self
 
-        if self._has_own_setattr:
+        if self._has_custom_setattr:
             # We need to write a __setattr__ but there already is one!
             raise ValueError(
                 "Can't combine custom __setattr__ with on_setattr hooks."
             )
 
-        cls = self._cls
-
+        # docstring comes from _add_method_dunders
         def __setattr__(self, name, val):
-            """
-            Method generated by attrs for class %s.
-            """ % (
-                cls.__name__,
-            )
             try:
                 a, hook = sa_attrs[name]
             except KeyError:
diff --git a/tests/test_setattr.py b/tests/test_setattr.py
index 02867135a..34d9ad723 100644
--- a/tests/test_setattr.py
+++ b/tests/test_setattr.py
@@ -340,6 +340,7 @@ def __setattr__(self, name, val):
 
         i = RemoveNeedForOurSetAttr(1)
 
+        assert not RemoveNeedForOurSetAttr.__attrs_own_setattr__
         assert 2 == i.x
 
     @pytest.mark.parametrize("slots", [True, False])
@@ -387,3 +388,48 @@ class HookAndCustomSetAttr(object):
 
                 def __setattr__(self, _, __):
                     pass
+
+    @pytest.mark.parametrize("a_slots", [True, False])
+    @pytest.mark.parametrize("b_slots", [True, False])
+    @pytest.mark.parametrize("c_slots", [True, False])
+    def test_setattr_inherited_do_not_reset_intermediate(
+        self, a_slots, b_slots, c_slots
+    ):
+        """
+        A user-provided intermediate __setattr__ is not reset to
+        object.__setattr__.
+
+        This only can work on Python 3+ with auto_detect activated, such that
+        attrs can know, that there is a user-provided __setattr__.
+        """
+
+        @attr.s(slots=a_slots)
+        class A(object):
+            x = attr.ib(on_setattr=setters.frozen)
+
+        @attr.s(slots=b_slots, auto_detect=True)
+        class B(A):
+            x = attr.ib(on_setattr=setters.NO_OP)
+
+            def __setattr__(self, key, value):
+                raise SystemError
+
+        @attr.s(slots=c_slots)
+        class C(B):
+            pass
+
+        assert getattr(A, "__attrs_own_setattr__", False) is True
+        assert getattr(B, "__attrs_own_setattr__", False) is False
+        assert getattr(C, "__attrs_own_setattr__", False) is False
+
+        with pytest.raises(SystemError):
+            C(1).x = 3
+
+    def test_docstring(self):
+        """
+        Generated __setattr__ has a useful docstring.
+        """
+        assert (
+            "Method generated by attrs for class WithOnSetAttrHook."
+            == WithOnSetAttrHook.__setattr__.__doc__
+        )