From 8fe369bee1b979d0a42d661586a3dfb2b0a52ecf Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Tue, 22 Sep 2026 23:19:51 -0400 Subject: [PATCH] docs: reconcile the Sphinx directives to the contract-versus-recipe tenet (#684) The API reference should keep usage and extensibility clear while hiding the recipe. Measured against that, `docs/source/` was wrong in both directions: it documented a handful of module-level privates on nobody's surface, and it omitted 39 class private attributes that are squarely contract -- several of them the only undocumented member of a group whose siblings all have directives. * Removed 7 directives, all module-level privates: `esp._resolve`, `esp._CRYPTO`, `ngap._convert`, `ngap._revert`, `ngap._PYCRATE`, `ngap._PDU_LOCK` and the private stream adapter `pypcapfile._NamedStream`. A private helper, a lazily imported backend flag, an internal lock and a wrapper nobody constructs or subclasses. `_NamedStream`'s two nested members go with it, which is correct -- a member of a private class is reachable only through that class. * Added 39 `autoattribute` directives for class private attributes that carry contract. The clearest are the ones that were alone in being left out: `Extractor._flag_f`, the only one of ten `_flag_*` without a directive; `PCAP_CT._backend` and `PyPCAP._backend`, whose every sibling on the same class is documented; and the built-in `PCAP` and `PCAPNG` engines, which carried none of the private state block that all six third-party engines carry. The rest back a documented property, or are read and written directly by collaborators -- `Extractor._vfunc`, `_fext` and `EngineBase._extractor` are what `ext.rst` already teaches engine authors to use by name. * Documented the const enums' `_missing_` fallback once, on the `pcapkit.const` landing page, rather than on each of the 121 enumerations under `pcapkit/const/` that implement it. Looking a value up by an unassigned-in-range number mints a member instead of raising, which is deliberate per #647 and is the extensibility behaviour of the whole package; `conf.py` already names `_missing_` in `exclude-members`, so per-class directives would be fighting the project's own configuration to say 128 times what one section says better. `registry.rst` gains a sentence pointing at it, since every `register_*` there takes a code the enum must already resolve. * `CONTRIBUTING.md` records the tenet: what counts as contract, that per-option `_read_*` / `_make_*` pairs publish the data format and stay, that most class private attributes stay, that anything abstract or implemented across subclasses must be documented, and that the recipe to hide is chiefly module-level privates. Deliberately not changed: the `_read_*` / `_make_*` families, which outline the constructor contract and the data format for each option, parameter, chunk and cause; every other class private attribute; and the six class members that looked like helpers but each document observable behaviour a caller needs -- `_split_key` interprets caller-supplied keying material, `Vendor._request` is the hook a subclass with a different registry source overrides, and `PyPCAPFile._decode` and `_get_decoder` record the warnings they emit and the guarantee they make. No `pcapkit/` line changed, so no changelog entry, following #686. `sphinx-build -b html` exits 0 with 56 warnings against 55 on `f0999858e`. The one added is `more than one target found for cross-reference 'Type'`, raised from the newly rendered `TraceFlow._foutio`, whose doc comment at `pcapkit/foundation/traceflow/traceflow.py:406` reads `#: Type[Dumper]: Dumper class.` -- Napoleon turns that bare `Type` into a cross-reference that five classes in the tree answer to. The same warning already fires four times at `f0999858e` from `engine.rst`, `reassembly.rst` and `traceflow.rst`, so this is a fifth instance of a standing ambiguity rather than a new kind of breakage, and no reference was orphaned. Qualifying that annotation would silence it, but the file is outside the scope of this change. Fixes #684 --- CONTRIBUTING.md | 52 +++++++++++++++++++ docs/source/pcapkit/const/index.rst | 30 +++++++++++ docs/source/pcapkit/corekit/fields/field.rst | 8 +++ docs/source/pcapkit/corekit/io.rst | 3 ++ .../pcapkit/foundation/engines/3rdparty.rst | 9 +--- .../pcapkit/foundation/engines/builtin.rst | 8 +++ .../pcapkit/foundation/engines/engine.rst | 2 + docs/source/pcapkit/foundation/extraction.rst | 13 +++++ docs/source/pcapkit/foundation/registry.rst | 6 +++ .../foundation/traceflow/traceflow.rst | 13 +++++ .../pcapkit/protocols/application/ngap.rst | 9 ---- .../source/pcapkit/protocols/internet/esp.rst | 4 -- docs/source/pcapkit/protocols/protocol.rst | 4 ++ 13 files changed, 141 insertions(+), 20 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0c2ab54c43..a48afa206c 100755 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -89,6 +89,58 @@ GitHub's own rendering and the release body rather than Sphinx. The issue and pu under `.github/` are Markdown for the same reason. The exception ends there: anything added under `docs/source/` is `.rst`. +### What belongs in the API reference: document the contract, hide the recipe + +The tenet is to **keep the usage and extensibility clear and straightforward, while hiding the +recipe**. A member earns an autodoc directive because a reader needs it, not because of its spelling — +so a leading underscore is not by itself a reason to leave something out, and being public is not by +itself a reason to put it in. + +**Contract — document it.** Anything a *caller* needs in order to use a class, and anything an +*implementer* needs in order to subclass it: + +- **Per-option and per-parameter `_read_*` / `_make_*` pairs.** These publish the data format: the + keyword arguments a caller passes to construct that option, and the fields they get back when + parsing it. There is nowhere else to look it up, so they stay documented even though the dispatch + reaches them through `getattr` rather than by name. +- **Most class private attributes.** A private attribute that carries a subclass's state, or that a + subclass sets or reads, is contract. Keep it unless there is a specific reason it is not. +- **Anything abstract, or implemented across subclasses.** An `@abstractmethod`, or an overridable + hook a subclass is expected to provide — `_make_data` has a concrete base implementation on + `ProtocolBase` and is overridden in 28 protocol modules, which is exactly the case this covers. +- **Members whose observable behaviour is documented**, such as a method whose docstring records the + warning it emits or a guarantee it makes. A reader who hits that warning looks it up here. + +**Recipe — leave it out.** The implementation detail that is on nobody's usage or extensibility +surface. In practice this is chiefly **module-level privates**: a private helper function, a lazily +imported backend flag, an internal lock, a private wrapper class nobody constructs or subclasses. +Removing one of these takes its members with it, which is correct — a member of a private class is +reachable only through that class. + +The sweep runs **in both directions**. A must-implement member with no directive is the same defect as +a recipe body with one, only quieter: add the missing directive rather than aiming for a small diff. + +Two mechanical points that decide real cases: + +- **A private base class must stay documented when a documented subclass carries + `:show-inheritance:`.** Sphinx renders that subclass's `Bases:` line as a link into the private + class's page, so dropping the directive breaks a link a public page really does render. This is what + keeps `pcapkit.protocols.schema.misc.pcapng._OPT_Option` and its five siblings, plus `_IPField`, + `_IPInterfaceField` and `_TextField`. +- **Dunders keep their directives.** A `__dunder__` is reached through public syntax rather than by + name — `__len__` is what `len()` calls, `__getitem__` is what `obj[key]` does — so overriding one + changes behaviour a caller observes without ever writing the name. PyPCAPKit's own `__proto__`, + `__option__`, `__schema__` and `__protocol_name__` family is the documented extension contract that + subclass authors and the `register_*` functions write to: public API in everything but spelling. + +One shape to know about because it is invisible: every const enum carries a `_missing_` fallback that +resolves an unregistered value and registers it, rather than raising as a plain `enum` would. That is +deliberate and it is the extensibility behaviour of the whole `pcapkit.const` package, so it is +documented once on the package's landing page rather than restated on each of the 121 enumerations +under `pcapkit/const/` that implement it. Note `docs/source/conf.py` already names `_missing_` in +`autodoc_default_options['exclude-members']`, so a per-class directive would be arguing with the +project's own configuration. + ## Coding style [PEP 8](https://peps.python.org/pep-0008/) is the baseline, but the repository's own linters are the diff --git a/docs/source/pcapkit/const/index.rst b/docs/source/pcapkit/const/index.rst index 6c435951b0..b2d6d96133 100644 --- a/docs/source/pcapkit/const/index.rst +++ b/docs/source/pcapkit/const/index.rst @@ -6,6 +6,36 @@ Constant Enumerations This module contains all constant enumerations of :mod:`pcapkit`, which are automatically generated from the :mod:`pcapkit.vendor` module. +.. _unrecognised-values: + +Unrecognised Values +------------------- + +Every enumeration below departs from :mod:`enum` in one deliberate way, and it is +the behaviour to know about before using any of them: looking one up by a value +the registry does not define does **not** raise. Each class overrides +``_missing_`` so that a value inside the registry's valid range is minted into a +new member on the fly -- via ``aenum.extend_enum``, named for the unassigned or +reserved band it falls in -- and returned. Only a value outside that range, or of +the wrong type, raises :exc:`ValueError`. + +That is what makes the enumerations usable against live capture data, where a +protocol number assigned after this release was generated is a routine +occurrence rather than an error. It also means an enumeration member is not a +closed set: the identity of a minted member is stable for the life of the +process, but it does not exist until something asks for it. + +The mechanism is uniform because it is generated -- see +:mod:`pcapkit.vendor.default`, whose template emits the ``_missing_`` override +for every registry -- while the valid range and the names of the unassigned bands +are per-registry, taken from that registry's own IANA data. The individual +overrides are therefore not documented per class. + +.. seealso:: + + :doc:`../foundation/registry` covers the other half of extending a registry: + once a code exists, registering a parser class, schema or engine against it. + Protocol Numbers ---------------- diff --git a/docs/source/pcapkit/corekit/fields/field.rst b/docs/source/pcapkit/corekit/fields/field.rst index 955a1613fd..dad63a2919 100644 --- a/docs/source/pcapkit/corekit/fields/field.rst +++ b/docs/source/pcapkit/corekit/fields/field.rst @@ -7,10 +7,18 @@ Base Fields :members: :show-inheritance: + .. autoattribute:: _length + .. autoattribute:: _length_callback + .. autoclass:: pcapkit.corekit.fields.field.FieldBase :members: :show-inheritance: + .. autoattribute:: _name + .. autoattribute:: _template + .. autoattribute:: _default + .. autoattribute:: _callback + Auxiliaries ----------- diff --git a/docs/source/pcapkit/corekit/io.rst b/docs/source/pcapkit/corekit/io.rst index 7e53af26f9..c7b36f1e60 100644 --- a/docs/source/pcapkit/corekit/io.rst +++ b/docs/source/pcapkit/corekit/io.rst @@ -14,6 +14,9 @@ implementation to :class:`io.BufferedReader`. .. autoproperty:: raw .. autoproperty:: closed + .. autoattribute:: _stream + .. autoattribute:: _closed + .. automethod:: read .. automethod:: read1 .. automethod:: readinto diff --git a/docs/source/pcapkit/foundation/engines/3rdparty.rst b/docs/source/pcapkit/foundation/engines/3rdparty.rst index 0c18f5bc21..ef99e55719 100644 --- a/docs/source/pcapkit/foundation/engines/3rdparty.rst +++ b/docs/source/pcapkit/foundation/engines/3rdparty.rst @@ -216,6 +216,7 @@ support, as is used by :class:`pcapkit.foundation.extraction.Extractor`. .. autoattribute:: _expkg .. autoattribute:: _extmp + .. autoattribute:: _backend .. autoattribute:: _dlink .. autoattribute:: _closed @@ -369,6 +370,7 @@ support, as is used by :class:`pcapkit.foundation.extraction.Extractor`. .. autoattribute:: _expkg .. autoattribute:: _handle .. autoattribute:: _extmp + .. autoattribute:: _backend .. autoattribute:: _dlink .. autoattribute:: _closed @@ -415,13 +417,6 @@ support, as is used by :class:`pcapkit.foundation.extraction.Extractor`. Internal Definitions -------------------- -.. autoclass:: pcapkit.foundation.engines.pypcapfile._NamedStream - :no-members: - :show-inheritance: - - .. autoattribute:: name - .. automethod:: read - .. automethod:: pcapkit.foundation.engines.pypcapfile.PyPCAPFile._get_decoder .. automethod:: pcapkit.foundation.engines.pypcapfile.PyPCAPFile._decode diff --git a/docs/source/pcapkit/foundation/engines/builtin.rst b/docs/source/pcapkit/foundation/engines/builtin.rst index 34e6e62ef3..1d21cf86b9 100644 --- a/docs/source/pcapkit/foundation/engines/builtin.rst +++ b/docs/source/pcapkit/foundation/engines/builtin.rst @@ -25,6 +25,11 @@ support, as is used by :class:`pcapkit.foundation.extraction.Extractor`. .. automethod:: run .. automethod:: read_frame + .. autoattribute:: _gbhdr + .. autoattribute:: _vinfo + .. autoattribute:: _dlink + .. autoattribute:: _nnsec + PCAP-NG Support =============== @@ -43,6 +48,9 @@ support, as is used by :class:`pcapkit.foundation.extraction.Extractor`. .. automethod:: run .. automethod:: read_frame + .. autoattribute:: _ctx + .. autoattribute:: _ctx_list + Internal Definitions -------------------- diff --git a/docs/source/pcapkit/foundation/engines/engine.rst b/docs/source/pcapkit/foundation/engines/engine.rst index ed551ca9a3..99e87b6bdd 100644 --- a/docs/source/pcapkit/foundation/engines/engine.rst +++ b/docs/source/pcapkit/foundation/engines/engine.rst @@ -51,6 +51,8 @@ all engine support functionality. .. autoproperty:: extractor + .. autoattribute:: _extractor + .. automethod:: unsupported_reason .. automethod:: run diff --git a/docs/source/pcapkit/foundation/extraction.rst b/docs/source/pcapkit/foundation/extraction.rst index 6944259e0c..48172bcc08 100644 --- a/docs/source/pcapkit/foundation/extraction.rst +++ b/docs/source/pcapkit/foundation/extraction.rst @@ -61,6 +61,7 @@ extracts parametres from a PCAP file. .. autoattribute:: _flag_a .. autoattribute:: _flag_d .. autoattribute:: _flag_e + .. autoattribute:: _flag_f .. autoattribute:: _flag_q .. autoattribute:: _flag_r .. autoattribute:: _flag_t @@ -70,10 +71,22 @@ extracts parametres from a PCAP file. .. autoattribute:: _ifile .. autoattribute:: _ofile + .. autoattribute:: _ifnm + .. autoattribute:: _ofnm + .. autoattribute:: _fext + .. autoattribute:: _offmt + .. autoattribute:: _magic .. autoattribute:: _frnum + .. autoattribute:: _frame .. autoattribute:: _reasm .. autoattribute:: _trace + .. autoattribute:: _ipv4 + .. autoattribute:: _ipv6 + .. autoattribute:: _tcp + + .. autoattribute:: _eof_mark + .. autoattribute:: _vfunc .. autoattribute:: _exnam .. autoattribute:: _exeng diff --git a/docs/source/pcapkit/foundation/registry.rst b/docs/source/pcapkit/foundation/registry.rst index e508286ab5..ef4d7ebd6c 100644 --- a/docs/source/pcapkit/foundation/registry.rst +++ b/docs/source/pcapkit/foundation/registry.rst @@ -7,6 +7,12 @@ This module (:mod:`pcapkit.foundation.registry`) provides the registry management for :mod:`pcapkit`, as the module contains various registry points. +Every registration below takes a code that is already an enumeration member. For +how a code the shipped registries do not define becomes one in the first place, +see :ref:`unrecognised-values` -- the constant enumerations mint an in-range +unknown value rather than rejecting it, which is what makes registering against a +newly assigned number possible without regenerating them. + Foundation Registries --------------------- diff --git a/docs/source/pcapkit/foundation/traceflow/traceflow.rst b/docs/source/pcapkit/foundation/traceflow/traceflow.rst index fd1b70944e..6c78081379 100644 --- a/docs/source/pcapkit/foundation/traceflow/traceflow.rst +++ b/docs/source/pcapkit/foundation/traceflow/traceflow.rst @@ -73,6 +73,19 @@ which is an abstract base class for all flow tracing classes. .. autoattribute:: _bidir :no-value: + .. autoattribute:: _fproot + :no-value: + .. autoattribute:: _foutio + :no-value: + .. autoattribute:: _fdpext + :no-value: + .. autoattribute:: _endian + :no-value: + .. autoattribute:: _nnsecd + :no-value: + .. autoattribute:: _analyse + :no-value: + .. automethod:: __call__ .. automethod:: __init_subclass__ diff --git a/docs/source/pcapkit/protocols/application/ngap.rst b/docs/source/pcapkit/protocols/application/ngap.rst index c943f43585..b5243eb096 100644 --- a/docs/source/pcapkit/protocols/application/ngap.rst +++ b/docs/source/pcapkit/protocols/application/ngap.rst @@ -106,15 +106,6 @@ Auxiliary Functions .. autofunction:: pcapkit.protocols.application.ngap.load_pycrate -.. autodata:: pcapkit.protocols.application.ngap._PYCRATE - -.. autodata:: pcapkit.protocols.application.ngap._PDU_LOCK - :no-value: - -.. autofunction:: pcapkit.protocols.application.ngap._convert - -.. autofunction:: pcapkit.protocols.application.ngap._revert - Auxiliary Data -------------- diff --git a/docs/source/pcapkit/protocols/internet/esp.rst b/docs/source/pcapkit/protocols/internet/esp.rst index 959213ac7a..bf5cc32225 100644 --- a/docs/source/pcapkit/protocols/internet/esp.rst +++ b/docs/source/pcapkit/protocols/internet/esp.rst @@ -223,8 +223,6 @@ MD5, DES-MAC, KPDK-MD5 MUST NOT **no** registered, delibe .. autodata:: pcapkit.protocols.internet.esp.INTEGRITY_SUITES -.. autofunction:: pcapkit.protocols.internet.esp._resolve - Known Limitations ----------------- @@ -251,8 +249,6 @@ Processing Status .. autofunction:: pcapkit.protocols.internet.esp.load_cryptography -.. autodata:: pcapkit.protocols.internet.esp._CRYPTO - Header Schemas -------------- diff --git a/docs/source/pcapkit/protocols/protocol.rst b/docs/source/pcapkit/protocols/protocol.rst index 0b8790c67d..2fcb6d765a 100644 --- a/docs/source/pcapkit/protocols/protocol.rst +++ b/docs/source/pcapkit/protocols/protocol.rst @@ -70,6 +70,10 @@ utility arguments and methods of specified protocols. .. autoattribute:: _data .. autoattribute:: _file .. autoattribute:: _info + .. autoattribute:: _next + .. autoattribute:: _protos + .. autoattribute:: _seekset + .. autoattribute:: _sigterm .. autoattribute:: __data__ .. automethod:: __init__