fix(corekit): stop EnumRegistry.get's default from minting

pcapkit/corekit/enum.py

@@ -264,15 +264,25 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self':
         returns the member the alias points at, not a separate object -- so
         two names for one assignment resolve to one enum.
 
-        It never mints -- see #864 for the ``default`` path, which does and
-        should not. Registering a member is :meth:`register`'s job and
-        nobody else's, which is the ruling #775 exists to carry out: *"so that
-        we dont create registered enums out of unrecognised/unregistered
-        values, unless user/caller explicitly created them"*. A value inside a
-        registry's declared-but-unassigned range still resolves, through that
-        registry's own ``_missing_`` and :meth:`_unregistered_member`, to a
-        member that is deliberately absent from the lookup tables -- for a
-        non-``str`` key; the ``str`` case is qualified below.
+        It never mints while resolving ``default``; ``key`` may still mint
+        through a ``_missing_`` that GitHub issue #775's ruling deliberately
+        kept minting, on three registries (``EtherType``, ``Socket``,
+        ``CGAType``). Registering a member any other way is
+        :meth:`register`'s job and nobody else's, which is the ruling #775
+        exists to carry out: *"so that we dont create registered enums out
+        of unrecognised/unregistered values, unless user/caller explicitly
+        created them"*. A value inside a registry's declared-but-unassigned
+        range still resolves, through that registry's own ``_missing_`` and
+        :meth:`_unregistered_member`, to a member that is deliberately
+        absent from the lookup tables -- true outside the three registries
+        named above, where such a value instead lands in *both* tables,
+        exactly as :meth:`register` would leave it -- for a non-``str`` key;
+        the ``str`` case is qualified below. Both describe ``key`` resolution
+        only. ``default`` never reaches ``_missing_`` on either branch: a
+        declared-but-unassigned ``default`` does not resolve to an
+        unregistered member the way such a ``key`` does -- it simply does
+        not resolve, and the lookup error ``key`` itself would have raised
+        propagates instead.
 
         For a ``str`` key, a name match wins over a value match -- the two are
         checked in that order, so a string that happens to be both a member's
@@ -286,9 +296,11 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self':
         mint a permanent member where it previously just raised. Restricting
         the value side of ``key`` to an already-registered value keeps *that
         side* non-minting on every ``str``-valued registry, not only the ones
-        without a minting ``_missing_`` -- it is a claim about the value side
-        alone, not about this method as a whole: an unregistered ``default``
-        still reaches ``cls(default)`` below, and can mint there just the same.
+        without a minting ``_missing_``. Since #864, that is no longer merely
+        a claim about the value side alone: ``default`` resolves through the
+        same kind of ``_value2member_map_`` lookup rather than
+        ``cls(default)``, so for a ``str`` key every path through this
+        method -- name, value and ``default`` alike -- is non-minting.
 
         That restriction has a cost the paragraph above glosses over: a
         *declared-but-unassigned* value -- the case resolved there through
@@ -304,9 +316,13 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self':
 
         Args:
             key: Name or value to look up.
-            default: Value to fall back to when ``key`` does not resolve.
-                :data:`NO_DEFAULT` stands for *no default*, in which case the
-                lookup error propagates instead.
+            default: An already-registered value to fall back to when
+                ``key`` does not resolve. Resolved through a plain
+                ``_value2member_map_`` lookup, never through
+                ``cls(default)``, so it cannot mint -- see #864.
+                :data:`NO_DEFAULT` stands for *no default*; that and a
+                ``default`` naming no registered member both fall through to
+                the same lookup error ``key`` itself would have raised.
 
         Returns:
             The canonical member for ``key``, or for ``default``.
@@ -324,15 +340,15 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self':
             except KeyError:
                 if key in cls._value2member_map_:
                     return cls._value2member_map_[key]
-                if default is NO_DEFAULT:
+                if default is NO_DEFAULT or default not in cls._value2member_map_:
                     raise
-                return cls(default)  # type: ignore[call-arg]
+                return cls._value2member_map_[default]
         try:
             return cls(key)  # type: ignore[call-arg]
         except ValueError:
-            if default is NO_DEFAULT:
+            if default is NO_DEFAULT or default not in cls._value2member_map_:
                 raise
-            return cls(default)  # type: ignore[call-arg]
+            return cls._value2member_map_[default]
 
     @classmethod
     def get_all(cls, key: 'Any') -> 'tuple[Self, ...]':

tests/const/test_const_enum_builtin_parity.py

@@ -676,6 +676,23 @@ def test_the_guard_leaves_the_other_lookup_paths_alone(self) -> None:
         and the integer path reaches it inside a ``try``. Both are exercised here
         for the four modules GitHub issue #647 touched, so a guard that had
         broken either would fail rather than merely go unmeasured.
