From c46ffe5b3c155d2b679487c7a26b8e598e229a47 Mon Sep 17 00:00:00 2001 From: Denis Laxalde Date: Mon, 3 Jun 2024 11:26:59 +0200 Subject: [PATCH] Fix inheritance resolution of cached properties in slotted class This resolves the case where a sub-class of a slotted class defining some cached properties has a custom __getattr__() method. In that case, we need to build the custom __getattr__ implementation (see in _make_cached_property_getattr()) using cached properties from all classes in the MRO. In order to keep references of cached properties defined the inheritance hierarchy, we store them in a new __attrs_cached_properties__ attribute and finally build the "cached_properties" value, passed to _make_cached_property_getattr(), by combining current class' cached properties with that of all its parents. Also, when building __attrs_cached_properties__, we now clear current class' __dict__ (name 'cd'), thus saving an extra loop. Fix https://github.com/python-attrs/attrs/issues/1288 --- changelog.d/1289.change.md | 1 + src/attr/_make.py | 26 ++++++++++++++++------ tests/test_slots.py | 44 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 changelog.d/1289.change.md diff --git a/changelog.d/1289.change.md b/changelog.d/1289.change.md new file mode 100644 index 000000000..a1a1e6857 --- /dev/null +++ b/changelog.d/1289.change.md @@ -0,0 +1 @@ +Fix inheritance resolution of cached properties in slotted class when subclasses do not define any `@cached_property` themselves but do define a custom `__getattr__()` method. diff --git a/src/attr/_make.py b/src/attr/_make.py index d3bfb440f..10f3863de 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -917,11 +917,27 @@ def _create_slots_class(self): names += ("__weakref__",) if PY_3_8_PLUS: + # Store class cached properties for further use by subclasses + # (below) while clearing them out from __dict__ to avoid name + # clashing. + cd["__attrs_cached_properties__"] = { + name: cd.pop(name).func + for name in [ + name + for name, cached_property in cd.items() + if isinstance(cached_property, functools.cached_property) + ] + } + # Gather cached properties from parent classes. cached_properties = { - name: cached_property.func - for name, cached_property in cd.items() - if isinstance(cached_property, functools.cached_property) + name: func + for base_cls in self._cls.__mro__[1:-1] + for name, func in base_cls.__dict__.get( + "__attrs_cached_properties__", {} + ).items() } + # Then from this class. + cached_properties.update(cd["__attrs_cached_properties__"]) else: # `functools.cached_property` was introduced in 3.8. # So can't be used before this. @@ -934,10 +950,6 @@ def _create_slots_class(self): # Add cached properties to names for slotting. names += tuple(cached_properties.keys()) - for name in cached_properties: - # Clear out function from class to avoid clashing. - del cd[name] - additional_closure_functions_to_update.extend( cached_properties.values() ) diff --git a/tests/test_slots.py b/tests/test_slots.py index 78215ea18..e0dbdd370 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -891,6 +891,50 @@ def f(self): assert b.z == "z" +@pytest.mark.skipif(not PY_3_8_PLUS, reason="cached_property is 3.8+") +def test_slots_getattr_in_subclass_without_cached_property(): + """ + Ensure that when a subclass of a slotted class with cached properties + defines a __getattr__ but has no cached property itself, parent's cached + properties are reachable. + + Cover definition and usage of __attrs_cached_properties__ internal + attribute. + + Regression test for issue https://github.com/python-attrs/attrs/issues/1288 + """ + + @attr.s(slots=True) + class A: + @functools.cached_property + def f(self): + return 0 + + @attr.s(slots=True) + class B(A): + def __getattr__(self, item): + return item + + @attr.s(slots=True) + class C(B): + @functools.cached_property + def g(self): + return 1 + + b = B() + assert b.f == 0 + assert b.z == "z" + + c = C() + assert c.f == 0 + assert c.g == 1 + assert c.a == "a" + + assert list(A.__attrs_cached_properties__) == ["f"] + assert not B.__attrs_cached_properties__ + assert list(C.__attrs_cached_properties__) == ["g"] + + @pytest.mark.skipif(not PY_3_8_PLUS, reason="cached_property is 3.8+") def test_slots_getattr_in_subclass_gets_superclass_cached_property(): """