Replace cached_property on slotted classes with a new descriptor instead of writing __getattr__

bench/test_benchmarks.py

@@ -214,3 +214,19 @@ def test_repeated_access(self):
 
         for _ in range(ROUNDS):
             _ = c.cached
+
+    def test_create_cached_property_class(self):
+        """
+        Benchmark creating a class with a cached property
+        """
+        for _ in range(ROUNDS):
+
+            @attrs.define
+            class LocalC:
+                x: int
+                y: str
+                z: dict[str, int]
+
+                @functools.cached_property
+                def cached(self):
+                    return 42

docs/how-does-it-work.md

@@ -110,8 +110,9 @@ Therefore, *attrs* converts `cached_property`-decorated methods when constructin
 
 Getting this working is achieved by:
 
+* Removing the wrapped methods from the original class and storing them.
 * Adding names to `__slots__` for the wrapped methods.
-* Adding a `__getattr__` method to set values on the wrapped methods.
+* Creating new `_SlottedCachedProperty` wrappers that wrap the slot descriptors together with the original functions.
 
 For most users, this should mean that it works transparently.
 

src/attr/_make.py

@@ -496,56 +496,47 @@ def _transform_attrs(
     return _Attributes(AttrsClass(attrs), base_attrs, base_attr_map)
 
 
-def _make_cached_property_getattr(cached_properties, original_getattr, 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",
-    ]
-    if original_getattr is not None:
-        lines.append(
-            "         return original_getattr(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)",
-            ]
-        )
+class _SlottedCachedProperty:
+    """
+    This is a class that is used to wrap both a slot and a cached property.
+    *It is not to be used directly.*
+    Users should just use `functools.cached_property`.
 
-    lines.extend(
-        [
-            "    return __getattr__",
-            "__getattr__ = wrapper(_cls)",
-        ]
-    )
+    attrs' slotting behaviour will remove cached_property instances,
+    add the names to `__slots__` and after constructing the class,
+    replace those slot descriptors with instances of this class
+    """
 
-    unique_filename = _generate_unique_filename(cls, "getattr")
+    def __init__(self, slot, func):
+        self.slot = slot
+        self.func = func
+        self.__doc__ = self.func.__doc__
+        self.__module__ = self.func.__module__
 
-    glob = {
-        "cached_properties": cached_properties,
-        "_cached_setattr_get": _OBJ_SETATTR.__get__,
-        "original_getattr": original_getattr,
-    }
+        self._slotget = slot.__get__
+        self._slotset = slot.__set__
+        self._slotdelete = slot.__delete__
 
-    return _linecache_and_compile(
-        "\n".join(lines), unique_filename, glob, locals={"_cls": cls}
-    )["__getattr__"]
+    def __get__(self, instance, owner=None):
+        if instance is None:
+            return self
+
+        try:
+            return self._slotget(instance, owner)
+        except AttributeError:
+            pass
+
+        result = self.func(instance)
+
+        self._slotset(instance, result)
+
+        return result
+
+    def __set__(self, obj, value):
+        self._slotset(obj, value)
+
+    def __delete__(self, obj):
+        self._slotdelete(obj)
 
 
 def _frozen_setattrs(self, name, value):
@@ -918,18 +909,11 @@ def _create_slots_class(self):
                 names += (name,)
                 # Clear out function from class to avoid clashing.
                 del cd[name]
-                additional_closure_functions_to_update.append(func)
                 annotation = inspect.signature(func).return_annotation
                 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)
-
-            cd["__getattr__"] = _make_cached_property_getattr(
-                cached_properties, original_getattr, self._cls
-            )
+                additional_closure_functions_to_update.append(func)
 
         # We only add the names of attributes that aren't inherited.
         # Setting __slots__ to inherited attributes wastes memory.
@@ -956,6 +940,14 @@ def _create_slots_class(self):
         # Create new class based on old class and our methods.
         cls = type(self._cls)(self._cls.__name__, self._cls.__bases__, cd)
 
+        # Now add back the wrapped cached properties
+        for name, func in cached_properties.items():
+            slot = getattr(cls, name)
+            if isinstance(slot, _SlottedCachedProperty):
+                slot = slot.slot
+            slotted_property = _SlottedCachedProperty(slot, func)
+            setattr(cls, name, slotted_property)
+
         # The following is a fix for
         # <https://github.com/python-attrs/attrs/issues/102>.
         # If a method mentions `__class__` or uses the no-arg super(), the

