Repository: python-attrs/attrs
Pull request #687 (2020-09-04 08:03:58)

NG: make frozen classes comfortably subclassable
This is a fix for an inconvenience that @euresti mentioned in https://github.com/python-attrs/attrs/issues/668#issuecomment-678689981.

`on_setattr=validate` gets into the way if you want to pass `define(frozen=True)` or if you subclass a frozen class using `define`.

The new logic should catch these problems.


Patch:
diff --git a/changelog.d/687.change.rst b/changelog.d/687.change.rst
new file mode 100644
index 0000000..5438db7
--- /dev/null
+++ b/changelog.d/687.change.rst
@@ -0,0 +1,2 @@
+The ergonomics of creating frozen classes using ``@define(frozen=True)`` and sub=classing frozen classes has been improved:
+you don't have to set ``on_setattr=None`` anymore.
diff --git a/src/attr/_next_gen.py b/src/attr/_next_gen.py
index 0e3c365..b5ff60e 100644
--- a/src/attr/_next_gen.py
+++ b/src/attr/_next_gen.py
@@ -10,7 +10,7 @@ from functools import partial
 from attr.exceptions import UnannotatedAttributeError
 
 from . import setters
-from ._make import NOTHING, attrib, attrs
+from ._make import NOTHING, _frozen_setattrs, attrib, attrs
 
 
 def define(
@@ -32,10 +32,10 @@ def define(
     order=False,
     auto_detect=True,
     getstate_setstate=None,
-    on_setattr=setters.validate,
+    on_setattr=None,
 ):
     r"""
-    The only behavioral difference is the handling of the *auto_attribs*
+    The only behavioral differences are the handling of the *auto_attribs*
     option:
 
     :param Optional[bool] auto_attribs: If set to `True` or `False`, it behaves
@@ -46,6 +46,7 @@ def define(
        2. Otherwise it assumes *auto_attribs=False* and tries to collect
           `attr.ib`\ s.
 
+    and that mutable classes (``frozen=False``) validate on ``__setattr__``.
 
     .. versionadded:: 20.1.0
     """
@@ -73,11 +74,36 @@ def define(
             on_setattr=on_setattr,
         )
 
-    if auto_attribs is not None:
-        return do_it(maybe_cls, auto_attribs)
-
     def wrap(cls):
-        # Making this a wrapper ensures this code runs during class creation.
+        """
+        Making this a wrapper ensures this code runs during class creation.
+
+        We also ensure that frozen-ness of classes is inherited.
+        """
+        nonlocal frozen, on_setattr
+
+        had_on_setattr = on_setattr not in (None, setters.NO_OP)
+
+        # By default, mutable classes validate on setattr.
+        if frozen is False and on_setattr is None:
+            on_setattr = setters.validate
+
+        # However, if we subclass a frozen class, we inherit the immutability
+        # and disable on_setattr.
+        for base_cls in cls.__bases__:
+            if base_cls.__setattr__ is _frozen_setattrs:
+                if had_on_setattr:
+                    raise ValueError(
+                        "Frozen classes can't use on_setattr "
+                        "(frozen-ness was inherited)."
+                    )
+
+                on_setattr = setters.NO_OP
+                break
+
+        if auto_attribs is not None:
+            return do_it(cls, auto_attribs)
+
         try:
             return do_it(cls, True)
         except UnannotatedAttributeError:


Test patch:
diff --git a/tests/test_next_gen.py b/tests/test_next_gen.py
index 1ed0108..941ec0a 100644
--- a/tests/test_next_gen.py
+++ b/tests/test_next_gen.py
@@ -2,6 +2,8 @@
 Python 3-only integration tests for provisional next generation APIs.
 """
 
+import re
+
 import pytest
 
 import attr
@@ -173,3 +175,66 @@ class TestNextGen:
 
         with pytest.raises(ValueError):
             C() == C()
+
+    def test_subclass_frozen(self):
+        """
+        It's possible to subclass an `attr.frozen` class and the frozen-ness is
+        inherited.
+        """
+
+        @attr.frozen
+        class A:
+            a: int
+
+        @attr.frozen
+        class B(A):
+            b: int
+
+        @attr.define(on_setattr=attr.setters.NO_OP)
+        class C(B):
+            c: int
+
+        assert B(1, 2) == B(1, 2)
+        assert C(1, 2, 3) == C(1, 2, 3)
+
+        with pytest.raises(attr.exceptions.FrozenInstanceError):
+            A(1).a = 1
+
+        with pytest.raises(attr.exceptions.FrozenInstanceError):
+            B(1, 2).a = 1
+
+        with pytest.raises(attr.exceptions.FrozenInstanceError):
+            B(1, 2).b = 2
+
+        with pytest.raises(attr.exceptions.FrozenInstanceError):
+            C(1, 2, 3).c = 3
+
+    def test_catches_frozen_on_setattr(self):
+        """
+        Passing frozen=True and on_setattr hooks is caught, even if the
+        immutability is inherited.
+        """
+
+        @attr.define(frozen=True)
+        class A:
+            pass
+
+        with pytest.raises(
+            ValueError, match="Frozen classes can't use on_setattr."
+        ):
+
+            @attr.define(frozen=True, on_setattr=attr.setters.validate)
+            class B:
+                pass
+
+        with pytest.raises(
+            ValueError,
+            match=re.escape(
+                "Frozen classes can't use on_setattr "
+                "(frozen-ness was inherited)."
+            ),
+        ):
+
+            @attr.define(on_setattr=attr.setters.validate)
+            class C(A):
+                pass