From f46335132dc9ca7b2b0d194abfd5a16d335488eb Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Mon, 19 Oct 2026 14:15:27 -0400 Subject: [PATCH] docs(const): link the remaining internal references in the AppType and OptionType templates (#1947) - Link TransportProtocol in AppType._dispatch and the crawler's flag docstring, and AppType.get in the per-transport __canonical__ comment - Give AppType.__new__, __repr__, __str__, __int__, __eq__ and OptionType.__new__ docstrings, so their entries on reg.rst and pcapng.rst render text - Edit the vendor templates and regenerate the const modules byte-identically from the committed crawled data - Add a project test pinning the roles, the docstrings and their parity with the templates; drop the #1940 test's docstring exemption for AppType.__eq__ Closes #1947 --- pcapkit/const/pcapng/option_type.py | 8 + pcapkit/const/reg/apptype/apptype.py | 22 ++- pcapkit/const/reg/apptype/dccp.py | 2 +- pcapkit/const/reg/apptype/sctp.py | 2 +- pcapkit/const/reg/apptype/tcp.py | 2 +- pcapkit/const/reg/apptype/udp.py | 2 +- pcapkit/vendor/pcapng/option_type.py | 8 + pcapkit/vendor/reg/apptype/apptype.py | 27 +++- .../test_docs_internal_refs_1940_unit.py | 6 - .../test_docs_internal_refs_1947_unit.py | 144 ++++++++++++++++++ 10 files changed, 209 insertions(+), 14 deletions(-) create mode 100644 tests/project/test_docs_internal_refs_1947_unit.py diff --git a/pcapkit/const/pcapng/option_type.py b/pcapkit/const/pcapng/option_type.py index 72e9cd9a87..a9166c3a45 100644 --- a/pcapkit/const/pcapng/option_type.py +++ b/pcapkit/const/pcapng/option_type.py @@ -37,6 +37,14 @@ class OptionType(EnumRegistry, StrEnum): __members_ns__: 'DefaultDict[str, dict[int, OptionType]]' = defaultdict(dict) def __new__(cls, value: 'int', name: 'str' = 'opt_unknown') -> 'Type[OptionType]': + """Create a member from a registry row and file it under its namespace. + + Args: + value: Option code. + name: Option name, whose text before the first underscore is the + namespace it is filed under in :attr:`__members_ns__`. + + """ temp = '%s [%d]' % (name, value) obj = str.__new__(cls, temp) diff --git a/pcapkit/const/reg/apptype/apptype.py b/pcapkit/const/reg/apptype/apptype.py index 861dd5b1ba..2d56763c34 100644 --- a/pcapkit/const/reg/apptype/apptype.py +++ b/pcapkit/const/reg/apptype/apptype.py @@ -2408,6 +2408,18 @@ class AppType(EnumRegistry, StrEnum): def __new__(cls, value: 'int', name: 'str' = '', proto: 'TransportProtocol' = TransportProtocol.undefined) -> 'Type[AppType]': + """Create a member from a registry row and add it to :attr:`__registry__`. + + Args: + value: Port number. + name: Service name. + proto: Transport protocol carrying the service. + + Raises: + ValueError: If called on the base registry, which holds no members + of its own, rather than on a per-transport subclass. + + """ temp = f'{name} [{value} - {proto.name}]' obj = str.__new__(cls, temp) @@ -2453,6 +2465,8 @@ def aliases(self) -> 'tuple[AppType, ...]': if member is not self) def __repr__(self) -> 'str': + """Show the member as ````, listing its + :attr:`aliases` before the closing bracket when it has any.""" # NOTE: GitHub issue #807. The suffix below is the only difference from # the plain form GitHub issue #806 already fixed: it is appended only # when :attr:`aliases` is non-empty, so every member that does not @@ -2468,6 +2482,8 @@ def __repr__(self) -> 'str': return f'{base} (aliases: {", ".join(alias.svc for alias in aliases)})>' def __str__(self) -> 'str': + """Show the member as ``service [port - transport]``, followed by its + :attr:`aliases` when it has any.""" # NOTE: same suffix as __repr__, for the same reason, and -- like that # suffix, and unlike everything else this formats -- never folded into # ``_value_``. GitHub issue #807's hard constraint: ``_value_`` is the @@ -2487,6 +2503,7 @@ def __str__(self) -> 'str': return f'{base} (aliases: {", ".join(alias.svc for alias in aliases)})' def __int__(self) -> 'int': + """Convert the member to its port number.""" return self.port def __lt__(self, other: 'AppType') -> 'bool': @@ -2502,6 +2519,7 @@ def __ge__(self, other: 'AppType') -> 'bool': return self.port >= other def __eq__(self, other: 'Any') -> 'bool': + """Compare the member's port number with ``other``.""" return self.port == other def __ne__(self, other: 'Any') -> 'bool': @@ -2519,7 +2537,9 @@ def _dispatch(cls, key: 'int', proto: 'TransportProtocol | str | int') -> 'Type[ every entry point rejects a non-port identically. proto: Transport protocol, as a member, its name, or a bare :class:`int`. That last shape is not merely defensive: GitHub - issue :issue:`808` dropped ``TransportProtocol``'s ``IntFlag`` base, so + issue :issue:`808` dropped + :class:`~pcapkit.const.reg.apptype.apptype.TransportProtocol`'s + ``IntFlag`` base, so ``TransportProtocol.a | TransportProtocol.b`` -- built by hand, the same as any caller passing a literal port-transport bitmask -- falls through to ``int.__or__`` and returns a bare diff --git a/pcapkit/const/reg/apptype/dccp.py b/pcapkit/const/reg/apptype/dccp.py index 6632a75e25..46904ddb1e 100644 --- a/pcapkit/const/reg/apptype/dccp.py +++ b/pcapkit/const/reg/apptype/dccp.py @@ -50,7 +50,7 @@ class DCCP(AppType): __registry__: 'MultiDict[int, DCCP]' = MultiDict() #: The canonical service for each port IANA assigns more than one to, i.e. - #: what ``AppType.get()`` answers with. + #: what :meth:`AppType.get() ` answers with. #: Taken from :file:`/etc/services`, which is what #: :func:`socket.getservbyport` reads -- IANA names no precedence among the #: services it registers on one port, and registry row order does not supply diff --git a/pcapkit/const/reg/apptype/sctp.py b/pcapkit/const/reg/apptype/sctp.py index d7e010e55f..28b8b7e2d3 100644 --- a/pcapkit/const/reg/apptype/sctp.py +++ b/pcapkit/const/reg/apptype/sctp.py @@ -54,7 +54,7 @@ class SCTP(AppType): __registry__: 'MultiDict[int, SCTP]' = MultiDict() #: The canonical service for each port IANA assigns more than one to, i.e. - #: what ``AppType.get()`` answers with. + #: what :meth:`AppType.get() ` answers with. #: Taken from :file:`/etc/services`, which is what #: :func:`socket.getservbyport` reads -- IANA names no precedence among the #: services it registers on one port, and registry row order does not supply diff --git a/pcapkit/const/reg/apptype/tcp.py b/pcapkit/const/reg/apptype/tcp.py index e2f528d157..be8d709353 100644 --- a/pcapkit/const/reg/apptype/tcp.py +++ b/pcapkit/const/reg/apptype/tcp.py @@ -906,7 +906,7 @@ class TCP(AppType): __registry__: 'MultiDict[int, TCP]' = MultiDict() #: The canonical service for each port IANA assigns more than one to, i.e. - #: what ``AppType.get()`` answers with. + #: what :meth:`AppType.get() ` answers with. #: Taken from :file:`/etc/services`, which is what #: :func:`socket.getservbyport` reads -- IANA names no precedence among the #: services it registers on one port, and registry row order does not supply diff --git a/pcapkit/const/reg/apptype/udp.py b/pcapkit/const/reg/apptype/udp.py index 883a39c77e..d6d5c1ee73 100644 --- a/pcapkit/const/reg/apptype/udp.py +++ b/pcapkit/const/reg/apptype/udp.py @@ -373,7 +373,7 @@ class UDP(AppType): __registry__: 'MultiDict[int, UDP]' = MultiDict() #: The canonical service for each port IANA assigns more than one to, i.e. - #: what ``AppType.get()`` answers with. + #: what :meth:`AppType.get() ` answers with. #: Taken from :file:`/etc/services`, which is what #: :func:`socket.getservbyport` reads -- IANA names no precedence among the #: services it registers on one port, and registry row order does not supply diff --git a/pcapkit/vendor/pcapng/option_type.py b/pcapkit/vendor/pcapng/option_type.py index 90d475e68c..033fcc5581 100644 --- a/pcapkit/vendor/pcapng/option_type.py +++ b/pcapkit/vendor/pcapng/option_type.py @@ -65,6 +65,14 @@ class {NAME}(EnumRegistry, StrEnum): __members_ns__: 'DefaultDict[str, dict[int, {NAME}]]' = defaultdict(dict) def __new__(cls, value: 'int', name: 'str' = 'opt_unknown') -> 'Type[{NAME}]': + """Create a member from a registry row and file it under its namespace. + + Args: + value: Option code. + name: Option name, whose text before the first underscore is the + namespace it is filed under in :attr:`__members_ns__`. + + """ temp = '%s [%d]' % (name, value) obj = str.__new__(cls, temp) diff --git a/pcapkit/vendor/reg/apptype/apptype.py b/pcapkit/vendor/reg/apptype/apptype.py index dd56f1153d..d2d0115fc0 100644 --- a/pcapkit/vendor/reg/apptype/apptype.py +++ b/pcapkit/vendor/reg/apptype/apptype.py @@ -362,6 +362,18 @@ class {NAME}(EnumRegistry, StrEnum): def __new__(cls, value: 'int', name: 'str' = '', proto: 'TransportProtocol' = TransportProtocol.undefined) -> 'Type[{NAME}]': + """Create a member from a registry row and add it to :attr:`__registry__`. + + Args: + value: Port number. + name: Service name. + proto: Transport protocol carrying the service. + + Raises: + ValueError: If called on the base registry, which holds no members + of its own, rather than on a per-transport subclass. + + """ temp = f'{{name}} [{{value}} - {{proto.name}}]' obj = str.__new__(cls, temp) @@ -407,6 +419,8 @@ def aliases(self) -> 'tuple[{NAME}, ...]': if member is not self) def __repr__(self) -> 'str': + """Show the member as ````, listing its + :attr:`aliases` before the closing bracket when it has any.""" # NOTE: GitHub issue #807. The suffix below is the only difference from # the plain form GitHub issue #806 already fixed: it is appended only # when :attr:`aliases` is non-empty, so every member that does not @@ -422,6 +436,8 @@ def __repr__(self) -> 'str': return f'{{base}} (aliases: {{", ".join(alias.svc for alias in aliases)}})>' def __str__(self) -> 'str': + """Show the member as ``service [port - transport]``, followed by its + :attr:`aliases` when it has any.""" # NOTE: same suffix as __repr__, for the same reason, and -- like that # suffix, and unlike everything else this formats -- never folded into # ``_value_``. GitHub issue #807's hard constraint: ``_value_`` is the @@ -441,6 +457,7 @@ def __str__(self) -> 'str': return f'{{base}} (aliases: {{", ".join(alias.svc for alias in aliases)}})' def __int__(self) -> 'int': + """Convert the member to its port number.""" return self.port def __lt__(self, other: '{NAME}') -> 'bool': @@ -456,6 +473,7 @@ def __ge__(self, other: '{NAME}') -> 'bool': return self.port >= other def __eq__(self, other: 'Any') -> 'bool': + """Compare the member's port number with ``other``.""" return self.port == other def __ne__(self, other: 'Any') -> 'bool': @@ -473,7 +491,9 @@ def _dispatch(cls, key: 'int', proto: 'TransportProtocol | str | int') -> 'Type[ every entry point rejects a non-port identically. proto: Transport protocol, as a member, its name, or a bare :class:`int`. That last shape is not merely defensive: GitHub - issue :issue:`808` dropped ``TransportProtocol``'s ``IntFlag`` base, so + issue :issue:`808` dropped + :class:`~pcapkit.const.reg.apptype.apptype.TransportProtocol`'s + ``IntFlag`` base, so ``TransportProtocol.a | TransportProtocol.b`` -- built by hand, the same as any caller passing a literal port-transport bitmask -- falls through to ``int.__or__`` and returns a bare @@ -1006,7 +1026,7 @@ class {NAME}(AppType): __registry__: 'MultiDict[int, {NAME}]' = MultiDict() #: The canonical service for each port IANA assigns more than one to, i.e. - #: what ``AppType.get()`` answers with. + #: what :meth:`AppType.get() ` answers with. #: Taken from :file:`/etc/services`, which is what #: :func:`socket.getservbyport` reads -- IANA names no precedence among the #: services it registers on one port, and registry row order does not supply @@ -1106,7 +1126,8 @@ def wrap_comment(text: 'str') -> 'str': @staticmethod def flag(protos: 'Iterable[str]') -> 'str': - """Render transport protocol names as a ``TransportProtocol`` expression. + """Render transport protocol names as a + :class:`~pcapkit.const.reg.apptype.apptype.TransportProtocol` expression. Args: protos: Transport protocol names. diff --git a/tests/project/test_docs_internal_refs_1940_unit.py b/tests/project/test_docs_internal_refs_1940_unit.py index f1db4fb3d9..b5b90aaf19 100644 --- a/tests/project/test_docs_internal_refs_1940_unit.py +++ b/tests/project/test_docs_internal_refs_1940_unit.py @@ -87,10 +87,6 @@ ('pcapkit/vendor/reg.rst', 'pcapkit.vendor.reg.ethertype.EtherType._insert_range'), ) -#: Helpers in :data:`HELPERS` documented without a docstring of their own: the vendor -#: template that generates their module is outside this change (#1920). -UNDOCSTRINGED = frozenset({'pcapkit.const.reg.apptype.apptype.AppType.__eq__'}) - #: ``(source, literal, the role that replaced it)``. The first two are the #: examples #1940 names. CONVERTED = ( @@ -236,8 +232,6 @@ def test_helpers_are_real_objects(self) -> 'None': def test_documented_functions_have_docstrings(self) -> 'None': """A helper's entry renders its docstring, so a function must have one of its own.""" for _, path in HELPERS: - if path in UNDOCSTRINGED: - continue owner, _, name = path.rpartition('.') parent = _resolve(owner) obj = vars(parent).get(name) if isinstance(parent, type) else getattr(parent, name) diff --git a/tests/project/test_docs_internal_refs_1947_unit.py b/tests/project/test_docs_internal_refs_1947_unit.py new file mode 100644 index 0000000000..f791faa743 --- /dev/null +++ b/tests/project/test_docs_internal_refs_1947_unit.py @@ -0,0 +1,144 @@ +# -*- coding: utf-8 -*- +"""Pins the internal references and special-method docstrings of the AppType and OptionType templates (#1947). + +#1940 converts library-internal literals to roles and gives every special +method that an API page names an entry; the AppType and OptionType templates, +and the const modules generated from them, are the part #1947 covers. + +* :class:`ConvertedReferenceTests` -- the references in those templates and + their generated modules, pinned as roles naming a declared entry. +* :class:`SpecialMethodDocstringTests` -- the special methods ``reg.rst`` and + ``pcapng.rst`` declare have docstrings to render. +* :class:`TemplateParityTests` -- each of those docstrings is the one its vendor + template renders, so regenerating the const module keeps it. + +""" + +from __future__ import annotations + +import ast +import importlib +import inspect +import unittest +from typing import TYPE_CHECKING + +from tests.project.test_docs_internal_refs_1940_unit import (DOCS, ROLE, ROOT, _resolve, declared, + declared_anywhere, is_declared) + +if TYPE_CHECKING: + from typing import Any + +TRANSPORT_PROTOCOL = ':class:`~pcapkit.const.reg.apptype.apptype.TransportProtocol`' +APPTYPE_GET = ':meth:`AppType.get() `' + +#: ``(source, literal in context, the same context with the role)``. +CONVERTED = ( + ('pcapkit/vendor/reg/apptype/apptype.py', "dropped ``TransportProtocol``'s", + f"dropped\n {TRANSPORT_PROTOCOL}'s"), + ('pcapkit/const/reg/apptype/apptype.py', "dropped ``TransportProtocol``'s", + f"dropped\n {TRANSPORT_PROTOCOL}'s"), + ('pcapkit/vendor/reg/apptype/apptype.py', 'as a ``TransportProtocol`` expression', + f'as a\n {TRANSPORT_PROTOCOL} expression'), +) + tuple( + (f'pcapkit/{kind}/reg/apptype/{name}.py', '#: what ``AppType.get()`` answers with.', + f'#: what {APPTYPE_GET} answers with.') + for kind, name in (('vendor', 'apptype'), ('const', 'tcp'), ('const', 'udp'), + ('const', 'sctp'), ('const', 'dccp')) +) + +#: ``(page, const class, vendor module, template, special methods)``. +SPECIAL = ( + ('pcapkit/const/reg.rst', 'pcapkit.const.reg.apptype.apptype.AppType', + 'pcapkit.vendor.reg.apptype.apptype', 'BASE', ('__new__', '__repr__', '__str__', '__int__', '__eq__')), + ('pcapkit/const/pcapng.rst', 'pcapkit.const.pcapng.option_type.OptionType', + 'pcapkit.vendor.pcapng.option_type', 'LINE', ('__new__',)), +) + +#: Stand-ins for the crawled parts of a template, enough to make it parse. +PARTS = {'TABLE': '', 'ENUM': 'pass', 'MISS': 'return None'} + + +def klass(path: 'str') -> 'type': + """The class at the dotted ``path``.""" + obj = _resolve(path) + assert isinstance(obj, type), path + return obj + + +def own(cls: 'type', name: 'str') -> 'Any': + """The function a class body defines as ``name``; for an enumeration's + ``__new__`` that is ``_new_member_``, where the enum machinery keeps it.""" + obj = vars(cls)['_new_member_' if name == '__new__' else name] + return getattr(obj, '__func__', obj) + + +def rendered_docstrings(modname: 'str', template: 'str', clsname: 'str') -> 'dict[str, str | None]': + """Docstring of each method of ``clsname`` in the module ``template`` renders.""" + module = importlib.import_module(modname) + vendor = getattr(module, clsname) + args = {'NAME': clsname, 'DOCS': vendor.__doc__, 'FLAG': vendor.FLAG, 'MODL': modname, **PARTS} + func = getattr(module, template) + names = func.__code__.co_varnames[:func.__code__.co_argcount] + tree = ast.parse(func(*(args[name] for name in names))) + body = next(node for node in tree.body if isinstance(node, ast.ClassDef) and node.name == clsname) + return {node.name: ast.get_docstring(node) for node in body.body if isinstance(node, ast.FunctionDef)} + + +class ConvertedReferenceTests(unittest.TestCase): + """Each reference is a role, not a literal, and names a declared entry.""" + + def test_literal_is_a_role(self) -> 'None': + for source, literal, role in CONVERTED: + with self.subTest(source=source, literal=literal): + text = (ROOT / source).read_text(encoding='utf-8') + self.assertIn(role, text) + self.assertNotIn(literal, text) + + def test_role_targets_are_declared_entries(self) -> 'None': + found = declared_anywhere() + for role in (TRANSPORT_PROTOCOL, APPTYPE_GET): + with self.subTest(role=role): + match = ROLE.fullmatch(role) + if match is None: + self.fail(f'{role} is not a Python-domain role') + target = match.group(2) + _resolve(target) + self.assertTrue(is_declared(target, found), f'no API page declares {target}') + + +class SpecialMethodDocstringTests(unittest.TestCase): + """Each special method an API page declares has a docstring of its own.""" + + def test_pages_declare_the_methods(self) -> 'None': + for page, path, _, _, methods in SPECIAL: + found = set(declared((DOCS / page).read_text(encoding='utf-8'))) + for name in methods: + with self.subTest(path=path, name=name): + self.assertIn(f'{path}.{name}', found) + + def test_methods_have_docstrings(self) -> 'None': + for _, path, _, _, methods in SPECIAL: + cls = klass(path) + for name in methods: + with self.subTest(path=path, name=name): + func = own(cls, name) + self.assertEqual(func.__qualname__, f'{cls.__qualname__}.{name}') + self.assertTrue((func.__doc__ or '').strip(), f'{path}.{name} has no docstring') + + +class TemplateParityTests(unittest.TestCase): + """Each docstring in the const module is the one its vendor template renders.""" + + def test_const_docstring_matches_template(self) -> 'None': + for _, path, modname, template, methods in SPECIAL: + cls = klass(path) + rendered = rendered_docstrings(modname, template, cls.__name__) + for name in methods: + with self.subTest(path=path, name=name): + self.assertIsNotNone(rendered.get(name), + f'{modname}.{template} renders no docstring for {name}') + self.assertEqual(inspect.cleandoc(own(cls, name).__doc__ or ''), rendered[name]) + + +if __name__ == '__main__': + unittest.main()