Tin/more-typeddict-coverage (#492)

* TypedDict coverage

* Improve typeddicts coverage

* Remove dead code

* TypedDict fix

* Remove dead code

Author: Tinche

src/cattrs/_compat.py

@@ -412,19 +412,22 @@ def is_counter(type):
             or getattr(type, "__origin__", None) is Counter
         )
 
-    def is_generic(obj) -> bool:
-        """Whether obj is a generic type."""
+    def is_generic(type) -> bool:
+        """Whether `type` is a generic type."""
         # Inheriting from protocol will inject `Generic` into the MRO
         # without `__orig_bases__`.
-        return isinstance(obj, (_GenericAlias, GenericAlias)) or (
-            is_subclass(obj, Generic) and hasattr(obj, "__orig_bases__")
+        return isinstance(type, (_GenericAlias, GenericAlias)) or (
+            is_subclass(type, Generic) and hasattr(type, "__orig_bases__")
         )
 
     def copy_with(type, args):
         """Replace a generic type's arguments."""
         if is_annotated(type):
             # typing.Annotated requires a special case.
             return Annotated[args]
+        if isinstance(args, tuple) and len(args) == 1:
+            # Some annotations can't handle 1-tuples.
+            args = args[0]
         return type.__origin__[args]
 
     def get_full_type_hints(obj, globalns=None, localns=None):

src/cattrs/gen/_generics.py

@@ -6,7 +6,8 @@
 
 
 def generate_mapping(cl: type, old_mapping: dict[str, type] = {}) -> dict[str, type]:
-    mapping = {}
+    """Generate a mapping of typevars to actual types for a generic class."""
+    mapping = dict(old_mapping)
 
     origin = get_origin(cl)
 
@@ -25,8 +26,6 @@ def generate_mapping(cl: type, old_mapping: dict[str, type] = {}) -> dict[str, t
                 continue
             mapping[p.__name__] = t
 
-        if not mapping:
-            return dict(old_mapping)
     elif is_generic(cl):
         # Origin is None, so this may be a subclass of a generic class.
         orig_bases = cl.__orig_bases__

src/cattrs/gen/typeddicts.py

@@ -53,7 +53,7 @@ def get_annots(cl) -> dict[str, Any]:
 if TYPE_CHECKING:  # pragma: no cover
     from typing_extensions import Literal
 
-    from cattr.converters import BaseConverter
+    from ..converters import BaseConverter
 
 __all__ = ["make_dict_unstructure_fn", "make_dict_structure_fn"]
 
@@ -209,7 +209,7 @@ def make_dict_unstructure_fn(
                 # No default or no override.
                 lines.append(f"  res['{kn}'] = {invoke}")
             else:
-                lines.append(f"  if '{kn}' in instance: res['{kn}'] = {invoke}")
+                lines.append(f"  if '{attr_name}' in instance: res['{kn}'] = {invoke}")
 
         internal_arg_line = ", ".join([f"{i}={i}" for i in internal_arg_parts])
         if internal_arg_line:
@@ -340,9 +340,7 @@ def make_dict_structure_fn(
             if nrb is not NOTHING:
                 t = nrb
 
-            if isinstance(t, TypeVar):
-                t = mapping.get(t.__name__, t)
-            elif is_generic(t) and not is_bare(t) and not is_annotated(t):
+            if is_generic(t) and not is_bare(t) and not is_annotated(t):
                 t = deep_copy_with(t, mapping)
 
             # For each attribute, we try resolving the type here and now.
@@ -554,8 +552,12 @@ def _required_keys(cls: type) -> set[str]:
 elif sys.version_info >= (3, 9):
     from typing_extensions import Annotated, NotRequired, Required, get_args
 
+    # Note that there is no `typing.Required` on 3.9 and 3.10, only in
+    # `typing_extensions`. Therefore, `typing.TypedDict` will not honor this
+    # annotation, only `typing_extensions.TypedDict`.
+
     def _required_keys(cls: type) -> set[str]:
-        """Own own processor for required keys."""
+        """Our own processor for required keys."""
         if _is_extensions_typeddict(cls):
             return cls.__required_keys__
 
@@ -564,6 +566,9 @@ def _required_keys(cls: type) -> set[str]:
         own_annotations = cls.__dict__.get("__annotations__", {})
         required_keys = set()
         for base in cls.__mro__[1:]:
+            if base in (object, dict):
+                # These have no required keys for sure.
+                continue
             required_keys |= _required_keys(base)
         for key in getattr(cls, "__required_keys__", []):
             annotation_type = own_annotations[key]
@@ -574,9 +579,7 @@ def _required_keys(cls: type) -> set[str]:
                     annotation_type = annotation_args[0]
                     annotation_origin = get_origin(annotation_type)
 
-            if annotation_origin is Required:
-                required_keys.add(key)
-            elif annotation_origin is NotRequired:
+            if annotation_origin is NotRequired:
                 pass
             elif cls.__total__:
                 required_keys.add(key)
@@ -588,7 +591,7 @@ def _required_keys(cls: type) -> set[str]:
     # On 3.8, typing.TypedDicts do not have __required_keys__.
 
     def _required_keys(cls: type) -> set[str]:
-        """Own own processor for required keys."""
+        """Our own processor for required keys."""
         if _is_extensions_typeddict(cls):
             return cls.__required_keys__
 

tests/test_typeddicts.py

@@ -1,15 +1,16 @@
 """Tests for TypedDict un/structuring."""
 from datetime import datetime, timezone
-from typing import Dict, Generic, Set, Tuple, TypedDict, TypeVar
+from typing import Dict, Generic, NewType, Set, Tuple, TypedDict, TypeVar
 
 import pytest
+from attrs import NOTHING
 from hypothesis import assume, given
 from hypothesis.strategies import booleans
 from pytest import raises
 from typing_extensions import NotRequired
 
 from cattrs import BaseConverter, Converter
-from cattrs._compat import ExtensionsTypedDict, is_generic
+from cattrs._compat import ExtensionsTypedDict, get_notrequired_base, is_generic
 from cattrs.errors import (
     ClassValidationError,
     ForbiddenExtraKeysError,
@@ -51,17 +52,27 @@ def get_annot(t) -> dict:
             args = t.__args__
             params = origin.__parameters__
             param_to_args = dict(zip(params, args))
-            return {
-                k: param_to_args[v] if v in param_to_args else v
-                for k, v in origin_annotations.items()
-            }
+            res = {}
+            for k, v in origin_annotations.items():
+                if (nrb := get_notrequired_base(v)) is not NOTHING:
+                    res[k] = (
+                        NotRequired[param_to_args[nrb]] if nrb in param_to_args else v
+                    )
+                else:
+                    res[k] = param_to_args[v] if v in param_to_args else v
+            return res
 
         # Origin is `None`, so this is a subclass for a generic typeddict.
         mapping = generate_mapping(t)
-        return {
-            k: mapping[v.__name__] if v.__name__ in mapping else v
-            for k, v in get_annots(t).items()
-        }
+        res = {}
+        for k, v in get_annots(t).items():
+            if (nrb := get_notrequired_base(v)) is not NOTHING:
+                res[k] = (
+                    NotRequired[mapping[nrb.__name__]] if nrb.__name__ in mapping else v
+                )
+            else:
+                res[k] = mapping[v.__name__] if v.__name__ in mapping else v
+        return res
     return get_annots(t)
 
 
@@ -196,6 +207,27 @@ class GenericTypedDict(TypedDict, Generic[T]):
         c.structure({"a": 1}, GenericTypedDict)
 
 
+@pytest.mark.skipif(not is_py311_plus, reason="3.11+ only")
+@given(detailed_validation=...)
+def test_deep_generics(detailed_validation: bool):
+    c = mk_converter(detailed_validation=detailed_validation)
+
+    Int = NewType("Int", int)
+
+    c.register_unstructure_hook_func(lambda t: t is Int, lambda v: v - 1)
+
+    T = TypeVar("T")
+    T1 = TypeVar("T1")
+
+    class GenericParent(TypedDict, Generic[T]):
+        a: T
+
+    class GenericChild(GenericParent[Int], Generic[T1]):
+        b: T1
+
+    assert c.unstructure({"b": 2, "a": 2}, GenericChild[Int]) == {"a": 1, "b": 1}
+
+
 @given(simple_typeddicts(total=True, not_required=True), booleans())
 def test_not_required(
     cls_and_instance: Tuple[type, Dict], detailed_validation: bool
@@ -273,36 +305,25 @@ def test_omit(cls_and_instance: Tuple[type, Dict], detailed_validation: bool) ->
     assert restructured == instance
 
 
-@given(simple_typeddicts(min_attrs=1, total=True), booleans())
+@given(simple_typeddicts(min_attrs=1, total=True, not_required=True), booleans())
 def test_rename(cls_and_instance: Tuple[type, Dict], detailed_validation: bool) -> None:
     """`override(rename=...)` works."""
     c = mk_converter(detailed_validation=detailed_validation)
 
     cls, instance = cls_and_instance
     key = next(iter(get_annot(cls)))
     c.register_unstructure_hook(
-        cls,
-        make_dict_unstructure_fn(
-            cls,
-            c,
-            _cattrs_detailed_validation=detailed_validation,
-            **{key: override(rename="renamed")},
-        ),
+        cls, make_dict_unstructure_fn(cls, c, **{key: override(rename="renamed")})
     )
 
     unstructured = c.unstructure(instance, unstructure_as=cls)
 
     assert key not in unstructured
-    assert "renamed" in unstructured
+    if key in instance:
+        assert "renamed" in unstructured
 
     c.register_structure_hook(
-        cls,
-        make_dict_structure_fn(
-            cls,
-            c,
-            _cattrs_detailed_validation=detailed_validation,
-            **{key: override(rename="renamed")},
-        ),
+        cls, make_dict_structure_fn(cls, c, **{key: override(rename="renamed")})
     )
     restructured = c.structure(unstructured, cls)
 

tests/typeddicts.py

@@ -1,9 +1,10 @@
 """Strategies for typed dicts."""
 from datetime import datetime, timezone
 from string import ascii_lowercase
-from typing import Any, Dict, Generic, List, Optional, Set, Tuple, TypeVar
+from typing import Any, Dict, Generic, List, Optional, Set, Tuple, Type, TypeVar
 
 from attrs import NOTHING
+from hypothesis import note
 from hypothesis.strategies import (
     DrawFn,
     SearchStrategy,
@@ -49,7 +50,7 @@ def gen_typeddict_attr_names():
 @composite
 def int_attributes(
     draw: DrawFn, total: bool = True, not_required: bool = False
-) -> Tuple[int, SearchStrategy, SearchStrategy]:
+) -> Tuple[Type[int], SearchStrategy, SearchStrategy]:
     if total:
         if not_required and draw(booleans()):
             return (NotRequired[int], integers() | just(NOTHING), text(ascii_lowercase))
@@ -176,6 +177,15 @@ def simple_typeddicts(
         else typeddict_cls
     )("HypTypedDict", attrs_dict, total=total)
 
+    note(
+        "\n".join(
+            [
+                "class HypTypedDict(TypedDict):",
+                *[f"    {n}: {a}" for n, a in attrs_dict.items()],
+            ]
+        )
+    )
+
     if draw(booleans()):
 
         class InheritedTypedDict(cls):
@@ -240,9 +250,8 @@ def generic_typeddicts(
             generics.append(typevar)
             if total and draw(booleans()):
                 # We might decide to make these NotRequired
-                actual_types.append(NotRequired[attr_type])
-            else:
-                actual_types.append(attr_type)
+                typevar = NotRequired[typevar]
+            actual_types.append(attr_type)
             attrs_dict[attr_name] = typevar
 
     cls = make_typeddict(
@@ -282,6 +291,14 @@ def make_typeddict(
         lines.append(f"  {n}: _{trimmed}_type")
 
     script = "\n".join(lines)
+
+    note_lines = script
+    for n, t in globs.items():
+        if n == "TypedDict":
+            continue
+        note_lines = note_lines.replace(n, repr(t))
+    note(note_lines)
+
     eval(compile(script, "name", "exec"), globs)
 
     return globs[cls_name]