+
+        This used to also probe the fallback with ``Flags.get(UNRESOLVABLE,
+        0)``, expecting it to resolve -- ``0`` is in-bounds for ``Flags``'s
+        own ``_missing_`` (``0 <= value <= 0xFFFF``), so before GitHub issue
+        #864 it reached ``cls(0)`` and resolved via the converted pseudo-member
+        path, uncaught. It is not a *registered* ``Flags`` member, though
+        (``Flags``'s smallest declared value is ``1 << 4``), so under #864's
+        restriction of ``default`` to a plain ``_value2member_map_`` lookup it
+        no longer resolves at all -- the same accepted cost as an unassigned
+        integer range, here on an :class:`~aenum.IntFlag` registry instead.
+        That is why the prior version of this assertion errored rather than
+        failed: the ``ValueError`` from the *key* (``UNRESOLVABLE``) now
+        propagates uncaught through a bare ``assertEqual`` that expected
+        success. Replaced with the same two-part check used elsewhere in this
+        batch: a *registered* default (``1 << 14``, ``SYN``) is still
+        genuinely consulted and resolves, while the unregistered ``0`` now
+        raises the same original-key error as no default at all.
         """
         from pcapkit.const.ftp.command import Command
         from pcapkit.const.http.method import Method
@@ -694,7 +711,14 @@ def test_the_guard_leaves_the_other_lookup_paths_alone(self) -> None:
 
         # And the fallback the guard's ValueError is what triggers: GitHub issue
         # #584's ``get(key, default)``, which an EnumError would have walked past.
-        self.assertEqual(int(Flags.get(UNRESOLVABLE, 0)), 0)
+        # A registered default is still genuinely consulted and resolves.
+        self.assertIs(Flags.get(UNRESOLVABLE, 1 << 14), Flags.SYN)
+        # Since #864: ``0`` is not a registered ``Flags`` member, so it no
+        # longer resolves either -- the original key's error propagates, the
+        # same as omitting the default outright.
+        with self.assertRaises(ValueError) as caught:
+            Flags.get(UNRESOLVABLE, 0)
+        self.assertIn(str(UNRESOLVABLE), str(caught.exception))
         with self.assertRaises(ValueError):
             Flags.get(UNRESOLVABLE)
 

tests/const/test_const_enum_get.py

@@ -120,7 +120,7 @@
 #: value at all that ``obj(fallback)`` would resolve to a *cached* identity
 #: for. Excused from the main sweep for that reason and covered by its own
 #: test instead, :meth:`ConstEnumGetDefaultTests.
-#: test_filter_type_default_is_consulted_without_a_cached_fallback`.
+#: test_filter_type_default_never_resolves_but_key_path_still_uses_an_uncached_member`.
 EXPECTED_WITHOUT_A_CACHEABLE_FALLBACK = frozenset({
     'pcapkit.const.pcapng.filter_type.FilterType',
 })
@@ -191,18 +191,27 @@ def test_the_reported_case_returns_the_default(self) -> None:
         from pcapkit.const.arp.hardware import Hardware
 
         # Before the fix this raised ValueError('99999 is not a valid Hardware'),
-        # dropping the caller's default entirely.
+        # dropping the caller's default entirely. Both ``0`` (``Reserved_0``)
+        # and ``1`` (``Ethernet``) are genuinely registered members, so #864
+        # leaves this half of the repro untouched: a registered default still
+        # resolves, exactly as #584 asked for.
         self.assertIs(Hardware.get(99999, 0), Hardware(0))
         self.assertIs(Hardware.get(99999, 1), Hardware.Ethernet)
 
-        # The default is genuinely consulted rather than merely swallowing the
-        # error: a default that is itself unresolvable is now what fails, and
-        # the message names it rather than the original key. #584 passed a
-        # ``str`` where the signature says ``int``, and that is still invalid.
+        # GitHub issue #864 changes what an *unresolvable* default does,
+        # though: #584 passed a ``str`` where the signature says ``int``,
+        # and ``'X'`` is not a registered value either way -- before #864
+        # that failed with ``cls('X')``'s own error, naming ``'X'``; #864's
+        # ruling ("get should not mint unless it falls through the
+        # _missing_'s minted ranges") replaces ``cls(default)`` with a plain
+        # ``_value2member_map_`` lookup for exactly this reason, so an
+        # unresolvable default no longer gets an attempt of its own to fail
+        # from -- it now falls through to the *original* key lookup's own
+        # error instead, same as if no default had been supplied at all.
         with self.assertRaises(ValueError) as caught:
             Hardware.get(99999, 'X')
-        self.assertIn('X', str(caught.exception))
-        self.assertNotIn('99999', str(caught.exception))
+        self.assertIn('99999', str(caught.exception))
+        self.assertNotIn('X', str(caught.exception))
 
         # A resolvable key is untouched.
         self.assertIs(Hardware.get(1), Hardware.Ethernet)
@@ -216,16 +225,35 @@ def test_the_reported_case_returns_the_default(self) -> None:
     def test_omitting_the_default_still_raises_for_the_original_key(self) -> None:
         """GitHub issue #857: ``-1`` used to be compared with ``==`` against
         :data:`~pcapkit.corekit.enum.NO_DEFAULT`, so a caller-supplied ``-1``
-        was silently read as *no default was supplied* -- "explicitly passing
-        the placeholder is the same as omitting it", as this test used to
-        assert. :data:`~pcapkit.corekit.enum.NO_DEFAULT` is now an exported
-        instance of the dedicated :class:`~pcapkit.corekit.enum.NoDefaultType`
-        compared with ``is``, so ``-1`` is a genuine default like any other:
-        omitting ``default`` entirely still raises for the *original* key
-        (unchanged, pinned below), while explicitly passing ``-1`` now raises
-        for the *attempted fallback* ``cls(-1)`` instead -- no longer the same
-        error, since every registry's domain here starts at ``0`` and ``-1``
-        is never a legitimate value.
+        was silently read as *no default was supplied*.
+        :data:`~pcapkit.corekit.enum.NO_DEFAULT` is now an exported instance
+        of the dedicated :class:`~pcapkit.corekit.enum.NoDefaultType`,
+        compared with ``is`` rather than ``==`` -- so a caller-supplied
+        ``-1`` is never confused with the sentinel. That fact is pinned
+        directly, independent of ``get()``, by
+        :class:`~tests.const.test_const_registry_protocol.NoDefaultSentinelTests
+        .test_no_default_is_not_equal_to_any_plausible_caller_value`.
+
+        Before GitHub issue #864, that distinction was *also* observable
+        through ``get()`` itself: a supplied-but-unresolvable ``-1`` failed
+        with its own name (``cls(-1)`` raising directly), differently from
+        the omitted case's original-key error. #864's ruling ("get should
+        not mint unless it falls through the _missing_'s minted ranges")
+        removes that particular observation for an *unregistered* default
+        specifically: ``default`` no longer reaches ``cls(default)`` at
+        all, and ``-1`` is not a registered value on either registry here
+        (both domains start at ``0``), so supplying it now converges on
+        exactly the *same* original-key error as omitting it outright,
+        rather than a distinguishable one of its own. The test's own title
+        is, if anything, more true after #864 than before: *omitting* the
+        default raises for the original key, and now so does supplying an
+        unregistered one -- pinned below for both.
+
+        The genuinely-consulted half of #857's claim is repinned with a
+        *registered* default instead (``0``, ``Reserved_0`` on both
+        registries), which still resolves rather than raising -- the
+        observable proof #864 leaves available once ``-1`` no longer
+        provides it.
         """
         from pcapkit.const.arp.hardware import Hardware
         from pcapkit.const.arp.operation import Operation
@@ -236,12 +264,18 @@ def test_omitting_the_default_still_raises_for_the_original_key(self) -> None:
                     enum.get(99999)
                 self.assertIn('99999', str(omitted.exception))
 
-                # No longer "the same as omitting it": this now names the
-                # failed fallback (``-1``), not the original key (``99999``).
+                # Since #864: an unregistered default (neither registry here
+                # has a member at ``-1``) converges on the *same*
+                # original-key error as omission, rather than a distinct one
+                # naming ``-1``.
                 with self.assertRaises(ValueError) as supplied:
                     enum.get(99999, -1)
-                self.assertIn('-1', str(supplied.exception))
-                self.assertNotIn('99999', str(supplied.exception))
+                self.assertIn('99999', str(supplied.exception))
+                self.assertNotIn('-1', str(supplied.exception))
+
+                # A *registered* default, by contrast, is still genuinely
+                # consulted rather than raising at all.
+                self.assertIs(enum.get(99999, 0), enum(0))
 
     def test_the_two_unverified_enums_from_the_issue(self) -> None:
         """#584 named ``Operation`` and ``LinkType`` but verified only ``Hardware``."""
@@ -343,20 +377,54 @@ def test_the_always_resolving_registries_have_nothing_to_fall_back_to(self) -> N
                     # every time, and the registry never grows for it.
                     self.assertIsNot(resolved, obj.get(UNRESOLVABLE, 0))
 
-    def test_filter_type_default_is_consulted_without_a_cached_fallback(self) -> None:
+    def test_filter_type_default_never_resolves_but_key_path_still_uses_an_uncached_member(self) -> None:
         """:class:`~pcapkit.const.pcapng.filter_type.FilterType`'s own
         replacement for the sweep above -- see
-        :data:`EXPECTED_WITHOUT_A_CACHEABLE_FALLBACK` for why it needs one."""
+        :data:`EXPECTED_WITHOUT_A_CACHEABLE_FALLBACK` for why it needs one.
+
+        FilterType declares *no* static members at all, so its own
+        ``_value2member_map_`` is permanently empty -- not ``0``, not
+        anything. Before GitHub issue #864, ``default=0`` still resolved
+        because ``get`` called ``cls(0)`` directly, reaching ``_missing_``
+        and its converted, uncached pseudo-member path. #864's ruling
+        closes exactly that path for ``default``: it is restricted to a
+        plain ``_value2member_map_`` lookup, and FilterType's is
+        permanently empty, so *no* default can ever resolve for this
+        registry any more -- the strongest instance of the ruling's own
+        accepted cost, since it is not one range that stops resolving but
+        the registry's entire domain.
+
+        ``key`` resolution is untouched by #864, though, and it is where
+        the uncached pseudo-member path this test was written to cover
+        still lives: ``FilterType.get(0)`` -- ``0`` as the *key*, no
+        default at all -- still reaches ``cls(0)`` -> ``_missing_``
+        directly, resolving to a fresh, equal-but-not-identical
+        pseudo-member every call, exactly as before. Restored here through
+        the path #864 does not touch, now that ``default`` can no longer
+        demonstrate it.
+        """
         from pcapkit.const.pcapng.filter_type import FilterType
 
         with self.assertRaises(ValueError):
             FilterType.get(UNRESOLVABLE)
 
-        # 0 is in-bounds (0x00-0xFF) but not a real member either -- FilterType
-        # declares none -- so this resolves via the same converted pseudo-member
-        # path as UNRESOLVABLE-with-a-default does, not via a cached identity.
+        # No default can resolve on FilterType any more -- it has no
+        # registered members at all -- so even a value that is in-bounds
+        # (0x00-0xFF) for the *key* path, like ``0``, now falls through to
+        # the *original* ``UNRESOLVABLE`` error rather than resolving.
         before = len(FilterType.__members__)
-        resolved = FilterType.get(UNRESOLVABLE, 0)
+        with self.assertRaises(ValueError) as caught:
+            FilterType.get(UNRESOLVABLE, 0)
+        self.assertIn(str(UNRESOLVABLE), str(caught.exception))
+        self.assertEqual(len(FilterType.__members__), before)
+
+        # The uncached pseudo-member path itself is unaffected: as a *key*
+        # (no default supplied), ``0`` still resolves via ``cls(0)`` ->
+        # ``_missing_``, returning a fresh, equal-but-not-identical member
+        # every time rather than a cached identity -- the strength this
+        # test was written to add over the generic sweep, now demonstrated
+        # through ``key`` instead of ``default``.
+        resolved = FilterType.get(0)
         self.assertEqual(int(resolved), 0)
         self.assertEqual(resolved, FilterType(0))
         self.assertIsNot(resolved, FilterType(0))

tests/const/test_const_enum_no_mint.py

@@ -657,15 +657,21 @@ def test_unresolvable_string_key_with_default_falls_back_by_value(self) -> None:
         self.assertEqual(before, len(Hardware.__members__))
 
     def test_unresolvable_string_key_with_default_in_unassigned_range(self) -> None:
-        """The fallback is itself a value lookup, so a default landing in a
-        bounded-unassigned range returns the same kind of pseudo-member the
-        int path does -- rather than minting a member literally named after
-        the caller's unresolved key, which is what this used to do."""
+        """The fallback is itself a value lookup (GitHub issue #864), so a
+        default landing in a bounded-unassigned range no longer returns the
+        pseudo-member the int path does -- it simply does not resolve, and
+        the original key's own error propagates instead. Before #864 this
+        resolved via ``cls(default)`` -> ``_missing_`` to an unregistered
+        pseudo-member; that is exactly the cost the owner's ruling accepts
+        ("a default naming a value with no registered member stops
+        resolving, on every registry... today such a default returns an
+        unregistered member via _missing_")."""
         from pcapkit.const.arp.hardware import Hardware
 
         before = len(Hardware.__members__)
-        result = Hardware.get('Definitely-Not-A-Member', 40)
-        self.assertEqual(result.value, 40)
+        with self.assertRaises(KeyError) as caught:
+            Hardware.get('Definitely-Not-A-Member', 40)
+        self.assertIn('Definitely-Not-A-Member', str(caught.exception))
         self.assertEqual(before, len(Hardware.__members__))
         self.assertNotIn('Definitely-Not-A-Member', Hardware.__members__)