From 9575bcf0c5b796caedc42898640f732ba6e22510 Mon Sep 17 00:00:00 2001 From: Sam McKelvie Date: Sun, 26 Jul 2026 14:52:14 -0700 Subject: [PATCH] Fix CatchAll fields breaking on reuse across structural contexts load_func_for_dataclass()/dump_func_for_dataclass() popped the CATCH_ALL marker off of the per-class alias dict returned by resolve_dataclass_field_to_alias_for_load()/_for_dump(). That dict is a direct reference into a WeakKeyDictionary cached once per class, not a copy, so the first codegen pass for a class permanently stripped the marker from the shared cache. Any later codegen pass for the same class in a different structural context (e.g. used as a nested field after being used as a top-level from_dict()/to_dict() target, or nested in a second, different enclosing class) then saw has_catch_all as False, let the field's CatchAll (a typing.NewType, not a class) fall through to the normal dispatch path, and hit issubclass(origin, t) with a non-class origin, raising TypeError. Use .get() instead of .pop() so the cached dict is only ever read, never mutated, fixing reuse in any number of structural contexts including the self-referential case (which hits two conflicting contexts in a single top-level call). --- dataclass_wizard/_dumpers.py | 8 +++++- dataclass_wizard/_loaders.py | 8 +++++- tests/unit/test_loaders.py | 48 ++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/dataclass_wizard/_dumpers.py b/dataclass_wizard/_dumpers.py index 68933a5a..d523a802 100644 --- a/dataclass_wizard/_dumpers.py +++ b/dataclass_wizard/_dumpers.py @@ -931,7 +931,13 @@ def dump_func_for_dataclass( skip_defaults = True if meta.skip_defaults else False skip_if = True if field_to_skip_if or skip_if_condition else False - catch_all_name: str | None = field_to_alias.pop(CATCH_ALL, None) + # Note: use `.get()` rather than `.pop()` here -- `field_to_alias` is a + # direct reference to a dict cached per-class (not a copy), so popping + # from it would permanently strip the `CATCH_ALL` marker from the shared + # cache the first time this runs, breaking any later codegen pass for + # this same class in a different structural context (e.g. nested vs. + # top-level). See: reuse-across-contexts regression test. + catch_all_name: str | None = field_to_alias.get(CATCH_ALL) has_catch_all = catch_all_name is not None if has_catch_all: diff --git a/dataclass_wizard/_loaders.py b/dataclass_wizard/_loaders.py index 996977a5..1d6b0573 100644 --- a/dataclass_wizard/_loaders.py +++ b/dataclass_wizard/_loaders.py @@ -1240,7 +1240,13 @@ def load_func_for_dataclass( on_unknown_key = meta.on_unknown_key - catch_all_field: str | None = field_to_aliases.pop(CATCH_ALL, None) + # Note: use `.get()` rather than `.pop()` here -- `field_to_aliases` is a + # direct reference to a dict cached per-class (not a copy), so popping + # from it would permanently strip the `CATCH_ALL` marker from the shared + # cache the first time this runs, breaking any later codegen pass for + # this same class in a different structural context (e.g. nested vs. + # top-level). See: reuse-across-contexts regression test. + catch_all_field: str | None = field_to_aliases.get(CATCH_ALL) has_catch_all = catch_all_field is not None if has_catch_all: diff --git a/tests/unit/test_loaders.py b/tests/unit/test_loaders.py index 2f5c1744..b36ded96 100644 --- a/tests/unit/test_loaders.py +++ b/tests/unit/test_loaders.py @@ -2806,6 +2806,54 @@ class _(JSONWizard.Meta): assert opt == Options(my_extras={}, the_email='x@y.com') +def test_catch_all_reused_across_structural_contexts(): + """ + A `CatchAll`-bearing dataclass must load/dump correctly regardless of how + many different "structural contexts" it's used in across a process: as + the direct top-level target of `from_dict`/`to_dict`, and as a field + nested inside one or more different enclosing classes. + + Regression test for a bug where `load_func_for_dataclass` / + `dump_func_for_dataclass` popped the `CATCH_ALL` marker off of the + per-class alias dict returned by + `resolve_dataclass_field_to_alias_for_load` / + `resolve_dataclass_field_to_alias_for_dump` -- since that dict is a + direct (not copied) reference to a cache shared across all codegen + passes for the class, the first pass would permanently strip the + marker, and any later codegen pass for the same class (e.g. nested + inside a different enclosing class) would then treat it as having no + `CatchAll` field at all, causing the field's `NewType` annotation to + reach `issubclass()` and raise `TypeError`. + """ + @dataclass + class Leaf(JSONWizard): + name: str + extra_data: CatchAll = field(default_factory=dict) + + @dataclass + class WrapperA(JSONWizard): + value: Leaf + + @dataclass + class WrapperB(JSONWizard): + other: Leaf + + # First use: direct top-level target. + leaf = Leaf.from_dict({'name': 'a', 'unrecognized': 'x'}) + assert leaf == Leaf(name='a', extra_data={'unrecognized': 'x'}) + assert leaf.to_dict() == {'name': 'a', 'unrecognized': 'x'} + + # Second use: nested inside a first enclosing class. + wa = WrapperA.from_dict({'value': {'name': 'b', 'unrecognized': 'y'}}) + assert wa == WrapperA(value=Leaf(name='b', extra_data={'unrecognized': 'y'})) + assert wa.to_dict() == {'value': {'name': 'b', 'unrecognized': 'y'}} + + # Third use: nested inside a second, different enclosing class. + wb = WrapperB.from_dict({'other': {'name': 'c', 'unrecognized': 'z'}}) + assert wb == WrapperB(other=Leaf(name='c', extra_data={'unrecognized': 'z'})) + assert wb.to_dict() == {'other': {'name': 'c', 'unrecognized': 'z'}} + + def test_from_dict_with_nested_object_alias_path(): """ Specifying a custom mapping of "nested" alias to dataclass field,