diff --git a/changelog.d/681.change.rst b/changelog.d/681.change.rst new file mode 100644 index 000000000..a4aaa3240 --- /dev/null +++ b/changelog.d/681.change.rst @@ -0,0 +1 @@ +It's now possible to define custom ``__setattr__`` methods on slotted classes again. diff --git a/src/attr/_make.py b/src/attr/_make.py index be7cbd8bf..b753095c7 100644 --- a/src/attr/_make.py +++ b/src/attr/_make.py @@ -669,24 +669,29 @@ def _create_slots_class(self): if k not in tuple(self._attr_names) + ("__dict__", "__weakref__") } - # Traverse the MRO to check for an existing __weakref__ and - # __setattr__. - custom_setattr_inherited = False + # Check the bases if one of them has an attrs-made __setattr__ that + # needs to be reset. + # XXX: This can be confused by subclassing a slotted attrs class with + # XXX: a non-attrs class and subclass the resulting class with an attrs + # XXX: class. See `test_slotted_confused for details. For now that's + # XXX: OK with us. + for base_cls in self._cls.__bases__: + if not self._has_own_setattr and getattr( + base_cls, "__dict__", {} + ).get("__attrs_own_setattr__", False): + cd["__setattr__"] = object.__setattr__ + break + + # Traverse the MRO to check for an existing __weakref__. weakref_inherited = False for base_cls in self._cls.__mro__[1:-1]: - d = getattr(base_cls, "__dict__", {}) - - weakref_inherited = weakref_inherited or "__weakref__" in d - custom_setattr_inherited = custom_setattr_inherited or not ( - d.get("__attrs_own_setattr__", False) - ) - - if weakref_inherited and custom_setattr_inherited: + if ( + getattr(base_cls, "__dict__", {}).get("__weakref__", None) + is not None + ): + weakref_inherited = True break - if not self._has_own_setattr and not custom_setattr_inherited: - cd["__setattr__"] = object.__setattr__ - names = self._attr_names if ( self._weakref_slot @@ -697,7 +702,7 @@ def _create_slots_class(self): names += ("__weakref__",) # We only add the names of attributes that aren't inherited. - # Settings __slots__ to inherited attributes wastes memory. + # Setting __slots__ to inherited attributes wastes memory. slot_names = [name for name in names if name not in base_names] if self._cache_hash: slot_names.append(_hash_cache_field) diff --git a/tests/test_setattr.py b/tests/test_setattr.py index aebd3890b..2fc781bf0 100644 --- a/tests/test_setattr.py +++ b/tests/test_setattr.py @@ -274,6 +274,45 @@ def test_pickling_retains_attrs_own(self, slots): assert True is WOSAH.__attrs_own_setattr__ + def test_slotted_class_can_have_custom_setattr(self): + """ + A slotted class can define a custom setattr and it doesn't get + overwritten. + + Regression test for #680. + """ + + @attr.s(slots=True) + class A(object): + def __setattr__(self, key, value): + raise SystemError + + with pytest.raises(SystemError): + A().x = 1 + + @pytest.mark.xfail + def test_slotted_confused(self): + """ + If we have a in-between non-attrs class, setattr reset detection + should still work, but currently doesn't. + + It works with dict classes because we can look the finished class and + patch it. With slotted classes we have to deduce it ourselves. + """ + + @attr.s(slots=True) + class A(object): + x = attr.ib(on_setattr=setters.frozen) + + class B(A): + pass + + @attr.s(slots=True) + class C(B): + x = attr.ib() + + C(1).x = 2 + @pytest.mark.skipif(PY2, reason="Python 3-only.") class TestSetAttrNoPy2(object): diff --git a/tox.ini b/tox.ini index 1f04ec79a..0508fdaa8 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,7 @@ [pytest] addopts = -ra testpaths = tests +xfail_strict = true filterwarnings = once::Warning ignore:::pympler[.*]