Define __getattribute__() instead of __getattr__() on slotted classes…

… with cached properties

The __getattribute__() is documented as the "way to to actually get
total control over attribute access", which is really what we want. Just
changing that preserves the current behaviour, according to the test
suite, but also makes sub-classing work better, e.g. when the subclass is
not an attr-class and also defines a custom __getattr__() as evidenced
in added test.

Fix #1288

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.

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.

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():
  """