From 097a99f0e2322f597c12de1e8973877e450f7415 Mon Sep 17 00:00:00 2001
From: Hynek Schlawack <hs@ox.cx>
Date: Sun, 16 May 2021 12:06:26 +0200
Subject: [PATCH] Optimize the case of on_setattr=validate & no validators

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

Fixes #816
---
 src/attr/_make.py        | 29 +++++++++++++++++++++-----
 tests/test_dunders.py    |  2 +-
 tests/test_functional.py | 44 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/src/attr/_make.py b/src/attr/_make.py
index ceb08f910..38d3b6588 100644
--- a/src/attr/_make.py
+++ b/src/attr/_make.py
@@ -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,
             )
         )
@@ -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,
             )
         )
@@ -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:
@@ -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:
@@ -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(
diff --git a/tests/test_dunders.py b/tests/test_dunders.py
index 70f26d044..b12ea0597 100644
--- a/tests/test_dunders.py
+++ b/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,
+        global_on_setattr=None,
         attrs_init=False,
     )
     return cls
diff --git a/tests/test_functional.py b/tests/test_functional.py
index a17023ffc..93e3ad3f2 100644
--- a/tests/test_functional.py
+++ b/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,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