diff --git a/pcapkit/foundation/reassembly/ip.py b/pcapkit/foundation/reassembly/ip.py index 45b233061..05f864974 100644 --- a/pcapkit/foundation/reassembly/ip.py +++ b/pcapkit/foundation/reassembly/ip.py @@ -67,9 +67,9 @@ class IP(ReassemblyBase[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Gene ` (:issue:`477`). An incomplete buffer expires after ``timeout`` seconds of capture time, 60 by default for both IPv4 - and IPv6. A datagram with fragment offset zero and **MF** clear is - reported even when its BUFID is already buffered, rather than dropped - (:issue:`2270`). + and IPv6. A datagram with fragment offset zero and **MF** (**M** in + IPv6) clear is reported even when its BUFID is already buffered, + rather than dropped (:issue:`2270`). """ if TYPE_CHECKING: @@ -156,8 +156,10 @@ def reassembly(self, info: 'Packet[_AT]') -> 'None': # belong to. self._dtgram.extend(self.expire(TS)) - # when non-fragmented (possibly discarded) packet received: it is - # reassembled on its own, in a buffer of its own (#2270) + # a datagram with fragment offset zero and MF clear is reported even + # when its BUFID is already buffered, rather than dropped: isolated + # from that buffer (IPv6 atomic fragments) or after flushing it first + # (IPv4) (#2270) isolated = False if not FO and not MF: if self.__isolate_atomic__: diff --git a/pcapkit/foundation/registry/protocols.py b/pcapkit/foundation/registry/protocols.py index 3a32851cf..7f6b7dd93 100644 --- a/pcapkit/foundation/registry/protocols.py +++ b/pcapkit/foundation/registry/protocols.py @@ -200,9 +200,9 @@ class rather than supplied by a caller, and this function is the funnel :class:`~pcapkit.foundation.reassembly.reassembly.ReassemblyMeta` and :class:`~pcapkit.foundation.traceflow.traceflow.TraceFlowMeta` fall back to :class:`~pcapkit.protocols.misc.raw.Raw`. The registry redesign of - :issue:`514` left this key unchanged and recorded the shared namespace as a - risk it did not fix, so the collision is reported here rather than - resolved. + :issue:`514` keeps this key, and its thread records the shared namespace as + a risk that design does not resolve, so the collision is reported here + rather than fixed. Args: protocol: Protocol class. @@ -256,10 +256,11 @@ class _NameLease: Each code registrar below ends by naming its class in the by-name registry, and :mod:`pcapkit.foundation.registry` has no ``unregister``: a code is undone by registering the entry it displaced (:issue:`1363`). Without a - record of that by-name write, handing a code back restored the code and left - the name behind (:issue:`1505`). A lease records what the write displaced and - which codes still dispatch to the class, so the last of them to be handed - back puts the displaced entry back, or removes the name if it was new. + record of that by-name write, handing a code back would restore the code and + leave the name behind (:issue:`1505`). A lease records what the write + displaced and which codes still dispatch to the class, so the last of them + to be handed back puts the displaced entry back, or removes the name if it + was new. A lease is one write, not one class: the same class can hold two leases on a name -- e.g. a built-in registered at one port over a custom class @@ -366,9 +367,9 @@ def _register_name(protocol: 'Type[ProtocolBase]', *targets: 'tuple[type, Any]') releasing whatever each target code held before, and followed by a lease when the write changed the name, so that registering the code back undoes it (:issue:`1505`). A name already showing ``protocol`` -- shipped, defined - by a :class:`~pcapkit.protocols.protocol.Protocol` subclass, or registered - directly -- is not leased, so no restore takes it away; one showing it - through a lease extends that lease to ``targets``. + by a :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass, or + registered directly -- is not leased, so no restore takes it away; one + showing it through a lease extends that lease to ``targets``. Args: protocol: Protocol class just registered -- and so validated -- at ``targets``. @@ -565,12 +566,15 @@ def register_protocol_code(protocol: 'Type[ProtocolBase]', code: 'Any') -> 'None def _check_code(code: 'Any', kind: 'type', target: 'Type[ProtocolBase]', *, number: 'bool' = False) -> 'None': - """Reject a registration key that is not a member of the registrar's enum. + """Reject a registration key that is not a member of the enum its + destination registry is keyed by. Each registrar typed on one enum calls this before touching any registry, - so a bad ``code`` leaves every registry as it was (:issue:`2186`). The check is on the type, - not the value: the registry enums compare equal to their values, so a - member of another enum would key the entry of the member it equals, e.g. + as does :func:`register_protocol_code` for an inferred or explicitly + named destination, so a bad ``code`` leaves every registry as it was + (:issue:`2186`). The check is on the type, not the value: the registry + enums compare equal to their values, so a member of another enum would + key the entry of the member it equals, e.g. :attr:`TransType.ICMP ` would replace :attr:`LinkType.ETHERNET ` (:issue:`2224`). @@ -580,7 +584,7 @@ def _check_code(code: 'Any', kind: 'type', target: 'Type[ProtocolBase]', *, kind: Enum the registrar is typed on. target: Protocol class whose registry ``code`` is destined for, named in the error message so a rejection says *whose* registry rejected the - key rather than only what it wanted -- the registrars share one + key rather than only what it wanted -- its callers share one message, and several of them are typed on the same enum. number: Whether a bare :class:`int` is a valid key too, as for the registrars keyed on a port number or a payload protocol identifier. diff --git a/pcapkit/protocols/application/httpv1.py b/pcapkit/protocols/application/httpv1.py index f1c48b70e..ad0336555 100644 --- a/pcapkit/protocols/application/httpv1.py +++ b/pcapkit/protocols/application/httpv1.py @@ -224,7 +224,8 @@ def make(self, # type: ignore[override] Raises: ProtocolError: If the values are neither a request's nor a - response's, or if ``raw_line`` or a ``raw_header`` entry that + response's, if they are a request's and ``uri`` is + :obj:`None`, or if ``raw_line`` or a ``raw_header`` entry that reads back as the given values carries a CR or LF that the values do not (other than the CRLF of an ``obs-fold``). @@ -328,7 +329,8 @@ def _make_start_line(self, http_version: 'str | bytes', *, Raises: ProtocolError: If the values are neither a request's nor a - response's. + response's, or if they are a request's and ``uri`` is + :obj:`None`. """ version = http_version.encode() if isinstance(http_version, str) else http_version diff --git a/pcapkit/protocols/internet/hip.py b/pcapkit/protocols/internet/hip.py index cad41bf60..163c328aa 100644 --- a/pcapkit/protocols/internet/hip.py +++ b/pcapkit/protocols/internet/hip.py @@ -1955,8 +1955,9 @@ def _make_lifetime_octet(self, code: 'Enum_Parameter', version: 'int', An explicit ``exponent`` is the octet and wins, as a parsed parameter's is; otherwise ``lifetime`` is a duration and - :meth:`_make_registration_lifetime` encodes it. Shared by the three - :rfc:`8003` makers, which between them carry four such octets. + :meth:`_make_registration_lifetime` encodes it. Shared by the + :rfc:`8003` registration makers, wherever a ``Lifetime`` octet + appears. Args: code: parameter code diff --git a/pcapkit/protocols/internet/internet.py b/pcapkit/protocols/internet/internet.py index a64b5c769..5e986a7a8 100644 --- a/pcapkit/protocols/internet/internet.py +++ b/pcapkit/protocols/internet/internet.py @@ -182,7 +182,7 @@ def register(cls, code: 'Enum_TransType', protocol: 'ModuleDescriptor[ProtocolBa Arguments: code: protocol code as in :class:`~pcapkit.const.reg.transtype.TransType` protocol: module descriptor or a - :class:`~pcapkit.protocols.protocol.Protocol` subclass + :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass Raises: pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a diff --git a/pcapkit/protocols/link/link.py b/pcapkit/protocols/link/link.py index cc4d148af..228a38b71 100644 --- a/pcapkit/protocols/link/link.py +++ b/pcapkit/protocols/link/link.py @@ -138,7 +138,7 @@ def register(cls, code: 'Enum_EtherType', protocol: 'ModuleDescriptor[ProtocolBa Arguments: code: protocol code as in :class:`~pcapkit.const.reg.ethertype.EtherType` protocol: module descriptor or a - :class:`~pcapkit.protocols.protocol.Protocol` subclass + :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass Raises: pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a @@ -150,7 +150,7 @@ def register(cls, code: 'Enum_EtherType', protocol: 'ModuleDescriptor[ProtocolBa replacement so a caller can tell *what* was lost. Fires only when the incumbent differs from the replacement -- see :meth:`ProtocolBase.register - ` for the + ` for the guard this shares with ``register_protocol``, and for the shipped descriptor that a restore puts back (:issue:`1504`). @@ -158,7 +158,7 @@ def register(cls, code: 'Enum_EtherType', protocol: 'ModuleDescriptor[ProtocolBa Takes the protocol as one ``protocol`` argument, a :class:`~pcapkit.corekit.module.ModuleDescriptor` or a - :class:`~pcapkit.protocols.protocol.Protocol` subclass, in place of + :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass, in place of the ``module`` and ``class_`` names. """ diff --git a/pcapkit/protocols/misc/null.py b/pcapkit/protocols/misc/null.py index fb689b4ca..4e57bbbc0 100644 --- a/pcapkit/protocols/misc/null.py +++ b/pcapkit/protocols/misc/null.py @@ -68,7 +68,17 @@ def protocol(self) -> 'NoReturn': @property def protochain(self) -> 'None': # type: ignore[override] """Protocol chain of current instance, which an empty payload does not - have.""" + have. + + .. versionchanged:: 1.5.0 + + Narrows :attr:`Protocol.protochain + `'s declared + type from :class:`~pcapkit.corekit.protochain.ProtoChain` to + :obj:`None`, the value ``self._protos`` already held here; the fix + is to the declaration, not the runtime value (:issue:`2317`). + + """ return None ########################################################################## diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index 4eda240b0..05f883e27 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -661,7 +661,15 @@ def protocol(self) -> 'Optional[str]': # protocol chain of current instance @property def protochain(self) -> 'ProtoChain': - """Protocol chain of current instance.""" + """Protocol chain of current instance. + + Note: + :class:`~pcapkit.protocols.misc.null.NoPayload` has no chain and + narrows this to :obj:`None`; :meth:`_decode_next_layer` passes it + on as ``basis``, which + :class:`~pcapkit.corekit.protochain.ProtoChain` accepts. + + """ return self._protos # packet data @@ -1031,7 +1039,7 @@ def register(cls, code: 'int', protocol: 'ModuleDescriptor | Type[ProtocolBase]' Arguments: code: protocol code protocol: module descriptor or a - :class:`~pcapkit.protocols.protocol.Protocol` subclass + :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass Raises: pcapkit.utilities.exceptions.RegistryError: If ``protocol`` is not a @@ -1271,9 +1279,12 @@ def __init__(self, file: 'Optional[IO[bytes] | bytes]' = None, length: 'Optional UnsupportedCall: When constructing (``file`` is :obj:`None`), if a keyword names no parameter of this protocol's :meth:`make`, :meth:`read`, :meth:`pack`, :meth:`unpack`, - :meth:`__post_init__` or :meth:`__init__`, anywhere in the MRO, - and is not listed in :attr:`__keywords__`. Parsing (``file`` is - given) is unaffected. + :meth:`__post_init__` or :meth:`__init__`, anywhere in the MRO + -- except a ``make`` whose class sets + :attr:`__own_make_keywords__`, which counts only while it is + the ``make`` of the protocol being constructed -- and is not + listed in :attr:`__keywords__`. Parsing (``file`` is given) is + unaffected. Note: The ``_layer``, ``_protocol`` and ``__context__`` keywords are diff --git a/pcapkit/protocols/transport/sctp.py b/pcapkit/protocols/transport/sctp.py index 1c7086f60..45d217f73 100644 --- a/pcapkit/protocols/transport/sctp.py +++ b/pcapkit/protocols/transport/sctp.py @@ -694,6 +694,10 @@ def make(self, Returns: Constructed packet data. + Raises: + ProtocolError: If ``srcport`` or ``dstport`` is a :obj:`bool` + (:issue:`2287`). + Note: There is no ``payload`` argument: SCTP carries its user data in the ``data`` field of a DATA chunk, so the payload is supplied as part @@ -730,7 +734,7 @@ def register(cls, code: 'Enum_PayloadProtocolIdentifier | int', protocol: 'Modul code: payload protocol identifier (PPID), as in :class:`~pcapkit.const.sctp.payload_protocol_identifier.PayloadProtocolIdentifier` protocol: module descriptor or a - :class:`~pcapkit.protocols.protocol.Protocol` subclass + :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass Important: SCTP overrides :meth:`Transport.register diff --git a/pcapkit/protocols/transport/tcp.py b/pcapkit/protocols/transport/tcp.py index 86abc1e57..82304c935 100644 --- a/pcapkit/protocols/transport/tcp.py +++ b/pcapkit/protocols/transport/tcp.py @@ -624,6 +624,10 @@ def make(self, Returns: Constructed packet data. + Raises: + ProtocolError: If ``srcport`` or ``dstport`` is a :obj:`bool` + (:issue:`2287`). + .. versionchanged:: 1.5.0 An out-of-range ``srcport`` or ``dstport`` raises @@ -2900,13 +2904,13 @@ def _make_mptcp_unknown(self, subtype: 'Enum_MPTCPOption', opt: 'Optional[Data_M # three octets of header -- ``kind``, ``length`` and the subtype/flags # octet whose low nibble takes ``data[0]`` -- and then ``data[1:]`` as # its body, so the option is never shorter than 3 octets -- the smallest - # Multipath TCP option :rfc:`8684` section 3 permits. The former - # ``2 + len(data)`` declared 2 for ``data=b''`` while still packing those - # three octets, and the segment could not be read back: - # ``MPTCPUnknown.data`` sizes itself ``pkt['length'] - 3``, which went - # negative and raised ``ProtocolError: Field data resolved to a negative - # length``. For every non-empty ``data`` the two expressions agree, so - # this only changes the empty case. + # Multipath TCP option :rfc:`8684` section 3 permits. A plain + # ``2 + len(data)`` would declare 2 for ``data=b''`` while those three + # octets are packed anyway, and the segment could not be read back: + # ``MPTCPUnknown.data`` sizes itself ``pkt['length'] - 3``, which would + # go negative and raise ``ProtocolError: Field data resolved to a + # negative length``. For every non-empty ``data`` the two expressions + # agree, so only the empty case tells them apart. return Schema_MPTCPUnknown( kind=cast('Enum_Option', Enum_Option.Multipath_TCP), length=3 + len(data[1:]), diff --git a/pcapkit/protocols/transport/transport.py b/pcapkit/protocols/transport/transport.py index e4b100874..0535c517a 100644 --- a/pcapkit/protocols/transport/transport.py +++ b/pcapkit/protocols/transport/transport.py @@ -79,7 +79,7 @@ def register(cls, code: 'int', protocol: 'ModuleDescriptor[ProtocolBase] | Type[ Arguments: code: port number protocol: module descriptor or a - :class:`~pcapkit.protocols.protocol.Protocol` subclass + :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass Important: This method must be called from a non-abstract class, as the @@ -99,7 +99,7 @@ def register(cls, code: 'int', protocol: 'ModuleDescriptor[ProtocolBase] | Type[ :class:`~pcapkit.corekit.module.ModuleDescriptor` counts as different from the class it names. See :meth:`ProtocolBase.register - ` for the + ` for the guard this shares with ``register_protocol``, and for the shipped descriptor that a restore puts back (:issue:`1504`). @@ -115,7 +115,7 @@ def register(cls, code: 'int', protocol: 'ModuleDescriptor[ProtocolBase] | Type[ Takes the protocol as one ``protocol`` argument, a :class:`~pcapkit.corekit.module.ModuleDescriptor` or a - :class:`~pcapkit.protocols.protocol.Protocol` subclass, in place of + :class:`~pcapkit.protocols.protocol.ProtocolBase` subclass, in place of the ``module`` and ``class_`` names. """ diff --git a/pcapkit/protocols/transport/udp.py b/pcapkit/protocols/transport/udp.py index 224d1c8ec..931420089 100644 --- a/pcapkit/protocols/transport/udp.py +++ b/pcapkit/protocols/transport/udp.py @@ -228,6 +228,10 @@ def make(self, Returns: Constructed packet data. + Raises: + ProtocolError: If ``srcport`` or ``dstport`` is a :obj:`bool` + (:issue:`2287`). + .. versionchanged:: 1.5.0 An out-of-range ``srcport`` or ``dstport`` raises diff --git a/tests/protocols/test_maker_invalid_format_messages_2296_unit.py b/tests/protocols/test_maker_invalid_format_messages_2296_unit.py index 355fac522..be490ed75 100644 --- a/tests/protocols/test_maker_invalid_format_messages_2296_unit.py +++ b/tests/protocols/test_maker_invalid_format_messages_2296_unit.py @@ -1,17 +1,17 @@ # -*- coding: utf-8 -*- """GitHub issue #2296: makers name the argument they rejected. -Several ``_make_*`` builders refused a caller-supplied argument with the -*reader's* text -- ``: [Chunk|Param|Cause|OptNo N] invalid format``. -Nothing had been parsed at that point, so the message described wire data that -did not exist and told the caller neither which keyword was wrong nor what it -had passed. Each site now uses the house form established by #2232 and #2257, -``: : must be X, not {value!r}``. - -The parse side is deliberately untouched: ``invalid format`` is the right thing -for a reader to say, which is why the ``SCTP: [Chunk 0] invalid format`` pin in +The ``_make_*`` builders pinned below refuse a caller-supplied argument in the +house form established by #2232 and #2257, +``: : must be X, not {value!r}``, which names +the keyword at fault and the value passed. A reader instead says +``: [Chunk|Param|Cause|OptNo N] invalid format``, which describes wire +data -- something a maker has not parsed -- and identifies neither. + +Readers are deliberately exempt: ``invalid format`` is the right thing for a +reader to say, which is why the ``SCTP: [Chunk 0] invalid format`` pin in :mod:`tests.protocols.transport.test_transport_edge_roundtrip_unit` -- a parse -case -- is left alone. :class:`ReadersKeepInvalidFormat` below pins that +case -- expects exactly that. :class:`ReadersKeepInvalidFormat` below pins that division for the one maker/reader pair where the two guards check the same thing. @@ -427,7 +427,7 @@ def test_raw_secrets_data_still_builds(self) -> None: class ReadersKeepInvalidFormat(MakerMessageTestCase): - """The parse side still says ``invalid format``, as #2296 asks. + """The parse side says ``invalid format``, as #2296 asks. ``_read_chunk_data`` and ``_make_chunk_data`` guard the same thing -- a DATA chunk with no user data -- so the pair is where a sweep over the makers is diff --git a/tests/protocols/test_maker_invalid_format_messages_2406_unit.py b/tests/protocols/test_maker_invalid_format_messages_2406_unit.py index aada9efb8..45661d2d7 100644 --- a/tests/protocols/test_maker_invalid_format_messages_2406_unit.py +++ b/tests/protocols/test_maker_invalid_format_messages_2406_unit.py @@ -1,27 +1,28 @@ # -*- coding: utf-8 -*- -"""GitHub issue #2406: the makers #2296 did not list name their argument. +"""GitHub issue #2406: the makers outside #2296's checklist name their argument. -#2296 swept ``_make_*`` guards off the *reader's* text -- -``: [... N] invalid format`` -- and onto the house form established by -#2232 and #2257, ``: : must be X, not {value!r}``. -The maker-side sites below were outside its checklist and kept the generic -wording: +The ``_make_*`` guards below carry the house form established by #2232 and +#2257, ``: : must be X, not {value!r}``, which +names the keyword at fault and the value passed -- not the *reader's* +``: [... N] invalid format``, which describes wire data a maker has not +parsed. Each sits on the maker side, outside #2296's checklist: * :meth:`HOPOPT._make_opt_pad ` and :meth:`IPv6_Opts._make_opt_pad `, the twins of - the ``Pad1`` length warning #2296 fixed on :class:`~pcapkit.protocols.internet.mh.MH`; + the ``Pad1`` length warning on :class:`~pcapkit.protocols.internet.mh.MH` (#2296); * :meth:`HTTP.make ` and :meth:`PCAPNG.make `, the dispatchers whose unsupported-``frame``/``block`` guard is the same shape as ``MH.make``'s. -The parse side is deliberately untouched, as in #2296: ``invalid format`` is the -right thing for a reader to say. :class:`ReadersKeepInvalidFormat` below pins -that division on the one pair where the reader and the maker guard the very same -thing -- ``_read_opt_pad`` and ``_make_opt_pad`` both reject a ``Pad1`` carrying -a non-zero length. +Readers are deliberately exempt, as in #2296: ``invalid format`` is the right +thing for a reader to say. :class:`ReadersKeepInvalidFormat` below pins that +division on the one pair where the reader and the maker guard the very same +thing -- ``_read_opt_pad`` and ``_make_opt_pad`` guard a ``Pad1`` carrying a +non-zero length, though only the reader raises; the maker warns and makes a +``PadN`` instead. Every message is pinned whole, and every maker's valid input is exercised in the same class, so a guard that starts refusing good input regresses here too. @@ -161,7 +162,7 @@ def test_an_omitted_frame_is_not_rejected(self) -> None: from pcapkit.protocols.application.httpv2 import HTTP # ``frame=None`` dispatches to the frame type's own constructor (#2274) - # rather than reaching the guard, so the guard has not widened to it. + # rather than reaching the guard, so the guard does not cover it. http = self.protocol(HTTP) self.assertEqual(http.make(type=Frame.DATA, frame=None).type, Frame.DATA) @@ -209,11 +210,12 @@ def test_raw_block_still_builds(self) -> None: class ReadersKeepInvalidFormat(MakerMessageTestCase): - """The parse side still says ``invalid format``, as #2296 asks. + """The parse side says ``invalid format``, as #2296 asks. - ``_read_opt_pad`` and ``_make_opt_pad`` reject the same thing -- a ``Pad1`` - option carrying a non-zero length -- so the pair is where a sweep over the - makers is most likely to catch a reader by accident. + ``_read_opt_pad`` and ``_make_opt_pad`` guard the same thing -- a ``Pad1`` + option carrying a non-zero length -- though only the reader raises; the + maker warns and makes a ``PadN`` instead. The pair is where a sweep over + the makers is most likely to catch a reader by accident. """ diff --git a/tests/protocols/test_protocol_code_registration_unit.py b/tests/protocols/test_protocol_code_registration_unit.py index 96c7f97ee..b54661121 100644 --- a/tests/protocols/test_protocol_code_registration_unit.py +++ b/tests/protocols/test_protocol_code_registration_unit.py @@ -270,7 +270,7 @@ def test_iterable_mixes_inferred_and_explicit_targets(self) -> None: Deliberately *not* ``L2TPv2`` (#548): v2 answers on port 1701 only, and registering it at ``TransType.L2TP`` points the :rfc:`2661` parser at a - v3-over-IP header. The mechanism under test is unchanged either way -- the + v3-over-IP header. The mechanism under test is the same either way -- the class here is a stand-in and nothing is really registered -- but the example should not recommend a binding the library refuses. """ @@ -365,7 +365,7 @@ def test_subclass_without_code_does_not_inherit_parent_registration(self) -> Non Mirrors part (a)'s ``Parent``/``Derived``/``DerivedOptIn`` measurement on the issue thread: a subclass of a registered class that itself passes no ``code=`` must not re-register under its own name -- there - is no name-derived fallback left to do that with, but the property is + is no name-derived fallback to do that with, but the property is worth pinning directly for ``Protocol`` too. """ from pcapkit.const.reg.transtype import TransType @@ -416,12 +416,14 @@ def test_backward_compatible_dispatch_tables_are_unaffected(self) -> None: class RegisterProtocolCodeKeyCheckTests(unittest.TestCase): """Each ``code=`` key is checked against its destination's key type, all before any registers (#2268). - The typed registrars reject a member of another enum (#2224), but - ``register_protocol_code`` handed every ``{destination: key}`` pair straight - to ``destination.register``. The registry enums compare equal to their - values, so ``{Frame: TransType.ICMP}`` replaced the shipped entry at - ``LinkType.ETHERNET`` with only a warning. It also registered each target as - it flattened them, so a bad later target left the earlier ones in place. + The typed registrars reject a member of another enum (#2224), and + ``register_protocol_code`` checks every ``{destination: key}`` pair against + its destination's key type before it registers anything. The registry enums + compare equal to their values, so a pair handed straight to + ``destination.register`` -- ``{Frame: TransType.ICMP}`` -- would replace the + shipped entry at ``LinkType.ETHERNET`` with only a warning; checking every + pair up front also leaves no earlier target registered when a later one is + bad. Each case snapshots every dispatch registry and the name registry, expects :exc:`~pcapkit.utilities.exceptions.RegistryError`, compares each registry @@ -679,15 +681,15 @@ def test_a_rejection_names_the_destination_registry(self) -> None: @unittest.skipUnless(HAS_RUNTIME, 'runtime dependencies not installed') class ProtoDestDeclarationTests(unittest.TestCase): - """``__proto_dest__``, the declaration #2392 replaced the lookup table with. - - Inference used to read a private lookup table in - :mod:`pcapkit.foundation.registry.protocols`, keyed by enum type. #2392 - moves it onto the enumeration registry classes, mirroring what the same - issue did in the other direction with ``__proto_key__``. These pin the - declarations themselves; the ``register_protocol_code`` behaviour they drive - is pinned by :class:`RegisterProtocolCodeInferenceTests` above, which is - unchanged by the move and is the point. + """``__proto_dest__``, the declaration inference reads (#2392). + + The enumeration registry classes name their destination class(es) + themselves, as a ``__proto_dest__`` dunder, so inference reads the pairing + off the key's own type -- the mirror of ``__proto_key__``, which each + destination declares to name the enum its own registry is keyed by. These + pin the declarations themselves; the ``register_protocol_code`` behaviour + they drive, which is the point of them, is pinned by + :class:`RegisterProtocolCodeInferenceTests` above. """ def setUp(self) -> None: @@ -708,7 +710,7 @@ def setUp(self) -> None: ) def test_each_registry_declares_the_destinations_it_keys(self) -> None: - """Resolved, and in order -- ``LinkType`` still fans out to both formats. + """Resolved, and in order -- ``LinkType`` fans out to both formats. A declaration is read off the class's own :func:`vars`, since an enumeration that inherited one would be declaring nothing. @@ -754,7 +756,7 @@ def test_the_declarations_name_classes_lazily_rather_than_importing_them(self) - self.assertNotIn('import pcapkit.protocols', source) def test_only_these_registries_declare_a_destination(self) -> None: - """Every other generated enumeration is untouched by the template change. + """No other generated enumeration declares a destination. The template renders nothing for a crawler that declares no ``PROTO_DEST``, so a declaration appearing anywhere else means either a diff --git a/tests/vendor/test_vendor_proto_dest_2392_unit.py b/tests/vendor/test_vendor_proto_dest_2392_unit.py index 53877a465..533259354 100644 --- a/tests/vendor/test_vendor_proto_dest_2392_unit.py +++ b/tests/vendor/test_vendor_proto_dest_2392_unit.py @@ -1,11 +1,10 @@ # -*- coding: utf-8 -*- """The ``__proto_dest__`` declaration the default template renders (#2392). -GitHub issue #2392 moves the enum type -> destination pairing out of a private -lookup table in :mod:`pcapkit.foundation.registry.protocols` and onto the -enumeration registry classes themselves, as a ``__proto_dest__`` dunder -- the -mirror of what the same issue did for the opposite direction with -``__proto_key__``. +The enum type -> destination pairing lives on the enumeration registry classes +themselves, as a ``__proto_dest__`` dunder (GitHub issue #2392) -- the mirror +of ``__proto_key__``, which each destination declares to name the enum its own +registry is keyed by. The :file:`pcapkit/const/` modules that carry it are generated, so the declaration is made in the crawler @@ -18,7 +17,7 @@ *nothing at all* for an empty :attr:`~pcapkit.vendor.default.Vendor.PROTO_DEST`. * :class:`TemplateProtoDestTests` -- the same through :data:`LINE`: the import line and the class-body declaration both appear when a crawler declares a - destination, and the rendered module is byte-identical to the pre-#2392 + destination, and the rendered module is byte-identical to the committed rendering when it does not. That second assertion is the load-bearing one: only a handful of registries declare a destination, so a template change that touched the other generated modules would rewrite most of :mod:`pcapkit.const` @@ -53,10 +52,9 @@ #: Whether the crawlers are importable at all. HAS_VENDOR_DEPS = all(importlib.util.find_spec(name) is not None for name in VENDOR_DEPS) -#: ``crawler module -> ((module, class), ...)``, the destinations #2392 settled -#: on. The same pairings the lookup table held, with ``LinkType`` still -#: naming two: a link-layer header type keys the next-layer registry of -#: both capture file formats. +#: ``crawler module -> ((module, class), ...)``, the destinations the declaring +#: crawlers name (#2392), with ``LinkType`` naming two: a link-layer header +#: type keys the next-layer registry of both capture file formats. EXPECTED = { 'pcapkit.vendor.reg.ethertype': (('pcapkit.protocols.link.link', 'Link'),), 'pcapkit.vendor.reg.transtype': (('pcapkit.protocols.internet.internet', 'Internet'),), @@ -115,7 +113,7 @@ def test_the_declaration_is_a_lazy_descriptor_not_an_import(self) -> None: generated module that imported :class:`~pcapkit.protocols.link.link.Link` to declare it would be a circular import. The lazy :class:`~pcapkit.corekit.module.ModuleDescriptor` is what avoids that, - and is why #2392 settled on it. + and is why the declaration uses it (#2392). """ from pcapkit.corekit.module import ModuleDescriptor from pcapkit.vendor.default import render_proto_dest @@ -144,12 +142,12 @@ def _render(dest: 'tuple' = ()) -> 'str': 'pcapkit.vendor.arp.hardware', dest) def test_no_declaration_leaves_the_rendering_untouched(self) -> None: - """The pre-#2392 output, character for character. + """The output for a crawler that declares none, character for character. Pinned against the committed :mod:`pcapkit.const.arp.hardware` rather than against a copy of the template: ``arp/hardware`` declares no - destination, so its import block and class body must still be what the - template rendered before #2392. + destination, so its import block and class body are what the template + renders with no declaration to add. """ rendered = self._render() @@ -235,7 +233,7 @@ def test_the_declaring_crawlers_name_the_destinations_the_registries_need(self) expected) def test_no_other_crawler_declares_one(self) -> None: - """The rest inherit the empty default, so their output is unchanged. + """The rest inherit the empty default, so none of their output declares one. Not a style point: a crawler that gained a destination by accident would add an import and an attribute to its generated module on the next