refactor(corekit): house all four sentinels in one shared module (#911)

docs/source/contributing/conventions.rst

@@ -148,16 +148,25 @@ four in the tree follow it:
      - Defined in
    * - ``NULL``
      - ``NullType``
-     - :mod:`pcapkit.corekit.module`
+     - :mod:`pcapkit.corekit.sentinels`
    * - ``NoValue``
      - ``NoValueType``
-     - :mod:`pcapkit.corekit.fields.field`
+     - :mod:`pcapkit.corekit.sentinels`
    * - ``NO_DEFAULT``
      - ``NoDefaultType``
-     - :mod:`pcapkit.corekit.enum`
+     - :mod:`pcapkit.corekit.sentinels`
    * - ``_Absent``
      - ``_AbsentType``
-     - :mod:`pcapkit.protocols.protocol`
+     - :mod:`pcapkit.corekit.sentinels`
+
+All four used to live beside the one class that used them --
+:mod:`pcapkit.corekit.module`, :mod:`pcapkit.corekit.fields.field`,
+:mod:`pcapkit.corekit.enum` and :mod:`pcapkit.protocols.protocol` respectively.
+GitHub issue #911's housing ruling, verbatim -- *"Okay one module for all four it
+is."* -- moved the four definitions into the single shared module the table now
+names; each original module keeps a re-export so every existing
+``from <module> import <name>`` 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
@@ -166,16 +175,16 @@ 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` -- 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/protocols/protocol.py`, line 95) says so:
+: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:
 
    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
    to name where ``object()`` would give it nothing. It follows
-   :class:`~pcapkit.corekit.fields.field.NoValueType`, which does the same job for an
-   unset field default; this is a sibling of it rather than a reuse [...]
+   :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
@@ -190,7 +199,7 @@ neither, which is what private means here.
 
 .. note::
 
-   Of the four, only :class:`~pcapkit.corekit.module.NullType` is a full worked
+   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__``
@@ -239,9 +248,12 @@ inconsistencies**:
    fails every ``is`` check. Worth having wherever the type is reachable by a caller at
    all -- which, since the type is kept out of ``__all__``, means wherever it is
    importable by its dotted path rather than wherever it is star-exported.
-   :class:`~pcapkit.corekit.module.NullType` documents the limit honestly: a module
+   :class:`~pcapkit.corekit.sentinels.NullType` documents the limit honestly: a module
    **reload** re-executes the class statement, so the guard does not survive one, and
-   code holding the pre-reload instance will fail ``is``.
+   code holding the pre-reload instance will fail ``is``. Since GitHub issue #911,
+   that means reloading :mod:`pcapkit.corekit.sentinels` itself -- reloading
+   :mod:`pcapkit.corekit.module`, which now only re-exports the sentinel, no longer
+   has any effect on it.
 
 ``__bool__`` returning :obj:`False`
    ``NULL``, ``NoValue`` and ``_Absent`` have it, because each stands for an *absent
@@ -253,7 +265,7 @@ inconsistencies**:
    prevent.
 
 ``__copy__`` / ``__deepcopy__`` / ``__reduce__``
-   :class:`~pcapkit.corekit.module.NullType` has them because ``NULL`` is stored in a
+   :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

pcapkit/corekit/enum.py

@@ -90,7 +90,7 @@
 
 from aenum import extend_enum
 
-from pcapkit.utilities.compat import final
+from pcapkit.corekit.sentinels import NO_DEFAULT, NoDefaultType  # pylint: disable=unused-import
 
 if TYPE_CHECKING:
     from typing import Any
@@ -100,171 +100,6 @@
 __all__ = ['NO_DEFAULT', 'EnumLookup', 'EnumRegistry']
 
 
