diff --git a/pcapkit/corekit/enum.py b/pcapkit/corekit/enum.py index 09d7eb17c..d4cd6429f 100644 --- a/pcapkit/corekit/enum.py +++ b/pcapkit/corekit/enum.py @@ -18,9 +18,9 @@ :meth:`~EnumRegistry._unregistered_member`. Every generated registry under :mod:`pcapkit.const` inherits from here. -That split follows the rules laid down in GitHub issue #877: a helper enumeration is -immutable by default, unless RFC or IANA documents its value space as open. -Such closed sets subclass a bare base enumeration in this module, and +That split follows the rules laid down in GitHub issue :issue:`877`: a helper +enumeration is immutable by default, unless RFC or IANA documents its value space as +open. Such closed sets subclass a bare base enumeration in this module, and :class:`EnumRegistry` subclasses that base for the mutable ones. Which methods land on which tier was settled in the same thread. The deciding @@ -44,21 +44,20 @@ .. note:: Re-parenting every non-registry enumeration onto :class:`EnumLookup` was - **phase 2** of GitHub issue #877, and it is now **complete**: introducing - the base above was deliberately behaviour-preserving on its own, so that it - could land while other work was still in flight on the files the re-parent - touches, and the phase itself landed in two pull requests for exactly that - reason -- the first for the 17 enumerations that were free to move at - once, and the second, `#930 - `__, for the - remaining seven once the files holding them freed up. - -The registry tier's own shape is the earlier design settled in GitHub issue #842: -``get``, ``get_all``, ``register`` and ``register_alias`` are to exist on every -const registry, so the abstraction is finished by moving them to the base class. -The closed sets split off by #877 are outside that contract. -``AppType``'s sub-base class carries the overrides and dispatching logic it -needs, and ``AppType``'s subclasses override again where their contracts + **phase 2** of GitHub issue :issue:`877`, and it is now **complete**: + introducing the base above was deliberately behaviour-preserving on its + own, so that it could land while other work was still in flight on the + files the re-parent touches, and the phase itself landed in two pull + requests for exactly that reason -- the first for the 17 enumerations that + were free to move at once, and the second, :issue:`930`, for the remaining + seven once the files holding them freed up. + +The registry tier's own shape is the earlier design settled in GitHub issue +:issue:`842`: ``get``, ``get_all``, ``register`` and ``register_alias`` are to +exist on every const registry, so the abstraction is finished by moving them to +the base class. The closed sets split off by :issue:`877` are outside that +contract. ``AppType``'s sub-base class carries the overrides and dispatching +logic it needs, and ``AppType``'s subclasses override again where their contracts differ. That is a three-tier hierarchy, of which this module is **tier one**: @@ -71,14 +70,15 @@ overrides below and a few hand-written ``get`` overrides. 2. ``AppType``'s sub-base -- overrides all four, routing the two lookups through its ``_dispatch``, because a port lookup needs a transport protocol - to be answerable at all. Landed as of GitHub issue #860: not in this module, - but in :class:`pcapkit.const.reg.apptype.apptype.AppType` itself, which - mixes in :class:`EnumRegistry` directly. ``register`` and ``register_alias`` - are overridden without that dispatch, staying on the registry they are - called on, so that a write never leaks onto a transport IANA never assigned - the service to -- a minted member for the first, an alias for the second. - Plus ``_unregistered_member``, for its own three extra attributes (``svc``, - ``port``, ``proto``) that the generic one below does not know to set. + to be answerable at all. Landed as of GitHub issue :issue:`860`: not in this + module, but in :class:`pcapkit.const.reg.apptype.apptype.AppType` itself, + which mixes in :class:`EnumRegistry` directly. ``register`` and + ``register_alias`` are overridden without that dispatch, staying on the + registry they are called on, so that a write never leaks onto a transport + IANA never assigned the service to -- a minted member for the first, an + alias for the second. Plus ``_unregistered_member``, for its own three extra + attributes (``svc``, ``port``, ``proto``) that the generic one below does + not know to set. 3. The ``AppType`` transport subclasses -- ``TCP``, ``UDP``, ``SCTP``, ``DCCP`` -- turned out to need no override of their own at all: ``_dispatch`` already returns ``cls`` unchanged the moment ``cls.__registry__`` is not @@ -89,14 +89,15 @@ :data:`pcapkit.vendor.default.LINE` and copied verbatim into each of the eleven crawlers that replace that template wholesale, none of which carried ``register``, ``register_alias`` or ``get_all`` at all. Adding one method meant -editing every bespoke template by hand, which is the cost #775 asks to remove. +editing every bespoke template by hand, which is the cost :issue:`775` asks to +remove. -The contracts are those set out on GitHub issue #842: ``get`` is a shortcut for -the ``[]`` operation and returns the canonical enumeration member; ``get_all`` -returns every matching member; ``register`` mints a new member on the class at -runtime under the name the caller specifies, so nothing has to be guessed; -``register_alias`` (and ``register_aliases``) adds further alias names to a -given member's mapping. +The contracts are those set out on GitHub issue :issue:`842`: ``get`` is a +shortcut for the ``[]`` operation and returns the canonical enumeration member; +``get_all`` returns every matching member; ``register`` mints a new member on +the class at runtime under the name the caller specifies, so nothing has to be +guessed; ``register_alias`` (and ``register_aliases``) adds further alias names +to a given member's mapping. """ from typing import TYPE_CHECKING @@ -168,7 +169,7 @@ class EnumLookup: def _validate_value(cls, value: 'Any') -> 'None': """Hook: reject ``value`` if this enumeration's contract does not allow it. - GitHub issue #877 requires some range-validation logic for the + GitHub issue :issue:`877` requires some range-validation logic for the inheriting classes to hook into. This is that hook, and it is what the bare tier carries **instead** of ``register``: what values are *legal* is something every enumeration has an opinion on, whereas who may *add* @@ -188,10 +189,10 @@ def _validate_value(cls, value: 'Any') -> 'None': type is :obj:`None` deliberately rather than the validated value, so that this hook cannot become a converter: a subclass that returned a changed value here would silently alter what a lookup resolves to, which is - exactly the case-folding the ruling on GitHub issue #877 rules out: - an enumeration keeps the original spellings its registrars use. - Case handling belongs in a deliberate ``get`` override with - an RFC behind it, not in a validation hook. + exactly the case-folding the ruling on GitHub issue :issue:`877` rules + out: an enumeration keeps the original spellings its registrars use. Case + handling belongs in a deliberate ``get`` override with an RFC behind it, + not in a validation hook. Raise from :mod:`pcapkit.utilities.exceptions`, per the same issue's ruling that in-library code raises in-library exceptions -- @@ -248,19 +249,20 @@ def _validate_value(cls, value: 'Any') -> 'None': def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': """Resolve ``key`` to the canonical member. - A shortcut for the ``[]`` operation, per the ruling on #842: given a - name it is ``cls[key]``, and given a value it is ``cls(key)``. Either - way the answer is the *canonical* member -- subscripting an alias - returns the member the alias points at, not a separate object -- so - two names for one assignment resolve to one enum. + A shortcut for the ``[]`` operation, per the ruling on :issue:`842`: + given a name it is ``cls[key]``, and given a value it is + ``cls(key)``. Either way the answer is the *canonical* member -- + subscripting an alias returns the member the alias points at, not a + separate object -- so two names for one assignment resolve to one + enum. It never mints while resolving ``default``; ``key`` may still mint - through a ``_missing_`` that GitHub issue #775's ruling deliberately - kept minting, on one registry (``CGAType``) -- the ruling's final - round converted the other two it originally held out, + through a ``_missing_`` that GitHub issue :issue:`775`'s ruling + deliberately kept minting, on one registry (``CGAType``) -- the + ruling's final round converted the other two it originally held out, ``EtherType`` and ``Socket``, so they no longer mint on any path - either. Registering a member any other way is :meth:`register`'s - job and nobody else's, which is the ruling #775 exists to carry + either. Registering a member any other way is :meth:`register`'s job + and nobody else's, which is the ruling :issue:`775` exists to carry out: an unrecognised or unregistered value does not become a registered member unless a user or caller explicitly creates one. A value inside a registry's declared-but-unassigned range still resolves, through that @@ -284,10 +286,10 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': unrecognised value, routing a failed *name* lookup through the constructor would let a mere ``get()`` call mint a permanent member where it previously just raised. Defensive rather than observed: of - the 127 classes that reach this method -- 125 until GitHub issue #880's - own PR added ``pcapkit/const/ngap/procedure_code.py`` and - ``pcapkit/const/ngap/protocol_ie.py``, remeasured while auditing - GitHub issue #903 -- the ``str``-valued ones + the 127 classes that reach this method -- 125 until GitHub issue + :issue:`880`'s own PR added ``pcapkit/const/ngap/procedure_code.py`` + and ``pcapkit/const/ngap/protocol_ie.py``, remeasured while auditing + GitHub issue :issue:`903` -- the ``str``-valued ones (:class:`~pcapkit.const.ftp.command.Command`, :class:`~pcapkit.const. ftp.command.FEATCode`, :class:`~pcapkit.const.http.method.Method`, :class:`~pcapkit.const.pcapng.option_type.OptionType`, @@ -296,32 +298,30 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': :class:`~pcapkit.const.reg.apptype.udp.UDP`, :class:`~pcapkit.const.reg.apptype.sctp.SCTP` and :class:`~pcapkit.const.reg.apptype.dccp.DCCP` -- completing that set as - of GitHub issue #860's own PR 2 -- and, newest of them, + of GitHub issue :issue:`860`'s own PR 2 -- and, newest of them, :class:`~pcapkit.const.pcapng.tls_key_label.TLSKeyLabel`, which GitHub - issue #877's own thread reclassified from a hand-written helper to a - generated registry once RFC 9850 §4.2 turned its member list into a - live IANA registry) no longer mint - on any path, so no live witness exists in this tree today. The one - registry that still mints directly via :func:`~aenum.extend_enum`, - :class:`~pcapkit.const.mh.cga_type.CGAType`, is - :class:`int`-valued, so a ``str`` name could not reach its mint - branch even if this restriction did not exist; it is not an - exception to it, just not reachable by it. GitHub issue #775's final - round converted the other two that used to share this footnote, - :class:`~pcapkit.const.ipx.socket.Socket` and - :class:`~pcapkit.const.reg.ethertype.EtherType`, so ``CGAType`` is - now the only one left. This is about a future - ``str``-valued registry (or a present one whose ``_missing_`` - someday changes) reaching this base with a minting ``_missing_`` of - its own, which the restriction below is written to stay correct - for regardless. 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_``. 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. + issue :issue:`877`'s own thread reclassified from a hand-written helper + to a generated registry once RFC 9850 §4.2 turned its member list into + a live IANA registry) no longer mint on any path, so no live witness + exists in this tree today. The one registry that still mints directly + via :func:`~aenum.extend_enum`, + :class:`~pcapkit.const.mh.cga_type.CGAType`, is :class:`int`-valued, so + a ``str`` name could not reach its mint branch even if this restriction + did not exist; it is not an exception to it, just not reachable by it. + GitHub issue :issue:`775`'s final round converted the other two that + used to share this footnote, :class:`~pcapkit.const.ipx.socket.Socket` + and :class:`~pcapkit.const.reg.ethertype.EtherType`, so ``CGAType`` is + now the only one left. This is about a future ``str``-valued registry + (or a present one whose ``_missing_`` someday changes) reaching this + base with a minting ``_missing_`` of its own, which the restriction + below is written to stay correct for regardless. 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_``. Since :issue:`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 @@ -348,17 +348,17 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': given on :meth:`_validate_value` itself. Both failure paths raise from :mod:`pcapkit.utilities.exceptions` - rather than a builtin, per the ruling recorded on GitHub issue #923: - in-library code raises from ``pcapkit.utilities.exceptions`` rather - than a builtin, and whether ``ValueError`` or ``KeyError`` applies + rather than a builtin, per the ruling recorded on GitHub issue + :issue:`923`: in-library code raises from ``pcapkit.utilities.exceptions`` + rather than a builtin, and whether ``ValueError`` or ``KeyError`` applies follows what stdlib's ``Enum`` raises in the same circumstance. The *shape* - is unchanged by that ruling and deliberately so -- a name miss - stays :exc:`KeyError`-derived and a value miss :exc:`ValueError`-derived, - matching ``E['nosuch']`` and ``E(999)`` on a stdlib - :class:`~enum.Enum`, and matching the 119 of this tree's 127 concrete - subclasses that already answered a name miss that way. Only the - provenance changed, so every ``except KeyError`` and ``except - ValueError`` around a call to this method keeps catching. + is unchanged by that ruling and deliberately so -- a name miss stays + :exc:`KeyError`-derived and a value miss :exc:`ValueError`-derived, + matching ``E['nosuch']`` and ``E(999)`` on a stdlib :class:`~enum.Enum`, + and matching the 119 of this tree's 127 concrete subclasses that already + answered a name miss that way. Only the provenance changed, so every + ``except KeyError`` and ``except ValueError`` around a call to this method + keeps catching. Two details of that conversion are worth stating, since neither is visible from the exception type alone: @@ -372,7 +372,7 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': *successful* call -- that override catches it in order to mint. A loud error there would put a :data:`logging.CRITICAL` record on every such call and set :data:`sys.tracebacklimit` to ``0`` process-wide, which - is exactly the GitHub issue #362 defect + is exactly the GitHub issue :issue:`362` defect :class:`~pcapkit.utilities.exceptions.BaseError` documents ``quiet`` for. The value miss takes no such fallback anywhere in this tree, so it stays loud. @@ -392,7 +392,7 @@ def get(cls, key: 'Any', default: 'Any' = NO_DEFAULT) -> 'Self': 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. + ``cls(default)``, so it cannot mint -- see :issue:`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. @@ -441,7 +441,7 @@ def get_all(cls, key: 'Any') -> 'tuple[Self, ...]': entry, since an alias registered by :meth:`register_alias` is a second *name* for the canonical member rather than a second member. The method still exists here, because all four methods are to exist on every const - registry (GitHub issue #842), and it is where a registry with genuinely + registry (GitHub issue :issue:`842`), and it is where a registry with genuinely several matches puts them: ``AppType`` overrides it to return every service IANA assigns to a port. @@ -476,8 +476,8 @@ class EnumRegistry(EnumLookup): An enumeration inherits from *here* when it may grow at runtime, and from :class:`EnumLookup` directly when it may not. The owner's ruling on GitHub - issue #877 is what draws that line: an enumeration is immutable unless - RFC or IANA says otherwise. + issue :issue:`877` is what draws that line: an enumeration is immutable + unless RFC or IANA says otherwise. Mixed in ahead of the enum base exactly as before -- ``class Foo(EnumRegistry, IntFlag)`` -- and gaining :class:`EnumLookup` as a parent @@ -492,7 +492,7 @@ def register(cls, value: 'Any', name: 'str') -> 'Self': The caller-named path, and the only one that grows the registry: it mints a new member on the class at runtime under the ``name`` the - caller specifies, so nothing has to be guessed (GitHub issue #842). + caller specifies, so nothing has to be guessed (GitHub issue :issue:`842`). Contrast :meth:`get` and ``_missing_``, which resolve without naming anything. Refuses a ``value`` that already has a member. Without this guard, @@ -582,17 +582,17 @@ def _extend(cls, value: 'Any', name: 'str') -> 'Self': def register_alias(cls, value: 'Any', name: 'str') -> 'Self': """Add ``name`` as a further name for the member already at ``value``. - Per GitHub issue #842, an alias adds a further name to a given member's - mapping -- so it needs an existing member to attach to, and this refuses a - value no member carries rather than falling through to :meth:`register`. - #842 settled on an alias always attaching to an existing member, leaving - open whether ``AppType`` or a concrete enumeration might need to alias a - value no member carries. ``AppType``'s override does not: it requires the - port to already carry a member of that very registry. What an alias means - also differs away from ``AppType``: on every other registry it is a custom - name the caller opts into, not one recorded by the IANA registrars. - Minting under the name of an aliasing call would manufacture exactly the - unrecorded member #775 removes. + Per GitHub issue :issue:`842`, an alias adds a further name to a given + member's mapping -- so it needs an existing member to attach to, and this + refuses a value no member carries rather than falling through to + :meth:`register`. :issue:`842` settled on an alias always attaching to an + existing member, leaving open whether ``AppType`` or a concrete + enumeration might need to alias a value no member carries. ``AppType``'s + override does not: it requires the port to already carry a member of that + very registry. What an alias means also differs away from ``AppType``: on + every other registry it is a custom name the caller opts into, not one + recorded by the IANA registrars. Minting under the name of an aliasing + call would manufacture exactly the unrecorded member :issue:`775` removes. Membership is tested against ``_value2member_map_`` rather than by calling ``cls(value)``: a declared-but-unassigned value resolves through diff --git a/pcapkit/corekit/fields/field.py b/pcapkit/corekit/fields/field.py index 17ef3836e..f3895e535 100644 --- a/pcapkit/corekit/fields/field.py +++ b/pcapkit/corekit/fields/field.py @@ -41,13 +41,13 @@ #: empty, tail past a truncated area and having it decode as zero -- that is #: how an over-long ``ihl``, or a capture cut short by the snapshot length, #: reads as end-of-option-list or ``Pad1`` instead of wedging or raising (see -#: #431). Every such read is of a fixed-width, few-octet field, always far -#: under this ceiling, so it is untouched; only a length past it -- which no -#: fixed-width field ever legitimately is -- gets refused. +#: :issue:`431`). Every such read is of a fixed-width, few-octet field, always +#: far under this ceiling, so it is untouched; only a length past it -- which +#: no fixed-width field ever legitimately is -- gets refused. #: #: A ceiling on one field says nothing about how many fields a parse may pad, #: which is what :data:`_MAX_ZERO_PAD_SHORTFALL` and -#: :data:`_ZERO_PAD_BUDGET_RATIO` below are for. See #573. +#: :data:`_ZERO_PAD_BUDGET_RATIO` below are for. See :issue:`573`. _MAX_ZERO_PAD_LENGTH = 0x40_000 #: int: Shortfall :meth:`FieldBase.unpack` will always zero-pad for, whatever @@ -60,17 +60,17 @@ #: declaring ``secrets_length`` of 262,142 against two supplied octets through #: ``UnknownSecrets.data`` (``pcapkit/protocols/schema/misc/pcapng.py``) -- #: retained 50.0 MiB, an amplification of 10,922x per block. Every individual -#: field was under the ceiling, so nothing refused any of them (#573). +#: field was under the ceiling, so nothing refused any of them (:issue:`573`). #: #: The sum therefore wants a budget, and this is the figure that makes one #: *safe*. A budget on its own is not: a capture cut short by its snapshot length -#: pads legitimately and must keep parsing (#431, and the reasoning that declined -#: #554), and it pads far more than it reads, so any running budget tight enough -#: to matter starts refusing real captures. Worse, it refuses them *sometimes* -- -#: measured on this tree with a running budget alone, the same legitimate -#: 54-octet frame parsed to one result on 37 of 40 calls and to another on calls -#: 26, 33 and 39, because whether it fit depended on what had been parsed before -#: it. A guard whose answer moves with history is not a guard. +#: pads legitimately and must keep parsing (:issue:`431`, and the reasoning that +#: declined :issue:`554`), and it pads far more than it reads, so any running +#: budget tight enough to matter starts refusing real captures. Worse, it refuses +#: them *sometimes* -- measured on this tree with a running budget alone, the +#: same legitimate 54-octet frame parsed to one result on 37 of 40 calls and to +#: another on calls 26, 33 and 39, because whether it fit depended on what had +#: been parsed before it. A guard whose answer moves with history is not a guard. #: #: 65,536 is what removes that. It is the whole span of a 16-bit wire length #: field -- which is how an IP header, an IPv6 payload, a TCP or IPv4 option and @@ -84,10 +84,11 @@ #: else a 16-bit field can ask for. #: #: What is left above it is the band a *32-bit* wire length reaches -- -#: PCAP-NG's own block and secrets lengths, which is where #573's amplification -#: lives -- and legitimately that is a once-per-file event, since only the last -#: block of a truncated capture is cut short. So the band gets the running budget -#: below, whose one-off term already covers any single such event outright. +#: PCAP-NG's own block and secrets lengths, which is where :issue:`573`'s +#: amplification lives -- and legitimately that is a once-per-file event, since +#: only the last block of a truncated capture is cut short. So the band gets the +#: running budget below, whose one-off term already covers any single such event +#: outright. _MAX_ZERO_PAD_SHORTFALL = 0x10_000 #: int: Zero padding *past* :data:`_MAX_ZERO_PAD_SHORTFALL` that @@ -105,10 +106,10 @@ #: #: *Repeating* one is not legitimate, and that is what 16 is chosen to catch. #: Truncation cuts the end of a file, so a capture has one short block, not two -#: hundred; #573's shape has two hundred because they are declared rather than -#: cut. 16 octets of further allowance per octet genuinely read leaves any real -#: file an allowance orders of magnitude past the one event it can want, while -#: bounding the sum for a file whose blocks all lie. +#: hundred; :issue:`573`'s shape has two hundred because they are declared +#: rather than cut. 16 octets of further allowance per octet genuinely read +#: leaves any real file an allowance orders of magnitude past the one event it +#: can want, while bounding the sum for a file whose blocks all lie. _ZERO_PAD_BUDGET_RATIO = 0x10 #: ContextVar[Optional[list[int]]]: Running ``[octets supplied, octets @@ -207,7 +208,7 @@ def _zero_pad_budget() -> 'Iterator[list[int]]': #: sign after it (``'>Xs'``), or any other malformed template -- which is what #: makes checking for it a reliable way to tell those cases apart from each #: other *before* :func:`struct.calcsize` is asked to size either one. See -#: #825. +#: :issue:`825`. _RE_NEGATIVE_LENGTH_TEMPLATE = re.compile(r'^[@=<>!]?-\d+') @@ -292,7 +293,7 @@ def length(self) -> 'int': :func:`struct.calcsize` cannot size such a template and raises a bare :exc:`struct.error`, uncatchable as a pcapkit-specific error; this re-raises it as the negative-length message below. - See #805. + See :issue:`805`. ProtocolError: If :attr:`template` is otherwise malformed -- anything else :func:`struct.calcsize` cannot size, such as a typo'd format character -- rather than the negative-length @@ -304,7 +305,7 @@ def length(self) -> 'int': :data:`_RE_NEGATIVE_LENGTH_TEMPLATE` against :attr:`template` itself -- which is known already, without needing anything :func:`struct.calcsize`'s own error says -- rather than by the - error message. See #825. + error message. See :issue:`825`. """ try: @@ -372,7 +373,7 @@ def __copy__(self) -> 'Self': directly is exactly what :func:`copy.copy` would have done once it found it, so this changes nothing about *when* a field is copied or what the copy contains -- only the redundant dispatch is removed. - See GitHub issue #730. + See GitHub issue :issue:`730`. Returns: A new field instance sharing this one's attribute values. diff --git a/pcapkit/corekit/fields/ipaddress.py b/pcapkit/corekit/fields/ipaddress.py index ece4ef935..3324cf950 100644 --- a/pcapkit/corekit/fields/ipaddress.py +++ b/pcapkit/corekit/fields/ipaddress.py @@ -90,20 +90,21 @@ def _reject_bool(value: 'object', description: str) -> 'None': warning**. On an IPv6-typed field the same conversion happens to raise instead, because the resulting :class:`~ipaddress.IPv4Address`'s version mismatches -- and that - asymmetry is exactly what let this slip past #469's otherwise + asymmetry is exactly what let this slip past :issue:`469`'s otherwise equivalent guard for :meth:`MH._make_opt_mn_id - ` (c.f. #491). + ` (c.f. + :issue:`491`). Every caller checks this *before* dispatching on the value's type, - for the same placement reason #469 gives: a correct check in the - wrong position does not fire, and that placement mistake has - already been made twice in this repository's history. + for the same placement reason :issue:`469` gives: a correct check + in the wrong position does not fire, and that placement mistake + has already been made twice in this repository's history. Guarding the field classes is necessary but not sufficient, because a ``_make_*`` that must know the address family before it can build the schema converts the argument itself and so never hands this module a :obj:`bool` at all. :func:`parse_ip_address` is where those callers - reach this guard (c.f. #508). + reach this guard (c.f. :issue:`508`). """ if isinstance(value, bool): @@ -142,46 +143,45 @@ def parse_ip_address(value: 'IPv4Address | IPv6Address | bytes | int | str', This is the sanctioned way for a ``_make_*`` method to turn a caller-supplied address into an :mod:`ipaddress` object, and it exists because doing it with :func:`ipaddress.ip_address` directly is what - #508 turned out to be: a ``_make_*`` that has to know the address - *family* before it can build the schema -- to size an option whose - length is the only thing on the wire that carries the family -- must - convert the argument itself, and that conversion happens **before** the - schema, so it launders a :obj:`bool` into an - :class:`~ipaddress.IPv4Address` that the guard added for #491 in - :meth:`_IPAddressField.pre_process` can then only see as a legitimate - address. Seven such call sites took ``True`` / ``False`` without - complaint as ``0.0.0.1`` / ``0.0.0.0`` -- or ``::1`` / ``::`` where the - wire format fixes the family as IPv6 -- and six of them went on to pack - those octets. The seventh, - :meth:`TCP._make_mptcp_addaddr + :issue:`508` turned out to be: a ``_make_*`` that has to know the + address *family* before it can build the schema -- to size an option + whose length is the only thing on the wire that carries the family -- + must convert the argument itself, and that conversion happens + **before** the schema, so it launders a :obj:`bool` into an + :class:`~ipaddress.IPv4Address` that the guard added for :issue:`491` + in :meth:`_IPAddressField.pre_process` can then only see as a + legitimate address. Seven such call sites took ``True`` / ``False`` + without complaint as ``0.0.0.1`` / ``0.0.0.0`` -- or ``::1`` / ``::`` + where the wire format fixes the family as IPv6 -- and six of them went + on to pack those octets. The seventh, :meth:`TCP._make_mptcp_addaddr `, built an equally corrupt schema and is only stopped from packing it by an unrelated defect of its own. Routing every one of them through here rather than giving each its own - :func:`isinstance` check is the whole point: #469 added exactly such a - check to :meth:`MH._make_opt_mn_id - `, and #491 was the - same defect surviving at every site that had not been thought of. A - guard that has to be remembered per call site is a guard that will be - forgotten at the next one. + :func:`isinstance` check is the whole point: :issue:`469` added + exactly such a check to :meth:`MH._make_opt_mn_id + `, and :issue:`491` + was the same defect surviving at every site that had not been thought + of. A guard that has to be remembered per call site is a guard that + will be forgotten at the next one. The :obj:`bool` rejection is the **first** statement here, ahead of any - dispatch on the value's type, for the placement reason #469 gives and - :func:`_reject_bool` repeats. + dispatch on the value's type, for the placement reason :issue:`469` + gives and :func:`_reject_bool` repeats. This raises :exc:`FieldValueError` and not :exc:`~pcapkit.utilities.exceptions.ProtocolError`, which is deliberate even though two sibling guards for the same mistake -- :meth:`MH._make_opt_mn_id - ` from #469 and - :class:`ESP's SecurityAssociation - ` from #491 -- raise - the latter. The layer decides: this is a field-level conversion, so it - answers with what :meth:`_IPAddressField.pre_process` answers with for - the identical value, and a caller sees one exception whether the - :obj:`bool` reached the field through the schema or through a - ``_make_*``. The two protocol-level guards answer for the *option*, + ` from :issue:`469` + and :class:`ESP's SecurityAssociation + ` from :issue:`491` + -- raise the latter. The layer decides: this is a field-level + conversion, so it answers with what :meth:`_IPAddressField.pre_process` + answers with for the identical value, and a caller sees one exception + whether the :obj:`bool` reached the field through the schema or through + a ``_make_*``. The two protocol-level guards answer for the *option*, alongside siblings that are not about addresses at all -- ``_make_opt_mn_id`` refuses a :obj:`bool` for all eight MN-ID subtypes, only one of which is address-typed -- so neither can route through here diff --git a/pcapkit/corekit/fields/misc.py b/pcapkit/corekit/fields/misc.py index ea194607f..d53977d10 100644 --- a/pcapkit/corekit/fields/misc.py +++ b/pcapkit/corekit/fields/misc.py @@ -595,7 +595,7 @@ def nested_packet_context(packet: 'dict[str, Any]') -> 'dict[str, Any]': :class:`dict` subclass adopted when the ``ChainMap`` was suspected of corrupting the shared :class:`~abc.ABCMeta` cache every :class:`Schema ` subclass used - to share on CPython <= 3.10 (issue #439), and then a + to share on CPython <= 3.10 (issue :issue:`439`), and then a :class:`~collections.ChainMap` again once that suspicion was doubted. A plain :class:`dict` ends the question: it satisfies every ``packet: 'dict[str, Any]'`` annotation on the rest of the field @@ -603,25 +603,25 @@ def nested_packet_context(packet: 'dict[str, Any]') -> 'dict[str, Any]': site, and it cannot interact with :class:`~abc.ABCMeta` at all because :class:`dict` is not an :class:`~abc.ABCMeta`-based class. - On the #439 suspicion itself, for the record, since it drove two - rewrites: it is *probably* wrong and no longer decidable. What is + On the :issue:`439` suspicion itself, for the record, since it drove + two rewrites: it is *probably* wrong and no longer decidable. What is directly measured is that the cache keys on the **exact type queried**, so asking about a :class:`~collections.ChainMap` instance caches lookups for :class:`~collections.ChainMap` and not for - :class:`dict`, and that the poisoning observed in #439 came from - ordinary code asking :func:`isinstance` about a plain :class:`dict` -- - :func:`~pcapkit.corekit.infoclass.Info.__update__` does exactly that. - Against that, swapping the ``ChainMap`` for a plain literal was, at - the time and on a real CPython 3.10 venv, enough to move - ``test_pcapng_remaining_constructor_branches_and_custom_dispatch`` + :class:`dict`, and that the poisoning observed in :issue:`439` came + from ordinary code asking :func:`isinstance` about a plain + :class:`dict` -- :func:`~pcapkit.corekit.infoclass.Info.__update__` + does exactly that. Against that, swapping the ``ChainMap`` for a plain + literal was, at the time and on a real CPython 3.10 venv, enough to + move ``test_pcapng_remaining_constructor_branches_and_custom_dispatch`` between passing and failing, toggled both ways. The likeliest reconciliation -- that the ``ChainMap`` was never causal but changed which concrete types flowed through unrelated :func:`isinstance` calls in the same run, and so changed *when* the pre-existing corruption fired -- is plausible rather than demonstrated, and cannot now be - tested: #439 has been fixed directly, every :class:`Schema` subclass - gets its own ``_abc_impl``, and the original conditions no longer - exist. It does not affect correctness either way. + tested: :issue:`439` has been fixed directly, every :class:`Schema` + subclass gets its own ``_abc_impl``, and the original conditions no + longer exist. It does not affect correctness either way. """ return {**packet, '__packet__': packet} diff --git a/pcapkit/corekit/fields/numbers.py b/pcapkit/corekit/fields/numbers.py index 893d3adc3..7f6ecc502 100644 --- a/pcapkit/corekit/fields/numbers.py +++ b/pcapkit/corekit/fields/numbers.py @@ -54,20 +54,20 @@ class NumberField(Field[int], Generic[_T]): ProtocolError: If ``bit_length`` is given negative. Left alone, ``(1 << bit_length) - 1`` raises a bare, uncatchable :exc:`ValueError` (``negative shift count``) here, before - :meth:`__call__`'s own negative-``length`` guard (#828) or - :attr:`~pcapkit.corekit.fields.field.FieldBase.length`'s (#805) - ever see anything -- this one fires at construction time, on the - argument itself rather than on a resolved wire length. See - GitHub issue #831. + :meth:`__call__`'s own negative-``length`` guard (:issue:`828`) or + :attr:`~pcapkit.corekit.fields.field.FieldBase.length`'s + (:issue:`805`) ever see anything -- this one fires at construction + time, on the argument itself rather than on a resolved wire length. + See GitHub issue :issue:`831`. Notes: A subclass such as :class:`UInt32Field` fixes the sign through ``__signed__``, so ``signed`` there is at best redundant. It used to be discarded outright, in both directions, which meant ``UInt32Field(signed=True)`` handed back an unsigned field whose values - only looked wrong once the high bit was set -- see GitHub issue #545. A - contradicting value is now rejected instead; omitting it, or passing the - sign the class already fixes, stays legal. + only looked wrong once the high bit was set -- see GitHub issue + :issue:`545`. A contradicting value is now rejected instead; omitting + it, or passing the sign the class already fixes, stays legal. """ @@ -153,27 +153,27 @@ def __call__(self, packet: 'dict[str, Any]') -> 'Self': a bare, uncatchable :exc:`ValueError` (``negative shift count``) when ``bit_length`` was not supplied, before :attr:`~pcapkit.corekit.fields.field.FieldBase.length` (see - its own :exc:`ProtocolError` guard, #805/#825) or - :meth:`build_template` ever sees the value: this method sets + its own :exc:`ProtocolError` guard, :issue:`805`/:issue:`825`) + or :meth:`build_template` ever sees the value: this method sets ``self._bit_length`` from the resolved length eagerly, as a cache, and shifts by it immediately, so the crash happens on - *this* line rather than on the later, already-guarded ones. - See GitHub issue #828. This guard runs regardless of whether - ``bit_length`` was supplied, so a field constructed with a - fixed ``bit_length`` *and* a callable ``length`` that resolves - negative raises the identical message as one with no + *this* line rather than on the later, already-guarded ones. See + GitHub issue :issue:`828`. This guard runs regardless of + whether ``bit_length`` was supplied, so a field constructed + with a fixed ``bit_length`` *and* a callable ``length`` that + resolves negative raises the identical message as one with no ``bit_length`` at all, rather than falling through to a ``template='...-1s'`` :exc:`ProtocolError` from :attr:`~pcapkit.corekit.fields.field.FieldBase.length` later -- - see GitHub issue #831. A resolved length of exactly ``0`` is a - legitimate empty field (e.g. ``len=4`` above resolving to - ``0``) and is left alone. + see GitHub issue :issue:`831`. A resolved length of exactly + ``0`` is a legitimate empty field (e.g. ``len=4`` above + resolving to ``0``) and is left alone. Notes: Rebuilding the template here is what applies a callable ``length``, and :meth:`build_template` recomputes ``self._need_process`` as it goes, so the flag and the template always describe the same width. - They did not always: see GitHub issue #591. + They did not always: see GitHub issue :issue:`591`. """ new_self = super().__call__(packet) @@ -215,7 +215,7 @@ def build_template(self, length: 'int', signed: 'bool') -> 'str': resolved the real width. :meth:`pre_process` consequently handed :obj:`bytes` to a template that had become ``>Q`` -- or ``>I``, ``>H``, ``>B`` -- and :func:`struct.pack` refused it. See GitHub - issue #591. + issue :issue:`591`. Assigning it is what tells a placeholder apart from a width that genuinely needs byte packing, without having to remember that a @@ -265,7 +265,7 @@ def pre_process(self, value: 'int', packet: 'dict[str, Any]') -> 'int | bytes': integer code for. The flag is therefore consulted **after** the rebuild rather than before it, since deciding first and rebuilding second is how the template and the value being returned came to - disagree in the first place. C.f. #591. + disagree in the first place. C.f. :issue:`591`. That width is a **ceiling** of the bit length over eight, and it is written as one. It used to read @@ -276,7 +276,7 @@ def pre_process(self, value: 'int', packet: 'dict[str, Any]') -> 'int | bytes': eight was therefore sized one octet short -- ``256`` at one octet, ``65536`` at two, and ``1`` itself at *zero* -- which :meth:`int.to_bytes` and :func:`struct.pack` both refuse. See GitHub - issue #599. + issue :issue:`599`. """ value = value & self._bit_mask @@ -576,14 +576,14 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'Stdli would have selected it. PCAP-NG repeats a block's total length at both ends precisely so that a reader can skip a block type it does not recognise; that skip is what this fallback restores. See GitHub - issue #701. + issue :issue:`701`. The fallback is the same nameless pseudo-member this method already builds for a field carrying no registry at all, so it is a value shape the package already produces and the dump layer already renders -- as ``:: [28]``, through :func:`~pcapkit.dumpkit.common.render_enum`, not through the - ``name is None`` branch #648 added, which a member named + ``name is None`` branch :issue:`648` added, which a member named ```` never takes -- and one an :class:`int`-keyed dispatch registry looks up by value like any declared member. It is built per value rather than grafted onto @@ -608,14 +608,14 @@ def post_process(self, value: 'int | bytes', packet: 'dict[str, Any]') -> 'Stdli from its guard today, and deliberately so: a generated guard raises a bare, unlogged :exc:`ValueError` precisely because the generated ``get()``'s ``except ValueError`` fallback has to keep catching it - (GitHub issues #584 and #647). The registries that *do* bound - themselves to a width and reject outside it are the bit-flag ones -- - :class:`pcapkit.const.tcp.flags.Flags` among them -- and none of - those is named as the namespace of a plain :class:`EnumField` - anywhere in the package, so no in-library guard loses its force - through this method. The distinction is therefore for a registry - registered from outside :mod:`pcapkit.const`, which has no such - obligation to stay quiet. + (GitHub issues :issue:`584` and :issue:`647`). The registries that + *do* bound themselves to a width and reject outside it are the + bit-flag ones -- :class:`pcapkit.const.tcp.flags.Flags` among them -- + and none of those is named as the namespace of a plain + :class:`EnumField` anywhere in the package, so no in-library guard + loses its force through this method. The distinction is therefore for + a registry registered from outside :mod:`pcapkit.const`, which has no + such obligation to stay quiet. """ value = super().post_process(value, packet) @@ -654,16 +654,16 @@ def _unregistered_member(namespace: 'Type[StdlibEnum] | Type[AenumEnum]', the registry's own ``get()`` -- resolved without anyone asking for a name. - GitHub issue #575: the owner's ruling is that an unassigned wire value - should resolve to a real member of the registry the field names -- - ``isinstance`` against it and every ancestor holds, and it renders and - dispatches exactly like a declared one -- provided building it never - grows the registry, which is the whole reason the field stopped - calling ``get()`` unconditionally in the first place. This is what - gets there: it calls ``namespace``'s own storage base's ``__new__`` - directly -- :class:`str` or :class:`int`, whichever ``namespace`` - derives from -- which skips ``namespace``'s *own* ``__new__`` - entirely, and with it the ``cls.__registry__.add(...)`` / + GitHub issue :issue:`575`: the owner's ruling is that an unassigned + wire value should resolve to a real member of the registry the field + names -- ``isinstance`` against it and every ancestor holds, and it + renders and dispatches exactly like a declared one -- provided + building it never grows the registry, which is the whole reason the + field stopped calling ``get()`` unconditionally in the first place. + This is what gets there: it calls ``namespace``'s own storage base's + ``__new__`` directly -- :class:`str` or :class:`int`, whichever + ``namespace`` derives from -- which skips ``namespace``'s *own* + ``__new__`` entirely, and with it the ``cls.__registry__.add(...)`` / ``cls.__members_ns__[...] = ...`` line every registry in this package uses to record a member it mints. No entry is added to ``_member_map_`` or ``_value2member_map_`` either, since those are diff --git a/pcapkit/corekit/infoclass.py b/pcapkit/corekit/infoclass.py index e5106198d..8db3076e6 100644 --- a/pcapkit/corekit/infoclass.py +++ b/pcapkit/corekit/infoclass.py @@ -35,8 +35,8 @@ class FinalisedState(EnumLookup, enum.IntEnum): """Finalised state. - Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub - issue #877's ruling that every non-registry enumeration shares that + Re-parented onto :class:`~pcapkit.corekit.enum.EnumLookup` per GitHub issue + :issue:`877`'s ruling that every non-registry enumeration shares that lookup contract -- pure re-parenting, since this class defines neither ``get`` nor ``_missing_`` of its own to reconcile with the base. diff --git a/pcapkit/corekit/io.py b/pcapkit/corekit/io.py index 7c051c8b4..eb597da63 100644 --- a/pcapkit/corekit/io.py +++ b/pcapkit/corekit/io.py @@ -414,7 +414,7 @@ def truncate(self, size: 'int | None' = None, /) -> 'int': for. It is also what CPython's own read-only buffered readers do: both the accelerated :class:`io.BufferedReader` and the pure-Python ``_pyio.BufferedReader`` raise :exc:`io.UnsupportedOperation`, the latter from - ``_BufferedIOMixin.truncate``'s ``_checkWritable()`` (#645). + ``_BufferedIOMixin.truncate``'s ``_checkWritable()`` (:issue:`645`). The resizing this used to perform is still reachable internally, as :meth:`_truncate_buffer`. It never touched the underlying stream in the first place @@ -432,12 +432,12 @@ def _truncate_buffer(self, size: 'int | None' = None, /) -> 'int': Note: This is the internal half of what :meth:`truncate` used to do, which is all of it: - nothing here writes to the underlying stream -- :meth:`write` raises -- so what this - resizes is the buffer, not the stream behind it. That is why :meth:`truncate` refuses - (#645) while this remains: the operation is a private-window one, not an - :class:`io.IOBase` write. The buffer is a sliding window over a stream that cannot be - seeked: its octet 0 sits at absolute offset ``_buffer_set``, its content occupies - ``[0:_buffer_cur]``, and everything past that is padding never read. + nothing here writes to the underlying stream -- :meth:`write` raises -- so what + this resizes is the buffer, not the stream behind it. That is why :meth:`truncate` + refuses (:issue:`645`) while this remains: the operation is a private-window one, + not an :class:`io.IOBase` write. The buffer is a sliding window over a stream that + cannot be seeked: its octet 0 sits at absolute offset ``_buffer_set``, its content + occupies ``[0:_buffer_cur]``, and everything past that is padding never read. Two consequences for which octets survive. An extension appends its zero octets at the **tail**, the new area being by definition the region past the old end. A @@ -494,10 +494,10 @@ def writable(self) -> 'bool': :meth:`truncate` will raise :exc:`OSError`. Note: - This was spelled ``writeable`` until #645, which is not how the :mod:`io` protocol - spells it, so it overrode nothing and :mod:`io` never consulted it -- the inherited - :meth:`io.IOBase.writable` answered instead. Both returned :data:`False`, so there - was no observable divergence to notice; the coincidence is what hid it. + This was spelled ``writeable`` until :issue:`645`, which is not how the :mod:`io` + protocol spells it, so it overrode nothing and :mod:`io` never consulted it -- the + inherited :meth:`io.IOBase.writable` answered instead. Both returned :data:`False`, + so there was no observable divergence to notice; the coincidence is what hid it. """ return False diff --git a/pcapkit/corekit/sentinels.py b/pcapkit/corekit/sentinels.py index 5a8505e64..bf8788740 100644 --- a/pcapkit/corekit/sentinels.py +++ b/pcapkit/corekit/sentinels.py @@ -17,7 +17,7 @@ :class:`NoValueType` in :mod:`pcapkit.corekit.fields.field`, :class:`NoDefaultType` in :mod:`pcapkit.corekit.enum` and :class:`AbsentType` in :mod:`pcapkit.protocols.protocol`. The owner's ruling -on GitHub issue #911, choosing one shared module over one module per +on GitHub issue :issue:`911`, choosing one shared module over one module per sentinel, moves the four *definitions* here; each original module keeps a three-line re-export so that no existing ``from import `` breaks, including the ``if TYPE_CHECKING:``-only imports of the *types* that @@ -37,10 +37,10 @@ its re-export exists only so that module's own code keeps reading ``ABSENT`` rather than a fully-qualified name; see :class:`AbsentType`'s own docstring below for why it stays private after the move. GitHub issue -#937 later dropped the leading underscore both used to carry (``_Absent``, -``_AbsentType``) in favour of SCREAMING_SNAKE/CamelCase like their two -siblings; the privacy this paragraph describes did not move with the name -- -see :class:`AbsentType`'s docstring for what carries it now. +:issue:`937` later dropped the leading underscore both used to carry +(``_Absent``, ``_AbsentType``) in favour of SCREAMING_SNAKE/CamelCase like +their two siblings; the privacy this paragraph describes did not move with the +name -- see :class:`AbsentType`'s docstring for what carries it now. """ from typing import TYPE_CHECKING @@ -65,7 +65,7 @@ class NullType: independently of each other -- so that ``is`` comparisons against it mean what they say: no :class:`str` a caller passes, including one that happens to spell ``'(null)'`` itself, can compare equal to this sentinel - by identity. See GitHub issue #833. + by identity. See GitHub issue :issue:`833`. Genuinely a singleton, not merely a class this module happens to instantiate once: :meth:`__new__` always hands back the one instance @@ -116,11 +116,11 @@ class NullType: of problem for the *class* it resolves, which is why it re-reads :data:`sys.modules` on every call rather than memoising; nothing equivalent is possible here, because unlike a resolved class there is no - live registry this sentinel could be re-read from. The pre-#833 ``str`` - sentinel had the same fragility for the same reason -- it is a property - of sharing one module-level binding across a reload, not something this - class's singleton guarantees claim to solve -- and nothing in this - package reloads :mod:`pcapkit.corekit.sentinels` after import. + live registry this sentinel could be re-read from. The pre-:issue:`833` + ``str`` sentinel had the same fragility for the same reason -- it is a + property of sharing one module-level binding across a reload, not + something this class's singleton guarantees claim to solve -- and nothing + in this package reloads :mod:`pcapkit.corekit.sentinels` after import. A second caveat, specific to this class now living apart from its one caller-visible re-export: reloading :mod:`pcapkit.corekit.module` @@ -190,10 +190,10 @@ def __reduce__(self) -> 'tuple[Callable[[], NullType], tuple[()]]': #: helpers in :mod:`pcapkit.foundation.registry.protocols` and #: :mod:`pcapkit.foundation.registry.foundation`. Housed here, alongside the #: package's other sentinels, rather than in :mod:`pcapkit.corekit.module` -#: where it used to live -- per the owner's ruling on GitHub issue #911, see -#: the module docstring above. :mod:`pcapkit.corekit.module` keeps a -#: re-export so every existing ``from pcapkit.corekit.module import NULL`` -#: keeps working. +#: where it used to live -- per the owner's ruling on GitHub issue +#: :issue:`911`, see the module docstring above. :mod:`pcapkit.corekit.module` +#: keeps a re-export so every existing +#: ``from pcapkit.corekit.module import NULL`` keeps working. NULL = NullType() @@ -212,7 +212,7 @@ def _get_null() -> 'NullType': class NoValueType: """Type of :data:`NO_VALUE`, the default value for :mod:`pcapkit.corekit.fields`. - Housed here per GitHub issue #911 rather than in + Housed here per GitHub issue :issue:`911` rather than in :mod:`pcapkit.corekit.fields.field`, where it used to be defined and where :attr:`FieldBase.default ` still documents it as the field-default sentinel. @@ -228,8 +228,8 @@ def __bool__(self) -> 'Literal[False]': #: :attr:`FieldBase.default `. #: :mod:`pcapkit.corekit.fields.field` keeps a re-export, since that #: attribute's own documentation is a published contract naming this object. -#: Renamed from ``NoValue`` to ``NO_VALUE`` by GitHub issue #937, which -#: normalised all four sentinel *objects* to SCREAMING_SNAKE. +#: Renamed from ``NoValue`` to ``NO_VALUE`` by GitHub issue :issue:`937`, +#: which normalised all four sentinel *objects* to SCREAMING_SNAKE. NO_VALUE = NoValueType() @@ -239,7 +239,7 @@ class NoDefaultType: :meth:`EnumLookup.get `. A dedicated class rather than a bare :class:`object`, per a ruling given in - review of the work for #857, which asked for a dedicated class that + review of the work for :issue:`857`, which asked for a dedicated class that follows the house convention. A bare :class:`object` compares under ``is`` exactly as safely as a dedicated class with no ``__eq__`` of its own does -- identity comparison was never the problem an earlier @@ -252,21 +252,21 @@ class NoDefaultType: Named ``NoDefaultType`` for the *class* because that half of the house convention is settled: both :class:`NullType` and :class:`NoValueType` use ``Type``. At the time, the *instance*'s own name was not - similarly settled -- a follow-up given in review of the work for #857 - was explicit that ``NULL`` (``SCREAMING_CASE``) and ``NoValue`` - (``CapWords``) disagreed, and that the choice should follow what each - sentinel is needed for rather than a settled rule. The need here was - continuity: - ``NO_DEFAULT`` was already the name on ``main`` -- referenced in + similarly settled -- a follow-up given in review of the work for + :issue:`857` was explicit that ``NULL`` (``SCREAMING_CASE``) and + ``NoValue`` (``CapWords``) disagreed, and that the choice should follow + what each sentinel is needed for rather than a settled rule. The need here + was continuity: ``NO_DEFAULT`` was already the name on ``main`` -- + referenced in :meth:`EnumLookup.get `'s signature, its docstring, and both comparison sites -- and that change was to *what the sentinel is*, not to *what it is called*, so it kept that name rather than being renamed to match either precedent's instance casing for its own sake. ``NULL``'s ``SCREAMING_CASE`` was the closer match regardless, since - :data:`NO_DEFAULT` was already spelled that way -- and GitHub issue #937 - later settled the question this paragraph left open: ``NoValue`` became - :data:`NO_VALUE` and ``_Absent`` became :data:`ABSENT`, so every instance - name now agrees on SCREAMING_SNAKE. + :data:`NO_DEFAULT` was already spelled that way -- and GitHub issue + :issue:`937` later settled the question this paragraph left open: + ``NoValue`` became :data:`NO_VALUE` and ``_Absent`` became :data:`ABSENT`, + so every instance name now agrees on SCREAMING_SNAKE. Genuinely a singleton, not merely a class this module happens to instantiate once: :meth:`__new__` always hands back the one instance that @@ -330,18 +330,19 @@ class NoDefaultType: ``held_default is NO_DEFAULT`` comparison against the post-reload global then reads :data:`False` where it once read :data:`True`. :meth:`EnumLookup.get ` looked - exactly like such a consumer before GitHub issue #864: an unrecognised, - non-``NO_DEFAULT`` value used to fall through to ``cls(default)``, so a - stale sentinel handed to that call could raise a :exc:`ValueError` a - caller had no reason to expect from an *omitted* argument. #864 closed a - different hole -- ``default`` could mint a new member -- by replacing - that call with a ``default not in cls._value2member_map_`` guard, and the - guard happens to close this one too: a :class:`NoDefaultType` instance, - stale or fresh, is never a registered enum value, so the guard's ``not - in`` half reads :data:`True` for it either way and ``get`` re-raises the - original lookup error correctly regardless of which :data:`NO_DEFAULT` - a caller's stale default is stale *against*. Measured on the current - tree, guard included:: + exactly like such a consumer before GitHub issue :issue:`864`: an + unrecognised, non-``NO_DEFAULT`` value used to fall through to + ``cls(default)``, so a stale sentinel handed to that call could raise a + :exc:`ValueError` a caller had no reason to expect from an *omitted* + argument. :issue:`864` closed a different hole -- ``default`` could mint + a new member -- by replacing that call with a + ``default not in cls._value2member_map_`` guard, and the guard happens to + close this one too: a :class:`NoDefaultType` instance, stale or fresh, is + never a registered enum value, so the guard's ``not in`` half reads + :data:`True` for it either way and ``get`` re-raises the original lookup + error correctly regardless of which :data:`NO_DEFAULT` a caller's stale + default is stale *against*. Measured on the current tree, guard + included:: >>> Hardware.get('Definitely-Not-A-Member') # before reload KeyError: 'Definitely-Not-A-Member' @@ -349,33 +350,34 @@ class NoDefaultType: >>> Hardware.get('Definitely-Not-A-Member') # after reload KeyError: 'Definitely-Not-A-Member' - So the docstring this class carried before GitHub issue #911's move -- - which claimed the second call above raises :exc:`ValueError` -- was - already wrong on ``main`` at ``d31c0aaf6``, independently of the move: - it described the pre-#864 ``cls(default)`` call, and nobody had - re-verified it against the guard #864 added afterwards. Fixed here as a - drive-by correction, not a consequence of the housing change itself. + So the docstring this class carried before GitHub issue :issue:`911`'s + move -- which claimed the second call above raises :exc:`ValueError` -- + was already wrong on ``main`` at ``d31c0aaf6``, independently of the + move: it described the pre-:issue:`864` ``cls(default)`` call, and + nobody had re-verified it against the guard :issue:`864` added + afterwards. Fixed here as a drive-by correction, not a consequence of + the housing change itself. None of that makes the underlying hazard theoretical elsewhere in this package: ``-1`` never had this failure mode at all, since ``-1 == -1`` compares by value rather than identity, and reload staleness is a *tracked* defect class here for other constructs -- see :meth:`pcapkit.protocols.protocol.ProtocolBase._lookup_next_layer`'s own - docstring note citing GitHub issues #425 and #555, and + docstring note citing GitHub issues :issue:`425` and :issue:`555`, and :mod:`tests.protocols.test_dispatch_default_resolution_unit`'s own ``test_no_stale_class_survives_a_module_reload``, which reloads a module deliberately to pin the fix for exactly that class of bug elsewhere. A *future* comparison site written the vulnerable way -- a bare ``is - NO_DEFAULT`` with no independent guard behind it, the way #864's fix - itself was not -- would still reproduce it. "Nothing in this package + NO_DEFAULT`` with no independent guard behind it, the way :issue:`864`'s + fix itself was not -- would still reproduce it. "Nothing in this package reloads :mod:`pcapkit.corekit.sentinels` after import" remains true today, but it is a caveat to keep honest rather than a guarantee this class enforces. .. note:: - GitHub issue #911 also changes *which* reload is the one that - matters, independently of the #864 finding above. + GitHub issue :issue:`911` also changes *which* reload is the one that + matters, independently of the :issue:`864` finding above. :mod:`pcapkit.corekit.enum` no longer defines ``NoDefaultType`` itself; it only reads :data:`NO_DEFAULT` off this module once, at its own import time, into its own module global. Reloading @@ -401,11 +403,11 @@ class enforces. ``if not default:`` instead of ``if default is NO_DEFAULT:`` would then read :data:`NO_DEFAULT` the same way it reads a caller's genuine falsy default -- ``0``, ``''``, ``None`` or ``False`` -- which is the exact - collision ``-1`` used to cause under ``==`` and the reason #857 exists. - Leaving ``__bool__`` undefined makes ``NoDefaultType()`` truthy (the - default for any object defining neither ``__bool__`` nor ``__len__``), - which at least does not *look* like one of the falsy values it must never - be mistaken for. + collision ``-1`` used to cause under ``==`` and the reason :issue:`857` + exists. Leaving ``__bool__`` undefined makes ``NoDefaultType()`` truthy + (the default for any object defining neither ``__bool__`` nor + ``__len__``), which at least does not *look* like one of the falsy values + it must never be mistaken for. """ @@ -452,18 +454,19 @@ class AbsentType: and means "no value was given", not "this key is not here". Defined here, alongside the package's other sentinels, per the owner's - ruling on GitHub issue #911. Originally named ``_AbsentType``/``_Absent``, - with the leading underscore standing in for "private" -- GitHub issue #937 - normalised every sentinel *object* to SCREAMING_SNAKE and dropped it, so - this pair now reads as CamelCase/SCREAMING_SNAKE like their two siblings - and privacy is no longer signalled by the name at all. The owner's ruling - on GitHub issue #719 accepted that rename, and held that documenting - ``ABSENT`` as a private type and class, not for public use, is enough - to replace the underscore. So this class and :data:`ABSENT` stay - exactly as private as they were: nothing outside - :mod:`pcapkit.protocols.protocol` reads :data:`ABSENT`, from here or from - there, and neither this module's nor that module's :attr:`__all__` names - either one. This docstring, and the "Naming a Sentinel" section of + ruling on GitHub issue :issue:`911`. Originally named + ``_AbsentType``/``_Absent``, with the leading underscore standing in for + "private" -- GitHub issue :issue:`937` normalised every sentinel *object* + to SCREAMING_SNAKE and dropped it, so this pair now reads as + CamelCase/SCREAMING_SNAKE like their two siblings and privacy is no longer + signalled by the name at all. The owner's ruling on GitHub issue + :issue:`719` accepted that rename, and held that documenting ``ABSENT`` as + a private type and class, not for public use, is enough to replace the + underscore. So this class and :data:`ABSENT` stay exactly as private as + they were: nothing outside :mod:`pcapkit.protocols.protocol` reads + :data:`ABSENT`, from here or from there, and neither this module's nor + that module's :attr:`__all__` names either one. This docstring, and the + "Naming a Sentinel" section of :file:`docs/source/contributing/conventions/sentinel-convention.rst`, are what now records that fact in place of the leading underscore. @@ -487,6 +490,6 @@ def __repr__(self) -> 'str': #: `. Never leaves #: :mod:`pcapkit.protocols.protocol`, which keeps a private re-export of it #: for exactly that one read. Private by convention and documentation only, -#: not by a leading underscore, which GitHub issue #937 dropped -- see -#: :class:`AbsentType`'s own docstring for why. +#: not by a leading underscore, which GitHub issue :issue:`937` dropped -- +#: see :class:`AbsentType`'s own docstring for why. ABSENT = AbsentType()