tests/test_slots.py

@@ -885,6 +885,178 @@ def __getattr__(self, item):
     assert a.z == "z"
 
 
+def test_slots_cached_property_retains_doc():
+    """
+    Cached property's docstring is retained
+
+    See: https://github.com/python-attrs/attrs/issues/1325
+    """
+
+    @attr.s(slots=True)
+    class A:
+        x = attr.ib()
+
+        @functools.cached_property
+        def f(self):
+            """
+            This is a docstring.
+            """
+            return self.x
+
+    assert "This is a docstring." in A.f.__doc__
+
+
+def test_slots_cached_property_super_works():
+    """
+    Calling super() with a cached property should correctly call from the parent
+
+    See: https://github.com/python-attrs/attrs/issues/1333
+    """
+
+    @attr.s(slots=True)
+    class Parent:
+        @functools.cached_property
+        def name(self) -> str:
+            return "Alice"
+
+    @attr.s(slots=True)
+    class Child(Parent):
+        @functools.cached_property
+        def name(self) -> str:
+            return f"Bob (son of {super().name})"
+
+    p = Parent()
+    c = Child()
+
+    assert p.name == "Alice"
+    assert c.name == "Bob (son of Alice)"
+
+
+def test_slots_cached_property_skips_child_getattr():
+    """
+    __getattr__ on child should not interfere with cached_properties
+
+    See: https://github.com/python-attrs/attrs/issues/1288
+    """
+
+    @attrs.define
+    class Bob:
+        @functools.cached_property
+        def howdy(self):
+            return 3
+
+    class Sup(Bob):
+        def __getattr__(self, name):
+            raise AttributeError(name)
+
+    b = Sup()
+
+    assert b.howdy == 3
+
+
+def test_slots_cached_property_direct():
+    """
+    Test getting the wrapped cached property directly
+    """
+    from attr._make import _SlottedCachedProperty
+
+    @attr.s(slots=True)
+    class Parent:
+        @functools.cached_property
+        def name(self) -> str:
+            return "Alice"
+
+    assert isinstance(Parent.name, _SlottedCachedProperty)
+
+
+def test_slots_cached_property_set_delete():
+    """
+    Set and Delete should work on the descriptor as on a regular
+    cached property
+    """
+
+    @attr.s(slots=True)
+    class Parent:
+        @functools.cached_property
+        def name(self) -> str:
+            return "Alice"
+
+    p = Parent()
+    p.name = "Bob"
+    assert p.name == "Bob"
+    del p.name
+    assert p.name == "Alice"
+
+
+@pytest.mark.parametrize(
+    "slotted",
+    [
+        pytest.param(
+            True,
+            marks=pytest.mark.xfail(
+                reason="field names are not checked for cached properties"
+            ),
+        ),
+        False,
+    ],
+)
+def test_cached_property_overriding_field(slotted):
+    """
+    Discrepancy in cached property overriding behaviour
+
+    attrs only considers fields that are not fields to become
+    cached properties.
+
+    c.name is not converted to a cached property
+    """
+
+    @attrs.define(slots=slotted)
+    class Parent:
+        name: str = "Alice"
+
+    @attrs.define(slots=slotted)
+    class Child(Parent):
+        @functools.cached_property
+        def name(self):
+            return "Bob"
+
+    # This isn't to imply that this is good
+    # just that it's consistent
+    p = Parent()
+    c = Child()
+
+    assert p.name == "Alice"
+    assert c.name == "Alice"
+    del c.name
+    assert c.name == "Bob"  # Errors under slots
+
+
+@pytest.mark.parametrize("slotted", [True, False])
+def test_field_overriding_cached_property(slotted):
+    """
+    Check that overriding a cached property with a field
+    works the same slotted or unslotted
+    """
+
+    @attrs.define(slots=slotted)
+    class Parent:
+        @functools.cached_property
+        def name(self):
+            return "Alice"
+
+    @attrs.define(slots=slotted)
+    class Child(Parent):
+        name: str = "Bob"
+
+    p = Parent()
+    c = Child()
+
+    assert p.name == "Alice"
+    assert c.name == "Bob"
+    del c.name
+    assert c.name == "Alice"
+
+
 def test_slots_getattr_in_superclass__is_called_for_missing_attributes_when_cached_property_present():
     """
     Ensure __getattr__ implementation is maintained in subclass.