Skip to content

Commit

Permalink
Clear __setattr__ if it was inherited && written by us
Browse files Browse the repository at this point in the history 
Signed-off-by: Hynek Schlawack <hs@ox.cx>
  • Loading branch information
@hynek
hynek committed Aug 4, 2020
1 parent e554373 commit 17191e4
Show file tree
Hide file tree
Showing 4 changed files with 414 additions and 192 deletions.
64 changes: 57 additions & 7 deletions src/attr/_make.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ class _ClassBuilder(object):
"_on_setattr",
"_slots",
"_weakref_slot",
"_has_own_setattr",
)

def __init__(
Expand All @@ -573,6 +574,7 @@ def __init__(
is_exc,
collect_by_mro,
on_setattr,
has_custom_setattr,
):
attrs, base_attrs, base_map = _transform_attrs(
cls, these, auto_attribs, kw_only, collect_by_mro,
Expand All @@ -585,20 +587,24 @@ def __init__(
self._base_attr_map = base_map
self._attr_names = tuple(a.name for a in attrs)
self._slots = slots
self._frozen = frozen or _has_frozen_base_class(cls)
self._frozen = frozen
self._weakref_slot = weakref_slot
self._cache_hash = cache_hash
self._has_post_init = bool(getattr(cls, "__attrs_post_init__", False))
self._delete_attribs = not bool(these)
self._is_exc = is_exc
self._on_setattr = on_setattr

self._has_own_setattr = has_custom_setattr

self._cls_dict["__attrs_attrs__"] = self._attrs

if frozen:
self._cls_dict["__setattr__"] = _frozen_setattrs
self._cls_dict["__delattr__"] = _frozen_delattrs

self._has_own_setattr = True

if getstate_setstate:
(
self._cls_dict["__getstate__"],
Expand Down Expand Up @@ -645,6 +651,13 @@ def _patch_original_class(self):
for name, value in self._cls_dict.items():
setattr(cls, name, value)

# If we've inherited an attrs __setattr__ and don't write our own,
# reset it to object's.
if not self._has_own_setattr and getattr(
cls, "__attrs_own_setattr__", False
):
cls.__setattr__ = object.__setattr__

return cls

def _create_slots_class(self):
Expand All @@ -658,14 +671,24 @@ 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
weakref_inherited = False

# Traverse the MRO to check for an existing __weakref__.
for base_cls in self._cls.__mro__[1:-1]:
if "__weakref__" in getattr(base_cls, "__dict__", ()):
weakref_inherited = True
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:
break

if not self._has_own_setattr and not custom_setattr_inherited:
cd["__setattr__"] = object.__setattr__

names = self._attr_names
if (
self._weakref_slot
Expand Down Expand Up @@ -836,7 +859,20 @@ def add_setattr(self):
if not sa_attrs:
return self

if self._has_own_setattr:
# We need to write a __setattr__ but there already is one!
raise ValueError(
"Can't combine custom __setattr__ with on_setattr hooks."
)

cls = self._cls

def __setattr__(self, name, val):
"""
Method generated by attrs for class %s.
""" % (
cls.__name__,
)
try:
a, hook = sa_attrs[name]
except KeyError:
Expand All @@ -846,7 +882,9 @@ def __setattr__(self, name, val):

_obj_setattr(self, name, nval)

self._cls_dict["__attrs_own_setattr__"] = True
self._cls_dict["__setattr__"] = self._add_method_dunders(__setattr__)
self._has_own_setattr = True

return self

Expand Down Expand Up @@ -1076,6 +1114,8 @@ def attrs(
circumvent that limitation by using
``object.__setattr__(self, "attribute_name", value)``.
5. Subclasses of a frozen class are frozen too.
:param bool weakref_slot: Make instances weak-referenceable. This has no
effect unless ``slots`` is also enabled.
:param bool auto_attribs: If ``True``, collect `PEP 526`_-annotated
Expand Down Expand Up @@ -1200,13 +1240,20 @@ def wrap(cls):
if getattr(cls, "__class__", None) is None:
raise TypeError("attrs only works with new-style classes.")

is_frozen = frozen or _has_frozen_base_class(cls)
is_exc = auto_exc is True and issubclass(cls, BaseException)
has_own_setattr = auto_detect and _has_own_attribute(
cls, "__setattr__"
)

if has_own_setattr and is_frozen:
raise ValueError("Can't freeze a class with a custom __setattr__.")

builder = _ClassBuilder(
cls,
these,
slots,
frozen,
is_frozen,
weakref_slot,
_determine_whether_to_implement(
cls,
Expand All @@ -1221,6 +1268,7 @@ def wrap(cls):
is_exc,
collect_by_mro,
on_setattr,
has_own_setattr,
)
if _determine_whether_to_implement(
cls, repr, auto_detect, ("__repr__",)
Expand Down Expand Up @@ -1263,7 +1311,9 @@ def wrap(cls):
" hashing must be either explicitly or implicitly "
"enabled."
)
elif hash is True or (hash is None and eq is True and frozen is True):
elif hash is True or (
hash is None and eq is True and is_frozen is True
):
# Build a __hash__ if told so, or if it's safe.
builder.add_hash()
else:
Expand Down
192 changes: 7 additions & 185 deletions tests/test_functional.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@

import attr

from attr import setters
from attr._compat import PY2, TYPE
from attr._make import NOTHING, Attribute
from attr.exceptions import FrozenAttributeError, FrozenInstanceError
from attr.validators import instance_of, matches_re
from attr.exceptions import FrozenInstanceError

from .strategies import optional_bool

Expand Down Expand Up @@ -112,9 +110,9 @@ class WithMetaSlots(object):
FromMakeClass = attr.make_class("FromMakeClass", ["x"])


class TestDarkMagic(object):
class TestFunctional(object):
"""
Integration tests.
Functional tests.
"""

@pytest.mark.parametrize("cls", [C2, C2Slots])
Expand Down Expand Up @@ -320,6 +318,7 @@ def test_pickle_object(self, cls, protocol):
obj = cls(123, 456)
else:
obj = cls(123)

assert repr(obj) == repr(pickle.loads(pickle.dumps(obj, protocol)))

def test_subclassing_frozen_gives_frozen(self):
Expand All @@ -332,6 +331,9 @@ def test_subclassing_frozen_gives_frozen(self):
assert i.x == "foo"
assert i.y == "bar"

with pytest.raises(FrozenInstanceError):
i.x = "baz"

@pytest.mark.parametrize("cls", [WithMeta, WithMetaSlots])
def test_metaclass_preserved(self, cls):
"""
Expand Down Expand Up @@ -683,183 +685,3 @@ class C(object):
"2021-06-01. Please use `eq` and `order` instead."
== w.message.args[0]
)


class TestSetAttr(object):
def test_change(self):
"""
The return value of a hook overwrites the value. But they are not run
on __init__.
"""

def hook(*a, **kw):
return "hooked!"

@attr.s
class Hooked(object):
x = attr.ib(on_setattr=hook)
y = attr.ib()

h = Hooked("x", "y")

assert "x" == h.x
assert "y" == h.y

h.x = "xxx"
h.y = "yyy"

assert "yyy" == h.y
assert "hooked!" == h.x

def test_frozen_attribute(self):
"""
Frozen attributes raise FrozenAttributeError, others are not affected.
"""

@attr.s
class PartiallyFrozen(object):
x = attr.ib(on_setattr=setters.frozen)
y = attr.ib()

pf = PartiallyFrozen("x", "y")

pf.y = "yyy"

assert "yyy" == pf.y

with pytest.raises(FrozenAttributeError):
pf.x = "xxx"

assert "x" == pf.x

@pytest.mark.parametrize(
"on_setattr",
[setters.validate, [setters.validate], setters.pipe(setters.validate)],
)
def test_validator(self, on_setattr):
"""
Validators are run and they don't alter the value.
"""

@attr.s(on_setattr=on_setattr)
class ValidatedAttribute(object):
x = attr.ib()
y = attr.ib(validator=[instance_of(str), matches_re("foo.*qux")])

va = ValidatedAttribute(42, "foobarqux")

with pytest.raises(TypeError) as ei:
va.y = 42

assert "foobarqux" == va.y

assert ei.value.args[0].startswith("'y' must be <")

with pytest.raises(ValueError) as ei:
va.y = "quxbarfoo"

assert ei.value.args[0].startswith("'y' must match regex '")

assert "foobarqux" == va.y

va.y = "foobazqux"

assert "foobazqux" == va.y

def test_pipe(self):
"""
Multiple hooks are possible, in that case the last return value is
used. They can be supplied using the pipe functions or by passing a
list to on_setattr.
"""

s = [setters.convert, lambda _, __, nv: nv + 1]

@attr.s
class Piped(object):
x1 = attr.ib(converter=int, on_setattr=setters.pipe(*s))
x2 = attr.ib(converter=int, on_setattr=s)

p = Piped("41", "22")

assert 41 == p.x1
assert 22 == p.x2

p.x1 = "41"
p.x2 = "22"

assert 42 == p.x1
assert 23 == p.x2

def test_make_class(self):
"""
on_setattr of make_class gets forwarded.
"""
C = attr.make_class("C", {"x": attr.ib()}, on_setattr=setters.frozen)

c = C(1)

with pytest.raises(FrozenAttributeError):
c.x = 2

def test_no_validator_no_converter(self):
"""
validate and convert tolerate missing validators and converters.
"""

@attr.s(on_setattr=[setters.convert, setters.validate])
class C(object):
x = attr.ib()

c = C(1)

c.x = 2

def test_validate_respects_run_validators_config(self):
"""
If run validators is off, validate doesn't run them.
"""

@attr.s(on_setattr=setters.validate)
class C(object):
x = attr.ib(validator=attr.validators.instance_of(int))

c = C(1)

attr.set_run_validators(False)

c.x = "1"

assert "1" == c.x

attr.set_run_validators(True)

with pytest.raises(TypeError) as ei:
c.x = "1"

assert ei.value.args[0].startswith("'x' must be <")

def test_frozen_on_setattr_class_is_caught(self):
"""
@attr.s(on_setattr=X, frozen=True) raises an ValueError.
"""
with pytest.raises(ValueError) as ei:

@attr.s(frozen=True, on_setattr=setters.validate)
class C(object):
x = attr.ib()

assert "Frozen classes can't use on_setattr." == ei.value.args[0]

def test_frozen_on_setattr_attribute_is_caught(self):
"""
attr.ib(on_setattr=X) on a frozen class raises an ValueError.
"""

with pytest.raises(ValueError) as ei:

@attr.s(frozen=True)
class C(object):
x = attr.ib(on_setattr=setters.validate)

assert "Frozen classes can't use on_setattr." == ei.value.args[0]
Loading

0 comments on commit 17191e4

Please sign in to comment.