From 7090f84bd912866fb6c6c83f1c62c62d142ff9f1 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 29 Sep 2026 21:24:48 -0400 Subject: [PATCH] refactor(corekit,protocols): normalise the four sentinel objects to SCREAMING_SNAKE (#937) Rename NoValue -> NO_VALUE and _Absent -> ABSENT (type _AbsentType -> AbsentType), per the owner's ruling on #937: accept the breaking change, no backport. - Update pcapkit/corekit/sentinels.py, its re-export shims (fields/field.py, protocols/protocol.py) and every other call site in pcapkit/ and tests/, including the 3 references in tests/protocols/internet/test_mh_unit.py (:1869, :1875, :1879) -- initially thought off-limits alongside its zero-reference source mh.py (owned by #935), but the test itself was never actually measured. - Keep ABSENT/AbsentType out of every __all__: dropping the underscore removes the mechanical privacy signal, so protocol.py and test_sentinel_exports_unit.py now enforce it by assertion alone. - Document AbsentType/ABSENT on sentinels.rst as explicitly private, since Sphinx no longer hides them automatically; add the object-naming rule to conventions.rst's sentinel-convention section and update its table/prose. - Simplify test_sentinel_exports_unit.py's _expected_type_name to one mechanical rule; add test_every_sentinel_object_is_screaming_snake, confirmed to fail on 382375811 (NoValue, _Absent) and pass here. Build: isort clean (order_by_type sorts NO_VALUE ahead of Field/FieldBase -- a pure casing change reordered 5 files); mypy identical (321/38) and pylint's non-cyclic-import diagnostics identical to origin/main -- cyclic-import counts vary run-to-run even on an unmodified baseline (236 differing lines measured). Targeted suites pass, including the CI-caught mh.py padding test. --- docs/source/contributing/conventions.rst | 69 +++++--- docs/source/pcapkit/corekit/fields/field.rst | 2 +- docs/source/pcapkit/corekit/sentinels.rst | 21 ++- pcapkit/corekit/fields/field.py | 14 +- pcapkit/corekit/fields/ipaddress.py | 10 +- pcapkit/corekit/fields/misc.py | 22 +-- pcapkit/corekit/fields/numbers.py | 6 +- pcapkit/corekit/fields/strings.py | 8 +- pcapkit/corekit/sentinels.py | 78 +++++---- pcapkit/protocols/internet/hopopt.py | 4 +- pcapkit/protocols/internet/ipv6_opts.py | 4 +- pcapkit/protocols/protocol.py | 6 +- .../protocols/schema/application/httpv2.py | 2 +- pcapkit/protocols/schema/internet/hopopt.py | 8 +- .../protocols/schema/internet/ipv6_opts.py | 8 +- pcapkit/protocols/schema/internet/mh.py | 6 +- pcapkit/protocols/schema/schema.py | 20 +-- tests/corekit/test_sentinel_exports_unit.py | 163 +++++++++++++----- tests/corekit/test_sentinels_housing_unit.py | 24 +-- .../test_httpv2_payload_length_unit.py | 16 +- .../internet/test_ipv6_extension_unit.py | 10 +- tests/protocols/internet/test_mh_unit.py | 6 +- tests/protocols/misc/test_pcapng_unit.py | 2 +- tests/protocols/schema/test_schema_unit.py | 6 +- .../test_construction_keyword_check_unit.py | 48 +++--- 25 files changed, 346 insertions(+), 217 deletions(-) diff --git a/docs/source/contributing/conventions.rst b/docs/source/contributing/conventions.rst index d7d7968de0..d12db5187f 100644 --- a/docs/source/contributing/conventions.rst +++ b/docs/source/contributing/conventions.rst @@ -146,12 +146,17 @@ Naming a sentinel A *sentinel* here is a module-level singleton whose only job is to be recognised by identity -- ``value is SENTINEL`` -- so that it can never be confused with a value a -caller might legitimately pass. The house rule, from the maintainer: +caller might legitimately pass. The house rule, from the maintainer, covers the type: Keep the sentinel object's type class naming as ``Type``. -That is, the class takes the instance's name in CamelCase with ``Type`` appended. The -four in the tree follow it: +That is, the class takes the instance's name in CamelCase with ``Type`` appended. It +says nothing about the **object**'s own name, which is what let three casings diverge +with no rule naming any of them wrong. GitHub issue #937 closed that gap, verbatim: +*"take SCREAMING_SNAKE and accept the breaking change (no backport needed)."* So the +object is named in SCREAMING_SNAKE, and the type-naming rule above derives from it +mechanically -- title-case each underscore-separated word and append ``Type``, no +per-sentinel exception needed. The four in the tree follow it: .. list-table:: :header-rows: 1 @@ -163,14 +168,14 @@ four in the tree follow it: * - ``NULL`` - ``NullType`` - :mod:`pcapkit.corekit.sentinels` - * - ``NoValue`` + * - ``NO_VALUE`` - ``NoValueType`` - :mod:`pcapkit.corekit.sentinels` * - ``NO_DEFAULT`` - ``NoDefaultType`` - :mod:`pcapkit.corekit.sentinels` - * - ``_Absent`` - - ``_AbsentType`` + * - ``ABSENT`` + - ``AbsentType`` - :mod:`pcapkit.corekit.sentinels` All four used to live beside the one class that used them -- @@ -182,17 +187,24 @@ names; each original module keeps a re-export so every existing ``from import `` keeps working, including the ``if TYPE_CHECKING:``-only imports of the types. -Note what the rule does **not** fix: the **instance** name's casing is deliberately -free, which is why ``NULL`` and ``NoValue`` disagree and both are correct. Pick -whichever reads better at the call site, and where a name already exists, keep it -- -renaming a published sentinel costs every caller for no gain. - -Nor does it fix the **leading underscore**. ``_Absent`` is private -- it is read in -``_declared_keywords`` and discarded there, never leaving -:mod:`pcapkit.protocols.protocol` even though its *definition* now does -- and it is -still held to the convention, which is why its type is ``_AbsentType`` and not -``_Absent_t`` or ``Absent``. It is a deliberate fourth rather than an accident, and -its own docstring (:file:`pcapkit/corekit/sentinels.py`, line 430) says so: +Before GitHub issue #937, the **instance** name's casing was deliberately free, which is +why ``NULL`` and ``NoValue`` disagreed and both were called correct -- three sentinels +had already picked three different casings (``NULL`` SCREAMING_SNAKE, ``NoValue`` +CamelCase, ``_Absent`` CamelCase with a leading underscore) before anyone ruled on it. +#937's ruling closes that: SCREAMING_SNAKE is now the one answer, and the two renames +it made -- ``NoValue`` to ``NO_VALUE``, ``_Absent`` to ``ABSENT`` -- are the breaking +change it accepted rather than deprecating. Where a sentinel name already exists and +already follows SCREAMING_SNAKE, keep it; renaming a published sentinel again costs +every caller for no further gain. + +The rename also dropped the **leading underscore** ``_Absent``/``_AbsentType`` used to +carry. ``ABSENT`` is private -- it is read in ``_declared_keywords`` and discarded +there, never leaving :mod:`pcapkit.protocols.protocol` -- and the underscore used to be +the mechanical signal of that. The maintainer's ruling on #937, verbatim: *"we can +change* ``_ABSENT`` *to* ``ABSENT`` *just document it as private type/class in the +documentation and not for public use is enough."* So privacy is documentation-only from +here on, carried by this paragraph and by :class:`AbsentType`'s own docstring +(:file:`pcapkit/corekit/sentinels.py`, line 439), which still says so: A distinct class rather than a bare :obj:`object` so that the sentinel has a name of its own in a traceback or a debugger, and so that a type checker has something @@ -200,23 +212,28 @@ its own docstring (:file:`pcapkit/corekit/sentinels.py`, line 430) says so: :class:`NoValueType`, which does the same job for an unset field default; this is a sibling of it rather than a reuse [...] -The private name is also why this table listed three for as long as it did: a sweep -filtered on capitalised names does not see it. When adding a sentinel, add it here -whether or not it is public. +It remains a deliberate fourth rather than an accident: the leading underscore's +absence is also why this table once listed three for as long as it did -- a sweep +filtered on capitalised names did not see ``_Absent`` -- and that history does not +change now that nothing in the name itself marks it out. When adding a sentinel, add +it here whether or not it is public. What reaches users is the **object only**. The maintainer's ruling: *"we should ONLY export the objects (like* ``NULL`` *) to users"* -- so a public sentinel names its instance in its module's ``__all__`` and leaves the type out of it (GitHub issue #911). The type stays importable by its dotted path, for an annotation or an ``is`` guard; it -is ``import *`` that no longer offers it. A private sentinel such as ``_Absent`` is in -neither, which is what private means here. +is ``import *`` that no longer offers it. A private sentinel such as ``ABSENT`` is in +neither, which is what private means here -- dropping its leading underscore did not +add it to either list, and :class:`AbsentType` and :data:`ABSENT` are documented on +:doc:`the sentinels API page ` as private and not for +public use rather than left off it, since the name alone no longer says so. .. note:: Of the four, only :class:`~pcapkit.corekit.sentinels.NullType` is a full worked example. ``NoValueType`` follows the naming rule but is **not** a singleton - (``NoValueType() is NoValue`` is :obj:`False`) and has no ``__repr__`` of its own, - so it demonstrates the name and nothing else; ``_AbsentType`` has a ``__repr__`` + (``NoValueType() is NO_VALUE`` is :obj:`False`) and has no ``__repr__`` of its own, + so it demonstrates the name and nothing else; ``AbsentType`` has a ``__repr__`` (````) but no singleton guard either. Copy ``NullType`` when you need a pattern to follow. @@ -270,7 +287,7 @@ inconsistencies**: has any effect on it. ``__bool__`` returning :obj:`False` - ``NULL``, ``NoValue`` and ``_Absent`` have it, because each stands for an *absent + ``NULL``, ``NO_VALUE`` and ``ABSENT`` have it, because each stands for an *absent value* and reads naturally in a boolean test. ``NO_DEFAULT`` deliberately does **not**: it is a marker meaning *no default was supplied*, it is only ever tested with ``is``, and making it falsy would invite ``if not default:`` -- which would @@ -282,7 +299,7 @@ inconsistencies**: :class:`~pcapkit.corekit.sentinels.NullType` has them because ``NULL`` is stored in a :class:`~pcapkit.corekit.module.ModuleDescriptor` field, so a caller's :func:`copy.deepcopy` or :mod:`pickle` can walk into it and would otherwise - reconstruct a second instance. ``NO_DEFAULT`` and ``_Absent`` have none, because + reconstruct a second instance. ``NO_DEFAULT`` and ``ABSENT`` have none, because neither is ever stored in any structure a caller copies -- one only ever appears as a default argument, and the other never leaves the module that reads it. Add them when, and only when, the sentinel becomes reachable from something diff --git a/docs/source/pcapkit/corekit/fields/field.rst b/docs/source/pcapkit/corekit/fields/field.rst index dad63a2919..06561c21f2 100644 --- a/docs/source/pcapkit/corekit/fields/field.rst +++ b/docs/source/pcapkit/corekit/fields/field.rst @@ -23,7 +23,7 @@ Auxiliaries ----------- .. autoclass:: pcapkit.corekit.fields.field.NoValueType -.. autodata:: pcapkit.corekit.fields.field.NoValue +.. autodata:: pcapkit.corekit.fields.field.NO_VALUE :no-value: Internal Definitions diff --git a/docs/source/pcapkit/corekit/sentinels.rst b/docs/source/pcapkit/corekit/sentinels.rst index 403a8a9d04..d92299283e 100644 --- a/docs/source/pcapkit/corekit/sentinels.rst +++ b/docs/source/pcapkit/corekit/sentinels.rst @@ -12,7 +12,7 @@ Each of the three below used to live beside the one class that consumed it -- :class:`NullType` in :mod:`pcapkit.corekit.module`, :class:`NoValueType` in :mod:`pcapkit.corekit.fields.field` and :class:`NoDefaultType` in :mod:`pcapkit.corekit.enum` -- until GitHub issue #911 moved all four -definitions here, the private ``_AbsentType``/``_Absent`` included. Each +definitions here, the private ``AbsentType``/``ABSENT`` included. Each original module keeps a re-export, so every existing ``from import `` keeps working unchanged. @@ -20,8 +20,25 @@ original module keeps a re-export, so every existing ``from import .. autodata:: pcapkit.corekit.sentinels.NULL .. autoclass:: pcapkit.corekit.sentinels.NoValueType -.. autodata:: pcapkit.corekit.sentinels.NoValue +.. autodata:: pcapkit.corekit.sentinels.NO_VALUE :no-value: .. autoclass:: pcapkit.corekit.sentinels.NoDefaultType .. autodata:: pcapkit.corekit.sentinels.NO_DEFAULT + +.. note:: + + :class:`AbsentType` and :data:`ABSENT` are **private** -- never imported outside + :mod:`pcapkit.protocols.protocol`, and named in no module's ``__all__``. Until + GitHub issue #937, the leading underscore they carried (``_AbsentType``/ + ``_Absent``) hid them from Sphinx automatically, the way it hides every other + ``_``-prefixed name; dropping the underscore for SCREAMING_SNAKE consistency with + the other three sentinels (see :ref:`sentinel-convention`) means Sphinx would + otherwise document them as though they were public. They are documented below + instead, explicitly marked private, per the maintainer's ruling on #937: *"we can + change* ``_ABSENT`` *to* ``ABSENT`` *just document it as private type/class in the + documentation and not for public use is enough."* Neither is for use outside this + package. + +.. autoclass:: pcapkit.corekit.sentinels.AbsentType +.. autodata:: pcapkit.corekit.sentinels.ABSENT diff --git a/pcapkit/corekit/fields/field.py b/pcapkit/corekit/fields/field.py index d765e49563..a6caaf72f1 100644 --- a/pcapkit/corekit/fields/field.py +++ b/pcapkit/corekit/fields/field.py @@ -8,10 +8,10 @@ import struct from typing import TYPE_CHECKING, Generic, TypeVar, cast -from pcapkit.corekit.sentinels import NoValue, NoValueType # pylint: disable=unused-import +from pcapkit.corekit.sentinels import NO_VALUE, NoValueType # pylint: disable=unused-import from pcapkit.utilities.exceptions import FieldValueError, NoDefaultValue, ProtocolError -__all__ = ['NoValue', 'Field'] +__all__ = ['NO_VALUE', 'Field'] if TYPE_CHECKING: from typing import IO, Any, Callable, Iterator, Optional @@ -248,7 +248,7 @@ class FieldBase(Generic[_T], metaclass=FieldMeta): # fields takes no default value of its own -- and reading :attr:`default` off # one of those raised :exc:`AttributeError` for a private attribute rather # than reporting that the field declares no default. See #422. - _default: '_T | NoValueType' = NoValue + _default: '_T | NoValueType' = NO_VALUE @property def name(self) -> 'str': @@ -273,7 +273,7 @@ def default(self, value: '_T | NoValueType') -> 'None': @default.deleter def default(self) -> 'None': """Delete field default value.""" - self._default = NoValue + self._default = NO_VALUE @property def template(self) -> 'str': @@ -347,7 +347,7 @@ def __init__(self, *args: 'Any', **kwargs: 'Any') -> 'None': if not hasattr(self, '_name'): self._name = f'<{type(self).__name__[:-5].lower()}>' - self._default = NoValue + self._default = NO_VALUE self._template = '0s' self._callback = lambda *_: None @@ -428,7 +428,7 @@ def pack(self, value: 'Optional[_T]', packet: 'dict[str, Any]') -> 'bytes': """ if value is None: - if self._default is NoValue: + if self._default is NO_VALUE: raise NoDefaultValue(f'Field {self.name} has no default value.') value = cast('_T', self._default) @@ -609,7 +609,7 @@ def template(self) -> 'str': return self._template def __init__(self, length: 'int | Callable[[dict[str, Any]], int]', - default: '_T | NoValueType' = NoValue, + default: '_T | NoValueType' = NO_VALUE, callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': #self._name = '' if not hasattr(self, '_name'): diff --git a/pcapkit/corekit/fields/ipaddress.py b/pcapkit/corekit/fields/ipaddress.py index 27cf8dbe74..4260a1963e 100644 --- a/pcapkit/corekit/fields/ipaddress.py +++ b/pcapkit/corekit/fields/ipaddress.py @@ -6,7 +6,7 @@ import ipaddress from typing import TYPE_CHECKING, Generic, TypeVar, cast -from pcapkit.corekit.fields.field import Field, NoValue +from pcapkit.corekit.fields.field import NO_VALUE, Field from pcapkit.utilities.exceptions import FieldValueError __all__ = [ @@ -310,7 +310,7 @@ def version(self) -> 'Literal[4]': """IP version number.""" return 4 - def __init__(self, default: 'IPv4Address | NoValueType' = NoValue, + def __init__(self, default: 'IPv4Address | NoValueType' = NO_VALUE, callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': super().__init__(4, default, callback) @@ -332,7 +332,7 @@ def version(self) -> 'Literal[6]': """IP version number.""" return 6 - def __init__(self, default: 'IPv6Address | NoValueType' = NoValue, + def __init__(self, default: 'IPv6Address | NoValueType' = NO_VALUE, callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': super().__init__(16, default, callback) @@ -367,7 +367,7 @@ def version(self) -> 'Literal[4]': """IP version number.""" return 4 - def __init__(self, default: 'IPv4Interface | NoValueType' = NoValue, + def __init__(self, default: 'IPv4Interface | NoValueType' = NO_VALUE, callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': super().__init__(8, default, callback) @@ -455,7 +455,7 @@ def version(self) -> 'Literal[6]': """IP version number.""" return 6 - def __init__(self, default: 'IPv6Interface | NoValueType' = NoValue, + def __init__(self, default: 'IPv6Interface | NoValueType' = NO_VALUE, callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': super().__init__(17, default, callback) diff --git a/pcapkit/corekit/fields/misc.py b/pcapkit/corekit/fields/misc.py index 48fa179960..ea194607f9 100644 --- a/pcapkit/corekit/fields/misc.py +++ b/pcapkit/corekit/fields/misc.py @@ -4,7 +4,7 @@ import io from typing import TYPE_CHECKING, TypeVar, cast -from pcapkit.corekit.fields.field import FieldBase, NoValue +from pcapkit.corekit.fields.field import NO_VALUE, FieldBase from pcapkit.utilities.exceptions import FieldError, NoDefaultValue from pcapkit.utilities.warnings import RegistryWarning, warn @@ -32,7 +32,7 @@ class NoValueField(FieldBase[_TN]): """Schema field for no value type (or :obj:`None`).""" - _default = NoValue + _default = NO_VALUE @property def template(self) -> 'str': @@ -105,7 +105,7 @@ def default(self, value: '_TC | NoValueType') -> 'None': @default.deleter def default(self) -> 'None': """Delete field default value.""" - self._field.default = NoValue + self._field.default = NO_VALUE @property def template(self) -> 'str': @@ -303,7 +303,7 @@ def protocol(self, protocol: 'Type[_TP] | str') -> 'None': self._protocol = protocol def __init__(self, length: 'int | Callable[[dict[str, Any]], int]' = lambda _: -1, - default: '_TP | NoValueType | bytes' = NoValue, + default: '_TP | NoValueType | bytes' = NO_VALUE, protocol: 'Optional[Type[_TP] | str]' = None, callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': #self._name = '' @@ -363,7 +363,7 @@ def pack(self, value: 'Optional[_TP | Schema | bytes]', packet: 'dict[str, Any]' """ if value is None: - if self._default is NoValue: + if self._default is NO_VALUE: raise NoDefaultValue(f'Field {self.name} has no default value.') value = cast('_TP', self._default) @@ -432,7 +432,7 @@ def default(self, value: '_TC | NoValueType') -> 'None': @default.deleter def default(self) -> 'None': """Delete field default value.""" - self._field.default = NoValue + self._field.default = NO_VALUE @property def template(self) -> 'str': @@ -489,7 +489,7 @@ def pre_process(self, value: '_TC', packet: 'dict[str, Any]') -> 'Any': # pylin """ if self._field is None: - return NoValue # type: ignore[unreachable] + return NO_VALUE # type: ignore[unreachable] return self._field.pre_process(value, packet) def pack(self, value: 'Optional[_TC]', packet: 'dict[str, Any]') -> 'bytes': @@ -519,7 +519,7 @@ def post_process(self, value: 'Any', packet: 'dict[str, Any]') -> '_TC': # pyli """ if self._field is None: - return NoValue # type: ignore[unreachable] + return NO_VALUE # type: ignore[unreachable] return self._field.post_process(value, packet) def unpack(self, buffer: 'bytes | IO[bytes]', packet: 'dict[str, Any]') -> '_TC': @@ -658,7 +658,7 @@ def schema(self) -> 'Type[_TS]': def __init__(self, length: 'int | Callable[[dict[str, Any]], int]' = lambda _: -1, schema: 'Optional[Type[_TS]]' = None, - default: '_TS | NoValueType | bytes' = NoValue, + default: '_TS | NoValueType | bytes' = NO_VALUE, packet: 'Optional[dict[str, Any]]' = None, callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': #self._name = '' @@ -720,7 +720,7 @@ def pack(self, value: 'Optional[_TS | bytes]', packet: 'dict[str, Any]') -> 'byt """ if value is None: - if self._default is NoValue: + if self._default is NO_VALUE: raise NoDefaultValue(f'Field {self.name} has no default value.') value = cast('_TS', self._default) @@ -788,7 +788,7 @@ def default(self, value: '_TC | NoValueType') -> 'None': @default.deleter def default(self) -> 'None': """Delete field default value.""" - self._field.default = NoValue + self._field.default = NO_VALUE @property def template(self) -> 'str': diff --git a/pcapkit/corekit/fields/numbers.py b/pcapkit/corekit/fields/numbers.py index 7cec395021..d2fdcde41f 100644 --- a/pcapkit/corekit/fields/numbers.py +++ b/pcapkit/corekit/fields/numbers.py @@ -8,7 +8,7 @@ import aenum -from pcapkit.corekit.fields.field import Field, NoValue +from pcapkit.corekit.fields.field import NO_VALUE, Field from pcapkit.utilities.exceptions import BaseError, FieldValueError, IntError, ProtocolError __all__ = [ @@ -81,7 +81,7 @@ def bit_length(self) -> 'int': return self._bit_length def __init__(self, length: 'Optional[int | Callable[[dict[str, Any]], int]]' = None, - default: 'int | NoValueType' = NoValue, signed: 'Optional[bool]' = None, + default: 'int | NoValueType' = NO_VALUE, signed: 'Optional[bool]' = None, byteorder: 'Literal["little", "big"]' = 'big', bit_length: 'Optional[int]' = None, callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': @@ -532,7 +532,7 @@ class EnumField(NumberField[Union[enum.IntEnum, aenum.IntEnum]]): """ def __init__(self, length: 'int | Callable[[dict[str, Any]], int]', - default: 'StdlibEnum | AenumEnum | NoValueType' = NoValue, + default: 'StdlibEnum | AenumEnum | NoValueType' = NO_VALUE, signed: 'Optional[bool]' = None, byteorder: 'Literal["little", "big"]' = 'big', bit_length: 'Optional[int]' = None, diff --git a/pcapkit/corekit/fields/strings.py b/pcapkit/corekit/fields/strings.py index e56a9e91c6..bfed576e11 100644 --- a/pcapkit/corekit/fields/strings.py +++ b/pcapkit/corekit/fields/strings.py @@ -4,7 +4,7 @@ import urllib.parse as urllib_parse from typing import TYPE_CHECKING, Any, Generic, TypeVar -from pcapkit.corekit.fields.field import Field, NoValue +from pcapkit.corekit.fields.field import NO_VALUE, Field from pcapkit.utilities.chardet import detect from pcapkit.utilities.compat import Dict from pcapkit.utilities.exceptions import FieldValueError @@ -41,7 +41,7 @@ class _TextField(Field[_T], Generic[_T]): """ def __init__(self, length: 'int | Callable[[dict[str, Any]], int]', - default: '_T | NoValueType' = NoValue, + default: '_T | NoValueType' = NO_VALUE, callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': super().__init__(length, default, callback) # type: ignore[arg-type] @@ -121,7 +121,7 @@ class StringField(_TextField[str]): """ def __init__(self, length: 'int | Callable[[dict[str, Any]], int]', - default: 'str | NoValueType' = NoValue, encoding: 'Optional[str]' = None, + default: 'str | NoValueType' = NO_VALUE, encoding: 'Optional[str]' = None, errors: 'Literal["strict", "ignore", "replace"]' = 'strict', unquote: 'bool' = False, callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': @@ -189,7 +189,7 @@ class BitField(_TextField[Dict[str, Any]]): """ def __init__(self, length: 'int', - default: 'dict[str, Any] | NoValueType' = NoValue, + default: 'dict[str, Any] | NoValueType' = NO_VALUE, namespace: 'Optional[dict[str, NamespaceEntry]]' = None, callback: 'Callable[[Self, dict[str, Any]], None]' = lambda *_: None) -> 'None': super().__init__(length, default, callback) diff --git a/pcapkit/corekit/sentinels.py b/pcapkit/corekit/sentinels.py index aa94c16b90..03c1aef676 100644 --- a/pcapkit/corekit/sentinels.py +++ b/pcapkit/corekit/sentinels.py @@ -16,7 +16,7 @@ used it: :class:`NullType` in :mod:`pcapkit.corekit.module`, :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 +:class:`AbsentType` in :mod:`pcapkit.protocols.protocol`. The owner's ruling on GitHub issue #911, verbatim -- *"Okay one module for all four it is."* -- moves the four *definitions* here; each original module keeps a three-line re-export so that no existing ``from import `` breaks, @@ -27,23 +27,27 @@ :mod:`~pcapkit.corekit.fields.numbers` and :mod:`~pcapkit.corekit.fields.strings` already carry. -``NoValue`` and ``_Absent`` differ in how far the ruling reaches. ``NoValue`` +``NO_VALUE`` and ``ABSENT`` differ in how far the ruling reaches. ``NO_VALUE`` is documented as the value of :attr:`FieldBase.default `, so it is a published contract and the re-export at :mod:`pcapkit.corekit.fields.field` is load-bearing for callers outside this -package. ``_Absent`` is private to :mod:`pcapkit.protocols.protocol` -- +package. ``ABSENT`` is private to :mod:`pcapkit.protocols.protocol` -- nothing outside that module ever imports it, from here or from there -- so 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. +``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. """ from typing import TYPE_CHECKING from pcapkit.utilities.compat import final -__all__ = ['NULL', 'NoValue', 'NO_DEFAULT'] +__all__ = ['NULL', 'NO_VALUE', 'NO_DEFAULT'] if TYPE_CHECKING: from typing import Any, Callable @@ -206,7 +210,7 @@ def _get_null() -> 'NullType': @final class NoValueType: - """Type of :data:`NoValue`, the default value for :mod:`pcapkit.corekit.fields`. + """Type of :data:`NO_VALUE`, the default value for :mod:`pcapkit.corekit.fields`. Housed here per GitHub issue #911 rather than in :mod:`pcapkit.corekit.fields.field`, where it used to be defined and where @@ -224,7 +228,9 @@ 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. -NoValue = NoValueType() +#: Renamed from ``NoValue`` to ``NO_VALUE`` by GitHub issue #937, which +#: normalised all four sentinel *objects* to SCREAMING_SNAKE. +NO_VALUE = NoValueType() @final @@ -244,17 +250,20 @@ class NoDefaultType: Named ``NoDefaultType`` for the *class* because that half of the house convention is settled: both :class:`NullType` and :class:`NoValueType` - use ``Type``. The *instance*'s own name is not similarly settled -- - the owner's follow-up on #859 is explicit that ``NULL`` (``SCREAMING_CASE``) - and ``NoValue`` (``CapWords``) disagree, and "mainly depends on how we need - it." The need here is continuity: ``NO_DEFAULT`` is already the name on - ``main`` -- referenced in :meth:`EnumLookup.get - `'s signature, its docstring, and both - comparison sites -- and this change is to *what the sentinel is*, not to - *what it is called*, so it keeps that name rather than being renamed to - match either precedent's instance casing for its own sake. ``NULL``'s - ``SCREAMING_CASE`` is the closer match regardless, since :data:`NO_DEFAULT` - was already spelled that way. + use ``Type``. At the time, the *instance*'s own name was not + similarly settled -- the owner's follow-up on #859 was explicit that + ``NULL`` (``SCREAMING_CASE``) and ``NoValue`` (``CapWords``) disagreed, and + "mainly depends on how we need it." 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. Genuinely a singleton, not merely a class this module happens to instantiate once: :meth:`__new__` always hands back the one instance that @@ -427,8 +436,8 @@ def __repr__(self) -> 'str': @final -class _AbsentType: - """Type of :data:`_Absent`, the absent-key sentinel. +class AbsentType: + """Type of :data:`ABSENT`, the absent-key sentinel. A distinct class rather than a bare :obj:`object` so that the sentinel has a name of its own in a traceback or a debugger, and so that a type checker has @@ -440,11 +449,20 @@ 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 -- but it stays exactly as private as it was - in :mod:`pcapkit.protocols.protocol`: nothing outside that module reads - :data:`_Absent`, from here or from there, and this module's own - :attr:`__all__` does not name it. The move relocates the *definition*, - not the visibility. + 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 #937, verbatim: *"we can change* ``_ABSENT`` *to* ``ABSENT`` *just + document it as private type/class in the documentation and not for public + use is enough."* 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.rst`, are what now records + that fact in place of the leading underscore. """ @@ -457,7 +475,7 @@ def __repr__(self) -> 'str': return '' -#: _AbsentType: Absent-versus-:obj:`None` sentinel for +#: AbsentType: Absent-versus-:obj:`None` sentinel for #: :func:`pcapkit.protocols.protocol._declared_keywords` to read a class's #: own ``__keywords__`` out of its :attr:`~object.__dict__`, where #: :obj:`None` is itself a meaningful value -- the opt-out that says the @@ -465,5 +483,7 @@ def __repr__(self) -> 'str': #: :attr:`ProtocolBase.__keywords__ #: `. Never leaves #: :mod:`pcapkit.protocols.protocol`, which keeps a private re-export of it -#: for exactly that one read. -_Absent = _AbsentType() +#: for exactly that one read. Private by convention and documentation only, +#: not by a leading underscore -- see :class:`AbsentType`'s own docstring for +#: why, per GitHub issue #937. +ABSENT = AbsentType() diff --git a/pcapkit/protocols/internet/hopopt.py b/pcapkit/protocols/internet/hopopt.py index 507fb0a79c..973075e01e 100644 --- a/pcapkit/protocols/internet/hopopt.py +++ b/pcapkit/protocols/internet/hopopt.py @@ -35,7 +35,7 @@ from pcapkit.const.ipv6.smf_dpd_mode import SMFDPDMode as Enum_SMFDPDMode from pcapkit.const.ipv6.tagger_id import TaggerID as Enum_TaggerID from pcapkit.const.reg.transtype import TransType as Enum_TransType -from pcapkit.corekit.fields.field import NoValue +from pcapkit.corekit.fields.field import NO_VALUE from pcapkit.corekit.multidict import OrderedMultiDict from pcapkit.protocols.data.internet.hopopt import HOPOPT as Data_HOPOPT from pcapkit.protocols.data.internet.hopopt import CALIPSOOption as Data_CALIPSOOption @@ -1089,7 +1089,7 @@ def _read_opt_mpl(self, schema: 'Schema_MPLOption', *, options: 'Option') -> 'Da drop=bool(schema.flags['drop']), ), seq=schema.seq, - seed_id=schema.seed if schema.seed is not NoValue else None, # type: ignore[comparison-overlap] + seed_id=schema.seed if schema.seed is not NO_VALUE else None, # type: ignore[comparison-overlap] ) return opt diff --git a/pcapkit/protocols/internet/ipv6_opts.py b/pcapkit/protocols/internet/ipv6_opts.py index 3eb42e6ff9..b8a0b65a1c 100644 --- a/pcapkit/protocols/internet/ipv6_opts.py +++ b/pcapkit/protocols/internet/ipv6_opts.py @@ -35,7 +35,7 @@ from pcapkit.const.ipv6.smf_dpd_mode import SMFDPDMode as Enum_SMFDPDMode from pcapkit.const.ipv6.tagger_id import TaggerID as Enum_TaggerID from pcapkit.const.reg.transtype import TransType as Enum_TransType -from pcapkit.corekit.fields.field import NoValue +from pcapkit.corekit.fields.field import NO_VALUE from pcapkit.corekit.multidict import OrderedMultiDict from pcapkit.protocols.data.internet.ipv6_opts import CALIPSOOption as Data_CALIPSOOption from pcapkit.protocols.data.internet.ipv6_opts import DFFFlags as Data_DFFFlags @@ -1073,7 +1073,7 @@ def _read_opt_mpl(self, schema: 'Schema_MPLOption', *, options: 'Option') -> 'Da drop=bool(schema.flags['drop']), ), seq=schema.seq, - seed_id=schema.seed if schema.seed is not NoValue else None, # type: ignore[comparison-overlap] + seed_id=schema.seed if schema.seed is not NO_VALUE else None, # type: ignore[comparison-overlap] ) return opt diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index a3ca55536a..2a5c5bacdc 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -32,7 +32,7 @@ from pcapkit.corekit.context import ContextRegistry from pcapkit.corekit.module import ModuleDescriptor from pcapkit.corekit.protochain import ProtoChain -from pcapkit.corekit.sentinels import _Absent, _AbsentType # pylint: disable=unused-import +from pcapkit.corekit.sentinels import ABSENT, AbsentType # pylint: disable=unused-import from pcapkit.protocols import data as data_module from pcapkit.protocols import schema as schema_module from pcapkit.protocols.data.data import Data @@ -155,7 +155,7 @@ def _declared_keywords(cls: 'type') -> 'Optional[frozenset[str]]': # as a parameter is invisible to :func:`inspect.signature`, so the class # says so itself. Read per class in the MRO, for the same reason the # methods are: a subclass should not have to repeat its parents'. - keywords = klass.__dict__.get('__keywords__', _Absent) + keywords = klass.__dict__.get('__keywords__', ABSENT) if keywords is None: # NOTE: The :obj:`None` opt-out is *not* inherited, unlike a set, # which is unioned down the MRO. It describes how the class that @@ -169,7 +169,7 @@ def _declared_keywords(cls: 'type') -> 'Optional[frozenset[str]]': # can be checked. A subclass that dispatches in turn says so itself. if klass is cls: unchecked = True - elif keywords is not _Absent: + elif keywords is not ABSENT: names.update(keywords) for method in _KEYWORD_CONSUMERS: diff --git a/pcapkit/protocols/schema/application/httpv2.py b/pcapkit/protocols/schema/application/httpv2.py index b5e6298e97..a24ee6f8b4 100644 --- a/pcapkit/protocols/schema/application/httpv2.py +++ b/pcapkit/protocols/schema/application/httpv2.py @@ -218,7 +218,7 @@ class Flags(FrameType.Flags): # ``b''`` and nothing raised or warned. The parentheses also keep # ``pkt['pad_len']`` from being read at all when ``PADDED`` is clear, where # the :class:`~pcapkit.corekit.fields.misc.ConditionalField` above has left - # it as :data:`~pcapkit.corekit.fields.field.NoValue`. See #668. + # it as :data:`~pcapkit.corekit.fields.field.NO_VALUE`. See #668. data: 'bytes' = BytesField(length=lambda pkt: pkt['__length__'] - ( pkt['pad_len'] if pkt['flags']['bit_3'] else 0 )) diff --git a/pcapkit/protocols/schema/internet/hopopt.py b/pcapkit/protocols/schema/internet/hopopt.py index 5215c46b21..d13cfe783e 100644 --- a/pcapkit/protocols/schema/internet/hopopt.py +++ b/pcapkit/protocols/schema/internet/hopopt.py @@ -13,7 +13,7 @@ from pcapkit.const.ipv6.tagger_id import TaggerID as Enum_TaggerID from pcapkit.const.reg.transtype import TransType as Enum_TransType from pcapkit.corekit.fields.collections import OptionField -from pcapkit.corekit.fields.field import NoValue +from pcapkit.corekit.fields.field import NO_VALUE from pcapkit.corekit.fields.ipaddress import IPv4AddressField, IPv6AddressField from pcapkit.corekit.fields.misc import (ConditionalField, ForwardMatchField, NoValueField, PayloadField, SchemaField, SwitchField) @@ -272,7 +272,7 @@ def pad_opt_data_len(pkt: 'dict[str, Any]') -> 'int': :attr:`Option.len` is declared as a :class:`~pcapkit.corekit.fields.misc.ConditionalField` and is skipped for it. A skipped conditional field is *recorded* in the packet data as - :data:`~pcapkit.corekit.fields.field.NoValue`, rather than being left + :data:`~pcapkit.corekit.fields.field.NO_VALUE`, rather than being left out of it, so the test below has to be on the **value** and not on the presence of the key: ``pkt.get('len', 0)`` on its own hands that :obj:`~pcapkit.corekit.fields.field.NoValueType` straight to @@ -283,7 +283,7 @@ def pad_opt_data_len(pkt: 'dict[str, Any]') -> 'int': """ length = pkt.get('len', 0) - if not isinstance(length, int): # ``NoValue`` (skipped) or :obj:`None` (unset) + if not isinstance(length, int): # ``NO_VALUE`` (skipped) or :obj:`None` (unset) return 0 return length @@ -706,7 +706,7 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': """ if self.flags['type'] == Enum_SeedID.IPV6_SOURCE_ADDRESS: - self.seed = packet.get('src', NoValue) + self.seed = packet.get('src', NO_VALUE) return self if TYPE_CHECKING: diff --git a/pcapkit/protocols/schema/internet/ipv6_opts.py b/pcapkit/protocols/schema/internet/ipv6_opts.py index 2f01bfcd0e..73e3939164 100644 --- a/pcapkit/protocols/schema/internet/ipv6_opts.py +++ b/pcapkit/protocols/schema/internet/ipv6_opts.py @@ -13,7 +13,7 @@ from pcapkit.const.ipv6.tagger_id import TaggerID as Enum_TaggerID from pcapkit.const.reg.transtype import TransType as Enum_TransType from pcapkit.corekit.fields.collections import OptionField -from pcapkit.corekit.fields.field import NoValue +from pcapkit.corekit.fields.field import NO_VALUE from pcapkit.corekit.fields.ipaddress import IPv4AddressField, IPv6AddressField from pcapkit.corekit.fields.misc import (ConditionalField, ForwardMatchField, NoValueField, PayloadField, SchemaField, SwitchField) @@ -272,7 +272,7 @@ def pad_opt_data_len(pkt: 'dict[str, Any]') -> 'int': :attr:`Option.len` is declared as a :class:`~pcapkit.corekit.fields.misc.ConditionalField` and is skipped for it. A skipped conditional field is *recorded* in the packet data as - :data:`~pcapkit.corekit.fields.field.NoValue`, rather than being left + :data:`~pcapkit.corekit.fields.field.NO_VALUE`, rather than being left out of it, so the test below has to be on the **value** and not on the presence of the key: ``pkt.get('len', 0)`` on its own hands that :obj:`~pcapkit.corekit.fields.field.NoValueType` straight to @@ -283,7 +283,7 @@ def pad_opt_data_len(pkt: 'dict[str, Any]') -> 'int': """ length = pkt.get('len', 0) - if not isinstance(length, int): # ``NoValue`` (skipped) or :obj:`None` (unset) + if not isinstance(length, int): # ``NO_VALUE`` (skipped) or :obj:`None` (unset) return 0 return length @@ -711,7 +711,7 @@ def post_process(self, packet: 'dict[str, Any]') -> 'Schema': """ if self.flags['type'] == Enum_SeedID.IPV6_SOURCE_ADDRESS: - self.seed = packet.get('src', NoValue) + self.seed = packet.get('src', NO_VALUE) return self if TYPE_CHECKING: diff --git a/pcapkit/protocols/schema/internet/mh.py b/pcapkit/protocols/schema/internet/mh.py index 60ef14c645..7b475ecfc3 100644 --- a/pcapkit/protocols/schema/internet/mh.py +++ b/pcapkit/protocols/schema/internet/mh.py @@ -374,7 +374,7 @@ def pad_opt_data_len(pkt: 'dict[str, Any]') -> 'int': :attr:`Option.length` is declared as a :class:`~pcapkit.corekit.fields.misc.ConditionalField` and is skipped for it. A skipped conditional field is *recorded* in the packet data as - :data:`~pcapkit.corekit.fields.field.NoValue`, rather than being left + :data:`~pcapkit.corekit.fields.field.NO_VALUE`, rather than being left out of it, so the test below has to be on the **value** and not on the presence of the key: ``pkt.get('length', 0)`` on its own hands that :class:`~pcapkit.corekit.fields.field.NoValueType` straight to @@ -385,7 +385,7 @@ def pad_opt_data_len(pkt: 'dict[str, Any]') -> 'int': """ length = pkt.get('length', 0) - if not isinstance(length, int): # ``NoValue`` (skipped) or :obj:`None` (unset) + if not isinstance(length, int): # ``NO_VALUE`` (skipped) or :obj:`None` (unset) return 0 return length @@ -409,7 +409,7 @@ def pad_subopt_data_len(pkt: 'dict[str, Any]') -> 'int': """ length = pkt.get('length', 0) - if not isinstance(length, int): # ``NoValue`` (skipped) or :obj:`None` (unset) + if not isinstance(length, int): # ``NO_VALUE`` (skipped) or :obj:`None` (unset) return 0 return length diff --git a/pcapkit/protocols/schema/schema.py b/pcapkit/protocols/schema/schema.py index de37185943..ddb595bc69 100644 --- a/pcapkit/protocols/schema/schema.py +++ b/pcapkit/protocols/schema/schema.py @@ -9,7 +9,7 @@ from typing import TYPE_CHECKING, Any, Generic, TypeVar, cast from pcapkit.corekit.fields.collections import ListField, OptionField -from pcapkit.corekit.fields.field import FieldBase, NoValue +from pcapkit.corekit.fields.field import NO_VALUE, FieldBase from pcapkit.corekit.fields.misc import ConditionalField, ForwardMatchField, PayloadField from pcapkit.corekit.fields.strings import PaddingField from pcapkit.corekit.infoclass import FinalisedState @@ -105,7 +105,7 @@ class marked with a user's own ``from typing import final`` on 3.10 is cls.__builtin__ = set(temp) cls.__excluded__.extend(cls.__builtin__) - args_ = [f'{key}=NoValue' for key in cls.__fields__] + args_ = [f'{key}=NO_VALUE' for key in cls.__fields__] dict_ = [f'{key}={key}' for key in cls.__fields__] # NOTE: We shall only attempt to generate ``__init__`` method if the class @@ -502,10 +502,10 @@ def __post_init__(self, packet: 'Optional[dict[str, Any]]' = None) -> 'None': # generated ``__init__`` is not the only caller: :meth:`from_dict` # seeds only the keys its argument carries, so a field the caller left # out is missing from ``__dict__`` entirely rather than holding - # ``NoValue``, and subscripting it raised :exc:`KeyError` naming the + # ``NO_VALUE``, and subscripting it raised :exc:`KeyError` naming the # field. # - # What is tested is ``NoValue`` alone, not ``NoValue`` or ``None``. + # What is tested is ``NO_VALUE`` alone, not ``NO_VALUE`` or ``None``. # This method fills in what the caller did not say, and a ``None`` the # caller passed *is* something said: on an optional field it is the # chosen value, meaning this packet does not carry the field. It is @@ -515,16 +515,16 @@ def __post_init__(self, packet: 'Optional[dict[str, Any]]' = None) -> 'None': # substituting the default here would leave a constructed schema # disagreeing with a parsed one about the same packet, and # ``from_dict(parsed.to_dict())`` no longer reproducing what it was - # given. Telling the two apart is what ``NoValue`` is for. - value = self.__dict__.get(name, NoValue) - if value is not NoValue: + # given. Telling the two apart is what ``NO_VALUE`` is for. + value = self.__dict__.get(name, NO_VALUE) + if value is not NO_VALUE: continue default = field.default - if default is not NoValue: + if default is not NO_VALUE: self.__dict__[name] = default else: - # NOTE: Nothing to fill an unset field with, so the ``NoValue`` + # NOTE: Nothing to fill an unset field with, so the ``NO_VALUE`` # the generated ``__init__`` seeded it with is dropped rather than # kept: it is a *field* sentinel, not a value a schema may hold. # Dropping it rather than storing ``None`` also keeps the name out @@ -967,7 +967,7 @@ def unpack(cls, data: 'bytes | IO[bytes]', if not field.test(packet): self.__buffer__[field.name] = b'' setattr(self, field.name, None) - packet[field.name] = NoValue + packet[field.name] = NO_VALUE continue field = field.field(packet) diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index edeb6e3bb3..58a62649ab 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -24,13 +24,13 @@ pins it so a later reading of the ruling cannot escalate into deleting the types. The population is **four**, not the three -:file:`docs/source/contributing/conventions.rst` documented -- ``_Absent`` / -``_AbsentType`` in :mod:`pcapkit.protocols.protocol` is the fourth, missed because a -sweep filtered on capitalised names does not see a leading underscore. -:class:`SentinelPopulationTests` pins the count and the doc together, so the next -sentinel cannot be added to one without the other. ``_Absent`` is private and stays -out of :attr:`__all__` in both directions, which is what the ruling means by "to -users". +:file:`docs/source/contributing/conventions.rst` documented -- ``ABSENT`` / +``AbsentType`` in :mod:`pcapkit.protocols.protocol` is the fourth, missed because a +sweep filtered on capitalised names did not see it when it was still spelled +``_Absent``/``_AbsentType``, with a leading underscore. :class:`SentinelPopulationTests` +pins the count and the doc together, so the next sentinel cannot be added to one +without the other. ``ABSENT`` is private and stays out of :attr:`__all__` in both +directions, which is what the ruling means by "to users". A follow-up to this same issue moved all four *definitions* into :mod:`pcapkit.corekit.sentinels`, per the owner's later ruling -- *"Okay one module @@ -41,6 +41,17 @@ checks against :data:`CANONICAL_MODULE` rather than against a different module per sentinel. +GitHub issue #937 later renamed two of the four *objects* to SCREAMING_SNAKE -- +``NoValue`` to ``NO_VALUE`` and ``_Absent`` to ``ABSENT``, the latter also dropping +its leading underscore -- so every instance name agrees on one casing. Privacy for +what is now ``ABSENT`` stopped being signalled by the name at all and became +documentation-only, per the owner's ruling on #937: *"we can change* ``_ABSENT`` *to* +``ABSENT`` *just document it as private type/class in the documentation and not for +public use is enough."* +:meth:`SentinelExportTests.test_the_private_sentinel_is_exported_neither_way` is what +now pins that privacy under the new name, since the mechanical underscore signal it +used to double-check is gone. + One sentinel is deliberately **not** held to any of this: ``_NOT_FOUND = object()`` at :file:`pcapkit/utilities/compat.py`, line 73, inside the ``cached_property`` backport for interpreters below 3.8. It is ported code and exempt, and @@ -68,6 +79,30 @@ survive the edit (``ModuleDescriptor``, ``Field``, ``EnumLookup``, ``EnumRegistry``), the types staying importable, and the naming convention itself. +GitHub issue #937 also adds one genuinely new assertion rather than only renaming +existing ones: :meth:`SentinelPopulationTests.test_every_sentinel_object_is_screaming_snake` +pins the object's own casing directly, which nothing before it did -- +``test_every_sentinel_follows_the_naming_convention`` only ever pinned that +``Type`` derives from whatever the object is called, by design, via +:func:`_expected_type_name`, which is precisely why it passed on ``main`` at +``382375811`` even though ``NoValue`` and ``_Absent`` were not SCREAMING_SNAKE +there -- its two special cases (a ``bare.isupper()`` guard and a leading-underscore +carry-through) existed *because* the objects disagreed, so the check that derives +the type from the object could never by itself catch the object's casing being +wrong. The new test fails twice on that commit, measured directly:: + + $ git show 382375811:pcapkit/corekit/sentinels.py | grep -n 'NoValue = \\|_Absent = ' + 227:NoValue = NoValueType() + 469:_Absent = _AbsentType() + $ python3 -c "import re; p = re.compile(r'^[A-Z][A-Z0-9_]*$'); \\ + print([n for n in ('NULL', 'NoValue', 'NO_DEFAULT', '_Absent') if not p.match(n)])" + ['NoValue', '_Absent'] + +Now that every object is SCREAMING_SNAKE, :func:`_expected_type_name` collapses to +the one mechanical rule those two special cases used to carve exceptions around; +see its own docstring below for what dropped and why the assertion strength survives +the drop. + """ from __future__ import annotations @@ -76,9 +111,9 @@ import unittest from pcapkit.corekit.enum import NO_DEFAULT, NoDefaultType -from pcapkit.corekit.fields.field import NoValue, NoValueType +from pcapkit.corekit.fields.field import NO_VALUE, NoValueType from pcapkit.corekit.module import NULL, NullType -from pcapkit.protocols.protocol import _Absent, _AbsentType +from pcapkit.protocols.protocol import ABSENT, AbsentType #: Repository root, for the two tests that read a file rather than import it. ROOT = pathlib.Path(__file__).resolve().parents[2] @@ -98,17 +133,18 @@ #: :data:`PUBLIC_SENTINELS` below for the (still distinct) *shim* locations. SENTINELS = ( ('NULL', NULL, NullType), - ('NoValue', NoValue, NoValueType), + ('NO_VALUE', NO_VALUE, NoValueType), ('NO_DEFAULT', NO_DEFAULT, NoDefaultType), - ('_Absent', _Absent, _AbsentType), + ('ABSENT', ABSENT, AbsentType), ) #: The public three of :data:`SENTINELS`, as ``(module, object name, type name)``. -#: ``_Absent`` is absent from it deliberately: it is private, so it is exported -#: neither way and the export rule does not reach it. +#: ``ABSENT`` is absent from it deliberately: it is private, so it is exported +#: neither way and the export rule does not reach it -- GitHub issue #937 dropped +#: its leading underscore, but not its privacy. PUBLIC_SENTINELS = ( ('pcapkit.corekit.module', 'NULL', 'NullType'), - ('pcapkit.corekit.fields.field', 'NoValue', 'NoValueType'), + ('pcapkit.corekit.fields.field', 'NO_VALUE', 'NoValueType'), ('pcapkit.corekit.enum', 'NO_DEFAULT', 'NoDefaultType'), ) @@ -116,18 +152,18 @@ def _expected_type_name(instance_name: 'str') -> 'str': """The type name the house convention derives from an instance name. - ``NULL`` gives ``NullType`` and ``NO_DEFAULT`` gives ``NoDefaultType``, so an - all-caps name is title-cased word by word. ``NoValue`` is already CamelCase and - is left as it is -- ``str.capitalize`` would lowercase its tail into - ``Novalue``. A leading underscore is carried through, which is what makes - ``_Absent`` give ``_AbsentType`` rather than ``AbsentType``. + One mechanical rule since GitHub issue #937 made every instance name + SCREAMING_SNAKE: split on ``_``, title-case each word, and append ``Type``. + ``NULL`` gives ``NullType``, ``NO_VALUE`` gives ``NoValueType``, ``NO_DEFAULT`` + gives ``NoDefaultType`` and ``ABSENT`` gives ``AbsentType``. Before #937 this + needed two special cases that are gone now: ``NoValue`` was already CamelCase, + so a ``bare.isupper()`` guard skipped re-title-casing it, and ``_Absent`` + carried a leading underscore that had to be stripped and re-added around that + guard. Neither input shape exists any more, so neither does the code that + handled it. """ - lead = '_' if instance_name.startswith('_') else '' - bare = instance_name.lstrip('_') - if bare.isupper(): - bare = ''.join(word.capitalize() for word in bare.split('_')) - return f'{lead}{bare}Type' + return ''.join(word.capitalize() for word in instance_name.split('_')) + 'Type' def _star_import(module: 'str') -> 'dict[str, object]': @@ -197,16 +233,18 @@ def test_enum_exports_the_object_and_not_the_type(self) -> 'None': self.assertIn('EnumRegistry', enum.__all__) def test_field_exports_the_object_and_not_the_type(self) -> 'None': - """The direction that is an *addition*: ``NoValue`` was exported by neither. + """The direction that is an *addition*: ``NO_VALUE`` was exported by neither. :attr:`FieldBase.default ` is documented as being this object, so the ruling reaches it: a value a caller is told to compare against is a value ``import *`` should provide. + Named ``NoValue`` in :attr:`__all__` until GitHub issue #937 renamed the + object to ``NO_VALUE``; the type stayed ``NoValueType`` throughout. """ import pcapkit.corekit.fields.field as field - self.assertIn('NoValue', field.__all__) + self.assertIn('NO_VALUE', field.__all__) self.assertNotIn('NoValueType', field.__all__) self.assertIn('Field', field.__all__) @@ -249,19 +287,24 @@ def test_every_sentinel_type_is_still_importable_by_name(self) -> 'None': self.assertEqual(type_.__module__, CANONICAL_MODULE) def test_the_private_sentinel_is_exported_neither_way(self) -> 'None': - """``_Absent`` is private, so the export rule does not reach it. + """``ABSENT`` is private, so the export rule does not reach it. Pinned rather than assumed: the ruling says *export the objects*, and a - literal reading of that would add ``_Absent`` to + literal reading of that would add ``ABSENT`` to :attr:`pcapkit.protocols.protocol.__all__`, which would publish a sentinel - whose own docstring says it never leaves the module. + whose own docstring says it never leaves the module. This kept pinning the + same fact under ``_Absent``/``_AbsentType`` before GitHub issue #937 dropped + the leading underscore that used to be the mechanical signal of "private" -- + the rename makes this test the *only* thing enforcing that any more, which + is why it still checks both the object and the type, and both directions of + ``import *``. """ import pcapkit.protocols.protocol as protocol - self.assertNotIn('_Absent', protocol.__all__) - self.assertNotIn('_AbsentType', protocol.__all__) - self.assertNotIn('_Absent', _star_import('pcapkit.protocols.protocol')) + self.assertNotIn('ABSENT', protocol.__all__) + self.assertNotIn('AbsentType', protocol.__all__) + self.assertNotIn('ABSENT', _star_import('pcapkit.protocols.protocol')) class SentinelPopulationTests(unittest.TestCase): @@ -273,6 +316,37 @@ def test_every_sentinel_follows_the_naming_convention(self) -> 'None': with self.subTest(sentinel=name): self.assertEqual(type_.__name__, _expected_type_name(name)) + def test_every_sentinel_object_is_screaming_snake(self) -> 'None': + """GitHub issue #937's own rule: every sentinel **object** is SCREAMING_SNAKE. + + The naming-convention test above already pins that ``Type`` derives + mechanically from the object's name, but that alone does not pin what the + object's own casing *is* -- a tree where every object were, say, CamelCase + would satisfy it just as well, so long as the type followed suit. This test + is what actually pins SCREAMING_SNAKE, independent of the type-derivation + rule, which is the rule this issue's ruling adds and the gap that let + ``NULL``, ``NoValue``, ``NO_DEFAULT`` and ``_Absent`` disagree on ``main`` + before it landed. + + Fails on the tree before this change, at ``382375811``: ``NoValue`` and + ``_Absent`` are the two objects that were not SCREAMING_SNAKE, measured + directly against that commit rather than assumed:: + + $ git show 382375811:pcapkit/corekit/sentinels.py | grep -n 'NoValue = \\|_Absent = ' + 227:NoValue = NoValueType() + 469:_Absent = _AbsentType() + + Neither ``NoValue`` (``N``, ``o``, ``V``, ... mixed case) nor ``_Absent`` + (leading underscore, mixed case) matches ``^[A-Z][A-Z0-9_]*$``, so this test + fails twice over on that tree -- once for each -- and passes on this one for + all four. + + """ + pattern = re.compile(r'^[A-Z][A-Z0-9_]*$') + for name, _, _ in SENTINELS: + with self.subTest(sentinel=name): + self.assertRegex(name, pattern) + def test_conventions_doc_lists_every_sentinel_in_the_tree(self) -> 'None': """The doc said "three" and listed three; ``_Absent`` was the fourth. @@ -342,8 +416,8 @@ class SentinelBehaviourTests(unittest.TestCase): """ def test_the_absent_value_sentinels_are_falsy(self) -> 'None': - """``NULL``, ``NoValue`` and ``_Absent`` each stand for an absent value.""" - for sentinel in (NULL, NoValue, _Absent): + """``NULL``, ``NO_VALUE`` and ``ABSENT`` each stand for an absent value.""" + for sentinel in (NULL, NO_VALUE, ABSENT): with self.subTest(sentinel=repr(sentinel)): self.assertFalse(sentinel) @@ -361,25 +435,26 @@ def test_the_private_sentinel_reprs_as_its_own_name(self) -> 'None': """````, not ````. The reason the house rule prefers a class at all, and the reason the doc - can now name ``_AbsentType`` as having a ``__repr__`` where ``NoValueType`` + can now name ``AbsentType`` as having a ``__repr__`` where ``NoValueType`` does not. """ - self.assertEqual(repr(_Absent), '') - self.assertNotIn('0x', repr(_Absent)) + self.assertEqual(repr(ABSENT), '') + self.assertNotIn('0x', repr(ABSENT)) self.assertNotIn('__repr__', vars(NoValueType)) class NoValueIsTheDocumentedFieldDefaultTests(unittest.TestCase): - """Why the ruling reaches ``NoValue`` at all. + """Why the ruling reaches ``NO_VALUE`` at all. - ``NoValue``'s own comment in :mod:`pcapkit.corekit.fields.field` reads *"Default + ``NO_VALUE``'s own comment in :mod:`pcapkit.corekit.fields.field` reads *"Default value for* :attr:`FieldBase.default `*"*, so it is the value a caller is told to compare a field's default against -- which is what makes withholding it from ``import *`` the defect rather than a preference. That contract had no test: the ``default`` setter and deleter were both uncovered, and the deleter is the only code path that puts the sentinel - *back*. + *back*. Named ``NoValue`` at the time #911 landed; GitHub issue #937 renamed the + object to ``NO_VALUE`` without touching this contract. :class:`~pcapkit.corekit.fields.strings.BytesField` is the concrete field under test, following :file:`tests/corekit/test_fields_field.py`: a real user-facing @@ -392,10 +467,10 @@ def test_an_undefaulted_field_reports_the_sentinel(self) -> 'None': """``is``, not ``==`` -- the whole point of a sentinel.""" from pcapkit.corekit.fields.strings import BytesField - self.assertIs(BytesField(length=4).default, NoValue) + self.assertIs(BytesField(length=4).default, NO_VALUE) def test_setting_and_deleting_a_default_round_trips_through_the_sentinel(self) -> 'None': - """Deleting a default restores ``NoValue``, rather than :obj:`None` or ``b''``. + """Deleting a default restores ``NO_VALUE``, rather than :obj:`None` or ``b''``. :obj:`None` and ``b''`` are both values a caller may legitimately want as a default, so either would be indistinguishable from "no default given" -- @@ -407,10 +482,10 @@ def test_setting_and_deleting_a_default_round_trips_through_the_sentinel(self) - field = BytesField(length=4) field.default = b'\x00\x01\x02\x03' self.assertEqual(field.default, b'\x00\x01\x02\x03') - self.assertIsNot(field.default, NoValue) + self.assertIsNot(field.default, NO_VALUE) del field.default - self.assertIs(field.default, NoValue) + self.assertIs(field.default, NO_VALUE) self.assertFalse(field.default) diff --git a/tests/corekit/test_sentinels_housing_unit.py b/tests/corekit/test_sentinels_housing_unit.py index 7d5a22ee9a..a4c4722f03 100644 --- a/tests/corekit/test_sentinels_housing_unit.py +++ b/tests/corekit/test_sentinels_housing_unit.py @@ -11,7 +11,7 @@ four -- :class:`~pcapkit.corekit.sentinels.NullType`, :class:`~pcapkit.corekit.sentinels.NoValueType`, :class:`~pcapkit.corekit.sentinels.NoDefaultType` and -:class:`~pcapkit.corekit.sentinels._AbsentType`, and their four instances -- and +:class:`~pcapkit.corekit.sentinels.AbsentType`, and their four instances -- and each of the four original modules (:mod:`pcapkit.corekit.module`, :mod:`pcapkit.corekit.fields.field`, :mod:`pcapkit.corekit.enum` and :mod:`pcapkit.protocols.protocol`) keeps a re-export so that no existing @@ -68,9 +68,9 @@ import pcapkit.corekit.sentinels as sentinels import pcapkit.protocols.protocol as protocol_module from pcapkit.corekit.enum import NO_DEFAULT, NoDefaultType -from pcapkit.corekit.fields.field import NoValue, NoValueType +from pcapkit.corekit.fields.field import NO_VALUE, NoValueType from pcapkit.corekit.module import NULL, NullType -from pcapkit.protocols.protocol import _Absent, _AbsentType +from pcapkit.protocols.protocol import ABSENT, AbsentType from tests._support import purge_modules #: Every sentinel, as ``(instance name, shim module, shim instance, shim type)``. @@ -80,9 +80,9 @@ #: in each test below rather than being folded into this tuple a fifth time. SHIMMED_SENTINELS = ( ('NULL', module_module, NULL, NullType), - ('NoValue', field_module, NoValue, NoValueType), + ('NO_VALUE', field_module, NO_VALUE, NoValueType), ('NO_DEFAULT', enum_module, NO_DEFAULT, NoDefaultType), - ('_Absent', protocol_module, _Absent, _AbsentType), + ('ABSENT', protocol_module, ABSENT, AbsentType), ) @@ -101,9 +101,9 @@ def test_shim_and_canonical_import_are_the_same_object(self) -> 'None': """ for name, shim_instance, canonical_name in ( ('NULL', NULL, 'NULL'), - ('NoValue', NoValue, 'NoValue'), + ('NO_VALUE', NO_VALUE, 'NO_VALUE'), ('NO_DEFAULT', NO_DEFAULT, 'NO_DEFAULT'), - ('_Absent', _Absent, '_Absent'), + ('ABSENT', ABSENT, 'ABSENT'), ): with self.subTest(sentinel=name): self.assertIs(shim_instance, getattr(sentinels, canonical_name)) @@ -121,7 +121,7 @@ def test_shim_and_canonical_type_are_the_same_class_object(self) -> 'None': ('NullType', NullType, 'NullType'), ('NoValueType', NoValueType, 'NoValueType'), ('NoDefaultType', NoDefaultType, 'NoDefaultType'), - ('_AbsentType', _AbsentType, '_AbsentType'), + ('AbsentType', AbsentType, 'AbsentType'), ): with self.subTest(sentinel=name): self.assertIs(shim_type, getattr(sentinels, canonical_name)) @@ -228,7 +228,7 @@ def test_sentinels_module_imports_cleanly_on_its_own(self) -> 'None': finally: purge_modules(['pcapkit']) self.assertTrue(hasattr(fresh, 'NULL')) - self.assertTrue(hasattr(fresh, 'NoValue')) + self.assertTrue(hasattr(fresh, 'NO_VALUE')) self.assertTrue(hasattr(fresh, 'NO_DEFAULT')) def test_each_consumer_module_still_imports_cleanly_on_its_own(self) -> 'None': @@ -264,13 +264,13 @@ class SentinelsModuleExportRuleTests(unittest.TestCase): """ def test_all_names_the_three_public_objects_and_no_types(self) -> 'None': - """``_Absent`` stays out too: it is private regardless of which module + """``ABSENT`` stays out too: it is private regardless of which module defines it.""" self.assertIn('NULL', sentinels.__all__) - self.assertIn('NoValue', sentinels.__all__) + self.assertIn('NO_VALUE', sentinels.__all__) self.assertIn('NO_DEFAULT', sentinels.__all__) for name in ('NullType', 'NoValueType', 'NoDefaultType', - '_Absent', '_AbsentType'): + 'ABSENT', 'AbsentType'): with self.subTest(name=name): self.assertNotIn(name, sentinels.__all__) diff --git a/tests/protocols/application/test_httpv2_payload_length_unit.py b/tests/protocols/application/test_httpv2_payload_length_unit.py index b096c00192..19ae9240cb 100644 --- a/tests/protocols/application/test_httpv2_payload_length_unit.py +++ b/tests/protocols/application/test_httpv2_payload_length_unit.py @@ -567,7 +567,7 @@ def fields(self) -> 'list[tuple[str, Any, str]]': def packet(self, *, padded: 'bool') -> 'dict[str, Any]': """A synthetic ``packet`` mapping for a length callback. - ``pad_len`` is :data:`~pcapkit.corekit.fields.field.NoValue` in the + ``pad_len`` is :data:`~pcapkit.corekit.fields.field.NO_VALUE` in the unpadded mapping because that is what the :class:`~pcapkit.corekit.fields.misc.ConditionalField` ahead of the payload actually leaves behind when ``PADDED`` is clear -- see @@ -580,14 +580,14 @@ def packet(self, *, padded: 'bool') -> 'dict[str, Any]': The mapping to hand the callback. """ - from pcapkit.corekit.fields.field import NoValue + from pcapkit.corekit.fields.field import NO_VALUE flags = {f'bit_{bit}': 0 for bit in range(8)} if padded: flags['bit_3'] = 1 return { '__length__': self.REMAINING, - 'pad_len': self.PAD_LEN if padded else NoValue, + 'pad_len': self.PAD_LEN if padded else NO_VALUE, 'flags': flags, } @@ -611,7 +611,7 @@ def test_the_unpadded_arm_returns_the_remaining_length(self) -> None: def test_the_unpadded_arm_does_not_consult_pad_len(self) -> None: """The unpadded arm must not evaluate ``pad_len``. - ``pad_len`` is :data:`~pcapkit.corekit.fields.field.NoValue` when + ``pad_len`` is :data:`~pcapkit.corekit.fields.field.NO_VALUE` when ``PADDED`` is clear, so the conditional has to short-circuit around it. It does so in the fixed form because the conditional *is* the right operand of the subtraction. This does not discriminate the fix from the @@ -621,10 +621,10 @@ def test_the_unpadded_arm_does_not_consult_pad_len(self) -> None: which raises on the value the field machinery actually supplies. """ - from pcapkit.corekit.fields.field import NoValue + from pcapkit.corekit.fields.field import NO_VALUE packet = self.packet(padded=False) - self.assertIs(packet['pad_len'], NoValue) + self.assertIs(packet['pad_len'], NO_VALUE) for label, schema, attribute in self.fields(): with self.subTest(field=label): @@ -660,14 +660,14 @@ def test_the_padded_arm_reaches_zero_and_is_not_clamped(self) -> None: shows up here and at that boundary. """ - from pcapkit.corekit.fields.field import NoValue + from pcapkit.corekit.fields.field import NO_VALUE packet = { '__length__': self.PAD_LEN, 'pad_len': self.PAD_LEN, 'flags': {**{f'bit_{bit}': 0 for bit in range(8)}, 'bit_3': 1}, } - self.assertIsNot(packet['pad_len'], NoValue) + self.assertIsNot(packet['pad_len'], NO_VALUE) for label, schema, attribute in self.fields(): with self.subTest(field=label): diff --git a/tests/protocols/internet/test_ipv6_extension_unit.py b/tests/protocols/internet/test_ipv6_extension_unit.py index bc9aad3f0b..d5cabe1e56 100644 --- a/tests/protocols/internet/test_ipv6_extension_unit.py +++ b/tests/protocols/internet/test_ipv6_extension_unit.py @@ -1124,7 +1124,7 @@ def _assert_option_readers_cover_branchy_values(self, protocol_cls: type) -> Non from pcapkit.const.ipv6.seed_id import SeedID from pcapkit.const.ipv6.smf_dpd_mode import SMFDPDMode from pcapkit.const.ipv6.tagger_id import TaggerID - from pcapkit.corekit.fields.field import NoValue + from pcapkit.corekit.fields.field import NO_VALUE from pcapkit.corekit.multidict import OrderedMultiDict from pcapkit.protocols.schema.internet import hopopt as hopopt_schema from pcapkit.protocols.schema.internet import ipv6_opts as opts_schema @@ -1233,7 +1233,7 @@ def assert_bad(reader, option_schema) -> None: flags={'type': SeedID.IPV6_SOURCE_ADDRESS, 'max': 1, 'drop': 0}, seq=1) - object.__setattr__(source_seed, 'seed', NoValue) + object.__setattr__(source_seed, 'seed', NO_VALUE) self.assertIsNone(proto._read_opt_mpl(source_seed, options=options).seed_id) for seed_type, length, seed in [ @@ -2225,20 +2225,20 @@ def _assert_padding_option_schema_sizes_itself(self, protocol_cls: type) -> None :func:`~pcapkit.protocols.schema.internet.hopopt.pad_opt_data_len` is the whole of the read-side fix, so it is tested directly as well as through a parse: it has to read a skipped conditional field -- which is - recorded as :data:`~pcapkit.corekit.fields.field.NoValue`, not omitted -- + recorded as :data:`~pcapkit.corekit.fields.field.NO_VALUE`, not omitted -- as zero padding octets rather than passing it on to :class:`~pcapkit.corekit.fields.strings.PaddingField`. """ from pcapkit.const.ipv6.option import Option - from pcapkit.corekit.fields.field import NoValue + from pcapkit.corekit.fields.field import NO_VALUE from pcapkit.protocols.schema.internet import hopopt as hopopt_schema from pcapkit.protocols.schema.internet import ipv6_opts as opts_schema schema = hopopt_schema if protocol_cls.__name__ == 'HOPOPT' else opts_schema self.assertEqual(schema.pad_opt_data_len({}), 0) - self.assertEqual(schema.pad_opt_data_len({'len': NoValue}), 0) + self.assertEqual(schema.pad_opt_data_len({'len': NO_VALUE}), 0) self.assertEqual(schema.pad_opt_data_len({'len': None}), 0) self.assertEqual(schema.pad_opt_data_len({'len': 4}), 4) diff --git a/tests/protocols/internet/test_mh_unit.py b/tests/protocols/internet/test_mh_unit.py index 3779b021e1..b592161610 100644 --- a/tests/protocols/internet/test_mh_unit.py +++ b/tests/protocols/internet/test_mh_unit.py @@ -1866,17 +1866,17 @@ def test_mh_padding_option_schema_sizes_itself(self) -> None: :func:`~pcapkit.protocols.schema.internet.mh.pad_opt_data_len` has to read a skipped conditional field -- which is recorded as - :data:`~pcapkit.corekit.fields.field.NoValue`, not omitted -- as zero + :data:`~pcapkit.corekit.fields.field.NO_VALUE`, not omitted -- as zero padding octets, rather than handing that singleton to :class:`~pcapkit.corekit.fields.strings.PaddingField` where it becomes an unusable :mod:`struct` template. """ from pcapkit.const.mh.option import Option - from pcapkit.corekit.fields.field import NoValue + from pcapkit.corekit.fields.field import NO_VALUE from pcapkit.protocols.schema.internet import mh as schema self.assertEqual(schema.pad_opt_data_len({}), 0) - self.assertEqual(schema.pad_opt_data_len({'length': NoValue}), 0) + self.assertEqual(schema.pad_opt_data_len({'length': NO_VALUE}), 0) self.assertEqual(schema.pad_opt_data_len({'length': None}), 0) self.assertEqual(schema.pad_opt_data_len({'length': 4}), 4) diff --git a/tests/protocols/misc/test_pcapng_unit.py b/tests/protocols/misc/test_pcapng_unit.py index ac5a47b7e8..4ff1e66737 100644 --- a/tests/protocols/misc/test_pcapng_unit.py +++ b/tests/protocols/misc/test_pcapng_unit.py @@ -2746,7 +2746,7 @@ def test_pcapng_padding_fields_are_padding_fields(self) -> None: # Regression for #366, cause 1: ``PacketBlock.padding_data`` and # ``DecryptionSecretsBlock.padding_data`` were declared ``BytesField``, # which ``Schema.pack`` does not fill in, so packing them handed - # ``struct.pack`` the ``NoValue`` sentinel. + # ``struct.pack`` the ``NO_VALUE`` sentinel. from pcapkit.corekit.fields.strings import PaddingField from pcapkit.protocols.schema.misc.pcapng import (DecryptionSecretsBlock, EnhancedPacketBlock, PacketBlock) diff --git a/tests/protocols/schema/test_schema_unit.py b/tests/protocols/schema/test_schema_unit.py index bd4d897c16..29935729b3 100644 --- a/tests/protocols/schema/test_schema_unit.py +++ b/tests/protocols/schema/test_schema_unit.py @@ -332,7 +332,7 @@ class HeaderSchema(Schema): # ``__post_init__`` ran: ``size`` carries its declared default, and the # two that declare none are left absent rather than holding the - # ``NoValue`` the generated ``__init__`` seeded them with + # ``NO_VALUE`` the generated ``__init__`` seeded them with self.assertEqual(schema.to_dict(), {'kind': 9, 'size': 0x0102}) # so the schema packs, where before the fix the fields left out reached @@ -367,7 +367,7 @@ class OptionalSchema(Schema): spare: int = UInt8Field() # a field the caller left out is filled from its declared default, and one - # declaring none is left absent rather than holding ``NoValue`` + # declaring none is left absent rather than holding ``NO_VALUE`` self.assertEqual(OptionalSchema(kind=9).to_dict(), {'kind': 9, 'maybe': 0xCC}) # a ``None`` the caller passed is a value they chose, not a field they @@ -390,7 +390,7 @@ class OptionalSchema(Schema): def test_schema_mapping_payload_list_and_default_edge_branches(self) -> None: NestedSchema, FeatureSchema, PayloadOnlySchema, _, _ = self._make_schema_classes() - from pcapkit.corekit.fields.field import NoValue + from pcapkit.corekit.fields.field import NO_VALUE from pcapkit.corekit.fields.numbers import UInt8Field from pcapkit.protocols.schema.schema import Schema, schema_final from pcapkit.utilities.exceptions import ProtocolUnbound diff --git a/tests/protocols/test_construction_keyword_check_unit.py b/tests/protocols/test_construction_keyword_check_unit.py index d31f406dc7..4a15a174a7 100644 --- a/tests/protocols/test_construction_keyword_check_unit.py +++ b/tests/protocols/test_construction_keyword_check_unit.py @@ -711,8 +711,8 @@ class SentinelTests(unittest.TestCase): ``Optional[frozenset[str]]`` and :class:`~pcapkit.protocols.application.http.HTTP` sets it -- so the third needs a marker of its own. - That marker is :data:`~pcapkit.protocols.protocol._Absent`, an instance of - :class:`~pcapkit.protocols.protocol._AbsentType` following + That marker is :data:`~pcapkit.protocols.protocol.ABSENT`, an instance of + :class:`~pcapkit.protocols.protocol.AbsentType` following :class:`~pcapkit.corekit.fields.field.NoValueType`, which is how this library already spells a singleton marker. It was a bare ``object()`` when #640 was first raised; a bare ``object()`` has no name in a traceback, no informative @@ -723,7 +723,7 @@ class SentinelTests(unittest.TestCase): :meth:`test_the_absent_marker_is_an_instance_of_its_own_type` and :meth:`test_the_absent_marker_cannot_be_confused_with_another_singleton` - pin the marker's *shape*. Both name ``_Absent``, so on the bare + pin the marker's *shape*. Both name ``ABSENT``, so on the bare ``object()`` they fail at the import -- which makes them a check that the rename happened, with the substance behind the gate. @@ -735,8 +735,8 @@ class SentinelTests(unittest.TestCase): they catch is the mistake this kind of swap actually makes -- comparing against a second instance of the right class, which reads correctly and typechecks -- measured by mutating the ``.get`` default to a fresh - ``_AbsentType()``, at which point both fail with ``TypeError: - '_AbsentType' object is not iterable`` from ``names.update(keywords)`` + ``AbsentType()``, at which point both fail with ``TypeError: + 'AbsentType' object is not iterable`` from ``names.update(keywords)`` while both shape tests pass through the mutation unharmed. So neither pair is sufficient alone: the first pair cannot tell a correct @@ -747,18 +747,18 @@ class SentinelTests(unittest.TestCase): def test_the_absent_marker_is_an_instance_of_its_own_type(self) -> None: """It has a type of its own, and the falsiness the convention carries.""" - from pcapkit.corekit.fields.field import NoValue, NoValueType - from pcapkit.protocols.protocol import _Absent, _AbsentType + from pcapkit.corekit.fields.field import NO_VALUE, NoValueType + from pcapkit.protocols.protocol import ABSENT, AbsentType - self.assertIsInstance(_Absent, _AbsentType) + self.assertIsInstance(ABSENT, AbsentType) # The point of #640's review comment: ``type(object())`` is ``object``, # which says nothing about what the value is for. - self.assertIsNot(type(_Absent), object) + self.assertIsNot(type(ABSENT), object) - # Falsy, exactly as ``NoValue`` is. - self.assertFalse(_Absent) - self.assertFalse(NoValue) + # Falsy, exactly as ``NO_VALUE`` is. + self.assertFalse(ABSENT) + self.assertFalse(NO_VALUE) # And ``@final``. ``typing.final`` only records ``__final__`` on the # decorated class from 3.11 on, and 3.10 is in the CI matrix, so the two @@ -768,17 +768,17 @@ def test_the_absent_marker_is_an_instance_of_its_own_type(self) -> None: # decorated at all. ``NoValueType`` is the probe for which case this is, # so the two classes cannot drift apart either way. if hasattr(NoValueType, '__final__'): - self.assertIs(_AbsentType.__final__, True) # type: ignore[attr-defined] + self.assertIs(AbsentType.__final__, True) # type: ignore[attr-defined] else: # pragma: no cover - self.assertFalse(hasattr(_AbsentType, '__final__')) + self.assertFalse(hasattr(AbsentType, '__final__')) # A bare ``object()`` reads as ````. - self.assertEqual(repr(_Absent), '') + self.assertEqual(repr(ABSENT), '') def test_the_absent_marker_cannot_be_confused_with_another_singleton(self) -> None: """No other singleton in the library answers an ``is`` against it. - :data:`~pcapkit.corekit.fields.field.NoValue` is the near neighbour and + :data:`~pcapkit.corekit.fields.field.NO_VALUE` is the near neighbour and the one deliberately *not* reused here: it is documented as the default value of :attr:`FieldBase.default ` and means "no value was @@ -786,20 +786,20 @@ def test_the_absent_marker_cannot_be_confused_with_another_singleton(self) -> No between the two would make either site's marker satisfy the other's test. """ - from pcapkit.corekit.fields.field import NoValue, NoValueType - from pcapkit.protocols.protocol import _Absent, _AbsentType + from pcapkit.corekit.fields.field import NO_VALUE, NoValueType + from pcapkit.protocols.protocol import ABSENT, AbsentType - self.assertIsNot(_Absent, NoValue) - self.assertNotIsInstance(_Absent, NoValueType) - self.assertNotIsInstance(NoValue, _AbsentType) + self.assertIsNot(ABSENT, NO_VALUE) + self.assertNotIsInstance(ABSENT, NoValueType) + self.assertNotIsInstance(NO_VALUE, AbsentType) # Nor does it compare *equal* to any of them: neither class defines # ``__eq__``, so identity is the only way either is ever true, and this # says so rather than leaving it to be assumed. - for other in (None, NotImplemented, Ellipsis, NoValue, object(), frozenset(), ''): + for other in (None, NotImplemented, Ellipsis, NO_VALUE, object(), frozenset(), ''): with self.subTest(other=type(other).__name__): - self.assertIsNot(_Absent, other) - self.assertFalse(_Absent == other) + self.assertIsNot(ABSENT, other) + self.assertFalse(ABSENT == other) def test_the_absent_marker_never_reaches_the_accepted_names(self) -> None: """A class with no ``__keywords__`` anywhere in its MRO still reads clean.