fix(const,vendor): Old Xerox ethertype range shadowed by IEEE802.3 range

pcapkit/const/reg/ethertype.py

@@ -528,13 +528,13 @@ def _missing_(cls, value: 'int') -> 'EtherType':
         """
         if not (isinstance(value, int) and 0x0000 <= value <= 0xFFFF):
             raise ValueError('%r is not a valid %s' % (value, cls.__name__))
-        if 0x0000 <= value <= 0x05DC:
-            #: IEEE802.3 Length Field [IEEE Std 802.3]
-            return extend_enum(cls, 'IEEE802_3_Length_Field_0x%s' % hex(value)[2:].upper().zfill(4), value)
         if 0x0101 <= value <= 0x01FF:
             #: Old Xerox Experimental values. Invalid as an Ethertype since 1983. [Neil
     #: Sembower]
             return cls._unregistered_member(value, 'Old_Xerox_Experimental_values_Invalid_as_an_Ethertype_since_1983')
+        if 0x0000 <= value <= 0x05DC:
+            #: IEEE802.3 Length Field [IEEE Std 802.3]
+            return extend_enum(cls, 'IEEE802_3_Length_Field_0x%s' % hex(value)[2:].upper().zfill(4), value)
         if 0x0888 <= value <= 0x088A:
             #: Xyplex [Neil Sembower]
             return extend_enum(cls, 'Xyplex_0x%s' % hex(value)[2:].upper().zfill(4), value)

pcapkit/vendor/reg/ethertype.py

@@ -79,6 +79,34 @@ def rename(self, name: 'str', code: 'str', *, original: 'Optional[str]' = None)
             name = f'{name}_0x{code}'
         return self.safe_name(name)
 
+    @staticmethod
+    def _insert_range(ranges: 'list[tuple[int, int, list[str]]]', entry: 'tuple[int, int, list[str]]') -> 'None':
+        """Insert a range block ahead of the first already-placed range that fully contains it.
+
+        The generated :meth:`~EtherType._missing_` tests these ``if`` blocks in
+        list order and returns on the first match, so a range that is fully
+        contained within an earlier, wider one would never be reached -- see
+        GitHub issue #862, where the source table lists ``0x0101-0x01FF``
+        after the ``0x0000-0x05DC`` range that wholly contains it.
+
+        The IANA CSV is not otherwise sorted by range, so this only reorders
+        the pairs where containment would otherwise shadow one of them;
+        every other pair keeps the source table's row order, appended in the
+        order it was read.
+
+        Args:
+            ranges: Range blocks already placed for emission, as
+                ``(start, stop, lines)``.
+            entry: The ``(start, stop, lines)`` tuple to place.
+
+        """
+        lo, hi, _ = entry
+        for index, (rlo, rhi, _) in enumerate(ranges):
+            if rlo <= lo and hi <= rhi:
+                ranges.insert(index, entry)
+                return
+        ranges.append(entry)
+
     def process(self, data: 'list[str]') -> 'tuple[list[str], list[str]]':
         """Process CSV data.
 
@@ -93,7 +121,7 @@ def process(self, data: 'list[str]') -> 'tuple[list[str], list[str]]':
         next(reader)  # header
 
         enum = []  # type: list[str]
-        miss = []  # type: list[str]
+        ranges = []  # type: list[tuple[int, int, list[str]]]
         for item in reader:
             name = item[4]
             rfcs = item[5]
@@ -125,12 +153,14 @@ def process(self, data: 'list[str]') -> 'tuple[list[str], list[str]]':
             except ValueError:
                 start, stop = item[1].split('-')
 
-                miss.append(f'if 0x{start} <= value <= 0x{stop}:')
-                miss.append(f'    #: {desc}')
+                block = [f'if 0x{start} <= value <= 0x{stop}:', f'    #: {desc}']
                 if name in self.UNASSIGNED_ROW_NAMES:
-                    miss.append(f"    return cls._unregistered_member(value, '{self.safe_name(name)}')")
+                    block.append(f"    return cls._unregistered_member(value, '{self.safe_name(name)}')")
                 else:
-                    miss.append(f"    return extend_enum(cls, '{self.safe_name(name)}_0x%s' % hex(value)[2:].upper().zfill(4), value)")  # pylint: disable=line-too-long
+                    block.append(f"    return extend_enum(cls, '{self.safe_name(name)}_0x%s' % hex(value)[2:].upper().zfill(4), value)")  # pylint: disable=line-too-long
+                self._insert_range(ranges, (int(start, base=16), int(stop, base=16), block))
+
+        miss = [line for _, _, block in ranges for line in block]  # type: list[str]
         return enum, miss
 
 

tests/const/test_const_enum_no_mint.py

@@ -342,23 +342,28 @@ def _first_unassigned_value(cls: 'type') -> 'int':
 #: generic "first branch that converts, by source order" probe can actually
 #: exercise -- i.e. the registry's own bounds contain at least one value that
 #: is not already a declared member, *and* nothing earlier in the same
