From 8576d3d7a3f52c2c206709de47ddae25d4c3a859 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Fri, 4 Sep 2020 09:59:25 +0200 Subject: [PATCH 1/4] NG: make frozen classes comfortably subclassable --- src/attr/_next_gen.py | 41 ++++++++++++++++++++++----- tests/test_next_gen.py | 64 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 7 deletions(-) diff --git a/src/attr/_next_gen.py b/src/attr/_next_gen.py index 0e3c365f3..b477e6060 100644 --- a/src/attr/_next_gen.py +++ b/src/attr/_next_gen.py @@ -10,7 +10,7 @@ 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,37 @@ def do_it(cls, auto_attribs): 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 is not None + + # 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)." + ) + + frozen = True + 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: diff --git a/tests/test_next_gen.py b/tests/test_next_gen.py index 1ed0108ad..80e9c875a 100644 --- a/tests/test_next_gen.py +++ b/tests/test_next_gen.py @@ -1,6 +1,7 @@ """ Python 3-only integration tests for provisional next generation APIs. """ +import re import pytest @@ -173,3 +174,66 @@ def __eq__(self, o): 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 + 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 From 482a079a7941327cbec98528384a4269df192440 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Fri, 4 Sep 2020 10:05:28 +0200 Subject: [PATCH 2/4] Add newsfragment --- changelog.d/687.change.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelog.d/687.change.rst diff --git a/changelog.d/687.change.rst b/changelog.d/687.change.rst new file mode 100644 index 000000000..24e1bf977 --- /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. From 482e91214aaaea40b475b9b1288b782f0d0a4d9b Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Fri, 4 Sep 2020 10:05:58 +0200 Subject: [PATCH 3/4] This ain't markdown --- changelog.d/687.change.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/687.change.rst b/changelog.d/687.change.rst index 24e1bf977..5438db7de 100644 --- a/changelog.d/687.change.rst +++ b/changelog.d/687.change.rst @@ -1,2 +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. +you don't have to set ``on_setattr=None`` anymore. From 57c3ff4346ae4e030339f2762dd1f60005263655 Mon Sep 17 00:00:00 2001 From: Hynek Schlawack Date: Sat, 5 Sep 2020 11:09:49 +0200 Subject: [PATCH 4/4] Address review --- src/attr/_next_gen.py | 3 +-- tests/test_next_gen.py | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/attr/_next_gen.py b/src/attr/_next_gen.py index b477e6060..b5ff60e88 100644 --- a/src/attr/_next_gen.py +++ b/src/attr/_next_gen.py @@ -82,7 +82,7 @@ def wrap(cls): """ nonlocal frozen, on_setattr - had_on_setattr = on_setattr is not None + 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: @@ -98,7 +98,6 @@ def wrap(cls): "(frozen-ness was inherited)." ) - frozen = True on_setattr = setters.NO_OP break diff --git a/tests/test_next_gen.py b/tests/test_next_gen.py index 80e9c875a..941ec0a57 100644 --- a/tests/test_next_gen.py +++ b/tests/test_next_gen.py @@ -1,6 +1,7 @@ """ Python 3-only integration tests for provisional next generation APIs. """ + import re import pytest @@ -189,7 +190,7 @@ class A: class B(A): b: int - @attr.define + @attr.define(on_setattr=attr.setters.NO_OP) class C(B): c: int