Even better hook factories (#495)

* Even better hook factories

* Improve hook factory types

* Test deque validation, for coverage

* Enum coverage

* Fix lint

HISTORY.md

@@ -21,7 +21,7 @@ Our backwards-compatibility policy can be found [here](https://github.com/python
   ([#432](https://github.com/python-attrs/cattrs/issues/432) [#472](https://github.com/python-attrs/cattrs/pull/472))
 - {meth}`BaseConverter.register_structure_hook`, {meth}`BaseConverter.register_unstructure_hook`,
 {meth}`BaseConverter.register_unstructure_hook_factory` and {meth}`BaseConverter.register_structure_hook_factory`
-can now be used as decorators and have gained new features when used this way.
+can now be used as decorators and have gained new features.
   See [here](https://catt.rs/en/latest/customizing.html#use-as-decorators) and [here](https://catt.rs/en/latest/customizing.html#id1) for more details.
   ([#487](https://github.com/python-attrs/cattrs/pull/487))
 - Introduce the [_msgspec_](https://jcristharif.com/msgspec/) {mod}`preconf converter <cattrs.preconf.msgspec>`.

docs/customizing.md

@@ -116,15 +116,15 @@ Traceback (most recent call last):
 cattrs.errors.ForbiddenExtraKeysError: Extra fields in constructor for E: else
 ```
 
+Hook factories can receive the current converter by exposing an additional required parameter.
+
 A complex use case for hook factories is described over at [](usage.md#using-factory-hooks).
 
 #### Use as Decorators
 
 {meth}`register_unstructure_hook_factory() <cattrs.BaseConverter.register_unstructure_hook_factory>` and
 {meth}`register_structure_hook_factory() <cattrs.BaseConverter.register_structure_hook_factory>` can also be used as decorators.
 
-When registered via decorators, hook factories can receive the current converter by exposing an additional required parameter.
-
 Here's an example of using an unstructure hook factory to handle unstructuring [queues](https://docs.python.org/3/library/queue.html#queue.Queue).
 
 ```{doctest}
@@ -158,7 +158,9 @@ Here's an example of using an unstructure hook factory to handle unstructuring [
 ## Using `cattrs.gen` Generators
 
 The {mod}`cattrs.gen` module allows for generating and compiling specialized hooks for unstructuring _attrs_ classes, dataclasses and typed dicts.
-The default {class}`Converter <cattrs.Converter>`, upon first encountering one of these types, will use the generation functions mentioned here to generate specialized hooks for it, register the hooks and use them.
+The default {class}`Converter <cattrs.Converter>`, upon first encountering one of these types,
+will use the generation functions mentioned here to generate specialized hooks for it,
+register the hooks and use them.
 
 One reason for generating these hooks in advance is that they can bypass a lot of _cattrs_ machinery and be significantly faster than normal _cattrs_.
 The hooks are also good building blocks for more complex customizations.

src/cattrs/converters.py

@@ -123,6 +123,17 @@ def is_literal_containing_enums(typ: type) -> bool:
     return is_literal(typ) and any(isinstance(val, Enum) for val in typ.__args__)
 
 
+def _is_extended_factory(factory: Callable) -> bool:
+    """Does this factory also accept a converter arg?"""
+    # We use the original `inspect.signature` to not evaluate string
+    # annotations.
+    sig = inspect_signature(factory)
+    return (
+        len(sig.parameters) >= 2
+        and (list(sig.parameters.values())[1]).default is Signature.empty
+    )
+
+
 class BaseConverter:
     """Converts between structured and unstructured data."""
 
@@ -344,41 +355,36 @@ def register_unstructure_hook_factory(
     ) -> UnstructureHookFactory:
         ...
 
+    @overload
     def register_unstructure_hook_factory(
-        self,
-        predicate: Callable[[Any], bool],
-        factory: UnstructureHookFactory | None = None,
-    ) -> (
-        Callable[[UnstructureHookFactory], UnstructureHookFactory]
-        | UnstructureHookFactory
-    ):
+        self, predicate: Callable[[Any], bool], factory: ExtendedUnstructureHookFactory
+    ) -> ExtendedUnstructureHookFactory:
+        ...
+
+    def register_unstructure_hook_factory(self, predicate, factory=None):
         """
         Register a hook factory for a given predicate.
 
-        May also be used as a decorator. When used as a decorator, the hook
-        factory may expose an additional required parameter. In this case,
+        The hook factory may expose an additional required parameter. In this case,
         the current converter will be provided to the hook factory as that
         parameter.
 
+        May also be used as a decorator.
+
         :param predicate: A function that, given a type, returns whether the factory
             can produce a hook for that type.
         :param factory: A callable that, given a type, produces an unstructuring
             hook for that type. This unstructuring hook will be cached.
 
         .. versionchanged:: 24.1.0
             This method may now be used as a decorator.
+            The factory may also receive the converter as a second, required argument.
         """
         if factory is None:
 
             def decorator(factory):
                 # Is this an extended factory (takes a converter too)?
-                # We use the original `inspect.signature` to not evaluate string
-                # annotations.
-                sig = inspect_signature(factory)
-                if (
-                    len(sig.parameters) >= 2
-                    and (list(sig.parameters.values())[1]).default is Signature.empty
-                ):
+                if _is_extended_factory(factory):
                     self._unstructure_func.register_func_list(
                         [(predicate, factory, "extended")]
                     )
@@ -388,7 +394,16 @@ def decorator(factory):
                     )
 
             return decorator
-        self._unstructure_func.register_func_list([(predicate, factory, True)])
+
+        self._unstructure_func.register_func_list(
+            [
+                (
+                    predicate,
+                    factory,
+                    "extended" if _is_extended_factory(factory) else True,
+                )
+            ]
+        )
         return factory
 
     def get_unstructure_hook(
@@ -483,36 +498,36 @@ def register_structure_hook_factory(
     ) -> StructureHookFactory:
         ...
 
+    @overload
     def register_structure_hook_factory(
-        self,
-        predicate: Callable[[Any], bool],
-        factory: HookFactory[StructureHook] | None = None,
-    ) -> Callable[[StructureHookFactory, StructureHookFactory]] | StructureHookFactory:
+        self, predicate: Callable[[Any], bool], factory: ExtendedStructureHookFactory
+    ) -> ExtendedStructureHookFactory:
+        ...
+
+    def register_structure_hook_factory(self, predicate, factory=None):
         """
         Register a hook factory for a given predicate.
 
-        May also be used as a decorator. When used as a decorator, the hook
-        factory may expose an additional required parameter. In this case,
+        The hook factory may expose an additional required parameter. In this case,
         the current converter will be provided to the hook factory as that
         parameter.
 
+        May also be used as a decorator.
+
         :param predicate: A function that, given a type, returns whether the factory
             can produce a hook for that type.
         :param factory: A callable that, given a type, produces a structuring
             hook for that type. This structuring hook will be cached.
 
         .. versionchanged:: 24.1.0
             This method may now be used as a decorator.
+            The factory may also receive the converter as a second, required argument.
         """
         if factory is None:
             # Decorator use.
             def decorator(factory):
                 # Is this an extended factory (takes a converter too)?
-                sig = signature(factory)
-                if (
-                    len(sig.parameters) >= 2
-                    and (list(sig.parameters.values())[1]).default is Signature.empty
-                ):
+                if _is_extended_factory(factory):
                     self._structure_func.register_func_list(
                         [(predicate, factory, "extended")]
                     )
@@ -522,7 +537,15 @@ def decorator(factory):
                     )
 
             return decorator
-        self._structure_func.register_func_list([(predicate, factory, True)])
+        self._structure_func.register_func_list(
+            [
+                (
+                    predicate,
+                    factory,
+                    "extended" if _is_extended_factory(factory) else True,
+                )
+            ]
+        )
         return factory
 
     def structure(self, obj: UnstructuredValue, cl: type[T]) -> T:

tests/test_converter.py

@@ -856,3 +856,67 @@ def my_structure_hook_factory(type: Any, converter: BaseConverter) -> StructureH
         return lambda v, _: Test(int_handler(v[0]))
 
     assert converter.structure((2,), Test) == Test(1)
+
+
+def test_hook_factories_with_converters(converter: BaseConverter):
+    """Hook factories with converters work."""
+
+    @define
+    class Test:
+        a: int
+
+    converter.register_unstructure_hook(int, lambda v: v + 1)
+
+    def my_hook_factory(type: Any, converter: BaseConverter) -> UnstructureHook:
+        int_handler = converter.get_unstructure_hook(int)
+        return lambda v: (int_handler(v.a),)
+
+    converter.register_unstructure_hook_factory(has, my_hook_factory)
+
+    assert converter.unstructure(Test(1)) == (2,)
+
+    converter.register_structure_hook(int, lambda v: v - 1)
+
+    def my_structure_hook_factory(type: Any, converter: BaseConverter) -> StructureHook:
+        int_handler = converter.get_structure_hook(int)
+        return lambda v, _: Test(int_handler(v[0]))
+
+    converter.register_structure_hook_factory(has, my_structure_hook_factory)
+
+    assert converter.structure((2,), Test) == Test(1)
+
+
+def test_hook_factories_with_converter_methods(converter: BaseConverter):
+    """What if the hook factories are methods (have `self`)?"""
+
+    @define
+    class Test:
+        a: int
+
+    converter.register_unstructure_hook(int, lambda v: v + 1)
+
+    class Converters:
+        @classmethod
+        def my_hook_factory(
+            cls, type: Any, converter: BaseConverter
+        ) -> UnstructureHook:
+            int_handler = converter.get_unstructure_hook(int)
+            return lambda v: (int_handler(v.a),)
+
+        def my_structure_hook_factory(
+            self, type: Any, converter: BaseConverter
+        ) -> StructureHook:
+            int_handler = converter.get_structure_hook(int)
+            return lambda v, _: Test(int_handler(v[0]))
+
+    converter.register_unstructure_hook_factory(has, Converters.my_hook_factory)
+
+    assert converter.unstructure(Test(1)) == (2,)
+
+    converter.register_structure_hook(int, lambda v: v - 1)
+
+    converter.register_structure_hook_factory(
+        has, Converters().my_structure_hook_factory
+    )
+
+    assert converter.structure((2,), Test) == Test(1)

tests/test_enums.py

@@ -0,0 +1,30 @@
+"""Tests for enums."""
+from hypothesis import given
+from hypothesis.strategies import data, sampled_from
+from pytest import raises
+
+from cattrs import BaseConverter
+from cattrs._compat import Literal
+
+from .untyped import enums_of_primitives
+
+
+@given(data(), enums_of_primitives())
+def test_structuring_enums(data, enum):
+    """Test structuring enums by their values."""
+    converter = BaseConverter()
+    val = data.draw(sampled_from(list(enum)))
+
+    assert converter.structure(val.value, enum) == val
+
+
+@given(enums_of_primitives())
+def test_enum_failure(enum):
+    """Structuring literals with enums fails properly."""
+    converter = BaseConverter()
+    type = Literal[next(iter(enum))]
+
+    with raises(Exception) as exc_info:
+        converter.structure("", type)
+
+    assert exc_info.value.args[0] == f" not in literal {type!r}"

tests/test_structure.py

@@ -1,7 +1,7 @@
 """Test structuring of collections and primitives."""
 from typing import Any, Dict, FrozenSet, List, MutableSet, Optional, Set, Tuple, Union
 
-import attr
+from attrs import define
 from hypothesis import assume, given
 from hypothesis.strategies import (
     binary,
@@ -27,7 +27,6 @@
 from .untyped import (
     deque_seqs_of_primitives,
     dicts_of_primitives,
-    enums_of_primitives,
     lists_of_primitives,
     primitive_strategies,
     seqs_of_primitives,
@@ -325,15 +324,6 @@ class Bar:
     assert exc.value.type_ is Bar
 
 
-@given(data(), enums_of_primitives())
-def test_structuring_enums(data, enum):
-    """Test structuring enums by their values."""
-    converter = BaseConverter()
-    val = data.draw(sampled_from(list(enum)))
-
-    assert converter.structure(val.value, enum) == val
-
-
 def test_structuring_unsupported():
     """Loading unsupported classes should throw."""
     converter = BaseConverter()
@@ -373,12 +363,12 @@ class Bar(Foo):
 def test_structure_union_edge_case():
     converter = BaseConverter()
 
-    @attr.s(auto_attribs=True)
+    @define
     class A:
         a1: Any
         a2: Optional[Any] = None
 
-    @attr.s(auto_attribs=True)
+    @define
     class B:
         b1: Any
         b2: Optional[Any] = None

tests/test_validation.py

@@ -1,6 +1,6 @@
 """Tests for the extended validation mode."""
 import pickle
-from typing import Dict, FrozenSet, List, Set, Tuple
+from typing import Deque, Dict, FrozenSet, List, Set, Tuple
 
 import pytest
 from attrs import define, field
@@ -82,6 +82,28 @@ def test_list_validation():
     ]
 
 
+def test_deque_validation():
+    """Proper validation errors are raised structuring deques."""
+    c = Converter(detailed_validation=True)
+
+    with pytest.raises(IterableValidationError) as exc:
+        c.structure(["1", 2, "a", 3.0, "c"], Deque[int])
+
+    assert repr(exc.value.exceptions[0]) == repr(
+        ValueError("invalid literal for int() with base 10: 'a'")
+    )
+    assert exc.value.exceptions[0].__notes__ == [
+        "Structuring typing.Deque[int] @ index 2"
+    ]
+
+    assert repr(exc.value.exceptions[1]) == repr(
+        ValueError("invalid literal for int() with base 10: 'c'")
+    )
+    assert exc.value.exceptions[1].__notes__ == [
+        "Structuring typing.Deque[int] @ index 4"
+    ]
+
+
 @given(...)
 def test_mapping_validation(detailed_validation: bool):
     """Proper validation errors are raised structuring mappings."""

tests/untyped.py

@@ -38,7 +38,7 @@
 
 
 @st.composite
-def enums_of_primitives(draw):
+def enums_of_primitives(draw: st.DrawFn) -> Enum:
     """Generate enum classes with primitive values."""
     names = draw(
         st.sets(st.text(min_size=1).filter(lambda s: not s.endswith("_")), min_size=1)