Make Converter a kind of adapter, fix converters.pipe (#1328)

* Make Converter a kind of adapter, fix converters.pipe

* Fix import cycle on 3.7/8

* stray space

* Create static __call__ on Converter instantiation

* Add tests for adapters doing passing correct args

* Add news fragment

Author: hynek

changelog.d/1328.change.md

@@ -0,0 +1 @@
+`attrs.converters.pipe()` (and it's syntactic sugar of passing a list for `attrs.field()`'s / `attr.ib()`'s *converter* argument) works again when passing `attrs.setters.convert` to *on_setattr* (which is default for `attrs.define`).

src/attr/_make.py

@@ -2709,6 +2709,25 @@ def __init__(self, converter, *, takes_self=False, takes_field=False):
             converter
         ).get_first_param_type()
 
+        self.__call__ = {
+            (False, False): self._takes_only_value,
+            (True, False): self._takes_instance,
+            (False, True): self._takes_field,
+            (True, True): self._takes_both,
+        }[self.takes_self, self.takes_field]
+
+    def _takes_only_value(self, value, instance, field):
+        return self.converter(value)
+
+    def _takes_instance(self, value, instance, field):
+        return self.converter(value, instance)
+
+    def _takes_field(self, value, instance, field):
+        return self.converter(value, field)
+
+    def _takes_both(self, value, instance, field):
+        return self.converter(value, instance, field)
+
     @staticmethod
     def _get_global_name(attr_name: str) -> str:
         """
@@ -2915,18 +2934,7 @@ def pipe(*converters):
 
     def pipe_converter(val, inst, field):
         for c in converters:
-            if isinstance(c, Converter):
-                val = c.converter(
-                    val,
-                    *{
-                        (False, False): (),
-                        (True, False): (c.takes_self,),
-                        (False, True): (c.takes_field,),
-                        (True, True): (c.takes_self, c.takes_field),
-                    }[c.takes_self, c.takes_field],
-                )
-            else:
-                val = c(val)
+            val = c(val, inst, field) if isinstance(c, Converter) else c(val)
 
         return val
 

src/attr/setters.py

@@ -4,7 +4,6 @@
 Commonly used hooks for on_setattr.
 """
 
-
 from . import _config
 from .exceptions import FrozenAttributeError
 
@@ -56,14 +55,20 @@ def validate(instance, attrib, new_value):
 
 def convert(instance, attrib, new_value):
     """
-    Run *attrib*'s converter -- if it has one --  on *new_value* and return the
+    Run *attrib*'s converter -- if it has one -- on *new_value* and return the
     result.
 
     .. versionadded:: 20.1.0
     """
     c = attrib.converter
     if c:
-        return c(new_value)
+        # This can be removed once we drop 3.8 and use attrs.Converter instead.
+        from ._make import Converter
+
+        if not isinstance(c, Converter):
+            return c(new_value)
+
+        return c(new_value, instance, attrib)
 
     return new_value
 

tests/test_converters.py

@@ -28,6 +28,7 @@ def test_pickle(self, takes_self, takes_field):
         assert c == new_c
         assert takes_self == new_c.takes_self
         assert takes_field == new_c.takes_field
+        assert c.__call__.__name__ == new_c.__call__.__name__
 
     @pytest.mark.parametrize(
         "scenario",
@@ -58,6 +59,38 @@ def test_fmt_converter_call(self, scenario):
 
         assert expect == c._fmt_converter_call("le_name", "le_value")
 
+    def test_works_as_adapter(self):
+        """
+        Converter instances work as adapters and pass the correct arguments to
+        the wrapped converter callable.
+        """
+        taken = None
+        instance = object()
+        field = object()
+
+        def save_args(*args):
+            nonlocal taken
+            taken = args
+            return args[0]
+
+        Converter(save_args)(42, instance, field)
+
+        assert (42,) == taken
+
+        Converter(save_args, takes_self=True)(42, instance, field)
+
+        assert (42, instance) == taken
+
+        Converter(save_args, takes_field=True)(42, instance, field)
+
+        assert (42, field) == taken
+
+        Converter(save_args, takes_self=True, takes_field=True)(
+            42, instance, field
+        )
+
+        assert (42, instance, field) == taken
+
 
 class TestOptional:
     """

tests/test_setattr.py

@@ -109,16 +109,34 @@ def test_pipe(self):
         used. They can be supplied using the pipe functions or by passing a
         list to on_setattr.
         """
+        taken = None
+
+        def takes_all(val, instance, attrib):
+            nonlocal taken
+            taken = val, instance, attrib
+
+            return val
 
         s = [setters.convert, lambda _, __, nv: nv + 1]
 
         @attr.s
         class Piped:
-            x1 = attr.ib(converter=int, on_setattr=setters.pipe(*s))
+            x1 = attr.ib(
+                converter=[
+                    attr.Converter(
+                        takes_all, takes_field=True, takes_self=True
+                    ),
+                    int,
+                ],
+                on_setattr=setters.pipe(*s),
+            )
             x2 = attr.ib(converter=int, on_setattr=s)
 
         p = Piped("41", "22")
 
+        assert ("41", p) == taken[:-1]
+        assert "x1" == taken[-1].name
+
         assert 41 == p.x1
         assert 22 == p.x2
 
@@ -417,3 +435,19 @@ def test_docstring(self):
             "Method generated by attrs for class WithOnSetAttrHook."
             == WithOnSetAttrHook.__setattr__.__doc__
         )
+
+    def test_setattr_converter_piped(self):
+        """
+        If a converter is used, it is piped through the on_setattr hooks.
+
+        Regression test for https://github.com/python-attrs/attrs/issues/1327
+        """
+
+        @attr.define  # converter on setattr is implied in NG
+        class C:
+            x = attr.field(converter=[int])
+
+        c = C("1")
+        c.x = "2"
+
+        assert 2 == c.x