diff --git a/changelog.d/1291.change.md b/changelog.d/1291.change.md new file mode 100644 index 000000000..a1a1e6857 --- /dev/null +++ b/changelog.d/1291.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..95b9e038c 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -598,55 +598,60 @@ def _transform_attrs( return _Attributes((AttrsClass(attrs), base_attrs, base_attr_map)) -def _make_cached_property_getattr(cached_properties, original_getattr, cls): +def _make_cached_property_getattribute( + cached_properties, original_getattribute, cls +): lines = [ # Wrapped to get `__class__` into closure cell for super() # (It will be replaced with the newly constructed class after construction). "def wrapper(_cls):", " __class__ = _cls", - " def __getattr__(self, item, cached_properties=cached_properties, original_getattr=original_getattr, _cached_setattr_get=_cached_setattr_get):", - " func = cached_properties.get(item)", - " if func is not None:", - " result = func(self)", - " _setter = _cached_setattr_get(self)", - " _setter(item, result)", - " return result", + " def __getattribute__(self, item, cached_properties=cached_properties, original_getattribute=original_getattribute, _cached_setattr_get=_cached_setattr_get):", + " try:", + " return object.__getattribute__(self, item)", + " except AttributeError:", + " func = cached_properties.get(item)", + " if func is not None:", + " result = func(self)", + " _setter = _cached_setattr_get(self)", + " _setter(item, result)", + " return result", ] - if original_getattr is not None: + if original_getattribute is not None: lines.append( - " return original_getattr(self, item)", + " return original_getattribute(self, item)", ) else: lines.extend( [ - " try:", - " return super().__getattribute__(item)", - " except AttributeError:", - " if not hasattr(super(), '__getattr__'):", - " raise", - " return super().__getattr__(item)", - " original_error = f\"'{self.__class__.__name__}' object has no attribute '{item}'\"", - " raise AttributeError(original_error)", + " try:", + " return super().__getattribute__(item)", + " except AttributeError:", + " if not hasattr(super(), '__getattribute__'):", + " raise", + " return super().__getattribute__(item)", + " original_error = f\"'{self.__class__.__name__}' object has no attribute '{item}'\"", + " raise AttributeError(original_error)", ] ) lines.extend( [ - " return __getattr__", - "__getattr__ = wrapper(_cls)", + " return __getattribute__", + "__getattribute__ = wrapper(_cls)", ] ) - unique_filename = _generate_unique_filename(cls, "getattr") + unique_filename = _generate_unique_filename(cls, "getattribute") glob = { "cached_properties": cached_properties, "_cached_setattr_get": _OBJ_SETATTR.__get__, - "original_getattr": original_getattr, + "original_getattribute": original_getattribute, } return _make_method( - "__getattr__", + "__getattribute__", "\n".join(lines), unique_filename, glob, @@ -948,12 +953,14 @@ def _create_slots_class(self): if annotation is not inspect.Parameter.empty: class_annotations[name] = annotation - original_getattr = cd.get("__getattr__") - if original_getattr is not None: - additional_closure_functions_to_update.append(original_getattr) + original_getattribute = cd.get("__getattribute__") + if original_getattribute is not None: + additional_closure_functions_to_update.append( + original_getattribute + ) - cd["__getattr__"] = _make_cached_property_getattr( - cached_properties, original_getattr, self._cls + cd["__getattribute__"] = _make_cached_property_getattribute( + cached_properties, original_getattribute, self._cls ) # We only add the names of attributes that aren't inherited. diff --git a/tests/test_slots.py b/tests/test_slots.py index 78215ea18..9bd2b3ef3 100644 --- a/tests/test_slots.py +++ b/tests/test_slots.py @@ -891,6 +891,63 @@ 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. + + Regression test for issue https://github.com/python-attrs/attrs/issues/1288 + """ + + # Reference behaviour, without attr. + class P: + __slots__ = () + + @functools.cached_property + def f(self) -> int: + return 0 + + class C(P): + def __getattr__(self, item: str) -> str: + return item + + assert not C.__slots__ + c = C() + assert c.x == "x" + assert c.__getattribute__("f") == 0 + assert c.f == 0 + + # Same with a base attr class. + @attr.s(slots=True) + class A: + @functools.cached_property + def f(self) -> int: + return 0 + + # But subclass is not an attr-class. + class B(A): + def __getattr__(self, item: str) -> str: + return item + + b = B() + assert b.z == "z" + assert b.__getattribute__("f") == 0 + assert b.f == 0 + + # And also if subclass is an attr-class. + @attr.s(slots=True) + class D(A): + def __getattr__(self, item: str) -> str: + return item + + d = D() + assert d.z == "z" + assert d.__getattribute__("f") == 0 + assert d.f == 0 + + @pytest.mark.skipif(not PY_3_8_PLUS, reason="cached_property is 3.8+") def test_slots_getattr_in_subclass_gets_superclass_cached_property(): """