From 2cf51800cdc2e335e6b0741df3748a9256836062 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 16 Oct 2026 20:03:09 -0400 Subject: [PATCH 1/3] fix(corekit): make every sentinel a singleton and give NO_VALUE a repr (#1841, #1842) - Add a private abstract base, `_Sentinel`, to `pcapkit.corekit.sentinels`. It carries `__new__`, `__copy__`, `__deepcopy__`, `__reduce__` and `__repr__`. So calling the type, copy, deepcopy and pickle at every protocol give back the one object for `NULL`, `NO_VALUE`, `NO_DEFAULT` and `ABSENT`. - `__reduce__` pickles a sentinel by its module-level name. `_get_null` stays so that pickles written before this change still load. - `NO_VALUE` reprs as ``, not as an object address. - Update the sentinel convention page and the sentinels API page. - Add tests: singleton behaviour, pickles written by the previous module, a parsed HOPOPT MPL schema and a deep-copied field, and the repr. Closes #1841 Closes #1842 --- .../conventions/sentinel-convention.rst | 55 ++-- docs/source/pcapkit/corekit/sentinels.rst | 9 +- pcapkit/corekit/sentinels.py | 294 ++++++++---------- tests/corekit/test_sentinel_exports_unit.py | 45 ++- .../test_sentinel_singleton_1842_unit.py | 238 ++++++++++++++ tests/corekit/test_sentinels_housing_unit.py | 11 +- 6 files changed, 452 insertions(+), 200 deletions(-) create mode 100644 tests/corekit/test_sentinel_singleton_1842_unit.py diff --git a/docs/source/contributing/conventions/sentinel-convention.rst b/docs/source/contributing/conventions/sentinel-convention.rst index 0a50fe129..6a3f5e475 100644 --- a/docs/source/contributing/conventions/sentinel-convention.rst +++ b/docs/source/contributing/conventions/sentinel-convention.rst @@ -69,12 +69,12 @@ public use rather than left off it, since the name alone no longer says so. .. note:: - Of the sentinels, only :class:`~pcapkit.corekit.sentinels.NullType` is a full worked - example. :class:`~pcapkit.corekit.sentinels.NoValueType` follows the naming rule but - is **not** a singleton (``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. + Every sentinel type subclasses :class:`~pcapkit.corekit.sentinels._Sentinel`, a private + abstract base that carries the singleton protocol and the repr. So a new sentinel is + short: subclass it, set ``_name`` to the object's name, and add ``__bool__`` if the + sentinel stands for an absent value. :class:`~pcapkit.corekit.sentinels.NoValueType` + is the smallest example. ``AbsentType`` also overrides ``__repr__``, to keep the + ```` it had before the base gave every sentinel one built from its name. Why a Class and Not ``object()`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -110,14 +110,13 @@ ever sees. The rule above is for sentinels this package writes itself. Per-Sentinel Dunder Methods ~~~~~~~~~~~~~~~~~~~~~~~~~~~ -The sentinels deliberately differ, and the differences are **needs, not -inconsistencies**: +Most of the protocol is shared and lives on ``_Sentinel``. Where the sentinels differ, +the differences are **needs, not inconsistencies**: ``__new__`` returning a cached instance - Guards against a caller constructing a second, non-identical sentinel that then - 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. + Every sentinel has it, from the base. It guards against a caller constructing a + second, non-identical sentinel that then fails every ``is`` check, and it hands back + the cached instance rather than raising, so that mistake corrects itself. :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``. The module that matters there @@ -134,20 +133,20 @@ inconsistencies**: prevent. ``__copy__`` / ``__deepcopy__`` / ``__reduce__`` - :class:`~pcapkit.corekit.sentinels.NullType` is the only sentinel that 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_VALUE``, ``NO_DEFAULT`` and ``ABSENT`` have none. - Measured, since the consequence differs: ``NO_DEFAULT`` survives a copy identically - anyway, because its ``__new__`` hands back the cached instance, while - ``copy.copy(NO_VALUE)`` and ``copy.copy(ABSENT)`` each build a distinct object that - fails both ``is`` and ``==``. Neither is reached by a copy that would notice: - ``ABSENT`` never leaves the - module that reads it, and ``NO_VALUE``, though it rides - :attr:`FieldBase.default ` through a - field copy once per field per packet, keeps its identity there because - :meth:`FieldBase.__copy__ ` is - shallow and shares the reference. Add them all when, and only when, the sentinel - becomes reachable from a copy that is not. + Every sentinel has them, from the base. The owner ruled on :issue:`1842` that every + sentinel is a true singleton, rather than gaining these only once a deep copy is + found to reach it -- as one had by then, through a parsed schema and a field that + hold ``NO_VALUE``. ``__copy__`` and ``__deepcopy__`` return the object itself, and + ``__reduce__`` returns its module-level name, so :mod:`pickle` stores a reference to + that name and every protocol loads the same object back. A guarded ``__new__`` + alone would not do, since pickle protocols 0 and 1 rebuild through + :func:`copyreg._reconstructor`, which calls :meth:`object.__new__` directly. That + path also limits pickles written before :issue:`1842`: one of ``NO_VALUE``, + ``NO_DEFAULT`` or ``ABSENT`` at protocol 0 or 1 still loads, but as a second + instance, because nothing on the class can intercept it. + +``__repr__`` + The base builds it from the object's name -- ````, ````, + ```` -- which is what a signature, :func:`help` and a traceback show. + ``ABSENT`` keeps its own ````. diff --git a/docs/source/pcapkit/corekit/sentinels.rst b/docs/source/pcapkit/corekit/sentinels.rst index 029eff79e..b9de06b8a 100644 --- a/docs/source/pcapkit/corekit/sentinels.rst +++ b/docs/source/pcapkit/corekit/sentinels.rst @@ -22,7 +22,6 @@ that use them do not re-export them (:issue:`1744`). .. autoclass:: pcapkit.corekit.sentinels.NoValueType .. autodata:: pcapkit.corekit.sentinels.NO_VALUE - :no-value: .. autoclass:: pcapkit.corekit.sentinels.NoDefaultType .. autodata:: pcapkit.corekit.sentinels.NO_DEFAULT @@ -37,3 +36,11 @@ that use them do not re-export them (:issue:`1744`). .. autoclass:: pcapkit.corekit.sentinels.AbsentType .. autodata:: pcapkit.corekit.sentinels.ABSENT + +Internal Definitions +-------------------- + +Every sentinel type above derives from this private abstract base, which makes +each one a true singleton (:issue:`1842`). + +.. autoclass:: pcapkit.corekit.sentinels._Sentinel diff --git a/pcapkit/corekit/sentinels.py b/pcapkit/corekit/sentinels.py index 8e06d4547..4f09bd47a 100644 --- a/pcapkit/corekit/sentinels.py +++ b/pcapkit/corekit/sentinels.py @@ -21,6 +21,10 @@ :mod:`pcapkit.protocols.protocol` import the object they use from here, but none lists it in ``__all__`` or documents it, and none binds the type. +Every sentinel type derives from :class:`_Sentinel`, which makes it a true +singleton: calling the type again, :func:`copy.copy`, :func:`copy.deepcopy` +and every :mod:`pickle` protocol give back the one instance (:issue:`1842`). + ``ABSENT`` is private to :mod:`pcapkit.protocols.protocol` -- nothing outside that module ever reads it. It carries no leading underscore; :class:`AbsentType`'s docstring says what marks it private instead. @@ -32,20 +36,99 @@ (:issue:`937`). """ -from typing import TYPE_CHECKING +import abc +from typing import TYPE_CHECKING, cast from pcapkit.utilities.compat import final __all__ = ['NULL', 'NO_VALUE', 'NO_DEFAULT'] if TYPE_CHECKING: - from typing import Any, Callable + from typing import Any + + from typing_extensions import Literal, Self + + +class _Sentinel(abc.ABC): + """Abstract base of every sentinel type here, carrying the singleton protocol. + + A subclass sets ``_name`` to the name of the module-level variable + that holds its one instance, and gets the rest from here: + + * :meth:`__new__` hands back the one instance, so calling the type again + cannot produce a second object that fails an ``is`` check. It returns + the existing instance rather than raising, so a caller who builds one + by mistake still gets the sentinel. + * :meth:`__copy__` and :meth:`__deepcopy__` return ``self``. + * :meth:`__reduce__` returns ``_name``, so :mod:`pickle` stores the + instance as a reference to that module-level name and loads it back as + the same object, at every protocol. Guarding :meth:`__new__` alone + would not cover protocols 0 and 1, which rebuild through + :func:`copyreg._reconstructor` and so call :meth:`object.__new__` + directly. + * :meth:`__repr__` wraps ``_name`` in angle brackets, as in ````. + + ``__bool__`` is left to each sentinel, since only some of them stand for + an absent value. + + Private, because every sentinel is defined in this module (:issue:`911`), + so nothing outside it has a reason to subclass this. + + A pickle written before :issue:`1842` still loads. A pickle of + :data:`NULL` names ``_get_null``, kept for it, and a pickle of another + sentinel at protocol 2 or above rebuilds through :meth:`__new__`; both + load as the sentinel. A pickle of another sentinel at protocol 0 or 1 + rebuilds through :func:`copyreg._reconstructor`, which no hook on the + class can reach, so it loads as a second instance of the type. + + """ + + #: '_Sentinel | None': The one instance of a subclass, once :meth:`__new__` + #: has made it. Assigned on the subclass, so each sentinel type has its own. + _instance: '_Sentinel | None' = None + + @property + @abc.abstractmethod + def _name(self) -> 'str': + """Name of the module-level variable that holds the one instance. + + A subclass overrides this with a plain class attribute, which is what + makes it concrete. + + """ + + def __new__(cls) -> 'Self': + """Return the one instance of this class there will ever be.""" + if cls._instance is None: + cls._instance = super().__new__(cls) + return cast('Self', cls._instance) - from typing_extensions import Literal + def __repr__(self) -> 'str': + """Return :obj:`str` representation of the sentinel.""" + return f'<{self._name}>' + + def __copy__(self) -> 'Self': + """Return ``self`` -- there is, and only ever will be, one of these.""" + return self + + def __deepcopy__(self, memo: 'dict[int, Any]') -> 'Self': + """Return ``self``, for the same reason as :meth:`__copy__`. + + Args: + memo: The :func:`copy.deepcopy` memo table. Unused: returning + ``self`` needs no entry, since nothing about this object is + ever copied. + + """ + return self + + def __reduce__(self) -> 'str': + """Reduce to ``_name``, which :mod:`pickle` stores as a global reference.""" + return self._name @final -class NullType: +class NullType(_Sentinel): """Type of :data:`NULL`, the omitted-``class_``/``module`` sentinel. A distinct class rather than a plain :class:`str` such as ``'(null)'``, @@ -53,32 +136,11 @@ class NullType: :class:`str` a caller passes, including one that happens to spell ``'(null)'`` itself, can be this sentinel. See GitHub issue :issue:`833`. - Genuinely a singleton, not merely a class this module happens to - instantiate once: :meth:`__new__` always hands back the one instance - that already exists, rather than building a new one, so no caller -- - direct, or :mod:`copy`/:mod:`pickle` reconstructing an instance behind - the scenes -- can end up holding a second object that fails an ``is - NULL`` check downstream. :meth:`ModuleDescriptor.klass - ` makes exactly that - check; a second instance would slip past it to :func:`getattr` and - downgrade a clean :exc:`~pcapkit.utilities.exceptions.ProtocolError` into - a bare :exc:`TypeError` (``attribute name must be string, not - 'NullType'``). A stricter guard that raises on a second call would be - truer to "singleton" in the abstract, but it would also mean the module's - own ``NULL = NullType()`` below is the only call that is ever allowed to - succeed -- fragile for no real benefit, since nothing here needs - *rejecting* a second construction, only preventing it from producing a - distinct object. - - :func:`copy.copy`, :func:`copy.deepcopy` and :mod:`pickle` protocols 2 - and up reconstruct through ``cls.__new__(cls)``, which the guard above - already keeps to one instance. Pickle protocols 0 and 1 do not: they - reconstruct through :func:`copyreg._reconstructor`, which calls - :func:`object.__new__` *directly*, bypassing :meth:`__new__` entirely. - :meth:`__reduce__` is defined so that every protocol is routed through - the same module-level getter instead of through reconstruction at all, - and :meth:`__copy__`/:meth:`__deepcopy__` return ``self`` without - reducing anything. + :class:`_Sentinel` keeps it to one instance. :meth:`ModuleDescriptor.klass + ` checks ``is NULL``; a + second instance would slip past it to :func:`getattr` and downgrade a + clean :exc:`~pcapkit.utilities.exceptions.ProtocolError` into a bare + :exc:`TypeError` (``attribute name must be string, not 'NullType'``). A caveat rather than a defect: :func:`importlib.reload` on this module re-executes ``NULL = NullType()`` below, producing a *second* singleton @@ -93,7 +155,9 @@ class NullType: live registry this sentinel could be re-read from. It is a property of sharing one module-level binding across a reload, not something this class's singleton guarantees claim to solve, and nothing in this package - reloads :mod:`pcapkit.corekit.sentinels` after import. + reloads :mod:`pcapkit.corekit.sentinels` after import. Pickling a + pre-reload :data:`NULL` raises :exc:`pickle.PicklingError`, since it is + no longer the object its name refers to. Reloading :mod:`pcapkit.corekit.module` instead has no effect on the singleton at all. That module only imports :data:`NULL` from here, and @@ -103,57 +167,12 @@ class NullType: """ - #: 'NullType | None': The one instance :meth:`__new__` ever returns, - #: including for the module-level ``NULL = NullType()`` 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: 'NullType | None' = None - - def __new__(cls) -> 'NullType': - """Return the one instance of this class there will ever be.""" - if cls._instance is None: - cls._instance = super().__new__(cls) - return cls._instance + _name = 'NULL' def __bool__(self) -> 'Literal[False]': """Return :obj:`False`.""" return False - def __repr__(self) -> 'str': - """Return :obj:`str` representation of the sentinel.""" - return '' - - def __copy__(self) -> 'NullType': - """Return ``self`` -- there is, and only ever will be, one of these.""" - return self - - def __deepcopy__(self, memo: 'dict[int, Any]') -> 'NullType': - """Return ``self``, for the same reason as :meth:`__copy__`. - - Args: - memo: The :func:`copy.deepcopy` memo table. Unused: returning - ``self`` needs no entry, since nothing about this object is - ever copied. - - """ - return self - - def __reduce__(self) -> 'tuple[Callable[[], NullType], tuple[()]]': - """Reduce to the module-level singleton getter, for every :mod:`pickle` protocol. - - A class that defines :meth:`__reduce__` has it honoured by - :meth:`object.__reduce_ex__` for every protocol uniformly, rather - than only for the ones that would otherwise call - :func:`copyreg._reconstructor` -- so naming :func:`_get_null` here - sidesteps reconstruction, and therefore :meth:`__new__`, altogether. - That makes this correct independent of whatever :meth:`__new__` does, - which is what actually covers protocols 0 and 1; see the class - docstring. - - """ - return (_get_null, ()) - #: NullType: Sentinel for an omitted ``class_`` argument to the ``register_*`` #: helpers in :mod:`pcapkit.foundation.registry.protocols` and @@ -163,26 +182,36 @@ def __reduce__(self) -> 'tuple[Callable[[], NullType], tuple[()]]': def _get_null() -> 'NullType': - """Return :data:`NULL`, for :meth:`NullType.__reduce__`. + """Return :data:`NULL`, for a pickle written before :issue:`1842`. - A module-level function rather than a lambda or a bound method, so every - :mod:`pickle` protocol -- including 0 and 1, which cannot reference - anything nested inside a class -- can name it. + ``NullType.__reduce__`` used to name this function, so such a pickle + calls it to load. Nothing here calls it any more; it stays so that those + pickles keep loading, as :data:`NULL` itself. """ return NULL @final -class NoValueType: +class NoValueType(_Sentinel): """Type of :data:`NO_VALUE`, the default value for :mod:`pcapkit.corekit.fields`. Housed here per GitHub issue :issue:`911`; :attr:`FieldBase.default ` documents it as the field-default sentinel. + .. versionchanged:: 1.5.0 + + A singleton: calling this type, :func:`copy.deepcopy` and :mod:`pickle` + give back :data:`NO_VALUE` rather than a second instance + (:issue:`1842`). It reprs as ```` rather than as an object + address, which is what a field constructor's signature and a parsed + schema's repr show for an unset value (:issue:`1841`). + """ + _name = 'NO_VALUE' + def __bool__(self) -> 'Literal[False]': """Return :obj:`False`.""" return False @@ -194,7 +223,7 @@ def __bool__(self) -> 'Literal[False]': @final -class NoDefaultType: +class NoDefaultType(_Sentinel): """Type of :data:`NO_DEFAULT`, the omitted-``default`` sentinel for :meth:`EnumLookup.get `. @@ -203,63 +232,19 @@ class NoDefaultType: :class:`object` compares under ``is`` exactly as safely as a dedicated class with no ``__eq__`` of its own does; what it lacks is a readable representation: it prints as ```` in a signature, - in :func:`help`, and in a traceback, where ``NoDefaultType()`` -- via - :meth:`__repr__` below -- prints as ````. The names follow - the convention every sentinel here shares, settled by GitHub issue - :issue:`937`: ``Type`` for the class and SCREAMING_SNAKE for the - instance. - - 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 + in :func:`help`, and in a traceback, where ``NoDefaultType()`` -- via the + ``__repr__`` :class:`_Sentinel` gives it -- prints as ````. + The names follow the convention every sentinel here shares, settled by + GitHub issue :issue:`937`: ``Type`` for the class and + SCREAMING_SNAKE for the instance. + + :class:`_Sentinel` keeps it to one instance. 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() ` - always returns the one canonical :data:`NO_DEFAULT`, so that mistake - self-corrects. - - Unlike :class:`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) - (, (,), 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) - (, (, , None)) - >>> pickle.loads(pickle.dumps(NO_DEFAULT, protocol=0)) is NO_DEFAULT - False - - That gap is exactly what :class:`NullType`'s own - :meth:`~NullType.__reduce__` exists to close, because :data:`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. + the *no default* they meant. A reload caveat that is dormant rather than live. :func:`importlib.reload` on this module re-executes both ``class NoDefaultType:`` and @@ -309,7 +294,7 @@ class with no ``__eq__`` of its own does; what it lacks is a readable :mod:`pcapkit.corekit.enum`'s own global stays pointed at whatever it read until something re-runs that import. - Also unlike both :class:`NullType` and :class:`NoValueType`, this + Unlike both :class:`NullType` and :class:`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 @@ -329,35 +314,19 @@ class with no ``__eq__`` of its own does; what it lacks is a readable """ - #: '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 '' + _name = '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 -#: :mod:`pcapkit.corekit` siblings do. +#: than a bare :class:`object`, and why it does not define the ``__bool__`` +#: that its :mod:`pcapkit.corekit` siblings do. NO_DEFAULT = NoDefaultType() @final -class AbsentType: +class AbsentType(_Sentinel): """Type of :data:`ABSENT`, the absent-key sentinel. A distinct class rather than a bare :obj:`object` so that the sentinel has a @@ -384,12 +353,19 @@ class AbsentType: """ + _name = 'ABSENT' + def __bool__(self) -> 'Literal[False]': """Return :obj:`False`.""" return False def __repr__(self) -> 'str': - """Return :obj:`str` representation of the sentinel.""" + """Return :obj:`str` representation of the sentinel. + + ````, as it was before :issue:`1842`, rather than the + ```` that :class:`_Sentinel` would give. + + """ return '' diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index c2765c1f4..28acb71c1 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -43,8 +43,8 @@ A follow-up to this same issue moved every sentinel's *definition* into :mod:`pcapkit.corekit.sentinels`: the owner chose one shared module over one module per sentinel, on #911. The shared module is where the ``Type`` naming -convention and the rules for when to add ``__bool__``, ``__copy__`` or ``__reduce__`` -can sit together, which a near-empty file per sentinel would scatter. +convention and the dunder methods the sentinels share or differ on can sit +together, which a near-empty file per sentinel would scatter. Every assertion above still holds unchanged, since it is about each original module's ``__all__``, which the re-export shims left untouched; what changed is only :attr:`type.__module__` for the four types, which @@ -126,6 +126,7 @@ from __future__ import annotations import importlib +import inspect import pathlib import re import unittest @@ -516,16 +517,46 @@ def test_no_default_is_deliberately_truthy(self) -> 'None': 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`` - does not. + The reason the house rule prefers a class at all. ``AbsentType`` keeps a + ``__repr__`` of its own, since the shared one would print ````. + This used to pin that ``NoValueType`` had none, until #1841 gave it one. """ live = _sentinels() - absent, no_value_type = live[3][1], live[1][2] + absent, absent_type = live[3][1], live[3][2] self.assertEqual(repr(absent), '') self.assertNotIn('0x', repr(absent)) - self.assertNotIn('__repr__', vars(no_value_type)) + self.assertIn('__repr__', vars(absent_type)) + + def test_no_value_reprs_as_its_own_name(self) -> 'None': + """````, not ````. + + GitHub issue #1841. ``NO_VALUE`` is the ``default`` of the field + constructors, so the address showed in :func:`inspect.signature`, in + :func:`help` and in a parsed schema's repr, and changed every process. + :file:`pcapkit/corekit/fields/strings.py` and + :file:`pcapkit/corekit/fields/ipaddress.py` are where the issue saw it. + + """ + from pcapkit.corekit.fields.ipaddress import IPv4AddressField + from pcapkit.corekit.fields.strings import StringField + from pcapkit.protocols.internet.hopopt import HOPOPT + + no_value = _sentinels()[1][1] + self.assertEqual(repr(no_value), '') + + for field in (StringField, IPv4AddressField): + with self.subTest(field=field.__name__): + parameter = inspect.signature(field).parameters['default'] + self.assertIs(parameter.default, no_value) + self.assertTrue(str(parameter).endswith(' = '), str(parameter)) + + # An MPL option whose seed is the IPv6 source address, parsed with no + # IPv6 header around it, so ``post_process`` leaves the seed unset. + octets = bytes.fromhex('3b006d0220070000') + option = HOPOPT(octets, len(octets), extension=True).schema.options[0] + self.assertIs(option.seed, no_value) + self.assertIn('seed=', repr(option)) class NoValueIsTheDocumentedFieldDefaultTests(_ReimportCase): diff --git a/tests/corekit/test_sentinel_singleton_1842_unit.py b/tests/corekit/test_sentinel_singleton_1842_unit.py new file mode 100644 index 000000000..c5f2a4045 --- /dev/null +++ b/tests/corekit/test_sentinel_singleton_1842_unit.py @@ -0,0 +1,238 @@ +# -*- coding: utf-8 -*- +"""GitHub issue #1842: every sentinel is a true singleton. + +The owner chose to make the sentinels singletons rather than amend the +convention page to excuse them, with the shared behaviour on one abstract +base. So for ``NULL``, ``NO_VALUE``, ``NO_DEFAULT`` and ``ABSENT`` alike, +calling the type again, :func:`copy.copy`, :func:`copy.deepcopy` and every +:mod:`pickle` protocol hand back the one object. + +Before the change only ``NULL`` passed all of that. ``NO_DEFAULT`` failed +pickle protocols 0 and 1, and ``NO_VALUE`` and ``ABSENT`` failed everything. + +:data:`OLD_PICKLES` are pickles written by the module as it stood at +``75a9f237b``, the commit this change is based on, so they pin that a pickle +made before the change still loads. They were produced by executing +``git show 75a9f237b:pcapkit/corekit/sentinels.py`` as a module named +``pcapkit.corekit.sentinels``, registering it in :data:`sys.modules`, and +calling :func:`pickle.dumps` on each sentinel at each protocol. + +""" +from __future__ import annotations + +import copy +import importlib +import inspect +import pickle +import unittest +from typing import TYPE_CHECKING + +from tests._support import reimport_once_per_class + +if TYPE_CHECKING: + from typing import Any + +#: Where every sentinel is defined. +CANONICAL_MODULE = 'pcapkit.corekit.sentinels' + +#: ``(object name, repr, truth value)`` for every sentinel. The repr and the truth +#: value are what the shared base must leave as they were. +SENTINELS = ( + ('NULL', '', False), + ('NO_VALUE', '', False), + ('NO_DEFAULT', '', True), + ('ABSENT', '', False), +) + +#: Pickles written at ``75a9f237b``, keyed by ``(object name, protocol)``. ``NULL`` +#: reduced to the module-level ``_get_null``; the others took the default reduce, +#: which is ``copyreg._reconstructor`` below protocol 2 and ``NEWOBJ`` from 2 on. +OLD_PICKLES = { + ('NULL', 0): b'cpcapkit.corekit.sentinels\n_get_null\np0\n(tRp1\n.', + ('NULL', 1): b'cpcapkit.corekit.sentinels\n_get_null\nq\x00)Rq\x01.', + ('NULL', 2): b'\x80\x02cpcapkit.corekit.sentinels\n_get_null\nq\x00)Rq\x01.', + ('NULL', 3): b'\x80\x03cpcapkit.corekit.sentinels\n_get_null\nq\x00)Rq\x01.', + ('NULL', 4): (b'\x80\x04\x95.\x00\x00\x00\x00\x00\x00\x00\x8c\x19pcapkit.corekit.sentinels' + b'\x94\x8c\t_get_null\x94\x93\x94)R\x94.'), + ('NULL', 5): (b'\x80\x05\x95.\x00\x00\x00\x00\x00\x00\x00\x8c\x19pcapkit.corekit.sentinels' + b'\x94\x8c\t_get_null\x94\x93\x94)R\x94.'), + ('NO_VALUE', 0): (b'ccopy_reg\n_reconstructor\np0\n(cpcapkit.corekit.sentinels\nNoValueType\n' + b'p1\nc__builtin__\nobject\np2\nNtp3\nRp4\n.'), + ('NO_VALUE', 1): (b'ccopy_reg\n_reconstructor\nq\x00(cpcapkit.corekit.sentinels\nNoValueType\n' + b'q\x01c__builtin__\nobject\nq\x02Ntq\x03Rq\x04.'), + ('NO_VALUE', 2): b'\x80\x02cpcapkit.corekit.sentinels\nNoValueType\nq\x00)\x81q\x01.', + ('NO_VALUE', 3): b'\x80\x03cpcapkit.corekit.sentinels\nNoValueType\nq\x00)\x81q\x01.', + ('NO_VALUE', 4): (b'\x80\x04\x950\x00\x00\x00\x00\x00\x00\x00\x8c\x19pcapkit.corekit.sentinels' + b'\x94\x8c\x0bNoValueType\x94\x93\x94)\x81\x94.'), + ('NO_VALUE', 5): (b'\x80\x05\x950\x00\x00\x00\x00\x00\x00\x00\x8c\x19pcapkit.corekit.sentinels' + b'\x94\x8c\x0bNoValueType\x94\x93\x94)\x81\x94.'), + ('NO_DEFAULT', 0): (b'ccopy_reg\n_reconstructor\np0\n(cpcapkit.corekit.sentinels\nNoDefaultType\n' + b'p1\nc__builtin__\nobject\np2\nNtp3\nRp4\n.'), + ('NO_DEFAULT', 1): (b'ccopy_reg\n_reconstructor\nq\x00(cpcapkit.corekit.sentinels\nNoDefaultType\n' + b'q\x01c__builtin__\nobject\nq\x02Ntq\x03Rq\x04.'), + ('NO_DEFAULT', 2): b'\x80\x02cpcapkit.corekit.sentinels\nNoDefaultType\nq\x00)\x81q\x01.', + ('NO_DEFAULT', 3): b'\x80\x03cpcapkit.corekit.sentinels\nNoDefaultType\nq\x00)\x81q\x01.', + ('NO_DEFAULT', 4): (b'\x80\x04\x952\x00\x00\x00\x00\x00\x00\x00\x8c\x19pcapkit.corekit.sentinels' + b'\x94\x8c\rNoDefaultType\x94\x93\x94)\x81\x94.'), + ('NO_DEFAULT', 5): (b'\x80\x05\x952\x00\x00\x00\x00\x00\x00\x00\x8c\x19pcapkit.corekit.sentinels' + b'\x94\x8c\rNoDefaultType\x94\x93\x94)\x81\x94.'), + ('ABSENT', 0): (b'ccopy_reg\n_reconstructor\np0\n(cpcapkit.corekit.sentinels\nAbsentType\n' + b'p1\nc__builtin__\nobject\np2\nNtp3\nRp4\n.'), + ('ABSENT', 1): (b'ccopy_reg\n_reconstructor\nq\x00(cpcapkit.corekit.sentinels\nAbsentType\n' + b'q\x01c__builtin__\nobject\nq\x02Ntq\x03Rq\x04.'), + ('ABSENT', 2): b'\x80\x02cpcapkit.corekit.sentinels\nAbsentType\nq\x00)\x81q\x01.', + ('ABSENT', 3): b'\x80\x03cpcapkit.corekit.sentinels\nAbsentType\nq\x00)\x81q\x01.', + ('ABSENT', 4): (b'\x80\x04\x95/\x00\x00\x00\x00\x00\x00\x00\x8c\x19pcapkit.corekit.sentinels' + b'\x94\x8c\nAbsentType\x94\x93\x94)\x81\x94.'), + ('ABSENT', 5): (b'\x80\x05\x95/\x00\x00\x00\x00\x00\x00\x00\x8c\x19pcapkit.corekit.sentinels' + b'\x94\x8c\nAbsentType\x94\x93\x94)\x81\x94.'), +} + +#: A standalone HOPOPT header carrying one MPL option whose seed is the IPv6 +#: source address. Parsed with no IPv6 header around it, ``post_process`` has no +#: address to take, so the parsed schema's ``seed`` holds ``NO_VALUE``. +MPL_HEADER = bytes.fromhex('3b006d0220070000') + + +def _module() -> 'Any': + """The sentinels module that is live now (GitHub issue #1332).""" + return importlib.import_module(CANONICAL_MODULE) + + +class _ReimportCase(unittest.TestCase): + """Base: give each test class one fresh ``pcapkit`` import.""" + + def setUp(self) -> 'None': + reimport_once_per_class(self) + + +class SingletonTests(_ReimportCase): + """Each way of getting a second object hands back the first. + + Failures are collected and asserted once, so a regression confined to one + sentinel or one protocol fails the test method itself under any runner. + + """ + + def test_calling_the_type_again_returns_the_sentinel(self) -> 'None': + module = _module() + bad = [name for name, _, _ in SENTINELS + if type(getattr(module, name))() is not getattr(module, name)] + self.assertEqual(bad, []) + + def test_copy_and_deepcopy_return_the_sentinel(self) -> 'None': + module = _module() + bad = [] + for name, _, _ in SENTINELS: + sentinel = getattr(module, name) + for copier in (copy.copy, copy.deepcopy): + if copier(sentinel) is not sentinel: + bad.append((name, copier.__name__)) + self.assertEqual(bad, []) + + def test_every_pickle_protocol_round_trips_to_the_sentinel(self) -> 'None': + module = _module() + bad = [] + for name, _, _ in SENTINELS: + sentinel = getattr(module, name) + for protocol in range(pickle.HIGHEST_PROTOCOL + 1): + if pickle.loads(pickle.dumps(sentinel, protocol=protocol)) is not sentinel: + bad.append((name, protocol)) + self.assertEqual(bad, []) + + def test_repr_and_truth_value_are_unchanged(self) -> 'None': + """The shared base supplies ``__repr__``; ``__bool__`` stays per sentinel.""" + module = _module() + for name, text, truth in SENTINELS: + with self.subTest(sentinel=name): + self.assertEqual(repr(getattr(module, name)), text) + self.assertIs(bool(getattr(module, name)), truth) + + def test_the_shared_base_is_private_and_abstract(self) -> 'None': + """Every sentinel type derives from it; it is not exported and not instantiable.""" + module = _module() + base = module._Sentinel # pylint: disable=protected-access + + self.assertTrue(inspect.isabstract(base)) + with self.assertRaises(TypeError): + base() + self.assertNotIn('_Sentinel', module.__all__) + for name, _, _ in SENTINELS: + with self.subTest(sentinel=name): + self.assertTrue(issubclass(type(getattr(module, name)), base)) + + +class OldPickleTests(_ReimportCase): + """A pickle written before the change still loads, as the sentinel where it can.""" + + def test_every_old_pickle_still_loads(self) -> 'None': + module = _module() + for (name, protocol), blob in OLD_PICKLES.items(): + if protocol > pickle.HIGHEST_PROTOCOL: # pragma: no cover + continue + with self.subTest(sentinel=name, protocol=protocol): + self.assertIsInstance(pickle.loads(blob), type(getattr(module, name))) + + def test_an_old_pickle_loads_as_the_sentinel(self) -> 'None': + """``NULL`` at every protocol, the others from protocol 2 on. + + ``NULL``'s old pickles call ``_get_null``, which the module keeps for them. + The others' old pickles from protocol 2 on rebuild through + ``cls.__new__(cls)``, which the shared base guards. + + """ + module = _module() + bad = [] + for (name, protocol), blob in OLD_PICKLES.items(): + if protocol > pickle.HIGHEST_PROTOCOL: # pragma: no cover + continue + if name != 'NULL' and protocol < 2: + continue + if pickle.loads(blob) is not getattr(module, name): + bad.append((name, protocol)) + self.assertEqual(bad, []) + + def test_an_old_default_reduce_below_protocol_2_cannot(self) -> 'None': + """Pinned so the documented limit stays true. + + Below protocol 2 the default reduce rebuilds through + :func:`copyreg._reconstructor`, which calls :meth:`object.__new__` + directly. No hook on the class runs, so these load as a second instance. + Pickles written after the change reduce to the object's own name instead. + + """ + module = _module() + for name in ('NO_VALUE', 'NO_DEFAULT', 'ABSENT'): + for protocol in (0, 1): + with self.subTest(sentinel=name, protocol=protocol): + self.assertIsNot(pickle.loads(OLD_PICKLES[name, protocol]), + getattr(module, name)) + + +class RealObjectTests(_ReimportCase): + """The objects #1842 measured, which hold ``NO_VALUE`` in practice.""" + + def test_a_parsed_mpl_seed_keeps_its_identity(self) -> 'None': + from pcapkit.protocols.internet.hopopt import HOPOPT + + no_value = _module().NO_VALUE + schema = HOPOPT(MPL_HEADER, len(MPL_HEADER), extension=True).schema + self.assertIs(schema.options[0].seed, no_value) + + bad = [] + if copy.deepcopy(schema).options[0].seed is not no_value: + bad.append('deepcopy') + for protocol in range(pickle.HIGHEST_PROTOCOL + 1): + if pickle.loads(pickle.dumps(schema, protocol=protocol)).options[0].seed is not no_value: + bad.append(protocol) + self.assertEqual(bad, []) + + def test_a_deep_copied_field_keeps_its_default(self) -> 'None': + from pcapkit.corekit.fields.strings import BytesField + + no_value = _module().NO_VALUE + self.assertIs(copy.deepcopy(BytesField(length=4)).default, no_value) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/corekit/test_sentinels_housing_unit.py b/tests/corekit/test_sentinels_housing_unit.py index bae6ca556..825277046 100644 --- a/tests/corekit/test_sentinels_housing_unit.py +++ b/tests/corekit/test_sentinels_housing_unit.py @@ -222,11 +222,12 @@ def test_all_names_the_three_public_objects_and_no_types(self) -> 'None': class PicklePayloadTests(unittest.TestCase): """A ``NULL`` pickle names :mod:`pcapkit.corekit.sentinels` and loads back. - :meth:`NullType.__reduce__` names its factory by module path. A pickle - written by 1.5.0b5 or b6, before GitHub issue #911 relocated the - definitions, named ``pcapkit.corekit.module _get_null`` instead; the - re-export that kept those loadable was dropped under GitHub issue #1744, - since nothing in the library reads that path. + ``NullType.__reduce__`` names ``NULL`` itself by module path, since GitHub + issue #1842 gave every sentinel that reduce. A pickle written by 1.5.0b5 + or b6, before GitHub issue #911 relocated the definitions, named + ``pcapkit.corekit.module _get_null`` instead; the re-export that kept + those loadable was dropped under GitHub issue #1744, since nothing in the + library reads that path. """ From 751e85767f1dfa4221f28a0fd4b5be0f04ccb2bf Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Fri, 16 Oct 2026 22:56:48 -0400 Subject: [PATCH 2/3] fix(corekit): give sentinels a public, immutable Sentinel base with built-in reprs (#1841, #1842) - Rename the private `_Sentinel` base to `Sentinel`, export it, and document it; no alias is kept under the old name. - Add `SentinelMeta`, an `abc.ABCMeta` subclass that makes each concrete sentinel's instance as its class statement runs, refuses attribute writes on every sentinel type with `TypeError`, and refuses subclassing or `register` on a concrete one, as `type(None)` does. - Sentinel instances get empty `__slots__` and raise `AttributeError` on every attribute write or delete, as `None` does. - A sentinel reprs and prints as its own name (`NO_VALUE`), as `None` does; `AbsentType` drops its own `__repr__`. - Update the sentinels API page, the sentinel convention page, and the tests that pinned the old reprs; add parity tests against the built-in singletons. Closes #1841 Closes #1842 --- .../conventions/sentinel-convention.rst | 48 ++- docs/source/pcapkit/corekit/sentinels.rst | 17 +- pcapkit/corekit/sentinels.py | 273 +++++++++++++++--- tests/const/test_const_registry_protocol.py | 4 +- tests/corekit/test_module.py | 2 +- tests/corekit/test_sentinel_exports_unit.py | 25 +- .../test_sentinel_singleton_1842_unit.py | 248 +++++++++++++++- tests/corekit/test_sentinels_housing_unit.py | 9 +- .../test_construction_keyword_check_unit.py | 5 +- 9 files changed, 532 insertions(+), 99 deletions(-) diff --git a/docs/source/contributing/conventions/sentinel-convention.rst b/docs/source/contributing/conventions/sentinel-convention.rst index 6a3f5e475..7e4dfc6a3 100644 --- a/docs/source/contributing/conventions/sentinel-convention.rst +++ b/docs/source/contributing/conventions/sentinel-convention.rst @@ -59,7 +59,9 @@ What reaches users is the **object only**. The owner ruled on GitHub issue :issu that the objects alone -- ``NULL`` and its siblings -- are exported to users, so a public sentinel names its instance in :mod:`pcapkit.corekit.sentinels`'s ``__all__`` and leaves the type out of it. The type stays importable by its dotted path there, for an -annotation or an ``is`` guard; it is only ``import *`` that does not offer it. A private +annotation or an ``is`` guard; it is only ``import *`` that does not offer it. The +``__all__`` there does name :class:`~pcapkit.corekit.sentinels.Sentinel`, the abstract +base every sentinel type shares, since that is no sentinel's own type. 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:`~pcapkit.corekit.sentinels.AbsentType` and @@ -69,12 +71,13 @@ public use rather than left off it, since the name alone no longer says so. .. note:: - Every sentinel type subclasses :class:`~pcapkit.corekit.sentinels._Sentinel`, a private - abstract base that carries the singleton protocol and the repr. So a new sentinel is - short: subclass it, set ``_name`` to the object's name, and add ``__bool__`` if the - sentinel stands for an absent value. :class:`~pcapkit.corekit.sentinels.NoValueType` - is the smallest example. ``AbsentType`` also overrides ``__repr__``, to keep the - ```` it had before the base gave every sentinel one built from its name. + Every sentinel type subclasses :class:`~pcapkit.corekit.sentinels.Sentinel`, the public + abstract base that carries the singleton protocol, the repr and the immutability, and + whose metaclass is :class:`~pcapkit.corekit.sentinels.SentinelMeta`. So a new sentinel + is short: subclass it, set ``__slots__ = ()`` and ``_name`` to the object's name, and + add ``__bool__`` if the sentinel stands for an absent value. + :class:`~pcapkit.corekit.sentinels.NoValueType` is the smallest example. Everything + goes in the ``class`` body, since the type refuses any attribute set on it afterwards. Why a Class and Not ``object()`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -90,7 +93,7 @@ appears in a signature, in :func:`help` output and in a traceback. Compare what default: 'Any' = # dedicated type with __repr__ - default: 'Any' = + default: 'Any' = NO_DEFAULT .. warning:: @@ -110,13 +113,15 @@ ever sees. The rule above is for sentinels this package writes itself. Per-Sentinel Dunder Methods ~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Most of the protocol is shared and lives on ``_Sentinel``. Where the sentinels differ, +Most of the protocol is shared and lives on ``Sentinel``. Where the sentinels differ, the differences are **needs, not inconsistencies**: ``__new__`` returning a cached instance - Every sentinel has it, from the base. It guards against a caller constructing a - second, non-identical sentinel that then fails every ``is`` check, and it hands back - the cached instance rather than raising, so that mistake corrects itself. + Every sentinel has it, from the base, and returns the instance the metaclass made as + the ``class`` statement ran. It guards against a caller constructing a second, + non-identical sentinel that then fails every ``is`` check, and it hands back the + cached instance rather than raising, as ``type(None)()`` does, so that mistake + corrects itself. :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``. The module that matters there @@ -146,7 +151,20 @@ the differences are **needs, not inconsistencies**: instance, because nothing on the class can intercept it. ``__repr__`` - The base builds it from the object's name -- ````, ````, - ```` -- which is what a signature, :func:`help` and a traceback show. - ``ABSENT`` keeps its own ````. + Every sentinel has it, from the base. It is the object's own name -- ``NULL``, + ``NO_VALUE``, ``NO_DEFAULT``, ``ABSENT`` -- as ``repr(None)`` is ``'None'``, and + :func:`str` gives the same. That is what a signature, :func:`help` and a traceback + show. + +``__setattr__`` / ``__delattr__`` + Every sentinel has them, from the base, and they raise :exc:`AttributeError` for any + name, as ``None.x = 1`` does. ``__slots__`` is empty, so there is no ``__dict__`` to + reach round them either. The metaclass does the same for the type, raising + :exc:`TypeError` as ``type(None).x = 1`` does, so neither ``_name`` nor the cached + instance can be swapped out. It also refuses to let a concrete sentinel type be + subclassed, by a ``class`` statement or by ``register``, as ``type(None)`` cannot be; + ``Sentinel`` itself stays subclassable, being abstract. The owner asked for this in + review of the change for :issue:`1842`: a sentinel's repr and its other behaviour + follow Python's own singletons, :obj:`None`, :obj:`Ellipsis` and + :obj:`NotImplemented`. diff --git a/docs/source/pcapkit/corekit/sentinels.rst b/docs/source/pcapkit/corekit/sentinels.rst index b9de06b8a..8151361d6 100644 --- a/docs/source/pcapkit/corekit/sentinels.rst +++ b/docs/source/pcapkit/corekit/sentinels.rst @@ -11,11 +11,19 @@ caller might legitimately pass. They are all defined here, and this is their only import path: the modules that use them do not re-export them (:issue:`1744`). +Each behaves like a built-in singleton such as :obj:`None`: calling its type, +copying it and pickling it give back the one instance, it reprs as its own +name, neither it nor its type can be changed, and its type cannot be +subclassed (:issue:`1842`). + .. versionadded:: 1.5.0 :class:`NoValueType` moves here from :mod:`pcapkit.corekit.fields.field` (:issue:`911`), and :data:`NO_VALUE` replaces its ``NoValue`` there - (:issue:`937`). + (:issue:`937`). :class:`Sentinel` is the public base of every sentinel + type (:issue:`1842`). + +.. autoclass:: pcapkit.corekit.sentinels.Sentinel .. autoclass:: pcapkit.corekit.sentinels.NullType .. autodata:: pcapkit.corekit.sentinels.NULL @@ -40,7 +48,6 @@ that use them do not re-export them (:issue:`1744`). Internal Definitions -------------------- -Every sentinel type above derives from this private abstract base, which makes -each one a true singleton (:issue:`1842`). - -.. autoclass:: pcapkit.corekit.sentinels._Sentinel +.. autoclass:: pcapkit.corekit.sentinels.SentinelMeta + :no-members: + :show-inheritance: diff --git a/pcapkit/corekit/sentinels.py b/pcapkit/corekit/sentinels.py index 4f09bd47a..ab4f2ad82 100644 --- a/pcapkit/corekit/sentinels.py +++ b/pcapkit/corekit/sentinels.py @@ -21,9 +21,12 @@ :mod:`pcapkit.protocols.protocol` import the object they use from here, but none lists it in ``__all__`` or documents it, and none binds the type. -Every sentinel type derives from :class:`_Sentinel`, which makes it a true -singleton: calling the type again, :func:`copy.copy`, :func:`copy.deepcopy` -and every :mod:`pickle` protocol give back the one instance (:issue:`1842`). +Every sentinel type derives from :class:`Sentinel`, which with its +metaclass :class:`SentinelMeta` makes it behave like the type of a built-in +singleton such as :obj:`None`. Calling the type again, :func:`copy.copy`, +:func:`copy.deepcopy` and every :mod:`pickle` protocol give back the one +instance (:issue:`1842`). The instance reprs as its own name, neither it nor +its type can be changed, and its type cannot be subclassed. ``ABSENT`` is private to :mod:`pcapkit.protocols.protocol` -- nothing outside that module ever reads it. It carries no leading underscore; @@ -33,7 +36,8 @@ :class:`NoValueType` moves here from :mod:`pcapkit.corekit.fields.field` (:issue:`911`), and :data:`NO_VALUE` replaces its ``NoValue`` there - (:issue:`937`). + (:issue:`937`). :class:`Sentinel` is the public base of every sentinel + type (:issue:`1842`). """ import abc @@ -41,38 +45,178 @@ from pcapkit.utilities.compat import final -__all__ = ['NULL', 'NO_VALUE', 'NO_DEFAULT'] +__all__ = ['Sentinel', 'NULL', 'NO_VALUE', 'NO_DEFAULT'] if TYPE_CHECKING: - from typing import Any + from typing import Any, NoReturn, Type from typing_extensions import Literal, Self +#: str: Key a sentinel type carries in its own :attr:`~object.__dict__` once +#: :meth:`SentinelMeta.__new__` has built it. From then on the type refuses +#: every attribute write. +_FROZEN = '_frozen' -class _Sentinel(abc.ABC): - """Abstract base of every sentinel type here, carrying the singleton protocol. - A subclass sets ``_name`` to the name of the module-level variable - that holds its one instance, and gets the rest from here: +def _not_a_base(cls: 'type') -> 'TypeError': + """The error deriving from ``cls`` raises, worded as for ``type(None)``.""" + return TypeError(f'type {cls.__name__!r} is not an acceptable base type') + + +def _read_only(obj: 'object', name: 'str') -> 'AttributeError': + """The error setting or deleting ``name`` on ``obj`` raises, worded as for :obj:`None`.""" + kind = type(obj).__name__ + if hasattr(type(obj), name): + return AttributeError(f'{kind!r} object attribute {name!r} is read-only') + return AttributeError(f'{kind!r} object has no attribute {name!r}') + + +class SentinelMeta(abc.ABCMeta): + """Metaclass of :class:`Sentinel`, so every sentinel type acts as ``type(None)`` does. + + * A concrete sentinel type -- one that sets ``_name``, and so has no + abstract member left -- cannot be subclassed, either by a ``class`` + statement or by :meth:`~abc.ABCMeta.register`. Both raise + :exc:`TypeError`, as ``class X(type(None))`` does. An abstract one, + :class:`Sentinel` included, can be. + * A concrete sentinel type makes its one instance while its ``class`` + statement runs, and keeps it as ``_instance``. + * Every sentinel type is immutable once its ``class`` statement has run. + Setting or deleting any attribute on it raises :exc:`TypeError`, as + ``type(None).x = 1`` does, so ``_name``, ``_instance`` and the methods + every sentinel inherits stay as they were defined. + + A concrete sentinel type also gets ``__final__`` set to :obj:`True` here. + :func:`~pcapkit.utilities.compat.final` would record the same, but a class + decorator runs only after the ``class`` statement, when the type already + refuses the write, and :func:`typing.final` skips an attribute it cannot + set. + + These guards, and :class:`Sentinel`'s own, are Python-level hooks. So + :meth:`type.__setattr__` called directly on the type, or + :meth:`object.__setattr__` setting ``__class__`` on an instance, still + reaches past them, where nothing reaches past a built-in type's. + + .. versionadded:: 1.5.0 + + """ + + def __new__(cls, name: 'str', bases: 'tuple[type, ...]', attrs: 'dict[str, Any]', /, + **kwargs: 'Any') -> 'SentinelMeta': + """Create the sentinel type, and its instance if it is concrete. + + Args: + name: Sentinel type name. + bases: Sentinel type bases. + attrs: Sentinel type attributes. + **kwargs: Arbitrary keyword arguments in class definition. + + Raises: + TypeError: If any of ``bases`` is a concrete sentinel type. + + """ + for base in bases: + if isinstance(base, SentinelMeta) and not base.__abstractmethods__: + raise _not_a_base(base) + + klass = super().__new__(cls, name, bases, attrs, **kwargs) + + # NOTE: ``type.__setattr__`` rather than an assignment, which would + # reach :meth:`__setattr__` below. The writes ``abc.ABCMeta.__new__`` + # made above went through it, which is why the type is marked frozen + # only now. + if not klass.__abstractmethods__: + type.__setattr__(klass, '_instance', object.__new__(cast('Type[Sentinel]', klass))) + type.__setattr__(klass, '__final__', True) + type.__setattr__(klass, _FROZEN, True) + return klass + + def __setattr__(cls, name: 'str', value: 'Any') -> 'None': + """Refuse to set an attribute on a built sentinel type. + + Args: + name: Attribute name. + value: Attribute value. + + Raises: + TypeError: Once the ``class`` statement of ``cls`` has run. + + """ + if cls.__dict__.get(_FROZEN): + raise TypeError(f'cannot set {name!r} attribute of immutable type {cls.__name__!r}') + super().__setattr__(name, value) + + def __delattr__(cls, name: 'str') -> 'None': + """Refuse to delete an attribute from a built sentinel type. + + Args: + name: Attribute name. + + Raises: + TypeError: Once the ``class`` statement of ``cls`` has run. + + """ + if cls.__dict__.get(_FROZEN): + raise TypeError(f'cannot delete {name!r} attribute of immutable type {cls.__name__!r}') + super().__delattr__(name) + + def register(cls, subclass: 'type[Any]') -> 'type[Any]': + """Register a virtual subclass, unless ``cls`` is a concrete sentinel type. + + Args: + subclass: Class to register. + + Returns: + ``subclass``, as :meth:`abc.ABCMeta.register` returns it. + + Raises: + TypeError: If ``cls`` is concrete, which a virtual subclass would + defeat as surely as a real one: :func:`isinstance` would then + accept an object that is not the sentinel. + + """ + if not cls.__abstractmethods__: + raise _not_a_base(cls) + return super().register(subclass) + + +class Sentinel(metaclass=SentinelMeta): + """Abstract base of every sentinel type, so each behaves like the type of :obj:`None`. + + A subclass sets ``_name`` to the name of the module-level variable that + holds its one instance, which is what makes it concrete, and gets the + rest from here and from :class:`SentinelMeta`: * :meth:`__new__` hands back the one instance, so calling the type again cannot produce a second object that fails an ``is`` check. It returns - the existing instance rather than raising, so a caller who builds one - by mistake still gets the sentinel. + the existing instance rather than raising, as ``type(None)()`` returns + :obj:`None`, so a caller who builds one by mistake still gets the + sentinel. * :meth:`__copy__` and :meth:`__deepcopy__` return ``self``. - * :meth:`__reduce__` returns ``_name``, so :mod:`pickle` stores the - instance as a reference to that module-level name and loads it back as - the same object, at every protocol. Guarding :meth:`__new__` alone - would not cover protocols 0 and 1, which rebuild through + * :meth:`__reduce__` returns ``_name``, as ``Ellipsis.__reduce__()`` + returns ``'Ellipsis'``, so :mod:`pickle` stores the instance as a + reference to that module-level name and loads it back as the same + object, at every protocol. Guarding :meth:`__new__` alone would not + cover protocols 0 and 1, which rebuild through :func:`copyreg._reconstructor` and so call :meth:`object.__new__` directly. - * :meth:`__repr__` wraps ``_name`` in angle brackets, as in ````. + * :meth:`__repr__` returns ``_name`` itself, as ``repr(None)`` is + ``'None'``. :func:`str` gives the same, since nothing here overrides + :meth:`object.__str__`. + * :meth:`__setattr__` and :meth:`__delattr__` raise + :exc:`AttributeError` for every name, as ``None.x = 1`` does, and + ``__slots__`` is empty, so an instance has no :attr:`~object.__dict__` + to change either. + * :class:`SentinelMeta` keeps the type itself immutable, and a concrete + one free of subclasses. ``__bool__`` is left to each sentinel, since only some of them stand for an absent value. - Private, because every sentinel is defined in this module (:issue:`911`), - so nothing outside it has a reason to subclass this. + Public, as the library's other abstract bases are, so that an annotation + or an :func:`isinstance` check can name every sentinel at once. Every + sentinel the library itself uses is defined in this module + (:issue:`911`). A pickle written before :issue:`1842` still loads. A pickle of :data:`NULL` names ``_get_null``, kept for it, and a pickle of another @@ -81,11 +225,16 @@ class _Sentinel(abc.ABC): rebuilds through :func:`copyreg._reconstructor`, which no hook on the class can reach, so it loads as a second instance of the type. + .. versionadded:: 1.5.0 + """ - #: '_Sentinel | None': The one instance of a subclass, once :meth:`__new__` - #: has made it. Assigned on the subclass, so each sentinel type has its own. - _instance: '_Sentinel | None' = None + __slots__ = () + + #: 'Sentinel | None': The one instance of a concrete subclass, which + #: :class:`SentinelMeta` makes and assigns on that subclass, so each + #: sentinel type has its own. :obj:`None` on an abstract one. + _instance: 'Sentinel | None' = None @property @abc.abstractmethod @@ -98,14 +247,44 @@ def _name(self) -> 'str': """ def __new__(cls) -> 'Self': - """Return the one instance of this class there will ever be.""" + """Return the one instance of this class there will ever be. + + Raises: + TypeError: If ``cls`` is abstract, as :meth:`object.__new__` does. + + """ if cls._instance is None: - cls._instance = super().__new__(cls) + return super().__new__(cls) return cast('Self', cls._instance) def __repr__(self) -> 'str': - """Return :obj:`str` representation of the sentinel.""" - return f'<{self._name}>' + """Return the sentinel's own name, as ``repr(None)`` is ``'None'``.""" + return self._name + + def __setattr__(self, name: 'str', value: 'Any') -> 'NoReturn': + """Refuse, as ``None.x = 1`` does: a sentinel has no state to change. + + Args: + name: Attribute name. + value: Attribute value. + + Raises: + AttributeError: For every ``name``. + + """ + raise _read_only(self, name) + + def __delattr__(self, name: 'str') -> 'NoReturn': + """Refuse, as ``del None.x`` does. + + Args: + name: Attribute name. + + Raises: + AttributeError: For every ``name``. + + """ + raise _read_only(self, name) def __copy__(self) -> 'Self': """Return ``self`` -- there is, and only ever will be, one of these.""" @@ -128,7 +307,7 @@ def __reduce__(self) -> 'str': @final -class NullType(_Sentinel): +class NullType(Sentinel): """Type of :data:`NULL`, the omitted-``class_``/``module`` sentinel. A distinct class rather than a plain :class:`str` such as ``'(null)'``, @@ -136,14 +315,15 @@ class NullType(_Sentinel): :class:`str` a caller passes, including one that happens to spell ``'(null)'`` itself, can be this sentinel. See GitHub issue :issue:`833`. - :class:`_Sentinel` keeps it to one instance. :meth:`ModuleDescriptor.klass + :class:`Sentinel` keeps it to one instance. :meth:`ModuleDescriptor.klass ` checks ``is NULL``; a second instance would slip past it to :func:`getattr` and downgrade a clean :exc:`~pcapkit.utilities.exceptions.ProtocolError` into a bare :exc:`TypeError` (``attribute name must be string, not 'NullType'``). A caveat rather than a defect: :func:`importlib.reload` on this module - re-executes ``NULL = NullType()`` below, producing a *second* singleton + re-executes the ``class`` statement and ``NULL = NullType()`` below, + producing a *second* singleton that the reloaded code compares against correctly but that every module which already imported the pre-reload :data:`NULL` still holds -- so a comparison spanning the reload sees two "singletons" that are not each @@ -167,6 +347,8 @@ class NullType(_Sentinel): """ + __slots__ = () + _name = 'NULL' def __bool__(self) -> 'Literal[False]': @@ -193,7 +375,7 @@ def _get_null() -> 'NullType': @final -class NoValueType(_Sentinel): +class NoValueType(Sentinel): """Type of :data:`NO_VALUE`, the default value for :mod:`pcapkit.corekit.fields`. Housed here per GitHub issue :issue:`911`; @@ -204,12 +386,16 @@ class NoValueType(_Sentinel): A singleton: calling this type, :func:`copy.deepcopy` and :mod:`pickle` give back :data:`NO_VALUE` rather than a second instance - (:issue:`1842`). It reprs as ```` rather than as an object - address, which is what a field constructor's signature and a parsed - schema's repr show for an unset value (:issue:`1841`). + (:issue:`1842`). It reprs as ``NO_VALUE``, its own name, as + ``repr(None)`` is ``'None'``, rather than as an object address, which + is what a field constructor's signature and a parsed schema's repr + show for an unset value (:issue:`1841`). Its attributes, and its + type's, cannot be set or deleted, and the type cannot be subclassed. """ + __slots__ = () + _name = 'NO_VALUE' def __bool__(self) -> 'Literal[False]': @@ -223,7 +409,7 @@ def __bool__(self) -> 'Literal[False]': @final -class NoDefaultType(_Sentinel): +class NoDefaultType(Sentinel): """Type of :data:`NO_DEFAULT`, the omitted-``default`` sentinel for :meth:`EnumLookup.get `. @@ -233,12 +419,12 @@ class NoDefaultType(_Sentinel): class with no ``__eq__`` of its own does; what it lacks is a readable representation: it prints as ```` in a signature, in :func:`help`, and in a traceback, where ``NoDefaultType()`` -- via the - ``__repr__`` :class:`_Sentinel` gives it -- prints as ````. + ``__repr__`` :class:`Sentinel` gives it -- prints as ``NO_DEFAULT``. The names follow the convention every sentinel here shares, settled by GitHub issue :issue:`937`: ``Type`` for the class and SCREAMING_SNAKE for the instance. - :class:`_Sentinel` keeps it to one instance. That guards against a caller + :class:`Sentinel` keeps it to one instance. 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`` @@ -314,6 +500,8 @@ class with no ``__eq__`` of its own does; what it lacks is a readable """ + __slots__ = () + _name = 'NO_DEFAULT' @@ -326,7 +514,7 @@ class with no ``__eq__`` of its own does; what it lacks is a readable @final -class AbsentType(_Sentinel): +class AbsentType(Sentinel): """Type of :data:`ABSENT`, the absent-key sentinel. A distinct class rather than a bare :obj:`object` so that the sentinel has a @@ -353,21 +541,14 @@ class AbsentType(_Sentinel): """ + __slots__ = () + _name = 'ABSENT' def __bool__(self) -> 'Literal[False]': """Return :obj:`False`.""" return False - def __repr__(self) -> 'str': - """Return :obj:`str` representation of the sentinel. - - ````, as it was before :issue:`1842`, rather than the - ```` that :class:`_Sentinel` would give. - - """ - return '' - #: AbsentType: Absent-versus-:obj:`None` sentinel for #: :func:`pcapkit.protocols.protocol._declared_keywords` to read a class's diff --git a/tests/const/test_const_registry_protocol.py b/tests/const/test_const_registry_protocol.py index b94f3fa8a..1dec8245a 100644 --- a/tests/const/test_const_registry_protocol.py +++ b/tests/const/test_const_registry_protocol.py @@ -1341,11 +1341,11 @@ def test_repr_is_the_readable_form_not_a_bare_object_address(self) -> None: """The concrete reason #859 asked for a dedicated class over a bare ``object()``: the latter prints as ```` wherever it turns up -- a signature, :func:`help`, a traceback -- - and this prints as ```` instead.""" + and this prints as ``NO_DEFAULT``, its own name, instead.""" from pcapkit.corekit.sentinels import NO_DEFAULT rendered = repr(NO_DEFAULT) - self.assertEqual(rendered, '') + self.assertEqual(rendered, 'NO_DEFAULT') self.assertNotIn('0x', rendered) def test_type_is_the_dedicated_sentinel_class_and_only_the_object_is_exported(self) -> None: diff --git a/tests/corekit/test_module.py b/tests/corekit/test_module.py index 7ae972916..6e03b691f 100644 --- a/tests/corekit/test_module.py +++ b/tests/corekit/test_module.py @@ -213,7 +213,7 @@ def test_null_sentinel_is_not_a_string(self) -> None: self.assertFalse(self.sentinels.NULL == '(null)') # pylint: disable=unneeded-not self.assertIsNot(self.sentinels.NULL, '(null)') self.assertFalse(bool(self.sentinels.NULL)) - self.assertEqual(repr(self.sentinels.NULL), '') + self.assertEqual(repr(self.sentinels.NULL), 'NULL') self.assertIs(self.sentinels.NullType(), self.sentinels.NULL) def test_null_sentinel_identity_survives_copy_and_pickle(self) -> None: diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index 28acb71c1..18c64de28 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -284,8 +284,8 @@ def test_field_exports_neither_the_object_nor_the_type(self) -> 'None': def test_star_import_binds_the_objects_and_not_the_types(self) -> 'None': """Through ``import *`` itself, which is the surface the ruling is about. - The canonical module binds every public object and no type; each former - re-export site binds neither. + The canonical module binds every public object and no sentinel's own + type; each former re-export site binds neither. """ canonical = importlib.import_module(CANONICAL_MODULE) @@ -515,21 +515,22 @@ def test_no_default_is_deliberately_truthy(self) -> 'None': self.assertTrue(_sentinels()[2][1]) def test_the_private_sentinel_reprs_as_its_own_name(self) -> 'None': - """````, not ````. + """``ABSENT``, not ````. - The reason the house rule prefers a class at all. ``AbsentType`` keeps a - ``__repr__`` of its own, since the shared one would print ````. - This used to pin that ``NoValueType`` had none, until #1841 gave it one. + The reason the house rule prefers a class at all. ``AbsentType`` used to + keep a ``__repr__`` of its own, printing ````; the shared one now + prints the object's own name, as ``repr(None)`` is ``'None'``, so it no + longer needs one. """ live = _sentinels() absent, absent_type = live[3][1], live[3][2] - self.assertEqual(repr(absent), '') + self.assertEqual(repr(absent), 'ABSENT') self.assertNotIn('0x', repr(absent)) - self.assertIn('__repr__', vars(absent_type)) + self.assertNotIn('__repr__', vars(absent_type)) def test_no_value_reprs_as_its_own_name(self) -> 'None': - """````, not ````. + """``NO_VALUE``, not ````. GitHub issue #1841. ``NO_VALUE`` is the ``default`` of the field constructors, so the address showed in :func:`inspect.signature`, in @@ -543,20 +544,20 @@ def test_no_value_reprs_as_its_own_name(self) -> 'None': from pcapkit.protocols.internet.hopopt import HOPOPT no_value = _sentinels()[1][1] - self.assertEqual(repr(no_value), '') + self.assertEqual(repr(no_value), 'NO_VALUE') for field in (StringField, IPv4AddressField): with self.subTest(field=field.__name__): parameter = inspect.signature(field).parameters['default'] self.assertIs(parameter.default, no_value) - self.assertTrue(str(parameter).endswith(' = '), str(parameter)) + self.assertTrue(str(parameter).endswith(' = NO_VALUE'), str(parameter)) # An MPL option whose seed is the IPv6 source address, parsed with no # IPv6 header around it, so ``post_process`` leaves the seed unset. octets = bytes.fromhex('3b006d0220070000') option = HOPOPT(octets, len(octets), extension=True).schema.options[0] self.assertIs(option.seed, no_value) - self.assertIn('seed=', repr(option)) + self.assertIn('seed=NO_VALUE', repr(option)) class NoValueIsTheDocumentedFieldDefaultTests(_ReimportCase): diff --git a/tests/corekit/test_sentinel_singleton_1842_unit.py b/tests/corekit/test_sentinel_singleton_1842_unit.py index c5f2a4045..2c8c81ca1 100644 --- a/tests/corekit/test_sentinel_singleton_1842_unit.py +++ b/tests/corekit/test_sentinel_singleton_1842_unit.py @@ -10,6 +10,14 @@ Before the change only ``NULL`` passed all of that. ``NO_DEFAULT`` failed pickle protocols 0 and 1, and ``NO_VALUE`` and ``ABSENT`` failed everything. +In review of that change the owner asked for the base to be public, named +``Sentinel`` after the library's other abstract bases, for every sentinel +and sentinel type to be immutable, and for their repr and related behaviour +to agree with Python's own singletons. :class:`BuiltinParityTests` runs the +same probes against :obj:`None`, :obj:`Ellipsis` and :obj:`NotImplemented` +and against each sentinel, and the classes after it pin each behaviour on +its own. + :data:`OLD_PICKLES` are pickles written by the module as it stood at ``75a9f237b``, the commit this change is based on, so they pin that a pickle made before the change still loads. They were produced by executing @@ -20,6 +28,7 @@ """ from __future__ import annotations +import abc import copy import importlib import inspect @@ -30,20 +39,24 @@ from tests._support import reimport_once_per_class if TYPE_CHECKING: - from typing import Any + from typing import Any, Callable #: Where every sentinel is defined. CANONICAL_MODULE = 'pcapkit.corekit.sentinels' -#: ``(object name, repr, truth value)`` for every sentinel. The repr and the truth -#: value are what the shared base must leave as they were. +#: ``(object name, repr, truth value)`` for every sentinel. The repr is the +#: object's own name, as ``repr(None)`` is ``'None'``; the truth value is each +#: sentinel's own, which the shared base leaves alone. SENTINELS = ( - ('NULL', '', False), - ('NO_VALUE', '', False), - ('NO_DEFAULT', '', True), - ('ABSENT', '', False), + ('NULL', 'NULL', False), + ('NO_VALUE', 'NO_VALUE', False), + ('NO_DEFAULT', 'NO_DEFAULT', True), + ('ABSENT', 'ABSENT', False), ) +#: Python's own singletons, which a sentinel is to behave like. +BUILTINS = (None, Ellipsis, NotImplemented) + #: Pickles written at ``75a9f237b``, keyed by ``(object name, protocol)``. ``NULL`` #: reduced to the module-level ``_get_null``; the others took the default reduce, #: which is ``copyreg._reconstructor`` below protocol 2 and ``NEWOBJ`` from 2 on. @@ -99,6 +112,48 @@ def _module() -> 'Any': return importlib.import_module(CANONICAL_MODULE) +def _star_import() -> 'dict[str, object]': + """The namespace ``from pcapkit.corekit.sentinels import *`` binds.""" + namespace = {} # type: dict[str, object] + exec(f'from {CANONICAL_MODULE} import *', namespace) # pylint: disable=exec-used + return namespace + + +def _raised(probe: 'Callable[[], object]') -> 'type[BaseException] | None': + """The type of the exception ``probe`` raises, or :obj:`None` if it returns.""" + try: + probe() + except Exception as error: # pylint: disable=broad-except + return type(error) + return None + + +def _subclass(base: 'type') -> 'type': + """Derive a class from ``base`` with a ``class`` statement.""" + class Derived(base): + """A subclass of ``base``.""" + return Derived + + +#: ``(description, probe)``: what each probe does to a singleton ``obj``. Each is +#: run against :data:`BUILTINS` and against every sentinel, and must fail with the +#: same exception type, or succeed, alike. +PROBES = ( + ('set a new attribute', lambda obj: setattr(obj, 'x', 1)), + ('set an existing attribute', lambda obj: setattr(obj, '__doc__', 'x')), + ('delete a new attribute', lambda obj: delattr(obj, 'x')), + ('delete an existing attribute', lambda obj: delattr(obj, '__doc__')), + ('set through object.__setattr__', lambda obj: object.__setattr__(obj, 'x', 1)), + ('read vars()', vars), + ('set a new type attribute', lambda obj: setattr(type(obj), 'x', 1)), + ('set an existing type attribute', lambda obj: setattr(type(obj), '__doc__', 'x')), + ('delete a new type attribute', lambda obj: delattr(type(obj), 'x')), + ('delete an existing type attribute', lambda obj: delattr(type(obj), '__doc__')), + ('subclass the type', lambda obj: _subclass(type(obj))), + ('call the type', lambda obj: type(obj)()), +) + + class _ReimportCase(unittest.TestCase): """Base: give each test class one fresh ``pcapkit`` import.""" @@ -140,28 +195,191 @@ def test_every_pickle_protocol_round_trips_to_the_sentinel(self) -> 'None': bad.append((name, protocol)) self.assertEqual(bad, []) - def test_repr_and_truth_value_are_unchanged(self) -> 'None': - """The shared base supplies ``__repr__``; ``__bool__`` stays per sentinel.""" + def test_repr_is_the_name_and_truth_value_is_unchanged(self) -> 'None': + """The shared base supplies ``__repr__``; ``__bool__`` stays per sentinel. + + :func:`str` and an empty :func:`format` give the repr, as they do for + :obj:`None`. + + """ module = _module() for name, text, truth in SENTINELS: with self.subTest(sentinel=name): - self.assertEqual(repr(getattr(module, name)), text) - self.assertIs(bool(getattr(module, name)), truth) + sentinel = getattr(module, name) + self.assertEqual(repr(sentinel), text) + self.assertEqual(str(sentinel), text) + self.assertEqual(f'{sentinel}', text) + self.assertIs(bool(sentinel), truth) - def test_the_shared_base_is_private_and_abstract(self) -> 'None': - """Every sentinel type derives from it; it is not exported and not instantiable.""" + def test_the_shared_base_is_public_and_abstract(self) -> 'None': + """Every sentinel type derives from ``Sentinel``, which is exported and not instantiable. + + Its metaclass derives from :class:`abc.ABCMeta`, as the library's other + abstract bases' do, and the private name it had before review is gone, + with no alias left behind. + + """ module = _module() - base = module._Sentinel # pylint: disable=protected-access + base = module.Sentinel self.assertTrue(inspect.isabstract(base)) + self.assertTrue(issubclass(type(base), abc.ABCMeta)) with self.assertRaises(TypeError): base() - self.assertNotIn('_Sentinel', module.__all__) + self.assertIn('Sentinel', module.__all__) + self.assertIs(_star_import()['Sentinel'], base) + self.assertFalse(hasattr(module, '_Sentinel')) for name, _, _ in SENTINELS: with self.subTest(sentinel=name): self.assertTrue(issubclass(type(getattr(module, name)), base)) +class BuiltinParityTests(_ReimportCase): + """A sentinel and its type behave as :obj:`None` and ``type(None)`` do. + + Run against :obj:`Ellipsis` and :obj:`NotImplemented` as well, so a probe + whose outcome is particular to :obj:`None` cannot set the bar. + + """ + + def test_every_probe_fails_or_succeeds_as_for_the_builtins(self) -> 'None': + module = _module() + bad = [] + for description, probe in PROBES: + expected = {_raised(lambda: probe(obj)) for obj in BUILTINS} # pylint: disable=cell-var-from-loop + self.assertEqual(len(expected), 1, (description, expected)) + for name, _, _ in SENTINELS: + got = _raised(lambda: probe(getattr(module, name))) # pylint: disable=cell-var-from-loop + if got not in expected: + bad.append((description, name, got, expected)) + self.assertEqual(bad, []) + + def test_reduce_names_the_object_as_for_ellipsis(self) -> 'None': + """``Ellipsis.__reduce__()`` is ``'Ellipsis'``; :obj:`None` has no reduce of its own.""" + module = _module() + self.assertEqual(Ellipsis.__reduce__(), 'Ellipsis') + for name, _, _ in SENTINELS: + with self.subTest(sentinel=name): + self.assertEqual(getattr(module, name).__reduce__(), name) + + +class InstanceImmutabilityTests(_ReimportCase): + """No attribute of a sentinel can be set or deleted, as for :obj:`None`.""" + + def test_setting_or_deleting_any_attribute_raises_attribute_error(self) -> 'None': + module = _module() + for name, _, _ in SENTINELS: + sentinel = getattr(module, name) + for attr in ('x', '_name', '_instance', '__class__'): + with self.subTest(sentinel=name, attr=attr): + with self.assertRaisesRegex(AttributeError, repr(type(sentinel).__name__)): + setattr(sentinel, attr, 1) + with self.assertRaisesRegex(AttributeError, repr(attr)): + delattr(sentinel, attr) + with self.subTest(sentinel=name): + self.assertEqual(sentinel._name, name) # pylint: disable=protected-access + self.assertFalse(hasattr(sentinel, 'x')) + self.assertIs(type(sentinel)(), sentinel) + + def test_there_is_no_instance_dict_to_reach_round_the_hooks(self) -> 'None': + """``__slots__`` is empty, so :meth:`object.__setattr__` has nowhere to write.""" + module = _module() + for name, _, _ in SENTINELS: + sentinel = getattr(module, name) + with self.subTest(sentinel=name): + self.assertFalse(hasattr(sentinel, '__dict__')) + with self.assertRaises(AttributeError): + object.__setattr__(sentinel, 'x', 1) + + +class TypeImmutabilityTests(_ReimportCase): + """No attribute of a sentinel type can be set or deleted, as for ``type(None)``.""" + + def test_setting_or_deleting_any_type_attribute_raises_type_error(self) -> 'None': + module = _module() + types = [type(getattr(module, name)) for name, _, _ in SENTINELS] + [module.Sentinel] + for type_ in types: + for attr in ('x', '_name', '_instance', '__repr__', '__new__'): + with self.subTest(type=type_.__name__, attr=attr): + message = f'{attr!r} attribute of immutable type {type_.__name__!r}' + with self.assertRaisesRegex(TypeError, message): + setattr(type_, attr, None) + with self.assertRaisesRegex(TypeError, message): + delattr(type_, attr) + + def test_a_refused_write_leaves_the_type_as_it_was(self) -> 'None': + module = _module() + for name, _, _ in SENTINELS: + sentinel = getattr(module, name) + type_ = type(sentinel) + with self.subTest(sentinel=name): + for attr, value in (('_name', 'x'), ('_instance', None)): + with self.assertRaises(TypeError): + setattr(type_, attr, value) + self.assertEqual(type_._name, name) # pylint: disable=protected-access + self.assertIs(type_._instance, sentinel) # pylint: disable=protected-access + self.assertIs(type_(), sentinel) + self.assertEqual(repr(sentinel), name) + + +class SubclassTests(_ReimportCase): + """A concrete sentinel type cannot be subclassed; the abstract base can.""" + + def test_a_concrete_sentinel_type_cannot_be_subclassed(self) -> 'None': + """As ``class X(type(None))`` raises :exc:`TypeError`, so does this. + + :meth:`~abc.ABCMeta.register` is refused too: a virtual subclass would + pass an :func:`isinstance` check against the type just as a real one + would. + + """ + module = _module() + for name, _, _ in SENTINELS: + type_ = type(getattr(module, name)) + message = f'type {type_.__name__!r} is not an acceptable base type' + with self.subTest(sentinel=name): + with self.assertRaisesRegex(TypeError, message): + _subclass(type_) + with self.assertRaisesRegex(TypeError, message): + type_.register(int) + self.assertNotIsInstance(1, type_) + self.assertIs(type_.__dict__.get('__final__'), True) + + def test_the_abstract_base_can_still_be_subclassed(self) -> 'None': + """An abstract subclass, and a concrete one below it, are built as the library's are.""" + module = _module() + + class Abstract(module.Sentinel): + """An abstract sentinel base.""" + + __slots__ = () + + class Concrete(Abstract): + """A concrete sentinel.""" + + __slots__ = () + + _name = 'CONCRETE' + + class Virtual: + """A class to register as a virtual subclass of :class:`Abstract`.""" + + self.assertTrue(inspect.isabstract(Abstract)) + self.assertIs(Abstract.register(Virtual), Virtual) + self.assertIsInstance(Virtual(), Abstract) + + instance = Concrete() + self.assertIs(Concrete(), instance) + self.assertIs(copy.deepcopy(instance), instance) + self.assertEqual(repr(instance), 'CONCRETE') + with self.assertRaises(TypeError): + _subclass(Concrete) + with self.assertRaises(TypeError): + Concrete._name = 'x' # pylint: disable=protected-access + with self.assertRaises(AttributeError): + instance.x = 1 # pylint: disable=attribute-defined-outside-init + + class OldPickleTests(_ReimportCase): """A pickle written before the change still loads, as the sentinel where it can.""" diff --git a/tests/corekit/test_sentinels_housing_unit.py b/tests/corekit/test_sentinels_housing_unit.py index 825277046..1a96d6f7b 100644 --- a/tests/corekit/test_sentinels_housing_unit.py +++ b/tests/corekit/test_sentinels_housing_unit.py @@ -197,7 +197,7 @@ def test_each_consumer_module_still_imports_cleanly_on_its_own(self) -> 'None': class SentinelsModuleExportRuleTests(unittest.TestCase): - """The canonical module follows its own house rule: objects only, in ``__all__``. + """The canonical module follows its own house rule: no sentinel's type in ``__all__``. Nothing forces :mod:`pcapkit.corekit.sentinels` to honour the ruling that the sentinel objects, and not their types, are what gets exported -- the ruling @@ -205,6 +205,9 @@ class SentinelsModuleExportRuleTests(unittest.TestCase): a brand new module that violates the rule its own docstring cites would be a strange way to land it, so this pins that it does not. + It does export :class:`~pcapkit.corekit.sentinels.Sentinel`, the abstract + base every sentinel type shares, since that is no sentinel's own type. + """ def test_all_names_the_three_public_objects_and_no_types(self) -> 'None': @@ -218,6 +221,10 @@ def test_all_names_the_three_public_objects_and_no_types(self) -> 'None': with self.subTest(name=name): self.assertNotIn(name, sentinels.__all__) + live = importlib.import_module('pcapkit.corekit.sentinels') + types = [name for name in live.__all__ if isinstance(getattr(live, name), type)] + self.assertEqual(types, ['Sentinel']) + class PicklePayloadTests(unittest.TestCase): """A ``NULL`` pickle names :mod:`pcapkit.corekit.sentinels` and loads back. diff --git a/tests/protocols/test_construction_keyword_check_unit.py b/tests/protocols/test_construction_keyword_check_unit.py index 64d4514a7..0cd0ad26f 100644 --- a/tests/protocols/test_construction_keyword_check_unit.py +++ b/tests/protocols/test_construction_keyword_check_unit.py @@ -736,8 +736,9 @@ def test_the_absent_marker_is_an_instance_of_its_own_type(self) -> None: else: # pragma: no cover self.assertFalse(hasattr(AbsentType, '__final__')) - # A bare ``object()`` reads as ````. - self.assertEqual(repr(ABSENT), '') + # A bare ``object()`` reads as ````; this reads + # as its own name, as ``None`` does. + self.assertEqual(repr(ABSENT), '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. From 93c2a68ed8c501372b6978f5b7ee299bce5e91de Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sat, 17 Oct 2026 02:21:41 -0400 Subject: [PATCH 3/3] test(corekit): read the field constructor's own signature in the NO_VALUE repr test (#1841) - `test_no_value_reprs_as_its_own_name` called `inspect.signature` on the field class. On Python 3.8 that reports `Generic.__new__`'s `(*args, **kwds)` for a `Generic` subclass, so `parameters['default']` raised `KeyError` on the KTLO leg. Read `field.__init__` instead, which every version reports alike. Closes #1841 --- tests/corekit/test_sentinel_exports_unit.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index 18c64de28..2f98618f5 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -546,9 +546,13 @@ def test_no_value_reprs_as_its_own_name(self) -> 'None': no_value = _sentinels()[1][1] self.assertEqual(repr(no_value), 'NO_VALUE') + # NOTE: ``__init__`` rather than the class itself. Python 3.8 reports + # ``Generic.__new__``'s ``(*args, **kwds)`` as the signature of a + # ``Generic`` subclass, which both of these are; 3.9 and later report + # ``__init__``'s, so reading ``__init__`` directly agrees everywhere. for field in (StringField, IPv4AddressField): with self.subTest(field=field.__name__): - parameter = inspect.signature(field).parameters['default'] + parameter = inspect.signature(field.__init__).parameters['default'] self.assertIs(parameter.default, no_value) self.assertTrue(str(parameter).endswith(' = NO_VALUE'), str(parameter))