Optimize the case of on_setattr=validate & no validators (#817)

* Optimize the case of on_setattr=validate & no validators

This is important because define/mutable have on_setattr=setters.validate on
default.

Fixes #816

Signed-off-by: Hynek Schlawack <[email protected]>

* Grammar

Author: hynek

changelog.d/817.change.rst

@@ -0,0 +1 @@
+If the class-level *on_setattr* is set to ``attr.setters.validate`` (default in ``@attr.define`` and ``@attr.mutable``) but no field defines a validator, pretend that it's not set.

src/attr/_make.py

@@ -654,7 +654,7 @@ class _ClassBuilder(object):
         "_on_setattr",
         "_slots",
         "_weakref_slot",
-        "_has_own_setattr",
+        "_wrote_own_setattr",
         "_has_custom_setattr",
     )
 
@@ -701,15 +701,23 @@ def __init__(
         self._on_setattr = on_setattr
 
         self._has_custom_setattr = has_custom_setattr
-        self._has_own_setattr = False
+        self._wrote_own_setattr = False
 
         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
+            self._wrote_own_setattr = True
+        elif on_setattr == setters.validate:
+            for a in attrs:
+                if a.validator is not None:
+                    break
+            else:
+                # If class-level on_setattr is set to validating, but there's
+                # no field to validate, pretend like there's no on_setattr.
+                self._on_setattr = None
 
         if getstate_setstate:
             (
@@ -759,7 +767,7 @@ def _patch_original_class(self):
 
         # 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(
+        if not self._wrote_own_setattr and getattr(
             cls, "__attrs_own_setattr__", False
         ):
             cls.__attrs_own_setattr__ = False
@@ -787,7 +795,7 @@ def _create_slots_class(self):
         # 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.
-        if not self._has_own_setattr:
+        if not self._wrote_own_setattr:
             cd["__attrs_own_setattr__"] = False
 
             if not self._has_custom_setattr:
@@ -958,8 +966,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,
             )
         )
@@ -978,8 +985,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,
             )
         )
@@ -1038,7 +1044,7 @@ def __setattr__(self, name, val):
 
         self._cls_dict["__attrs_own_setattr__"] = True
         self._cls_dict["__setattr__"] = self._add_method_dunders(__setattr__)
-        self._has_own_setattr = True
+        self._wrote_own_setattr = True
 
         return self
 
@@ -2008,10 +2014,14 @@ def _make_init(
     cache_hash,
     base_attr_map,
     is_exc,
-    has_global_on_setattr,
+    cls_on_setattr,
     attrs_init,
 ):
-    if frozen and has_global_on_setattr:
+    has_cls_on_setattr = (
+        cls_on_setattr is not None and cls_on_setattr is not setters.NO_OP
+    )
+
+    if frozen and has_cls_on_setattr:
         raise ValueError("Frozen classes can't use on_setattr.")
 
     needs_cached_setattr = cache_hash or frozen
@@ -2030,7 +2040,7 @@ def _make_init(
 
             needs_cached_setattr = True
         elif (
-            has_global_on_setattr and a.on_setattr is not setters.NO_OP
+            has_cls_on_setattr and a.on_setattr is not setters.NO_OP
         ) or _is_slot_attr(a.name, base_attr_map):
             needs_cached_setattr = True
 
@@ -2046,7 +2056,7 @@ def _make_init(
         base_attr_map,
         is_exc,
         needs_cached_setattr,
-        has_global_on_setattr,
+        has_cls_on_setattr,
         attrs_init,
     )
     if cls.__module__ in sys.modules:
@@ -2183,7 +2193,7 @@ def _attrs_to_init_script(
     base_attr_map,
     is_exc,
     needs_cached_setattr,
-    has_global_on_setattr,
+    has_cls_on_setattr,
     attrs_init,
 ):
     """
@@ -2257,7 +2267,7 @@ def fmt_setter_with_converter(
 
         attr_name = a.name
         has_on_setattr = a.on_setattr is not None or (
-            a.on_setattr is not setters.NO_OP and has_global_on_setattr
+            a.on_setattr is not setters.NO_OP and has_cls_on_setattr
         )
         arg_name = a.name.lstrip("_")
 

tests/test_dunders.py

@@ -94,7 +94,7 @@ def _add_init(cls, frozen):
         cache_hash=False,
         base_attr_map={},
         is_exc=False,
-        has_global_on_setattr=False,
+        cls_on_setattr=None,
         attrs_init=False,
     )
     return cls

tests/test_functional.py

@@ -4,6 +4,7 @@
 
 from __future__ import absolute_import, division, print_function
 
+import inspect
 import pickle
 
 from copy import deepcopy
@@ -687,3 +688,48 @@ 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
+        assert object.__setattr__ == D.__setattr__
+
+    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
+        assert object.__setattr__ != D.__setattr__