From 71bf550882eab172a14bf1bb0686d95afbaae085 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 29 Sep 2026 22:42:00 -0400 Subject: [PATCH] docs(conventions,const): resolve part B's unresolved cross-references (#934) Part B of #934: the remaining nitpicky sphinx-build misses in conventions.rst that are neither the sentinels module refs part A (#936) fixed nor the six aenum roles part C already ruled on (plain literals, since aenum's objects.inv carries zero py: objects and conf.py excludes it deliberately). * Qualified the three unqualified sentinel refs -- :class:`AbsentType`, :class:`NoValueType` and :data:`ABSENT` -- to their real dotted path under pcapkit.corekit.sentinels, so they resolve against the page #936 added. Rewrapped the two lines that grew past this file's ~88-column convention; no wording changed. * Added an autoclass entry for FEATCode to docs/source/pcapkit/const/ftp.rst, and widened the FTP Command section's intro clause to name both classes it now documents -- FEATCode is a companion of Command's, not a peer listed in the page's own overview table, so it stays folded into that section rather than getting its own heading; every other section in this file pairs one heading with one autoclass, and inventing a repeated `.. module::` for a second heading on the same submodule would be a novel shape this file has nowhere else. * Demoted Method.get and part C's six aenum roles to plain double-backtick literals: Method.get carries `:meta private:` deliberately (same pattern as Command.get, OptionType's and AppType's private get overrides), and aenum cannot be cross-referenced at all, so no target can exist for either. * Rebasing onto #940 (merged after this branch started) surfaced a seventh broken reference: #940 deleted FastBindingAcknowledgmentStatus.get outright rather than just widening it, so the :meth: role citing it in the #923 retrospective joined the unresolved set. Demoted to a plain literal too, matching the two sibling examples already written that way in the same sentence (TransportProtocol.get, Criticality.get). * Added test_ftp_featcode_doc_page_934_unit.py, pinning the new autoclass entry the way test_sentinels_doc_page_934_unit.py pins part A's page; proven to fail against the pre-fix (83c7552b8) page. * Added AenumRoleExclusionTests to test_conventions_doc_claims.py: pins that no :mod:/:class:/etc. role names aenum on this page (the plain-literal demotion is settled policy per conf.py, and nothing else enforced it), and that the four qualified sentinel targets stay qualified. Both assertions proven to fail against the pre-fix (83c7552b8) page. Nitpicky sphinx-build: conventions.rst had 14 unresolved references against 83c7552b8, 15 against b337cdbc2 (this branch's rebased base) once #940's deletion is counted; all resolve here. Three more resolve as a side effect of documenting FEATCode: stale FEATCode references inside Command._unregistered_member's and Method._unregistered_member's own docstrings, plus one in a rendered `feat: Optional[FEATCode]` parameter annotation with no clear file attribution. Two pre-existing bugs inside FEATCode's own docstring are newly exposed rather than introduced -- a line-wrapped :meth: role and a reference to the vendor Command.process, deliberately excluded from vendor/ftp.rst's own :members: allowlist. FEATCode's :show-inheritance: does genuinely introduce one new warning of its own (an aenum._enum.StrEnum base that cannot resolve), joining five identical ones already present for Command/Method/etc. Recording rather than fixing any of these: out of scope for this file. mypy 321 errors/38 files, pylint 8.67/10 exit 30, isort clean -- all matching the b337cdbc2 baseline (R0401 cyclic-import churn aside, which is non-deterministic on an unmodified tree). Targeted tests: 40 passed, 1 skipped across test_conventions_doc_claims (incl. the two new AenumRoleExclusionTests methods), test_sentinel_exports_unit, test_sentinels_doc_page_934_unit and the FEATCode page test. --- docs/source/contributing/conventions.rst | 26 +++--- docs/source/pcapkit/const/ftp.rst | 12 ++- tests/project/test_conventions_doc_claims.py | 81 +++++++++++++++++++ .../test_ftp_featcode_doc_page_934_unit.py | 64 +++++++++++++++ 4 files changed, 169 insertions(+), 14 deletions(-) create mode 100644 tests/project/test_ftp_featcode_doc_page_934_unit.py diff --git a/docs/source/contributing/conventions.rst b/docs/source/contributing/conventions.rst index d12db5187f..d9f90306c2 100644 --- a/docs/source/contributing/conventions.rst +++ b/docs/source/contributing/conventions.rst @@ -203,14 +203,15 @@ there, never leaving :mod:`pcapkit.protocols.protocol` -- and the underscore use the mechanical signal of that. The maintainer's ruling on #937, verbatim: *"we can change* ``_ABSENT`` *to* ``ABSENT`` *just document it as private type/class in the documentation and not for public use is enough."* So privacy is documentation-only from -here on, carried by this paragraph and by :class:`AbsentType`'s own docstring +here on, carried by this paragraph and by +:class:`~pcapkit.corekit.sentinels.AbsentType`'s own docstring (:file:`pcapkit/corekit/sentinels.py`, line 439), which still says so: A distinct class rather than a bare :obj:`object` so that the sentinel has a name of its own in a traceback or a debugger, and so that a type checker has something to name where ``object()`` would give it nothing. It follows - :class:`NoValueType`, which does the same job for an unset field default; - this is a sibling of it rather than a reuse [...] + :class:`~pcapkit.corekit.sentinels.NoValueType`, which does the same job for an unset + field default; this is a sibling of it rather than a reuse [...] It remains a deliberate fourth rather than an accident: the leading underscore's absence is also why this table once listed three for as long as it did -- a sweep @@ -224,7 +225,8 @@ instance in its module's ``__all__`` and leaves the type out of it (GitHub issue The type stays importable by its dotted path, for an annotation or an ``is`` guard; it is ``import *`` that no longer offers it. A private sentinel such as ``ABSENT`` is in neither, which is what private means here -- dropping its leading underscore did not -add it to either list, and :class:`AbsentType` and :data:`ABSENT` are documented on +add it to either list, and :class:`~pcapkit.corekit.sentinels.AbsentType` and +:data:`~pcapkit.corekit.sentinels.ABSENT` are documented on :doc:`the sentinels API page ` as private and not for public use rather than left off it, since the name alone no longer says so. @@ -358,7 +360,7 @@ what was ruled. :class:`~pcapkit.corekit.enum.EnumRegistry` leaves the member data type exactly where it was -- ``LinkType -> EnumRegistry -> EnumLookup -> IntEnum -> int`` -- so ``_member_type_`` still comes from the enum base. Had either tier subclassed -:class:`~aenum.Enum` in order to "be an enum", it would have become the member type +``aenum.Enum`` in order to "be an enum", it would have become the member type itself and broken ``int``, ``str`` and flag registries at once. :meth:`~pcapkit.corekit.enum.EnumLookup._validate_value` is what the base carries @@ -388,7 +390,7 @@ Three things about it are easy to get wrong: With **no usable** ``default``, the rejection reaches the caller **unwrapped**. ``get`` re-raises a :exc:`ValueError` that is already a :exc:`~pcapkit.utilities.exceptions.BaseError` exactly as the override raised it, - and converts only :mod:`aenum`'s and :mod:`enum`'s own "no member carries this + and converts only ``aenum``'s and :mod:`enum`'s own "no member carries this value". Two things follow, and both are the point of the discrimination rather than side effects: the override's **own message** survives to the caller instead of being replaced by the base's, and the error is **logged once** rather than @@ -407,7 +409,7 @@ Three things about it are easy to get wrong: `#877 `__, and it is now **complete**: the phase landed for 24 of the 24 non-registry enumerations. **Zero enumerations remain outside the hierarchy**, measured by the same runtime - walk over both the :mod:`enum` and :mod:`aenum` flavours that once found seven. + walk over both the :mod:`enum` and ``aenum`` flavours that once found seven. It landed in two pull requests rather than one. Seven of the 24 sat in files other pull requests were editing around the same time: ``CommandType`` and @@ -454,7 +456,7 @@ Do not "improve" on the shape by making both misses report identically. Converti one into the other is exactly what #923 retired, and it was retired in three places at once: ``TransportProtocol.get`` and ``Criticality.get`` had each turned the base's :exc:`KeyError` into a :exc:`ValueError`, and -:meth:`~pcapkit.protocols.internet.mh.FastBindingAcknowledgmentStatus.get` raised +``FastBindingAcknowledgmentStatus.get`` raised :exc:`~pcapkit.utilities.exceptions.EnumValueError` for a name miss so that "the two ways of getting it wrong reported identically". @@ -463,7 +465,7 @@ exception class: the **name** miss is raised quietly (:class:`~pcapkit.utilities.exceptions.BaseError`'s ``quiet=True``, so nothing is logged and :data:`sys.tracebacklimit` is left alone) while the **value** miss stays loud. A name miss is in-library control flow at several call sites, and at -:meth:`~pcapkit.const.http.method.Method.get` it is part of a *successful* call -- +``Method.get`` it is part of a *successful* call -- that override catches it in order to mint. A loud error there would put a :data:`logging.CRITICAL` record on every such call and set :data:`sys.tracebacklimit` to ``0`` process-wide, which is the @@ -553,9 +555,9 @@ verbatim: *"we should audit all registries and then decide if case (in)sensitive The population it covers, with the counting convention spelled out because the figures move: **127** :class:`~pcapkit.corekit.enum.EnumRegistry` subclasses, every one of them under :mod:`pcapkit.const`, across 124 files -- 117 :class:`int`-valued -(of which 5 are flag registries) and 10 :class:`~aenum.StrEnum`-valued. Plus **24** +(of which 5 are flag registries) and 10 ``aenum.StrEnum``-valued. Plus **24** non-registry enumerations counted by a runtime walk over both the :mod:`enum` and -:mod:`aenum` flavours and including nested classes: 17 top level (3 of them under +``aenum`` flavours and including nested classes: 17 top level (3 of them under :mod:`pcapkit.const` itself) and 7 nested, the nested ones being ``FrameType.Flags`` in :mod:`pcapkit.protocols.schema.application.httpv2` plus its 6 concrete per-frame subclasses. 151 enumerations in total. @@ -716,7 +718,7 @@ rather than changing it. .. note:: The obstacle this page used to record -- that the base's string-key path does not - fall through to a value lookup, so a :class:`~aenum.StrEnum` registry would stop + fall through to a value lookup, so an ``aenum.StrEnum`` registry would stop resolving a valid value that is not also a name -- **no longer applies.** :meth:`~pcapkit.corekit.enum.EnumLookup.get` now checks ``_value2member_map_`` when the name lookup misses, so such a value resolves: diff --git a/docs/source/pcapkit/const/ftp.rst b/docs/source/pcapkit/const/ftp.rst index cd05d5ef98..531655423a 100644 --- a/docs/source/pcapkit/const/ftp.rst +++ b/docs/source/pcapkit/const/ftp.rst @@ -20,14 +20,22 @@ FTP Command .. module:: pcapkit.const.ftp.command -This module contains the constant enumeration for **FTP Command**, -which is automatically generated from :class:`pcapkit.vendor.ftp.command.Command`. +This module contains the constant enumeration for **FTP Command**, which is +automatically generated from :class:`pcapkit.vendor.ftp.command.Command`, plus the +companion :class:`~pcapkit.const.ftp.command.FEATCode` enumeration it also declares -- +the ``FEAT`` response keywords the ``FEAT code`` column of the same IANA registry +names, c.f., :rfc:`5797#section-3`. .. autoclass:: pcapkit.const.ftp.command.Command :members: :undoc-members: :show-inheritance: +.. autoclass:: pcapkit.const.ftp.command.FEATCode + :members: + :undoc-members: + :show-inheritance: + FTP Server Return Code ============================ diff --git a/tests/project/test_conventions_doc_claims.py b/tests/project/test_conventions_doc_claims.py index 19afcfac41..50858f9069 100644 --- a/tests/project/test_conventions_doc_claims.py +++ b/tests/project/test_conventions_doc_claims.py @@ -28,6 +28,15 @@ * :class:`FailedLookupExceptionTests` -- the worked example the page gives for a name miss, which named :exc:`KeyError` until #918 and now names :exc:`~pcapkit.utilities.exceptions.EnumKeyError`. +* :class:`AenumRoleExclusionTests` -- GitHub issue #934 part C's ruling that + ``aenum`` cannot be cross-referenced at all (``conf.py`` excludes it: its + ``objects.inv`` carries zero ``py:`` objects), converted to plain literals rather + than roles. A forbidden role needs a test or it comes back, exactly as + :class:`RetiredNameTests` guards a retired name. Pins the other half of the same + fix alongside it: the four sentinel references #934 part B qualified stay + qualified, since none of ``AbsentType``, ``NoValueType`` or ``ABSENT`` resolves + unqualified outside :file:`docs/source/pcapkit/corekit/sentinels.rst`'s own module + context. Deliberately **not** checked here: whether the page's cross-references resolve. That is a property of the built inventory rather than of the source, for the reason @@ -472,5 +481,77 @@ def test_a_declared_but_unassigned_value_still_resolves_through_the_constructor( self.assertEqual(FEATCode('ZZ-NOT-REAL').value, 'ZZ-NOT-REAL') +class AenumRoleExclusionTests(unittest.TestCase): + """GitHub issue #934 part C's ruling: ``aenum`` cannot be cross-referenced. + + ``docs/source/conf.py`` excludes ``aenum`` from ``intersphinx_mapping`` + deliberately -- its ``objects.inv`` carries zero ``py:`` objects, so no + ``:mod:``/``:class:``/``:func:``/etc. role naming it could ever resolve. Part + B converted the six such roles this page carried to plain double-backtick + literals rather than leaving them promising a link that can never exist. A + forbidden role needs a test, or a later edit reintroduces one without + noticing -- exactly the failure mode :class:`RetiredNameTests` guards a + retired name against. + + The other half of the same fix is pinned alongside it: the four references to + ``AbsentType``, ``NoValueType`` and ``ABSENT`` that part B qualified to their + real dotted path under ``pcapkit.corekit.sentinels`` have to stay qualified, + since none of the three resolves by its bare name outside + :file:`docs/source/pcapkit/corekit/sentinels.rst`'s own ``.. module::`` + context. + + """ + + #: Any Sphinx py-domain role whose target starts with ``aenum``, tilde-prefixed + #: or not. Matches ``:mod:`aenum``` and ``:class:`~aenum.Enum``` alike; would + #: also catch a role type never seen on this page (``:func:`aenum.something```), + #: since the ban is on naming ``aenum`` in a role at all, not on the six + #: specific roles #934 part C found. + FORBIDDEN_AENUM_ROLE = re.compile(r':(?:mod|class|meth|func|attr|exc|obj|data):`~?aenum\b') + + #: The four qualified sentinel references part B's fix relies on: the role, + #: the dotted target, and how many times that exact pairing has to appear. + QUALIFIED_SENTINEL_REFS = ( + (':class:', '~pcapkit.corekit.sentinels.AbsentType', 2), + (':class:', '~pcapkit.corekit.sentinels.NoValueType', 1), + (':data:', '~pcapkit.corekit.sentinels.ABSENT', 1), + ) + + def test_no_aenum_role_appears(self) -> 'None': + """A reintroduced ``:mod:`aenum``` or ``:class:`~aenum.X``` fails here. + + Checked as a role, not as the bare word: ``aenum`` still appears as a + plain double-backtick literal (``` ``aenum.Enum`` ```) and in prose + (*"the aenum flavours"*) throughout this page, which is exactly the point + of the fix -- only the unresolvable *role* form is banned. + + """ + text = _page() + offenders = self.FORBIDDEN_AENUM_ROLE.findall(text) + self.assertEqual(offenders, [], + f'{CONVENTIONS.name} names aenum in a role again: ' + f'{offenders!r}; GitHub issue #934 part C ruled this ' + 'unresolvable (conf.py excludes aenum -- zero py: objects ' + 'in its objects.inv) and converted every such role to a ' + 'plain literal') + + def test_the_qualified_sentinel_targets_stay_qualified(self) -> 'None': + """A later edit unqualifying one of these reintroduces #934 part B's miss. + + ``AbsentType``, ``NoValueType`` and ``ABSENT`` each resolve only against + the sentinels page's own module context (GitHub issue #936); written bare + anywhere on this page, none of the three resolves at all. + + """ + text = _page() + for role, target, count in self.QUALIFIED_SENTINEL_REFS: + with self.subTest(target=target): + needle = f'{role}`{target}`' + actual = text.count(needle) + self.assertEqual(actual, count, + f'{needle!r} appears {actual} time(s) in ' + f'{CONVENTIONS.name}, expected {count}') + + if __name__ == '__main__': unittest.main() diff --git a/tests/project/test_ftp_featcode_doc_page_934_unit.py b/tests/project/test_ftp_featcode_doc_page_934_unit.py new file mode 100644 index 0000000000..b1d20ec1f6 --- /dev/null +++ b/tests/project/test_ftp_featcode_doc_page_934_unit.py @@ -0,0 +1,64 @@ +# -*- coding: utf-8 -*- +"""Pins :class:`~pcapkit.const.ftp.command.FEATCode` as a documented API target. + +GitHub issue #934, part B: :file:`docs/source/contributing/conventions.rst` +cross-references :class:`~pcapkit.const.ftp.command.FEATCode` three times (the +``:class:`` role, at what were lines 592, 733 and 740 on ``83c7552b8``), and none of +them used to resolve, because no page under :file:`docs/source/` documented that +class -- confirmed by an explicit nitpicky ``sphinx-build`` before +:file:`docs/source/pcapkit/const/ftp.rst` gained an ``autoclass`` directive for it, +and again after, to confirm the fix. ``FEATCode`` is not in +:mod:`pcapkit.const.ftp.command`'s ``__all__`` (only ``Command`` is), which is why the +page's existing ``autoclass:: pcapkit.const.ftp.command.Command`` directive never +pulled it in as a side effect. + +This does **not** re-test Sphinx's cross-reference resolution itself -- as +:file:`tests/project/test_sentinels_doc_page_934_unit.py` explains for the sibling +half of this same issue, whether a reference resolves is a property of the built +inventory rather than of the source, and the honest check is the rendered HTML from +an actual ``sphinx-build`` run, recorded in the pull request rather than +reimplemented here. What this pins is the purely textual precondition a later edit +could silently break even though a plain (non-nitpicky) build would keep passing +either way: the page has to keep declaring an ``autoclass`` for the class. + +""" + +from __future__ import annotations + +import pathlib +import re +import unittest + +ROOT = pathlib.Path(__file__).resolve().parents[2] + +#: The page that gained the ``FEATCode`` entry for GitHub issue #934. +PAGE = ROOT / 'docs' / 'source' / 'pcapkit' / 'const' / 'ftp.rst' + + +class FEATCodeAPIPageTests(unittest.TestCase): + """The ``ftp.rst`` page's ``autoclass`` entry for ``FEATCode``.""" + + def test_page_exists(self) -> 'None': + """The API page itself is present on disk.""" + self.assertTrue(PAGE.is_file(), f'{PAGE} does not exist') + + def test_page_documents_the_featcode_class(self) -> 'None': + """The page declares an ``autoclass`` for the class conventions.rst names. + + Without this directive, ``:class:`~pcapkit.const.ftp.command.FEATCode``` + in :file:`docs/source/contributing/conventions.rst` has no target to + resolve against -- the exact defect GitHub issue #934 reports for this + class, distinct from the sibling ``sentinels`` module-level miss part A + of the same issue fixed. + + """ + text = PAGE.read_text(encoding='utf-8') + self.assertRegex( + text, r'\.\.\s+autoclass::\s+pcapkit\.const\.ftp\.command\.FEATCode\b', + f'{PAGE} does not declare an autoclass for ' + '"pcapkit.const.ftp.command.FEATCode"', + ) + + +if __name__ == '__main__': + unittest.main()