-#: ``_missing_`` masks it. Two different reasons hold registries out of this
-#: list:
+#: ``_missing_`` masks it. One reason holds registries out of this list: 13
+#: have no reachable gap at all -- the same situation :data:`ALL_REGISTRIES`
+#: already documents for :class:`~pcapkit.const.hip.transport.Transport`:
+#: every value inside the guard's own bounds names a real member, so
+#: ``_missing_`` can never actually run for them (e.g. :class:`~pcapkit.const.
+#: ipv4.tos_del.ToSDelay` is bounded to ``0 <= value <= 1`` and both 0 and 1
+#: are declared). They are proved by :class:`RulingConversionSourceTests` (a
+#: source sweep) instead.
 #:
-#: * 13 have no reachable gap at all -- the same situation :data:`ALL_
-#:   REGISTRIES` already documents for :class:`~pcapkit.const.hip.transport.
-#:   Transport`: every value inside the guard's own bounds names a real
-#:   member, so ``_missing_`` can never actually run for them (e.g. :class:
-#:   `~pcapkit.const.ipv4.tos_del.ToSDelay` is bounded to ``0 <= value <= 1``
-#:   and both 0 and 1 are declared). They are proved by :class:
-#:   `RulingConversionSourceTests` (a source sweep) instead.
-#: * :class:`~pcapkit.const.reg.ethertype.EtherType` is excluded for a third
-#:   reason: its *first* converted branch by source order ("Old Xerox
-#:   Experimental...", 0x0101-0x01FF) is itself unreachable, masked by the
-#:   wider 0x0000-0x05DC branch before it -- see :data:`ETHERTYPE_UNASSIGNED_
-#:   PROBES`'s comment. A naive probe would silently exercise the wrong
-#:   branch, so :class:`EtherTypeMixedMintTests` covers it explicitly instead
-#:   (behaviourally for ``DEC Unassigned``, by source for the masked row).
+#: :class:`~pcapkit.const.reg.ethertype.EtherType` used to be held out for a
+#: second, distinct reason on top of those 13: its *first* converted branch by
+#: source order ("Old Xerox Experimental...", 0x0101-0x01FF) was itself
+#: unreachable, masked by the wider 0x0000-0x05DC branch before it, so a naive
+#: probe would have silently exercised the wrong branch. GitHub issue #862
+#: (fixed by #865) reordered :meth:`pcapkit.vendor.reg.ethertype.EtherType.
+#: process` so the narrower range is tested first, which makes that branch
+#: directly probeable like any other reachable-gap registry -- the same shape
+#: :class:`~pcapkit.const.ipx.socket.Socket` (also mixed) already demonstrates
+#: in this list below. EtherType has therefore joined this set; see
+#: :data:`ETHERTYPE_UNASSIGNED_PROBES`'s comment for the probe itself, still
+#: covered explicitly by :class:`EtherTypeMixedMintTests` as well since it
+#: remains a mixed registry.
 #:
 #: Derived the same way as :data:`RULING_CONVERTED_REGISTRIES`: computed once
 #: by walking each candidate's own bounds for a gap, not hand-picked.
@@ -426,6 +431,7 @@ def _first_unassigned_value(cls: 'type') -> 'int':
     ('pcapkit.const.pcapng.record_type', 'RecordType'),
     ('pcapkit.const.pcapng.secrets_type', 'SecretsType'),
     ('pcapkit.const.pcapng.verdict_type', 'VerdictType'),
+    ('pcapkit.const.reg.ethertype', 'EtherType'),
     ('pcapkit.const.reg.linktype', 'LinkType'),
     ('pcapkit.const.reg.transtype', 'TransType'),
     ('pcapkit.const.tcp.checksum', 'Checksum'),
@@ -441,18 +447,26 @@ def _first_unassigned_value(cls: 'type') -> 'int':
 #: stays a mint; :data:`ETHERTYPE_KEPT_PROBE` pins one (``Xyplex``) as a
 #: regression guard.
 #:
-#: "Old Xerox Experimental" (0x0101-0x01FF) is not behaviourally probeable:
-#: it is a strict subset of the earlier, wider "IEEE802.3 Length Field" branch
-#: (0x0000-0x05DC), which the ``_missing_`` if-chain matches first and so
-#: masks it completely -- the same shape of pre-existing ordering bug GitHub
+#: "Old Xerox Experimental" (0x0101-0x01FF) used to not be behaviourally
+#: probeable: it was a strict subset of the earlier, wider "IEEE802.3 Length
+#: Field" branch (0x0000-0x05DC), which the ``_missing_`` if-chain matched
+#: first and so masked it completely -- the same shape of ordering bug GitHub
 #: issue #841 found in :mod:`pcapkit.const.ipx.socket`, present on ``main``
