Skip to content

Commit

Permalink
Optimize the case of on_setattr=validate & no validators
Browse files Browse the repository at this point in the history
This is important because define/mutable have on_setattr=setters.validate on
default.

Fixes #816
  • Loading branch information
hynek committed May 16, 2021
1 parent 8ae2d6f commit 097a99f
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 6 deletions.
29 changes: 24 additions & 5 deletions src/attr/_make.py
Expand Up @@ -958,8 +958,7 @@ def add_init(self):
self._cache_hash,
self._base_attr_map,
self._is_exc,
self._on_setattr is not None
and self._on_setattr is not setters.NO_OP,
self._on_setattr,
attrs_init=False,
)
)
Expand All @@ -978,8 +977,7 @@ def add_attrs_init(self):
self._cache_hash,
self._base_attr_map,
self._is_exc,
self._on_setattr is not None
and self._on_setattr is not setters.NO_OP,
self._on_setattr,
attrs_init=True,
)
)
Expand Down Expand Up @@ -2008,13 +2006,19 @@ def _make_init(
cache_hash,
base_attr_map,
is_exc,
has_global_on_setattr,
global_on_setattr,
attrs_init,
):
has_global_on_setattr = (
global_on_setattr is not None
and global_on_setattr is not setters.NO_OP
)

if frozen and has_global_on_setattr:
raise ValueError("Frozen classes can't use on_setattr.")

needs_cached_setattr = cache_hash or frozen
has_validator = False
filtered_attrs = []
attr_dict = {}
for a in attrs:
Expand All @@ -2023,6 +2027,7 @@ def _make_init(

filtered_attrs.append(a)
attr_dict[a.name] = a
has_validator = has_validator or a.validator is not None

if a.on_setattr is not None:
if frozen is True:
Expand All @@ -2034,6 +2039,20 @@ def _make_init(
) or _is_slot_attr(a.name, base_attr_map):
needs_cached_setattr = True

# The default in define/mutable is a global on_setattr=setters.validate,
# however it's quite likely, that no attribute has actually a validator.
# Therefore, in the one case where on_setattr is validate and NO attribute
# has a validator defined, we pretend as if no class-level on_setattr is
# set.
if (
not frozen
and not has_validator
and global_on_setattr == setters.validate
):
needs_cached_setattr = False
has_global_on_setattr = False
global_on_setattr = setters.NO_OP

unique_filename = _generate_unique_filename(cls, "init")

script, globs, annotations = _attrs_to_init_script(
Expand Down
2 changes: 1 addition & 1 deletion tests/test_dunders.py
Expand Up @@ -94,7 +94,7 @@ def _add_init(cls, frozen):
cache_hash=False,
base_attr_map={},
is_exc=False,
has_global_on_setattr=False,
global_on_setattr=None,
attrs_init=False,
)
return cls
Expand Down
44 changes: 44 additions & 0 deletions tests/test_functional.py
Expand Up @@ -4,6 +4,7 @@

from __future__ import absolute_import, division, print_function

import inspect
import pickle

from copy import deepcopy
Expand Down Expand Up @@ -687,3 +688,46 @@ class C(object):
"2021-06-01. Please use `eq` and `order` instead."
== w.message.args[0]
)

@pytest.mark.parametrize("slots", [True, False])
def test_no_setattr_if_validate_without_validators(self, slots):
"""
If a class has on_setattr=attr.setters.validate (default in NG APIs)
but sets no validators, don't use the (slower) setattr in __init__.
Regression test for #816.
"""

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

@attr.s(on_setattr=attr.setters.validate)
class D(C):
y = attr.ib()

src = inspect.getsource(D.__init__)

assert "setattr" not in src
assert "self.x = x" in src
assert "self.y = y" in src

def test_on_setattr_detect_inherited_validators(self):
"""
_make_init detects the presence of a validator even if the field is
inherited.
"""

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

@attr.s(on_setattr=attr.setters.validate)
class D(C):
y = attr.ib()

src = inspect.getsource(D.__init__)

assert "_setattr = _cached_setattr" in src
assert "_setattr('x', x)" in src
assert "_setattr('y', y)" in src

0 comments on commit 097a99f

Please sign in to comment.