diff --git a/docs/source/contributing/conventions/sentinel-convention.rst b/docs/source/contributing/conventions/sentinel-convention.rst index c0298dc275..c12512d94b 100644 --- a/docs/source/contributing/conventions/sentinel-convention.rst +++ b/docs/source/contributing/conventions/sentinel-convention.rst @@ -35,6 +35,9 @@ derives from it mechanically -- title-case each underscore-separated word and ap * - ``ABSENT`` - :class:`~pcapkit.corekit.sentinels.AbsentType` - :mod:`pcapkit.corekit.sentinels` + * - ``MISSING`` + - :class:`~pcapkit.corekit.sentinels.MissingType` + - :mod:`pcapkit.corekit.sentinels` GitHub issue :issue:`911`'s housing ruling -- one module for all of them -- is why the table names a single defining module, and it is the only import path. The modules that *use* a @@ -129,13 +132,13 @@ Where the sentinels differ, the differences are **needs, not inconsistencies**: of the modules that imports a sentinel from there has no effect on it. ``__bool__`` returning :obj:`False` - ``NULL``, ``NO_VALUE`` and ``ABSENT`` have it, because each stands for an *absent - value* and reads naturally in a boolean test. ``NO_DEFAULT`` deliberately does - **not**: it is a marker meaning *no default was supplied*, it is only ever tested - with ``is``, and making it falsy would invite ``if not default:`` -- which would - then treat a caller's genuine falsy default (``0``, ``''``, :obj:`None`, - :obj:`False`) the same as the sentinel, the very confusion the sentinel exists to - prevent. + ``NULL``, ``NO_VALUE``, ``ABSENT`` and ``MISSING`` have it, because each stands for + an *absent value* and reads naturally in a boolean test. ``NO_DEFAULT`` + deliberately does **not**: it is a marker meaning *no default was supplied*, it is + only ever tested with ``is``, and making it falsy would invite + ``if not default:`` -- which would then treat a caller's genuine falsy default + (``0``, ``''``, :obj:`None`, :obj:`False`) the same as the sentinel, the very + confusion the sentinel exists to prevent. ``__copy__`` / ``__deepcopy__`` / ``__reduce__`` Every sentinel has them, from the base. The owner ruled on :issue:`1842` that every @@ -152,9 +155,9 @@ Where the sentinels differ, the differences are **needs, not inconsistencies**: ``__repr__`` 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 - :class:`str` gives the same. That is what a signature, :func:`help` and a traceback - show. + ``NO_VALUE``, ``NO_DEFAULT``, ``ABSENT``, ``MISSING`` -- as ``repr(None)`` is + ``'None'``, and :class:`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 diff --git a/docs/source/pcapkit/corekit/sentinels.rst b/docs/source/pcapkit/corekit/sentinels.rst index 0f85a9592b..d7c83e081a 100644 --- a/docs/source/pcapkit/corekit/sentinels.rst +++ b/docs/source/pcapkit/corekit/sentinels.rst @@ -44,15 +44,19 @@ subclassed (:issue:`1842`). .. note:: - :class:`AbsentType` and :data:`ABSENT` are **private** -- never imported outside - :mod:`pcapkit.protocols.protocol`, and named in no module's ``__all__``. For - SCREAMING_SNAKE consistency with the other sentinels (see - :ref:`sentinel-convention`), neither carries the leading underscore that would - hide it from Sphinx, so both are documented below and marked private. + :class:`AbsentType` and :data:`ABSENT` are **private** to + :mod:`pcapkit.protocols.protocol`, and :class:`MissingType` and :data:`MISSING` to + :mod:`pcapkit.foundation.registry.protocols`: never imported outside it, and named + in no module's ``__all__``. For SCREAMING_SNAKE consistency with the other + sentinels (see :ref:`sentinel-convention`), none carries the leading underscore + that would hide it from Sphinx, so each is documented below and marked private. .. autoclass:: pcapkit.corekit.sentinels.AbsentType .. autodata:: pcapkit.corekit.sentinels.ABSENT +.. autoclass:: pcapkit.corekit.sentinels.MissingType +.. autodata:: pcapkit.corekit.sentinels.MISSING + Internal Definitions -------------------- diff --git a/pcapkit/corekit/sentinels.py b/pcapkit/corekit/sentinels.py index 19f87a9690..893bd8e106 100644 --- a/pcapkit/corekit/sentinels.py +++ b/pcapkit/corekit/sentinels.py @@ -28,9 +28,10 @@ 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; -:class:`AbsentType`'s docstring says what marks it private instead. +``ABSENT`` is private to :mod:`pcapkit.protocols.protocol`, and ``MISSING`` to +:mod:`pcapkit.foundation.registry.protocols` -- nothing outside its module ever +reads either. Neither carries a leading underscore; :class:`AbsentType`'s +docstring says what marks them private instead. .. versionadded:: 1.5.0 @@ -577,3 +578,32 @@ def __bool__(self) -> 'Literal[False]': #: that one read. Private by documentation rather than by a #: leading underscore -- see :class:`AbsentType`'s own docstring. ABSENT = AbsentType() + + +@final +class MissingType(Sentinel): + """Type of :data:`MISSING`, the sentinel for a name the protocol registry did not hold. + + A sibling of :class:`AbsentType` rather than a reuse, since that one is + documented as never leaving :mod:`pcapkit.protocols.protocol`. Private in + the same way, by documentation: nothing outside + :mod:`pcapkit.foundation.registry.protocols` reads :data:`MISSING`, and no + module's :attr:`__all__` names either one (:issue:`2555`). + + """ + + __slots__ = () + + _name = 'MISSING' + + def __bool__(self) -> 'Literal[False]': + """Return :obj:`False`.""" + return False + + +#: MissingType: Default for reading :data:`pcapkit.protocols.__proto__` in +#: :mod:`pcapkit.foundation.registry.protocols`, recorded as what a code +#: registration displaced when the name was new, so that handing the code back +#: removes the name rather than restoring an entry. Private by documentation +#: rather than by a leading underscore -- see :class:`MissingType`. +MISSING = MissingType() diff --git a/pcapkit/foundation/registry/protocols.py b/pcapkit/foundation/registry/protocols.py index 811ccbc181..8d9b71e25b 100644 --- a/pcapkit/foundation/registry/protocols.py +++ b/pcapkit/foundation/registry/protocols.py @@ -49,7 +49,7 @@ from pcapkit.const.tcp.mp_tcp_option import MPTCPOption as TCP_MPTCPOption from pcapkit.const.tcp.option import Option as TCP_Option from pcapkit.corekit.module import ModuleDescriptor -from pcapkit.corekit.sentinels import NULL +from pcapkit.corekit.sentinels import MISSING, NULL from pcapkit.protocols import __proto__ as protocol_registry from pcapkit.protocols.application.httpv2 import HTTPv2 from pcapkit.protocols.internet.hip import HIP @@ -279,10 +279,6 @@ class rather than supplied by a caller, and this function is the funnel logger.debug('registered protocol: %s', protocol.__name__) -#: Sentinel for a name :data:`pcapkit.protocols.__proto__` did not hold. -_MISSING = object() - - class _NameLease: """A :data:`pcapkit.protocols.__proto__` entry a code registration put there. @@ -308,7 +304,7 @@ def __init__(self, name: 'str', protocol: 'Type[ProtocolBase]', displaced: 'Any' self.name = name #: The class written under it. self.protocol = protocol - #: The entry it displaced, or :data:`_MISSING` for a new name. + #: The entry it displaced, or :data:`~pcapkit.corekit.sentinels.MISSING` for a new name. self.displaced = displaced #: ``(destination, code)`` registrations still holding the lease. self.codes = set() # type: set[tuple[type, Any]] @@ -384,7 +380,7 @@ def _release_name(target: 'tuple[type, Any]') -> 'None': if above.displaced is above.protocol: _fold_lease(stack, index) elif protocol_registry.get(lease.name) is lease.protocol: - if lease.displaced is _MISSING: + if lease.displaced is MISSING: protocol_registry.pop(lease.name, None) else: protocol_registry[lease.name] = lease.displaced @@ -420,7 +416,7 @@ def _register_name(protocol: 'Type[ProtocolBase]', *targets: 'tuple[type, Any]') if current is None or _CODE_LEASES.get(target) is not current: _release_name(target) - incumbent = protocol_registry.get(name, _MISSING) + incumbent = protocol_registry.get(name, MISSING) register_protocol(protocol) if protocol_registry.get(name) is not protocol: return diff --git a/tests/corekit/test_sentinel_bare_object_2555_unit.py b/tests/corekit/test_sentinel_bare_object_2555_unit.py new file mode 100644 index 0000000000..c937de7e61 --- /dev/null +++ b/tests/corekit/test_sentinel_bare_object_2555_unit.py @@ -0,0 +1,79 @@ +# -*- coding: utf-8 -*- +"""No sentinel the package writes for itself is a bare ``object()``. C.f. #2555. + +The "Naming a Sentinel" convention puts every such sentinel in +:mod:`pcapkit.corekit.sentinels` as a named class instance, public or not; its +"Why a Class and Not ``object()``" section gives the reason, a repr that is an +address. A sentinel is a singleton, so the scan flags an argument-less +``object()`` that runs when its module is imported -- in a module or class body, +a default value or a decorator -- and skips function bodies, which build a fresh +object per call. Without #2555 it flags ``_MISSING`` in +:mod:`pcapkit.foundation.registry.protocols`. + +The page exempts a verbatim upstream port; the tree holds none that needs it. + +""" + +from __future__ import annotations + +import ast +import pathlib +import textwrap +import unittest + +#: Repository root. +ROOT = pathlib.Path(__file__).resolve().parents[2] + + +def _import_time_bare_objects(node: 'ast.AST') -> 'list[int]': + """Line of every argument-less ``object()`` under ``node`` that runs at import. + + A function's defaults and decorators run where it is defined, and its body + only when it is called, so the body is not walked. + + """ + lines = [] # type: list[int] + if (isinstance(node, ast.Call) and isinstance(node.func, ast.Name) + and node.func.id == 'object' and not node.args and not node.keywords): + lines.append(node.lineno) + + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)): + children = [*node.args.defaults, *filter(None, node.args.kw_defaults), + *getattr(node, 'decorator_list', ())] # type: list[ast.AST] + else: + children = list(ast.iter_child_nodes(node)) + for child in children: + lines.extend(_import_time_bare_objects(child)) + return lines + + +class BareObjectSentinelTests(unittest.TestCase): + """A sentinel is a :class:`~pcapkit.corekit.sentinels.Sentinel`, never ``object()``.""" + + def test_no_module_makes_a_bare_object_at_import(self) -> 'None': + found = [] # type: list[str] + for path in sorted((ROOT / 'pcapkit').rglob('*.py')): + tree = ast.parse(path.read_text(encoding='utf-8'), filename=str(path)) + found.extend(f'{path.relative_to(ROOT).as_posix()}:{line}' + for line in _import_time_bare_objects(tree)) + self.assertEqual(found, [], 'define the sentinel in pcapkit.corekit.sentinels ' + 'and list it in sentinel-convention.rst') + + def test_the_scan_covers_import_time_and_skips_function_bodies(self) -> 'None': + """Module, class, default and decorator ``object()`` calls; no function or lambda body.""" + source = textwrap.dedent('''\ + A = object() + class K: + B = object() + def f(x=object(), *, y=object()): + local = object() + g = lambda z=object(): object() + @decorate(object()) + def h(): pass + ''') + self.assertEqual(sorted(_import_time_bare_objects(ast.parse(source))), + [1, 3, 4, 4, 6, 7]) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/corekit/test_sentinel_exports_unit.py b/tests/corekit/test_sentinel_exports_unit.py index 970ed7cff9..a4f351a13e 100644 --- a/tests/corekit/test_sentinel_exports_unit.py +++ b/tests/corekit/test_sentinel_exports_unit.py @@ -110,6 +110,7 @@ def _sentinels() -> 'tuple[tuple[str, object, type], ...]': ('NO_VALUE', canonical.NO_VALUE, canonical.NoValueType), ('NO_DEFAULT', canonical.NO_DEFAULT, canonical.NoDefaultType), ('ABSENT', canonical.ABSENT, canonical.AbsentType), + ('MISSING', canonical.MISSING, canonical.MissingType), ) @@ -285,6 +286,14 @@ def test_the_private_sentinel_is_exported_neither_way(self) -> 'None': self.assertNotIn('AbsentType', protocol.__all__) self.assertNotIn('ABSENT', _star_import('pcapkit.protocols.protocol')) + def test_the_registry_sentinel_is_exported_neither_way(self) -> 'None': + """``MISSING`` is private to the protocol registry, as ``ABSENT`` is to its module (#2555).""" + import pcapkit.foundation.registry.protocols as registry + + self.assertNotIn('MISSING', registry.__all__) + self.assertNotIn('MissingType', registry.__all__) + self.assertNotIn('MISSING', _star_import('pcapkit.foundation.registry.protocols')) + class SentinelPopulationTests(_ReimportCase): """They follow the naming rule, and the docs say so.""" @@ -439,9 +448,9 @@ class SentinelBehaviourTests(_ReimportCase): """ def test_the_absent_value_sentinels_are_falsy(self) -> 'None': - """``NULL``, ``NO_VALUE`` and ``ABSENT`` each stand for an absent value.""" + """``NULL``, ``NO_VALUE``, ``ABSENT`` and ``MISSING`` each stand for an absent value.""" live = {name: obj for name, obj, _ in _sentinels()} - for sentinel in (live['NULL'], live['NO_VALUE'], live['ABSENT']): + for sentinel in (live['NULL'], live['NO_VALUE'], live['ABSENT'], live['MISSING']): with self.subTest(sentinel=repr(sentinel)): self.assertFalse(sentinel) diff --git a/tests/corekit/test_sentinel_py_abc_1960_unit.py b/tests/corekit/test_sentinel_py_abc_1960_unit.py index 18756cff4b..e55546f897 100644 --- a/tests/corekit/test_sentinel_py_abc_1960_unit.py +++ b/tests/corekit/test_sentinel_py_abc_1960_unit.py @@ -62,6 +62,7 @@ ('NoValueType', 'NO_VALUE'), ('NoDefaultType', 'NO_DEFAULT'), ('AbsentType', 'ABSENT'), + ('MissingType', 'MISSING'), ) #: Run in the child. It prints one JSON object: whether the fallback was taken, diff --git a/tests/corekit/test_sentinel_singleton_1842_unit.py b/tests/corekit/test_sentinel_singleton_1842_unit.py index 92e48feb45..2caba33848 100644 --- a/tests/corekit/test_sentinel_singleton_1842_unit.py +++ b/tests/corekit/test_sentinel_singleton_1842_unit.py @@ -57,6 +57,7 @@ ('NO_VALUE', 'NO_VALUE', False), ('NO_DEFAULT', 'NO_DEFAULT', True), ('ABSENT', 'ABSENT', False), + ('MISSING', 'MISSING', False), ) #: Python's own singletons, which a sentinel is to behave like. diff --git a/tests/corekit/test_sentinels_housing_unit.py b/tests/corekit/test_sentinels_housing_unit.py index 881bd92aef..7a89d74829 100644 --- a/tests/corekit/test_sentinels_housing_unit.py +++ b/tests/corekit/test_sentinels_housing_unit.py @@ -10,11 +10,13 @@ So :mod:`pcapkit.corekit.sentinels` is the single defining module for every sentinel -- :class:`~pcapkit.corekit.sentinels.NullType`, :class:`~pcapkit.corekit.sentinels.NoValueType`, -:class:`~pcapkit.corekit.sentinels.NoDefaultType` and -:class:`~pcapkit.corekit.sentinels.AbsentType`, and their instances. The +:class:`~pcapkit.corekit.sentinels.NoDefaultType`, +:class:`~pcapkit.corekit.sentinels.AbsentType` and +:class:`~pcapkit.corekit.sentinels.MissingType`, and their instances. The modules that use them (:mod:`pcapkit.corekit.module`, -:mod:`pcapkit.corekit.fields.field`, :mod:`pcapkit.corekit.enum` and -:mod:`pcapkit.protocols.protocol`) import the *object* they use from here and +:mod:`pcapkit.corekit.fields.field`, :mod:`pcapkit.corekit.enum`, +:mod:`pcapkit.protocols.protocol` and +:mod:`pcapkit.foundation.registry.protocols`) import the *object* they use from here and keep no re-export of it (GitHub issue #1744), under the ruling on #1715 that a moved name keeps no re-export at its old path; ``tests/project/test_no_moved_name_re_exports_1744_unit.py`` pins that. @@ -65,9 +67,10 @@ import pcapkit.corekit.fields.field as field_module import pcapkit.corekit.module as module_module import pcapkit.corekit.sentinels as sentinels +import pcapkit.foundation.registry.protocols as registry_module import pcapkit.protocols.protocol as protocol_module -from pcapkit.corekit.sentinels import (ABSENT, NO_DEFAULT, NO_VALUE, NULL, AbsentType, - NoDefaultType, NoValueType, NullType) +from pcapkit.corekit.sentinels import (ABSENT, MISSING, NO_DEFAULT, NO_VALUE, NULL, AbsentType, + MissingType, NoDefaultType, NoValueType, NullType) from tests._support import purge_modules #: Test tier (#1958): exercises pickling and copying, whose protocols differ by @@ -82,6 +85,7 @@ ('NO_VALUE', field_module, NO_VALUE, NoValueType), ('NO_DEFAULT', enum_module, NO_DEFAULT, NoDefaultType), ('ABSENT', protocol_module, ABSENT, AbsentType), + ('MISSING', registry_module, MISSING, MissingType), ) @@ -129,8 +133,8 @@ def test_sentinels_module_imports_none_of_its_consumers(self) -> 'None': A cycle here would need :mod:`pcapkit.corekit.sentinels` to import from :mod:`pcapkit.corekit.module`, :mod:`pcapkit.corekit.fields.field`, - :mod:`pcapkit.corekit.enum` or :mod:`pcapkit.protocols.protocol` -- each - of which imports *it*. Walked with :mod:`ast` rather than grepped for the + :mod:`pcapkit.corekit.enum`, :mod:`pcapkit.protocols.protocol` or + :mod:`pcapkit.foundation.registry.protocols` -- each of which imports *it*. Walked with :mod:`ast` rather than grepped for the dotted name as plain text, because the sentinels module's own docstring names those modules *in prose*, explaining that they import from it -- a text search would flag that explanation as if it were the cycle this @@ -151,7 +155,8 @@ def test_sentinels_module_imports_none_of_its_consumers(self) -> 'None': imported_modules.append(node.module) forbidden = ('pcapkit.corekit.module', 'pcapkit.corekit.fields.field', - 'pcapkit.corekit.enum', 'pcapkit.protocols.protocol') + 'pcapkit.corekit.enum', 'pcapkit.protocols.protocol', + 'pcapkit.foundation.registry.protocols') for name in forbidden: with self.subTest(forbidden=name): self.assertFalse( @@ -189,7 +194,8 @@ def test_each_consumer_module_still_imports_cleanly_on_its_own(self) -> 'None': """ for name in ('pcapkit.corekit.module', 'pcapkit.corekit.fields.field', - 'pcapkit.corekit.enum', 'pcapkit.protocols.protocol'): + 'pcapkit.corekit.enum', 'pcapkit.protocols.protocol', + 'pcapkit.foundation.registry.protocols'): with self.subTest(first_import=name): purge_modules(['pcapkit']) try: @@ -214,13 +220,13 @@ class SentinelsModuleExportRuleTests(unittest.TestCase): """ def test_all_names_the_public_objects_and_no_types(self) -> 'None': - """``ABSENT`` stays out too: it is private regardless of which module - defines it.""" + """``ABSENT`` and ``MISSING`` stay out too: they are private regardless + of which module defines them.""" self.assertIn('NULL', sentinels.__all__) self.assertIn('NO_VALUE', sentinels.__all__) self.assertIn('NO_DEFAULT', sentinels.__all__) for name in ('NullType', 'NoValueType', 'NoDefaultType', - 'ABSENT', 'AbsentType'): + 'ABSENT', 'AbsentType', 'MISSING', 'MissingType'): with self.subTest(name=name): self.assertNotIn(name, sentinels.__all__) diff --git a/tests/foundation/registry/test_registry_name_restore_unit.py b/tests/foundation/registry/test_registry_name_restore_unit.py index 73d82eb2ee..617a49f610 100644 --- a/tests/foundation/registry/test_registry_name_restore_unit.py +++ b/tests/foundation/registry/test_registry_name_restore_unit.py @@ -88,6 +88,18 @@ def test_restoring_a_code_removes_the_name_it_added(self) -> None: self.assertNotIn('CUSTOM', self.names) self.assert_names_as_found() + def test_a_new_name_records_the_shared_missing_sentinel(self) -> None: + """A lease on a new name displaced ``MISSING``, not a module-local ``object()`` (#2555).""" + from pcapkit.corekit.sentinels import MISSING + from pcapkit.foundation.registry import protocols, register_tcp + from pcapkit.protocols.transport.tcp import TCP + + shipped = TCP.__proto__[80] + self._call(register_tcp, 80, self._custom('Custom')) + self.assertIs(protocols._CODE_LEASES[TCP, 80].displaced, MISSING) # pylint: disable=protected-access + self._call(register_tcp, 80, shipped) + self.assert_names_as_found() + def test_restoring_a_code_puts_back_the_name_it_displaced(self) -> None: """A custom class named like a built-in hands the built-in its name back.""" from pcapkit.foundation.registry import register_tcp