-@final
-class NoDefaultType:
-    """Type of :data:`NO_DEFAULT`, the omitted-``default`` sentinel for :meth:`EnumLookup.get`.
-
-    A dedicated class rather than a bare :class:`object`, per the owner's ruling
-    on #859: *"use dedicated class rather than bare object. Follow 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 revision's docstring here
-    overstated it to be. What a bare :class:`object` actually lacks is a
-    readable representation: it prints as ``<object object at 0x...>`` in a
-    signature, in :func:`help`, and in a traceback, where ``NoDefaultType()``
-    -- via :meth:`__repr__` below -- prints as ``<NO_DEFAULT>``.
-
-    Named ``NoDefaultType`` for the *class* because that half of the house
-    convention is settled: both :class:`~pcapkit.corekit.module.NullType` and
-    :class:`~pcapkit.corekit.fields.field.NoValueType` use ``<Name>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.
-
-    Genuinely a singleton, not merely a class this module happens to
-    instantiate once: :meth:`__new__` always hands back the one instance that
-    already exists, rather than building a new one. That guards against a
-    caller writing ``registry.get(key, default=NoDefaultType())`` -- perhaps
-    not realising :data:`NO_DEFAULT` already exists -- and getting back a
-    *second*, non-identical sentinel that silently fails ``is NO_DEFAULT``
-    inside :meth:`EnumLookup.get`, so their call is treated as supplying a
-    real (if useless) default instead of the *no default* they meant. With the
-    guard, :class:`NoDefaultType() <NoDefaultType>` always returns the one
-    canonical :data:`NO_DEFAULT`, so that mistake self-corrects.
-
-    Unlike :class:`~pcapkit.corekit.module.NullType`, this does *not* also
-    define ``__copy__``, ``__deepcopy__`` or ``__reduce__`` -- but not because
-    :func:`copy.deepcopy` or :mod:`pickle` "bypass" :meth:`__new__`; they do
-    not, for protocol 2 and above. :meth:`object.__reduce_ex__` at protocol 2
-    reduces through :func:`copyreg.__newobj__`, which reconstructs by calling
-    ``cls.__new__(cls)`` -- exactly the guarded path above -- so
-    ``copy.copy``, ``copy.deepcopy`` and every pickle protocol from 2 on
-    already come back as the one canonical instance with no extra code.
-    Measured::
-
-        >>> NO_DEFAULT.__reduce_ex__(2)
-        (<function __newobj__ at 0x...>, (<class '...NoDefaultType'>,), None, None, None)
-        >>> copy.deepcopy(NO_DEFAULT) is NO_DEFAULT
-        True
-
-    The one path :meth:`__new__` cannot see is pickle protocol 0 (and 1),
-    which reduces through :func:`copyreg._reconstructor` instead, and *that*
-    calls :func:`object.__new__` directly::
-
-        >>> NO_DEFAULT.__reduce_ex__(0)
-        (<function _reconstructor at 0x...>, (<class '...NoDefaultType'>, <class 'object'>, None))
-        >>> pickle.loads(pickle.dumps(NO_DEFAULT, protocol=0)) is NO_DEFAULT
-        False
-
-    That gap is exactly what :class:`~pcapkit.corekit.module.NullType`'s own
-    :meth:`~pcapkit.corekit.module.NullType.__reduce__` exists to close, because
-    :data:`~pcapkit.corekit.module.NULL` is stored as a
-    :class:`~pcapkit.corekit.module.ModuleDescriptor` field that a caller's own
-    :func:`copy.deepcopy` or :mod:`pickle` call can walk into and reconstruct.
-    :data:`NO_DEFAULT` is left unhandled here not because the gap cannot occur
-    in principle, but because nothing in this package ever pickles it at
-    protocol 0: it is reachable -- from :meth:`EnumLookup.get`'s own bound
-    parameter default (``EnumLookup.get.__defaults__[0]``, or any
-    subclass's, e.g. ``Hardware.get.__func__.__defaults__[0]``) and from
-    ``inspect.signature(Hardware.get).parameters['default'].default`` -- but
-    neither is a field any object here gets pickled *as*, and both still
-    survive :func:`copy.deepcopy` with identity intact regardless, because
-    deep-copying either still reduces the sentinel itself through the same,
-    guarded protocol-2 path measured above.
-
-    A caveat, not a defect, but a real and *worse* one than the marker it
-    replaces: :func:`importlib.reload` on this module does not merely leave
-    the class stale, it breaks :meth:`EnumLookup.get`'s own no-default
-    contract for every subclass whose ``get`` was already resolved before the
-    reload. Reload re-executes both ``class NoDefaultType:`` and
-    ``NO_DEFAULT = NoDefaultType()`` below, so the *module global*
-    :meth:`EnumLookup.get`'s body compares against becomes a fresh, distinct
-    object. But each subclass's own ``get`` inherited its **bound parameter
-    default** -- ``default: 'Any' = NO_DEFAULT`` -- at function-definition
-    time, before the reload, and a bound default is frozen then, not looked
-    up again per call. So after the reload, calling ``get`` with ``default``
-    *omitted* no longer compares the pre-reload default against ``NO_DEFAULT``
-    truthfully: it is comparing the stale pre-reload sentinel against the
-    fresh post-reload one, ``is`` reads *False*, and ``get`` falls through to
-    attempting ``cls(default)`` on the stale sentinel instead of re-raising
-    the original lookup error. Measured::
-
-        >>> Hardware.get('Definitely-Not-A-Member')              # before reload
-        KeyError: 'Definitely-Not-A-Member'
-        >>> importlib.reload(pcapkit.corekit.enum)
-        >>> Hardware.get('Definitely-Not-A-Member')               # after reload
-        ValueError: <NO_DEFAULT> is not a valid Hardware
-
-    ``-1`` never had this failure mode: ``-1 == -1`` holds no matter which
-    module execution produced either side, since it compares by value, not by
-    identity, so a reload could not disturb it. Trading that immunity away is
-    the real cost of moving to an identity-compared sentinel, and it is not
-    hypothetical to this tree specifically: reload staleness is a *tracked*
-    defect class here, not a theoretical one -- see
-    :meth:`pcapkit.protocols.protocol.ProtocolBase._lookup_next_layer`'s own
-    docstring note citing GitHub issues #425, #428 and #560, 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. So
-    "nothing in this package reloads :mod:`pcapkit.corekit.enum` after
-    import" is the only thing standing between this sentinel and that same
-    defect class -- true today, but a caveat to keep honest rather than a
-    guarantee this class enforces.
-
-    Also unlike both :class:`~pcapkit.corekit.module.NullType` and
-    :class:`~pcapkit.corekit.fields.field.NoValueType`, this deliberately does
-    *not* define ``__bool__``. Both of those model an *absent* value, so
-    reading falsy in a boolean context is the point. :data:`NO_DEFAULT` models
-    something different: a marker meaning *no default was supplied*, checked
-    exclusively by ``is`` at :meth:`EnumLookup.get`'s two comparison sites --
-    nothing here ever evaluates it for truthiness. Giving it ``__bool__ ->
-    False`` for symmetry with the other two would invite exactly the
-    conflation this sentinel exists to rule out: code that writes ``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.
-
-    """
-
-    #: 'NoDefaultType | None': The one instance :meth:`__new__` ever returns,
-    #: including for the module-level ``NO_DEFAULT = NoDefaultType()`` below
-    #: that creates it in the first place. Kept on the class rather than as a
-    #: module global so :meth:`__new__` can read and write it without a
-    #: ``global`` statement.
-    _instance: 'NoDefaultType | None' = None
-
-    def __new__(cls) -> 'NoDefaultType':
-        """Return the one instance of this class there will ever be."""
-        if cls._instance is None:
-            cls._instance = super().__new__(cls)
-        return cls._instance
-
-    def __repr__(self) -> 'str':
-        """Return :obj:`str` representation of the sentinel."""
-        return '<NO_DEFAULT>'
-
-
-#: NoDefaultType: The ``default`` argument value that means *no default*, i.e.
-#: let an unresolvable key propagate its lookup error rather than falling
-#: back. See :class:`NoDefaultType` for why this is a dedicated class rather
-#: than a bare :class:`object`, why it is a guarded singleton, and why it
-#: defines neither the copy/pickle hooks nor the ``__bool__`` that its two
-#: :mod:`pcapkit.corekit` precedents do.
-NO_DEFAULT = NoDefaultType()
-
-
 class EnumLookup:
     """Bare lookup protocol, shared by open registries and closed sets alike.
 

pcapkit/corekit/fields/field.py

@@ -8,33 +8,20 @@
 import struct
 from typing import TYPE_CHECKING, Generic, TypeVar, cast
 
-from pcapkit.utilities.compat import final
+from pcapkit.corekit.sentinels import NoValue, NoValueType  # pylint: disable=unused-import
 from pcapkit.utilities.exceptions import FieldValueError, NoDefaultValue, ProtocolError
 
 __all__ = ['NoValue', 'Field']
 
 if TYPE_CHECKING:
     from typing import IO, Any, Callable, Iterator, Optional
 
-    from typing_extensions import Literal, Self
+    from typing_extensions import Self
 
     from pcapkit.protocols.schema.schema import Schema
 
 _T = TypeVar('_T')
 
-
-@final
-class NoValueType:
-    """Default value for fields."""
-
-    def __bool__(self) -> 'Literal[False]':
-        """Return :obj:`False`."""
-        return False
-
-
-#: NoValueType: Default value for :attr:`FieldBase.default`.
-NoValue = NoValueType()
-
 #: int: Ceiling on the zero-padding :meth:`FieldBase.unpack` will still perform
 #: for a field whose declared length outruns its buffer.
 #: