Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clear __setattr__ if it was inherited && written by us #663

Merged
merged 1 commit into from
Aug 13, 2020
Merged

Clear __setattr__ if it was inherited && written by us #663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 57 additions & 7 deletions src/attr/_make.py
View file
Open in desktop
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
Open in desktop
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