-#: before this change and not part of #775/#847's ruling, so it is left as a
-#: follow-up rather than reordered here. :class:`EtherTypeMixedMintTests`
-#: proves that row by source instead of by calling it.
+#: before this change and not part of #775/#847's ruling, so it was left as a
+#: follow-up rather than reordered as part of that ruling. GitHub issue #862
+#: is that follow-up, fixed by #865: :meth:`pcapkit.vendor.reg.ethertype.
+#: EtherType.process` now tests the narrower Old Xerox range before the wider
+#: IEEE802.3 one, so 0x0101 resolves to Old Xerox and is directly probeable --
+#: it is included below alongside ``DEC Unassigned``, giving it the same
+#: behavioural no-mint proof :class:`EtherTypeMixedMintTests` already gives
+#: DEC Unassigned, rather than the weaker source-text check that stood in for
+#: it before #865 (an order-insensitive string search that would keep passing
+#: even if the ordering regressed).
+ETHERTYPE_OLD_XEROX_LABEL = 'Old_Xerox_Experimental_values_Invalid_as_an_Ethertype_since_1983'
 ETHERTYPE_UNASSIGNED_PROBES = {
     0x8039: 'DEC_Unassigned',
+    0x0101: ETHERTYPE_OLD_XEROX_LABEL,
 }
-ETHERTYPE_MASKED_UNASSIGNED_LABEL = 'Old_Xerox_Experimental_values_Invalid_as_an_Ethertype_since_1983'
 ETHERTYPE_KEPT_PROBE = (0x0888, 'Xyplex_0x0888')
 
 #: :class:`~pcapkit.const.ipx.socket.Socket` probes for the owner's ruling:
@@ -899,11 +913,16 @@ class RulingConversionSourceTests(unittest.TestCase):
     REGISTRIES` -- which keep some ``extend_enum`` calls by design."""
 
     def test_no_reachable_gap_registries_no_longer_call_extend_enum(self) -> None:
-        # EtherType is also absent from RULING_CONVERTED_WITH_REACHABLE_GAP,
-        # but for the *masked-branch* reason documented on that data, not the
-        # no-gap-at-all reason this test is about -- and unlike the true 13 it
-        # still legitimately keeps other extend_enum calls, so it is excluded
-        # from this set too and covered by EtherTypeMixedMintTests instead.
+        # EtherType now belongs to RULING_CONVERTED_WITH_REACHABLE_GAP: its
+        # masked-branch exclusion (Old Xerox shadowed by the wider IEEE802.3
+        # range) was fixed by GitHub issue #862/#865, and its first converted
+        # branch by source order is directly probeable like any other
+        # reachable-gap registry now. The `_MIXED_REGISTRIES` subtraction
+        # below is a defensive no-op today -- neither mixed registry needs
+        # removing from `RCR - RG` any more (EtherType is already in RG;
+        # Socket never appeared in RULING_CONVERTED_REGISTRIES to begin with)
+        # -- kept so a mixed registry that regains a masked or absent branch
+        # in the future does not silently inflate this count.
         excluded = set(RULING_CONVERTED_REGISTRIES) - set(RULING_CONVERTED_WITH_REACHABLE_GAP) - _MIXED_REGISTRIES
         self.assertEqual(len(excluded), 13, 'expected exactly 13 registries with no reachable gap')
         for module_name, class_name in sorted(excluded):
@@ -949,16 +968,19 @@ def test_unassigned_rows_do_not_mint(self) -> None:
                 self.assertEqual(before, len(EtherType.__members__))
                 self.assertNotIn(value, EtherType._value2member_map_)  # type: ignore[attr-defined]
 
-    def test_masked_old_xerox_row_converts_by_source(self) -> None:
-        """0x0101-0x01FF is unreachable at runtime -- masked by the wider
-        0x0000-0x05DC branch that precedes it, a pre-existing ordering issue
-        (see :data:`ETHERTYPE_UNASSIGNED_PROBES`'s comment) unrelated to this
-        ruling. Proved by source instead of by calling it."""
-        from pcapkit.const.reg.ethertype import EtherType
-
-        source = inspect.getsource(EtherType._missing_)  # type: ignore[attr-defined]
-        self.assertIn(f"cls._unregistered_member(value, '{ETHERTYPE_MASKED_UNASSIGNED_LABEL}')", source)
-        self.assertNotIn(f"extend_enum(cls, '{ETHERTYPE_MASKED_UNASSIGNED_LABEL}", source)
+    # ``test_masked_old_xerox_row_converts_by_source`` retired here, not simply
+    # deleted: it used to prove 0x0101-0x01FF converts *by source* (an
+    # order-insensitive string search for the two calls in
+    # EtherType._missing_'s text) because the row was unreachable at runtime,
+    # masked by the wider 0x0000-0x05DC branch that preceded it. GitHub issue
+    # #862, fixed by #865, reordered the generator so that row is no longer
+    # masked, and 0x0101 is now one of the values ``test_unassigned_rows_do_
+    # not_mint`` above probes directly via ``ETHERTYPE_UNASSIGNED_PROBES`` --
+    # a strictly stronger, behavioural proof of the same fact (it also catches
+    # a regression the old source check would have missed: the string search
+    # never checked *order*, so it would keep passing even if the ordering
+    # regressed and 0x0101 started minting again). Nothing this test checked
+    # is left unchecked; it is subsumed rather than replaced by a weaker test.
 
     def test_attributed_vendor_block_still_mints(self) -> None:
         from pcapkit.const.reg.ethertype import EtherType