From 8ef38142dd5bf1bb876fac4ca4896c4f745a4ed6 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Thu, 24 Sep 2026 11:53:58 -0400 Subject: [PATCH] refactor: import each *Base class under its own name, not the public one Library modules imported a *Base class aliased to its public name, so the source read `class PCAP(Engine)` while actually inheriting `EngineBase`. Per the ruling on #514 the split is permanent and library classes inherit the base, so the base is now imported under its own name. * rewrote 28 of the 82 alias imports -- EngineBase 8 of 8, ReassemblyBase 3 of 3, DumperBase 2 of 2, TraceFlowBase 2 of 2, ProtocolBase 13 of 67 * renamed the code and annotation references that followed, including `cast()` targets, `TypeVar(bound=)` and `# type:` comments, but not the string values inside `Literal[...]` * corrected four `docs/source` index pages that named the public class while their own class diagram roots the hierarchy at the base * added `tests/test_base_class_contract.py`, pinning the contract per suite The four non-protocols families are complete. The remaining 54 sites are all `ProtocolBase` ones in paths #726 and #742 own, recorded as prefixes in `PENDING_ALIAS_PATHS` so the entry fails once its pull request lands. No behaviour change: name registry 38 keys before and after, `descendants(Public)` 0 in all five suites, 45 dispatches identical and still identical after clearing the name registry, mypy at its 112-error baseline with none introduced. --- .../pcapkit/foundation/engines/index.rst | 2 +- .../pcapkit/foundation/reassembly/index.rst | 2 +- .../pcapkit/foundation/traceflow/index.rst | 2 +- docs/source/pcapkit/protocols/index.rst | 2 +- pcapkit/corekit/fields/misc.py | 4 +- pcapkit/corekit/protochain.py | 38 +- pcapkit/dumpkit/null.py | 4 +- pcapkit/dumpkit/pcap.py | 4 +- pcapkit/foundation/engines/dpkt.py | 4 +- pcapkit/foundation/engines/pcap.py | 4 +- pcapkit/foundation/engines/pcap_ct.py | 4 +- pcapkit/foundation/engines/pcapng.py | 4 +- pcapkit/foundation/engines/pypcap.py | 4 +- pcapkit/foundation/engines/pypcapfile.py | 4 +- pcapkit/foundation/engines/pyshark.py | 4 +- pcapkit/foundation/engines/scapy.py | 4 +- pcapkit/foundation/reassembly/data/data.py | 8 +- pcapkit/foundation/reassembly/data/ip.py | 6 +- pcapkit/foundation/reassembly/data/tcp.py | 6 +- pcapkit/foundation/reassembly/ip.py | 4 +- pcapkit/foundation/reassembly/reassembly.py | 10 +- pcapkit/foundation/reassembly/tcp.py | 4 +- pcapkit/foundation/traceflow/tcp.py | 4 +- pcapkit/interface/core.py | 22 +- pcapkit/utilities/decorators.py | 12 +- tests/test_base_class_contract.py | 381 ++++++++++++++++++ 26 files changed, 465 insertions(+), 82 deletions(-) create mode 100644 tests/test_base_class_contract.py diff --git a/docs/source/pcapkit/foundation/engines/index.rst b/docs/source/pcapkit/foundation/engines/index.rst index 601ed7ee3..d4368279c 100644 --- a/docs/source/pcapkit/foundation/engines/index.rst +++ b/docs/source/pcapkit/foundation/engines/index.rst @@ -21,7 +21,7 @@ support. builtin 3rdparty -All engines are implemented as :class:`~pcapkit.foundation.engines.engine.Engine` +All engines are implemented as :class:`~pcapkit.foundation.engines.engine.EngineBase` subclasses, which are responsible for parsing the input files and extracting the network packets for further processing. Below is a brief diagram of the class hierarchy of :mod:`pcapkit.foundation.engines`: diff --git a/docs/source/pcapkit/foundation/reassembly/index.rst b/docs/source/pcapkit/foundation/reassembly/index.rst index 24fc4925c..32056e1dc 100644 --- a/docs/source/pcapkit/foundation/reassembly/index.rst +++ b/docs/source/pcapkit/foundation/reassembly/index.rst @@ -20,7 +20,7 @@ of IP and TCP packets. ip/index tcp -All reassembly classes are implemented as :class:`~pcapkit.foundation.reassembly.reassembly.Reassembly` +All reassembly classes are implemented as :class:`~pcapkit.foundation.reassembly.reassembly.ReassemblyBase` subclasses, which are responsible for processing extracted packets and reassemble the datagrams to a nonfragmented packet. Below is a brief diagram of the class hierarchy of :mod:`pcapkit.foundation.reassembly`: diff --git a/docs/source/pcapkit/foundation/traceflow/index.rst b/docs/source/pcapkit/foundation/traceflow/index.rst index dc3d6dcb8..8e2d673b8 100644 --- a/docs/source/pcapkit/foundation/traceflow/index.rst +++ b/docs/source/pcapkit/foundation/traceflow/index.rst @@ -25,7 +25,7 @@ for :mod:`pcapkit` package. traceflow tcp -All flow tracing classes are implemented as :class:`~pcapkit.foundation.traceflow.traceflow.TraceFlow` +All flow tracing classes are implemented as :class:`~pcapkit.foundation.traceflow.traceflow.TraceFlowBase` subclasses, which are responsible for processing extracted packets and follow the flow and/or stream to provide more insights. Below is a brief diagram of the class hierarchy of :mod:`pcapkit.foundation.traceflow`: diff --git a/docs/source/pcapkit/protocols/index.rst b/docs/source/pcapkit/protocols/index.rst index a1534db98..52cf40c28 100644 --- a/docs/source/pcapkit/protocols/index.rst +++ b/docs/source/pcapkit/protocols/index.rst @@ -18,7 +18,7 @@ with detailed implementation and methods. application/index misc/index -All protocol classes are implemented as :class:`~pcapkit.protocols.protocol.Protocol` +All protocol classes are implemented as :class:`~pcapkit.protocols.protocol.ProtocolBase` subclasses, which are responsible for processing extracted binary packet data and/or construct protocol packet from given information. Below is a brief diagram of the class hierarchy of :mod:`pcapkit.protocols`: diff --git a/pcapkit/corekit/fields/misc.py b/pcapkit/corekit/fields/misc.py index 4a0364de6..532e78307 100644 --- a/pcapkit/corekit/fields/misc.py +++ b/pcapkit/corekit/fields/misc.py @@ -19,12 +19,12 @@ from typing_extensions import Self from pcapkit.corekit.fields.field import NoValueType - from pcapkit.protocols.protocol import ProtocolBase as Protocol + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.schema.schema import Schema _TC = TypeVar('_TC') _TS = TypeVar('_TS', bound='Schema') -_TP = TypeVar('_TP', bound='Protocol') +_TP = TypeVar('_TP', bound='ProtocolBase') _TN = TypeVar('_TN', bound='NoValueType') diff --git a/pcapkit/corekit/protochain.py b/pcapkit/corekit/protochain.py index 5a1e51531..47d567598 100644 --- a/pcapkit/corekit/protochain.py +++ b/pcapkit/corekit/protochain.py @@ -20,7 +20,7 @@ from typing_extensions import Self - from pcapkit.protocols.protocol import ProtocolBase as Protocol + from pcapkit.protocols.protocol import ProtocolBase __all__ = ['ProtoChain'] @@ -36,14 +36,14 @@ class ProtoChain(collections.abc.Sequence): """ #: Internal data storage for protocol chain. - __data__: 'tuple[tuple[str, Type[Protocol]], ...]' + __data__: 'tuple[tuple[str, Type[ProtocolBase]], ...]' ########################################################################## # Properties. ########################################################################## @cached_property - def protocols(self) -> 'tuple[Type[Protocol], ...]': + def protocols(self) -> 'tuple[Type[ProtocolBase], ...]': """List of protocols in the chain.""" return tuple(data[1] for data in self.__data__) @@ -62,7 +62,7 @@ def chain(self) -> 'str': ########################################################################## @classmethod - def from_list(cls, data: 'list[Protocol | Type[Protocol]]') -> 'Self': + def from_list(cls, data: 'list[ProtocolBase | Type[ProtocolBase]]') -> 'Self': """Create a protocol chain from a list. Args: @@ -70,11 +70,11 @@ def from_list(cls, data: 'list[Protocol | Type[Protocol]]') -> 'Self': """ from pcapkit.protocols.protocol import \ - ProtocolBase as Protocol # pylint: disable=import-outside-toplevel + ProtocolBase # pylint: disable=import-outside-toplevel temp_data = [] for proto in data: - if isinstance(proto, Protocol): + if isinstance(proto, ProtocolBase): alias = proto.alias proto = type(proto) else: @@ -86,7 +86,7 @@ def from_list(cls, data: 'list[Protocol | Type[Protocol]]') -> 'Self': obj.__data__ = tuple(temp_data) return obj - def index(self, value: 'str | Protocol | Type[Protocol]', + def index(self, value: 'str | ProtocolBase | Type[ProtocolBase]', start: 'Optional[int]' = None, stop: 'Optional[int]' = None) -> 'int': """First index of ``value``. @@ -109,8 +109,8 @@ def index(self, value: 'str | Protocol | Type[Protocol]', # prepare comparison values from pcapkit.protocols.protocol import \ - ProtocolBase as Protocol # pylint: disable=import-outside-toplevel - comp = Protocol.expand_comp(value) + ProtocolBase # pylint: disable=import-outside-toplevel + comp = ProtocolBase.expand_comp(value) pool = self.__data__[start:stop] for idx, (alias, proto) in enumerate(pool): @@ -120,7 +120,7 @@ def index(self, value: 'str | Protocol | Type[Protocol]', return start + idx raise IndexNotFound(f'{value!r} is not in {self.__class__.__name__!r}') - def count(self, value: 'str | Protocol | Type[Protocol]') -> int: + def count(self, value: 'str | ProtocolBase | Type[ProtocolBase]') -> int: """Number of occurrences of ``value``. Args: @@ -129,8 +129,8 @@ def count(self, value: 'str | Protocol | Type[Protocol]') -> int: """ # prepare comparison values from pcapkit.protocols.protocol import \ - ProtocolBase as Protocol # pylint: disable=import-outside-toplevel - comp = Protocol.expand_comp(value) + ProtocolBase # pylint: disable=import-outside-toplevel + comp = ProtocolBase.expand_comp(value) cnt = 0 for alias, proto in self.__data__: @@ -145,7 +145,7 @@ def count(self, value: 'str | Protocol | Type[Protocol]') -> int: # Data models. ########################################################################## - def __init__(self, proto: 'Protocol | Type[Protocol]', alias: 'Optional[str]' = None, *, + def __init__(self, proto: 'ProtocolBase | Type[ProtocolBase]', alias: 'Optional[str]' = None, *, basis: 'Optional[ProtoChain]' = None): """Initialisation. @@ -156,8 +156,8 @@ def __init__(self, proto: 'Protocol | Type[Protocol]', alias: 'Optional[str]' = """ from pcapkit.protocols.protocol import \ - ProtocolBase as Protocol # pylint: disable=import-outside-toplevel - if isinstance(proto, Protocol): + ProtocolBase # pylint: disable=import-outside-toplevel + if isinstance(proto, ProtocolBase): if alias is None: alias = proto.alias proto = type(proto) @@ -192,7 +192,7 @@ def __str__(self) -> 'str': """ return ':'.join(map(lambda p: p[0], self.__data__)) - def __contains__(self, name: 'str | Protocol | Type[Protocol]') -> 'bool': # type: ignore[override] + def __contains__(self, name: 'str | ProtocolBase | Type[ProtocolBase]') -> 'bool': # type: ignore[override] """Returns if ``name`` is in the chain. Args: @@ -203,8 +203,8 @@ def __contains__(self, name: 'str | Protocol | Type[Protocol]') -> 'bool': # ty """ from pcapkit.protocols.protocol import \ - ProtocolBase as Protocol # pylint: disable=import-outside-toplevel - comp = Protocol.expand_comp(name) + ProtocolBase # pylint: disable=import-outside-toplevel + comp = ProtocolBase.expand_comp(name) for alias, proto in self.__data__: test_comp = (proto, alias.upper(), *(name.upper() for name in proto.id())) @@ -232,7 +232,7 @@ def __getitem__(self, index: 'int | slice') -> 'str | tuple[str, ...]': return tuple(data[0] for data in self.__data__[index]) return self.__data__[index][0] - def __iter__(self) -> 'Iterator[tuple[str, Type[Protocol]]]': + def __iter__(self) -> 'Iterator[tuple[str, Type[ProtocolBase]]]': """Iterator support. Returns: diff --git a/pcapkit/dumpkit/null.py b/pcapkit/dumpkit/null.py index f5b77c9df..d453e0f5f 100644 --- a/pcapkit/dumpkit/null.py +++ b/pcapkit/dumpkit/null.py @@ -15,7 +15,7 @@ """ from typing import TYPE_CHECKING -from pcapkit.dumpkit.common import DumperBase as Dumper +from pcapkit.dumpkit.common import DumperBase if TYPE_CHECKING: from typing import IO, Any, Optional @@ -25,7 +25,7 @@ __all__ = ['NotImplementedIO'] -class NotImplementedIO(Dumper): +class NotImplementedIO(DumperBase): """Unspecified output format.""" ########################################################################## diff --git a/pcapkit/dumpkit/pcap.py b/pcapkit/dumpkit/pcap.py index 6dd470cf9..dc28bc2fb 100644 --- a/pcapkit/dumpkit/pcap.py +++ b/pcapkit/dumpkit/pcap.py @@ -13,7 +13,7 @@ import sys from typing import TYPE_CHECKING -from pcapkit.dumpkit.common import DumperBase as Dumper +from pcapkit.dumpkit.common import DumperBase from pcapkit.protocols.data.misc.pcap.header import Header as Data_Header from pcapkit.protocols.misc.pcap.header import Header @@ -49,7 +49,7 @@ _UINT32_MASK = 0xFFFF_FFFF -class PCAPIO(Dumper): +class PCAPIO(DumperBase): """PCAP file dumper. Args: diff --git a/pcapkit/foundation/engines/dpkt.py b/pcapkit/foundation/engines/dpkt.py index 634922107..623e55721 100644 --- a/pcapkit/foundation/engines/dpkt.py +++ b/pcapkit/foundation/engines/dpkt.py @@ -13,7 +13,7 @@ from typing import TYPE_CHECKING, cast from pcapkit.const.reg.linktype import LinkType as Enum_LinkType -from pcapkit.foundation.engines.engine import EngineBase as Engine +from pcapkit.foundation.engines.engine import EngineBase from pcapkit.utilities.exceptions import FormatError, stacklevel from pcapkit.utilities.logging import get_logger from pcapkit.utilities.warnings import AttributeWarning, DPKTWarning, warn @@ -36,7 +36,7 @@ logger = get_logger(__name__) -class DPKT(Engine['DPKTPacket']): +class DPKT(EngineBase['DPKTPacket']): """DPKT engine support. Args: diff --git a/pcapkit/foundation/engines/pcap.py b/pcapkit/foundation/engines/pcap.py index ea878588f..059ec8e64 100644 --- a/pcapkit/foundation/engines/pcap.py +++ b/pcapkit/foundation/engines/pcap.py @@ -10,7 +10,7 @@ """ from typing import TYPE_CHECKING -from pcapkit.foundation.engines.engine import EngineBase as Engine +from pcapkit.foundation.engines.engine import EngineBase from pcapkit.protocols.misc.pcap.frame import Frame from pcapkit.protocols.misc.pcap.header import Header from pcapkit.utilities.logging import get_logger @@ -26,7 +26,7 @@ logger = get_logger(__name__) -class PCAP(Engine[Frame]): +class PCAP(EngineBase[Frame]): """PCAP file extraction support. Args: diff --git a/pcapkit/foundation/engines/pcap_ct.py b/pcapkit/foundation/engines/pcap_ct.py index 6e66ec318..c22414f2b 100644 --- a/pcapkit/foundation/engines/pcap_ct.py +++ b/pcapkit/foundation/engines/pcap_ct.py @@ -15,7 +15,7 @@ from pcapkit.const.reg.linktype import LinkType as Enum_LinkType from pcapkit.foundation.engines import _pcap_backend -from pcapkit.foundation.engines.engine import EngineBase as Engine +from pcapkit.foundation.engines.engine import EngineBase from pcapkit.foundation.reassembly import ReassemblyManager from pcapkit.foundation.traceflow import TraceFlowManager from pcapkit.utilities.exceptions import FormatError, UnsupportedCall, stacklevel @@ -38,7 +38,7 @@ RawFrame = tuple[float, bytes] -class PCAP_CT(Engine['RawFrame']): +class PCAP_CT(EngineBase['RawFrame']): """pcap-ct engine support. `pcap-ct`_ is a :mod:`ctypes` reimplementation of the `PyPCAP`_ API on top of diff --git a/pcapkit/foundation/engines/pcapng.py b/pcapkit/foundation/engines/pcapng.py index 508c686ea..24f4acd68 100644 --- a/pcapkit/foundation/engines/pcapng.py +++ b/pcapkit/foundation/engines/pcapng.py @@ -12,7 +12,7 @@ from pcapkit.const.pcapng.block_type import BlockType as Enum_BlockType from pcapkit.corekit.infoclass import Info, info_final -from pcapkit.foundation.engines.engine import EngineBase as Engine +from pcapkit.foundation.engines.engine import EngineBase from pcapkit.protocols.misc.pcapng import PCAPNG as P_PCAPNG from pcapkit.utilities.exceptions import FormatError, stacklevel from pcapkit.utilities.logging import get_logger @@ -84,7 +84,7 @@ def __post_init__(self) -> None: def __init__(self, section: 'Data_SectionHeaderBlock') -> 'None': ... -class PCAPNG(Engine[P_PCAPNG]): +class PCAPNG(EngineBase[P_PCAPNG]): """PCAP-NG file extraction support. Args: diff --git a/pcapkit/foundation/engines/pypcap.py b/pcapkit/foundation/engines/pypcap.py index 498f07081..604849e47 100644 --- a/pcapkit/foundation/engines/pypcap.py +++ b/pcapkit/foundation/engines/pypcap.py @@ -22,7 +22,7 @@ from pcapkit.const.reg.linktype import LinkType as Enum_LinkType from pcapkit.foundation.engines import _pcap_backend -from pcapkit.foundation.engines.engine import EngineBase as Engine +from pcapkit.foundation.engines.engine import EngineBase from pcapkit.foundation.reassembly import ReassemblyManager from pcapkit.foundation.traceflow import TraceFlowManager from pcapkit.utilities.exceptions import FormatError, UnsupportedCall, stacklevel @@ -44,7 +44,7 @@ RawFrame = tuple[float, bytes] -class PyPCAP(Engine['RawFrame']): +class PyPCAP(EngineBase['RawFrame']): """PyPCAP engine support. `PyPCAP`_ is a binding over :manpage:`libpcap(3)`, primarily aimed at live diff --git a/pcapkit/foundation/engines/pypcapfile.py b/pcapkit/foundation/engines/pypcapfile.py index 81fca5498..ebabf2a69 100644 --- a/pcapkit/foundation/engines/pypcapfile.py +++ b/pcapkit/foundation/engines/pypcapfile.py @@ -15,7 +15,7 @@ from typing import TYPE_CHECKING, cast from pcapkit.const.reg.linktype import LinkType as Enum_LinkType -from pcapkit.foundation.engines.engine import EngineBase as Engine +from pcapkit.foundation.engines.engine import EngineBase from pcapkit.foundation.reassembly import ReassemblyManager from pcapkit.utilities.exceptions import FormatError, stacklevel from pcapkit.utilities.warnings import AttributeWarning, warn @@ -62,7 +62,7 @@ def read(self, size: 'int' = -1) -> 'bytes': return self._stream.read(size) -class PyPCAPFile(Engine['PCAPFilePacket']): +class PyPCAPFile(EngineBase['PCAPFilePacket']): """PyPCAPFile engine support. `PyPCAPFile`_ is a pure Python savefile reader. It decodes Ethernet, IPv4, diff --git a/pcapkit/foundation/engines/pyshark.py b/pcapkit/foundation/engines/pyshark.py index 99c81d106..b1723be67 100644 --- a/pcapkit/foundation/engines/pyshark.py +++ b/pcapkit/foundation/engines/pyshark.py @@ -13,7 +13,7 @@ import sys from typing import TYPE_CHECKING, cast -from pcapkit.foundation.engines.engine import EngineBase as Engine +from pcapkit.foundation.engines.engine import EngineBase from pcapkit.foundation.reassembly import ReassemblyManager from pcapkit.utilities.exceptions import stacklevel from pcapkit.utilities.logging import get_logger @@ -34,7 +34,7 @@ logger = get_logger(__name__) -class PyShark(Engine['PySharkPacket']): +class PyShark(EngineBase['PySharkPacket']): """PyShark engine support. Args: diff --git a/pcapkit/foundation/engines/scapy.py b/pcapkit/foundation/engines/scapy.py index 8c893f5d1..ae7aa38e3 100644 --- a/pcapkit/foundation/engines/scapy.py +++ b/pcapkit/foundation/engines/scapy.py @@ -39,7 +39,7 @@ """ from typing import TYPE_CHECKING, cast -from pcapkit.foundation.engines.engine import EngineBase as Engine +from pcapkit.foundation.engines.engine import EngineBase from pcapkit.utilities.exceptions import stacklevel from pcapkit.utilities.logging import get_logger from pcapkit.utilities.warnings import AttributeWarning, warn @@ -58,7 +58,7 @@ logger = get_logger(__name__) -class Scapy(Engine['ScapyPacket']): +class Scapy(EngineBase['ScapyPacket']): """Scapy engine support. Args: diff --git a/pcapkit/foundation/reassembly/data/data.py b/pcapkit/foundation/reassembly/data/data.py index 9f79e9919..b96235d17 100644 --- a/pcapkit/foundation/reassembly/data/data.py +++ b/pcapkit/foundation/reassembly/data/data.py @@ -14,7 +14,7 @@ from pcapkit.const.reg.transtype import TransType from pcapkit.foundation.reassembly.data.ip import Datagram as IP_Datagram from pcapkit.foundation.reassembly.data.tcp import Datagram as TCP_Datagram - from pcapkit.protocols.protocol import ProtocolBase as Protocol + from pcapkit.protocols.protocol import ProtocolBase class Completion(StrEnum): @@ -130,13 +130,13 @@ class Deferred: __slots__ = ('analyze', 'proto', 'payload') - def __init__(self, analyze: 'Callable[[TransType, bytes], Protocol]', + def __init__(self, analyze: 'Callable[[TransType, bytes], ProtocolBase]', proto: 'TransType', payload: 'bytes') -> 'None': self.analyze = analyze self.proto = proto self.payload = payload - def __call__(self) -> 'Protocol': + def __call__(self) -> 'ProtocolBase': """Run the postponed analysis. Returns: @@ -171,7 +171,7 @@ class DeferredPacket: # supplies all three. Neither mypy nor pylint can see that from here, and # pylint calls it an *error* rather than a warning. - def __analyse__(self) -> 'Optional[Protocol]': + def __analyse__(self) -> 'Optional[ProtocolBase]': """Resolve a deferred analysis, at most once. Returns: diff --git a/pcapkit/foundation/reassembly/data/ip.py b/pcapkit/foundation/reassembly/data/ip.py index 821df38ba..26598a570 100644 --- a/pcapkit/foundation/reassembly/data/ip.py +++ b/pcapkit/foundation/reassembly/data/ip.py @@ -18,7 +18,7 @@ from typing_extensions import TypeAlias from pcapkit.const.reg.transtype import TransType - from pcapkit.protocols.protocol import ProtocolBase as Protocol + from pcapkit.protocols.protocol import ProtocolBase _AT = TypeVar('_AT', 'IPv4Address', 'IPv6Address') @@ -104,7 +104,7 @@ class Datagram(DeferredPacket, Info, Generic[_AT]): #: Parsed IP payload. Analysed on first read, not at construction time; a #: :class:`Deferred` may be passed in its place, and reading this attribute #: then runs it and keeps the result. - packet: 'Optional[Protocol]' + packet: 'Optional[ProtocolBase]' #: Octet ranges, absolute into the reassembled payload and both #: **inclusive** -- the same convention as :attr:`Packet.fo` combined with #: its length -- on which two fragments disagreed, i.e. an arriving @@ -146,7 +146,7 @@ class Datagram(DeferredPacket, Info, Generic[_AT]): # from. Overloads keyed on a literal cannot be selected from a ``completed`` # computed at runtime anyway, so they only made the reassemblers' own calls # untypeable while promising a correlation the code does not keep. - def __init__(self, completed: 'Completion', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[Protocol | Deferred]', conflict: 'tuple[tuple[int, int], ...]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin + def __init__(self, completed: 'Completion', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[ProtocolBase | Deferred]', conflict: 'tuple[tuple[int, int], ...]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin @info_final class Buffer(Info, Generic[_AT]): diff --git a/pcapkit/foundation/reassembly/data/tcp.py b/pcapkit/foundation/reassembly/data/tcp.py index be666b217..feaf16a11 100644 --- a/pcapkit/foundation/reassembly/data/tcp.py +++ b/pcapkit/foundation/reassembly/data/tcp.py @@ -18,7 +18,7 @@ from typing_extensions import TypeAlias - from pcapkit.protocols.protocol import ProtocolBase as Protocol + from pcapkit.protocols.protocol import ProtocolBase _AT = TypeVar('_AT', 'IPv4Address', 'IPv6Address') @@ -106,7 +106,7 @@ class Datagram(DeferredPacket, Info, Generic[_AT]): #: Parsed TCP payload. Analysed on first read rather than at construction; #: a :class:`Deferred` may be passed in its place, and reading this #: attribute then runs it and keeps the result. - packet: 'Optional[Protocol]' + packet: 'Optional[ProtocolBase]' #: Sequence ranges on which two segments disagreed, i.e. where an arriving #: segment overlapped bytes already buffered but did not repeat them. #: Each entry is ``(first, last)``, absolute TCP sequence numbers and both @@ -127,7 +127,7 @@ class Datagram(DeferredPacket, Info, Generic[_AT]): # :class:`~pcapkit.foundation.reassembly.data.ip.Datagram`, which applies # here identically: ``strict=False`` reports an incomplete payload buffer as # one contiguous ``bytes`` and analyses it. - def __init__(self, completed: 'Completion', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[Protocol | Deferred]', conflict: 'tuple[tuple[int, int], ...]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin + def __init__(self, completed: 'Completion', id: 'DatagramID[_AT]', index: 'tuple[int, ...]', header: 'bytes', payload: 'bytes | tuple[bytes, ...]', packet: 'Optional[ProtocolBase | Deferred]', conflict: 'tuple[tuple[int, int], ...]') -> 'None': ... # pylint: disable=unused-argument,super-init-not-called,multiple-statements,line-too-long,redefined-builtin @info_final diff --git a/pcapkit/foundation/reassembly/ip.py b/pcapkit/foundation/reassembly/ip.py index a3737aa29..8fff49ec1 100644 --- a/pcapkit/foundation/reassembly/ip.py +++ b/pcapkit/foundation/reassembly/ip.py @@ -19,7 +19,7 @@ from pcapkit.foundation.reassembly.data.data import Completion from pcapkit.foundation.reassembly.data.ip import (_AT, Buffer, BufferID, Datagram, DatagramID, Deferred, Packet) -from pcapkit.foundation.reassembly.reassembly import ReassemblyBase as Reassembly +from pcapkit.foundation.reassembly.reassembly import ReassemblyBase if TYPE_CHECKING: from typing import Type @@ -30,7 +30,7 @@ __all__ = ['IP'] -class IP(Reassembly[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Generic[_AT]): # pylint: disable=abstract-method +class IP(ReassemblyBase[Packet[_AT], Datagram[_AT], BufferID, Buffer[_AT]], Generic[_AT]): # pylint: disable=abstract-method """Reassembly for IP payload. Args: diff --git a/pcapkit/foundation/reassembly/reassembly.py b/pcapkit/foundation/reassembly/reassembly.py index 9ff532501..fe4fdff72 100644 --- a/pcapkit/foundation/reassembly/reassembly.py +++ b/pcapkit/foundation/reassembly/reassembly.py @@ -41,7 +41,7 @@ from pcapkit.corekit.infoclass import Info from pcapkit.corekit.module import ModuleDescriptor - from pcapkit.protocols.protocol import ProtocolBase as Protocol + from pcapkit.protocols.protocol import ProtocolBase CallbackFn = Callable[[list[_DT]], None] @@ -64,7 +64,7 @@ class ReassemblyMeta(abc.ABCMeta): #: Protocol name of current reassembly object. __protocol_name__: 'str' #: Protocol of current reassembly object. - __protocol_type__: 'Type[Protocol]' + __protocol_type__: 'Type[ProtocolBase]' @property def name(cls) -> 'str': @@ -74,7 +74,7 @@ def name(cls) -> 'str': return cls.__name__ @property - def protocol(cls) -> 'Type[Protocol]': + def protocol(cls) -> 'Type[ProtocolBase]': """Protocol of current reassembly object.""" if hasattr(cls, '__protocol_type__'): return cls.__protocol_type__ @@ -128,7 +128,7 @@ class ReassemblyBase(Generic[_PT, _DT, _IT, _BT], metaclass=ReassemblyMeta): #: Protocol name of current reassembly object. __protocol_name__: 'str' #: Protocol of current reassembly object. - __protocol_type__: 'Type[Protocol]' + __protocol_type__: 'Type[ProtocolBase]' #: List of callback functions upon reassembled datagram. __callback_fn__: 'list[CallbackFn]' @@ -181,7 +181,7 @@ def name(self) -> 'str': return type(self).name # type: ignore[return-value] @property - def protocol(self) -> 'Type[Protocol]': + def protocol(self) -> 'Type[ProtocolBase]': """Protocol of current reassembly object. Note: diff --git a/pcapkit/foundation/reassembly/tcp.py b/pcapkit/foundation/reassembly/tcp.py index fdd4df2a3..fdf692a69 100644 --- a/pcapkit/foundation/reassembly/tcp.py +++ b/pcapkit/foundation/reassembly/tcp.py @@ -16,7 +16,7 @@ from pcapkit.foundation.reassembly.data.data import Completion, Deferred from pcapkit.foundation.reassembly.data.tcp import (Buffer, BufferID, Datagram, DatagramID, Fragment, HoleDescriptor, Packet) -from pcapkit.foundation.reassembly.reassembly import ReassemblyBase as Reassembly +from pcapkit.foundation.reassembly.reassembly import ReassemblyBase from pcapkit.protocols.transport.tcp import TCP as TCP_Protocol if TYPE_CHECKING: @@ -25,7 +25,7 @@ __all__ = ['TCP'] -class TCP(Reassembly[Packet, Datagram, BufferID, Buffer]): +class TCP(ReassemblyBase[Packet, Datagram, BufferID, Buffer]): """Reassembly for TCP payload. Args: diff --git a/pcapkit/foundation/traceflow/tcp.py b/pcapkit/foundation/traceflow/tcp.py index f1fa80a05..8cf3f4ee6 100644 --- a/pcapkit/foundation/traceflow/tcp.py +++ b/pcapkit/foundation/traceflow/tcp.py @@ -13,7 +13,7 @@ from pcapkit.foundation.traceflow.data.data import Deferred from pcapkit.foundation.traceflow.data.tcp import _AT, Buffer, BufferID, Index, Packet -from pcapkit.foundation.traceflow.traceflow import TraceFlowBase as TraceFlow +from pcapkit.foundation.traceflow.traceflow import TraceFlowBase from pcapkit.protocols.transport.tcp import TCP as TCP_Protocol from pcapkit.utilities.logging import get_logger @@ -32,7 +32,7 @@ logger = get_logger(__name__) -class TCP(TraceFlow[BufferID, Buffer[_AT], Index, Packet[_AT]], Generic[_AT]): +class TCP(TraceFlowBase[BufferID, Buffer[_AT], Index, Packet[_AT]], Generic[_AT]): """Trace TCP flows. Args: diff --git a/pcapkit/interface/core.py b/pcapkit/interface/core.py index 97c8e81a4..425e0bf43 100644 --- a/pcapkit/interface/core.py +++ b/pcapkit/interface/core.py @@ -18,7 +18,7 @@ from pcapkit.foundation.reassembly.ipv6 import IPv6 as IPv6_Reassembly from pcapkit.foundation.reassembly.tcp import TCP as TCP_Reassembly from pcapkit.foundation.traceflow.tcp import TCP as TCP_TraceFlow -from pcapkit.protocols.protocol import ProtocolBase as Protocol +from pcapkit.protocols.protocol import ProtocolBase from pcapkit.utilities.exceptions import FormatError if TYPE_CHECKING: @@ -28,8 +28,8 @@ from pcapkit.corekit.context import ContextRegistry, ProtocolContext from pcapkit.foundation.extraction import Engines, Formats, Layers, Protocols, VerboseHandler - from pcapkit.foundation.reassembly.reassembly import ReassemblyBase as Reassembly - from pcapkit.foundation.traceflow.traceflow import TraceFlowBase as TraceFlow + from pcapkit.foundation.reassembly.reassembly import ReassemblyBase + from pcapkit.foundation.traceflow.traceflow import TraceFlowBase __all__ = [ 'extract', 'reassemble', 'trace', # interface functions @@ -73,7 +73,7 @@ def extract(fin: 'Optional[str | IO[bytes]]' = None, fout: 'Optional[str]' = None, format: 'Optional[Formats]' = None, # basic settings # pylint: disable=redefined-builtin auto: 'bool' = True, extension: 'bool' = True, store: 'bool' = True, # internal settings # pylint: disable=line-too-long files: 'bool' = False, nofile: 'bool' = False, verbose: 'bool | VerboseHandler' = False, # output settings # pylint: disable=line-too-long - engine: 'Optional[Engines]' = None, layer: 'Optional[Layers] | Type[Protocol]' = None, # extraction settings # pylint: disable=line-too-long + engine: 'Optional[Engines]' = None, layer: 'Optional[Layers] | Type[ProtocolBase]' = None, # extraction settings # pylint: disable=line-too-long protocol: 'Optional[Protocols]' = None, # extraction settings # pylint: disable=line-too-long reassembly: 'bool' = False, reasm_strict: 'bool' = True, reasm_store: 'bool' = True, # reassembly settings # pylint: disable=line-too-long reasm_timeout: 'Optional[float]' = None, # reassembly settings # pylint: disable=line-too-long @@ -157,7 +157,7 @@ def extract(fin: 'Optional[str | IO[bytes]]' = None, fout: 'Optional[str]' = Non An :class:`~pcapkit.foundation.extraction.Extractor` object. """ - if isinstance(layer, type) and issubclass(layer, Protocol): + if isinstance(layer, type) and issubclass(layer, ProtocolBase): layer = (layer.__layer__ or 'none').lower() # type: ignore[assignment] return Extractor(fin=fin, fout=fout, format=format, @@ -174,8 +174,8 @@ def extract(fin: 'Optional[str | IO[bytes]]' = None, fout: 'Optional[str]' = Non no_eof=no_eof, context=context) -def reassemble(protocol: 'str | Type[Protocol]', strict: 'bool' = False, - timeout: 'Optional[float]' = None) -> 'Reassembly': +def reassemble(protocol: 'str | Type[ProtocolBase]', strict: 'bool' = False, + timeout: 'Optional[float]' = None) -> 'ReassemblyBase': """Reassemble fragmented datagrams. Arguments: @@ -191,7 +191,7 @@ def reassemble(protocol: 'str | Type[Protocol]', strict: 'bool' = False, FormatError: If ``protocol`` is **NOT** any of IPv4, IPv6 or TCP. """ - if isinstance(protocol, type) and issubclass(protocol, Protocol): + if isinstance(protocol, type) and issubclass(protocol, ProtocolBase): protocol = protocol.id()[0] if protocol == 'IPv4': @@ -203,10 +203,10 @@ def reassemble(protocol: 'str | Type[Protocol]', strict: 'bool' = False, raise FormatError(f'Unsupported reassembly protocol: {protocol}') -def trace(protocol: 'str | Type[Protocol]', fout: 'Optional[str]', +def trace(protocol: 'str | Type[ProtocolBase]', fout: 'Optional[str]', format: 'Optional[str]', # pylint: disable=redefined-builtin byteorder: 'Literal["little", "big"]' = sys.byteorder, - nanosecond: bool = False, bidirectional: 'bool' = True) -> 'TraceFlow': + nanosecond: bool = False, bidirectional: 'bool' = True) -> 'TraceFlowBase': """Trace flows. Arguments: @@ -225,7 +225,7 @@ def trace(protocol: 'str | Type[Protocol]', fout: 'Optional[str]', FormatError: If ``protocol`` is **NOT** TCP. """ - if isinstance(protocol, type) and issubclass(protocol, Protocol): + if isinstance(protocol, type) and issubclass(protocol, ProtocolBase): protocol = protocol.id()[0] if protocol == 'TCP': diff --git a/pcapkit/utilities/decorators.py b/pcapkit/utilities/decorators.py index d696e8121..b68ed030b 100644 --- a/pcapkit/utilities/decorators.py +++ b/pcapkit/utilities/decorators.py @@ -25,12 +25,12 @@ from typing_extensions import Concatenate, ParamSpec - from pcapkit.protocols.protocol import ProtocolBase as Protocol + from pcapkit.protocols.protocol import ProtocolBase from pcapkit.protocols.schema.schema import Schema P = ParamSpec('P') R_seekset = TypeVar('R_seekset') - R_beholder = TypeVar('R_beholder', bound=Protocol) + R_beholder = TypeVar('R_beholder', bound=ProtocolBase) R_prepare = TypeVar('R_prepare', bound=Schema) __all__ = ['seekset', 'beholder', 'prepare'] @@ -41,7 +41,7 @@ logger = get_logger(__name__) -def seekset(func: 'Callable[Concatenate[Protocol, P], R_seekset]') -> 'Callable[P, R_seekset]': +def seekset(func: 'Callable[Concatenate[ProtocolBase, P], R_seekset]') -> 'Callable[P, R_seekset]': """Read file from start then set back to original. Important: @@ -67,7 +67,7 @@ def seekset(func: 'Callable[Concatenate[Protocol, P], R_seekset]') -> 'Callable[ @functools.wraps(func) def seekcur(*args: 'P.args', **kw: 'P.kwargs') -> 'R_seekset': # extract self object - self = cast('Protocol', args[0]) + self = cast('ProtocolBase', args[0]) # move file pointer seek_cur = self._file.tell() @@ -82,7 +82,9 @@ def seekcur(*args: 'P.args', **kw: 'P.kwargs') -> 'R_seekset': return seekcur -def beholder(func: 'Callable[Concatenate[Protocol, int, Optional[int], P], R_beholder]') -> 'Callable[P, R_beholder]': +def beholder( + func: 'Callable[Concatenate[ProtocolBase, int, Optional[int], P], R_beholder]', +) -> 'Callable[P, R_beholder]': """Behold extraction procedure. Important: diff --git a/tests/test_base_class_contract.py b/tests/test_base_class_contract.py new file mode 100644 index 000000000..7d2e55757 --- /dev/null +++ b/tests/test_base_class_contract.py @@ -0,0 +1,381 @@ +# -*- coding: utf-8 -*- +"""The ``*Base``/public split, asserted rather than left to convention. + +Five class pairs carry the same contract, and GitHub issue #514 settled what it +is: **library classes inherit the ``*Base``; users inherit the public class, and +only a subclass of the public class auto-registers.** So +``descendants(Protocol) == 0`` is the invariant to enforce, not a defect to fix. + +================= ============================================== ================= +Suite ``*Base`` public +================= ============================================== ================= +protocols :class:`pcapkit.protocols.protocol.ProtocolBase` ``Protocol`` +engines :class:`pcapkit.foundation.engines.engine.EngineBase` ``Engine`` +reassembly :class:`pcapkit.foundation.reassembly.reassembly.ReassemblyBase` ``Reassembly`` +traceflow :class:`pcapkit.foundation.traceflow.traceflow.TraceFlowBase` ``TraceFlow`` +dumpers :class:`pcapkit.dumpkit.common.DumperBase` ``Dumper`` +================= ============================================== ================= + +Two different things are checked here, and the split between them is deliberate +because only one of them is a behaviour claim. + +:class:`BaseClassAliasTests` is the part with teeth. Until #514 part (c), 82 +import statements across the library read ``from ... import ProtocolBase as +Protocol`` -- binding the *base* to the *public* name -- so a module went on to +say ``class Application(Protocol)`` while in fact inheriting ``ProtocolBase``. +Nothing was wrong at runtime; the classes were correct. What was wrong is that +the source said the opposite of what it did, and that is not a cosmetic +complaint: the aliasing was a *convention* rather than a mechanism, so a sweep +over it could miss a site and nothing would fail. It missed one twice -- +issue #506 (a runtime import in ``schema/schema.py`` unreachable for three +years) and issue #513 (the ``extraction.py`` gates rejecting the library's own +built-ins for three years). These two tests replace the convention with +something that fails. + +The sweep is complete but the migration is not: 28 of the 82 sites are renamed, +and the other 54 sit in paths that open pull requests own -- see +:data:`PENDING_ALIAS_PATHS`, which is the whole of the remaining work and +shrinks to nothing as those land. The four non-``protocols`` families +(``Engine`` 8 sites, ``Reassembly`` 3, ``Dumper`` 2, ``TraceFlow`` 2) are +finished; every one of the 54 outstanding is a ``ProtocolBase`` site. + +:class:`RegistrationGateTests` is a regression pin, not a new behaviour. Every +assertion in it already held before part (c), because parts (a) and (b) +(issues #547 and #570) made registration opt-in on a keyword. It is written down +because the ruling promotes it from an accident of where the hook happens to live +into the specification, and an unasserted specification is one refactor away from +being untrue. The property worth noticing is the last one: a library-style +subclass cannot opt in *even if it tries*, because the ``*Base`` hook does not +accept the keyword at all. That is the ``final``-substitute the split was built +to provide, and it is stronger than a convention. + +.. note:: + + :class:`RegistrationGateTests` mutates the process-global registries, so each + test restores exactly the keys it added. It deliberately does not assert on + ``descendants(Public)`` directly -- any other module in the same session that + defines a subclass of a public class (``DummyProtocol`` in + :file:`tests/protocols/schema/test_schema_unit.py` is one, and it exists on + purpose) would make that count non-zero and the assertion order-dependent. + The invariant is asserted over the *library's own* classes instead, which is + what it is actually about and is immune to collection order. + +""" +from __future__ import annotations + +import ast +import pkgutil +import unittest +from typing import TYPE_CHECKING + +from tests._tiers import ROOT + +import pcapkit +from pcapkit.dumpkit.common import Dumper, DumperBase +from pcapkit.foundation.engines.engine import Engine, EngineBase +from pcapkit.foundation.extraction import Extractor +from pcapkit.foundation.reassembly.reassembly import Reassembly, ReassemblyBase +from pcapkit.foundation.traceflow.traceflow import TraceFlow, TraceFlowBase +from pcapkit.protocols.protocol import Protocol, ProtocolBase + +if TYPE_CHECKING: + from typing import Any, Iterator + +#: Every ``*Base`` name mapped to the public name it must never be aliased to. +#: Keyed on the base rather than the public name because the base is what an +#: import statement names; the alias is the thing being outlawed. +BASE_TO_PUBLIC = { + 'ProtocolBase': 'Protocol', + 'EngineBase': 'Engine', + 'ReassemblyBase': 'Reassembly', + 'DumperBase': 'Dumper', + 'TraceFlowBase': 'TraceFlow', +} + +#: ``FieldBase``/``Field`` is *not* in this table, and the omission is the +#: point. That pair is not registration-motivated -- ``Field`` has no +#: ``__init_subclass__`` and adds a real ``__init__`` and a ``template`` +#: property -- so aliasing it is a legitimate abbreviation rather than a +#: misstatement. #514 excludes it explicitly. +NOT_IN_SCOPE = frozenset({'FieldBase'}) + +#: Paths still carrying the alias because another open pull request owns them, +#: mapped to that pull request. A prefix rather than a file list, because #726 +#: owns the whole ``pcapkit/protocols/`` subtree and enumerating its ~50 sites +#: would turn this into a changelog. +#: +#: Each entry is a promise, not an exemption: when the named pull request lands, +#: the alias comes out and the entry goes with it. **An empty dict is the end +#: state**, and :meth:`BaseClassAliasTests.test_no_library_module_imports_a_base_under_its_public_name` +#: fails on an entry that has stopped matching anything, so a stale promise +#: cannot sit here unnoticed once its pull request merges. +PENDING_ALIAS_PATHS = { + 'pcapkit/protocols/': 726, + 'pcapkit/foundation/registry/protocols.py': 726, + 'pcapkit/foundation/extraction.py': 742, + 'pcapkit/foundation/traceflow/traceflow.py': 742, +} + + +def is_pending(rel: 'str') -> 'bool': + """Is ``rel`` owned by an open pull request? + + Args: + rel: repo-relative POSIX path. + + Returns: + Whether some entry of :data:`PENDING_ALIAS_PATHS` covers it. + + """ + return any(rel.startswith(prefix) for prefix in PENDING_ALIAS_PATHS) + + +def iter_library_sources() -> 'Iterator[tuple[str, ast.Module]]': + """Every module under :file:`pcapkit/`, as a parsed tree. + + Yields: + The repo-relative POSIX path and the parsed module. + + """ + for path in sorted((ROOT / 'pcapkit').rglob('*.py')): + rel = path.relative_to(ROOT).as_posix() + yield rel, ast.parse(path.read_text(encoding='utf-8')) + + +def find_alias_imports(tree: 'ast.Module') -> 'list[tuple[int, str, str]]': + """Locate every ``import Base as `` in ``tree``. + + Args: + tree: parsed module to search. + + Returns: + One ``(lineno, base name, alias)`` triple per offending import. + + """ + found = [] + for node in ast.walk(tree): + if not isinstance(node, (ast.Import, ast.ImportFrom)): + continue + for alias in node.names: + name = alias.name.rpartition('.')[2] + if name in NOT_IN_SCOPE or alias.asname is None: + continue + if BASE_TO_PUBLIC.get(name) == alias.asname: + found.append((node.lineno, name, alias.asname)) + return found + + +class BaseClassAliasTests(unittest.TestCase): + """No library module may call a ``*Base`` class by its public name.""" + + def test_no_library_module_imports_a_base_under_its_public_name(self) -> None: + """Source-level sweep: the alias must not appear. + + This is the source-level half, and it is the half that sees the ~53 + aliases that live inside ``if TYPE_CHECKING:``. Those have no runtime + effect at all -- they exist so an annotation can say ``Protocol`` -- so + :meth:`test_library_modules_do_not_bind_the_public_name_at_runtime` + cannot see them and this one has to. + + """ + offenders = {} + for rel, tree in iter_library_sources(): + hits = find_alias_imports(tree) + if hits: + offenders[rel] = hits + + unexpected = {rel: hits for rel, hits in offenders.items() + if not is_pending(rel)} + self.assertEqual( + unexpected, {}, + 'a library module imports a *Base class under its public name, so its ' + 'source now says the opposite of what it does; import the base under ' + 'its own name and rename the references. C.f. #514.' + ) + + # A pending entry that has stopped matching anything is worth failing on + # too: it means the pull request landed and the promise above is now a lie. + stale = sorted(prefix for prefix in PENDING_ALIAS_PATHS + if not any(rel.startswith(prefix) for rel in offenders)) + self.assertEqual( + stale, [], + 'PENDING_ALIAS_PATHS names a path that no longer carries the alias; ' + 'drop the entry now that its pull request has landed.' + ) + + def test_library_modules_do_not_bind_the_public_name_at_runtime(self) -> None: + """Runtime sweep: importing the module must not expose the public name. + + The complement to the source-level check, and not redundant with it. + This one is what notices a module that reaches the base through some + other spelling -- ``Protocol = ProtocolBase``, a re-export, a + ``globals()`` write -- rather than through an ``import ... as ...`` an + AST walk can recognise. + + A module legitimately binds the public name when it imports the *public + class itself*, so identity is what is asserted, not absence: the name + may exist, it just may not be the base. + + """ + pending = tuple(rel.removesuffix('.py').replace('/', '.') + for rel in PENDING_ALIAS_PATHS) + + offenders = [] + for info in pkgutil.walk_packages(pcapkit.__path__, prefix='pcapkit.'): + if info.name.startswith(pending) or info.name.startswith('pcapkit.vendor.'): + continue + module = __import__(info.name, fromlist=['__name__']) + for base_name, public_name in BASE_TO_PUBLIC.items(): + base = globals()[base_name] + bound = getattr(module, public_name, None) + if bound is base: + offenders.append(f'{info.name}.{public_name} is {base_name}') + + self.assertEqual( + offenders, [], + 'a module binds a public class name to the *Base class at runtime. ' + 'C.f. #514.' + ) + + +class RegistrationGateTests(unittest.TestCase): + """Only a subclass of the public class registers -- pinned, per suite.""" + + #: ``(label, base, public, keyword, registry accessor)`` per suite. The + #: protocols suite is absent on purpose: its name registry is + #: ``pcapkit.protocols.__proto__`` and its hook takes no registration + #: keyword of this shape, so it is covered by + #: :meth:`test_library_classes_are_not_descendants_of_the_public_class` and + #: by ``tests/protocols/`` instead. + SUITES = ( + ('engines', EngineBase, Engine, 'engine', 'ENGINE'), + ('reassembly', ReassemblyBase, Reassembly, 'protocol', 'REASSEMBLY'), + ('traceflow', TraceFlowBase, TraceFlow, 'protocol', 'TRACEFLOW'), + ('dumpers', DumperBase, Dumper, 'fmt', 'OUTPUT'), + ) + + @staticmethod + def registry(which: 'str') -> 'dict[str, Any]': + """The name-keyed registry for a suite. + + Args: + which: suite tag from :attr:`SUITES`. + + Returns: + The live registry mapping, not a copy. + + """ + return { + 'ENGINE': Extractor.__engine__, + 'REASSEMBLY': Extractor.__reassembly__, + 'TRACEFLOW': Extractor.__traceflow__, + 'OUTPUT': Extractor.__output__, + }[which] + + def make_subclass(self, name: 'str', base: 'type', **kwargs: 'Any') -> 'type': + """Create a subclass, restoring any registry key it adds. + + Args: + name: name for the new class. + base: the class to subclass. + **kwargs: class keywords to pass at definition. + + Returns: + The new class. + + """ + snapshots = [(tag, dict(self.registry(tag))) for _, _, _, _, tag in self.SUITES] + + def restore() -> 'None': + for tag, before in snapshots: + live = self.registry(tag) + for key in set(live) - set(before): + del live[key] + + self.addCleanup(restore) + return type(name, (base,), {}, **kwargs) + + def test_library_style_subclass_does_not_register(self) -> None: + """``class X(Base)`` -- the library's own shape -- registers nothing.""" + for label, base, _, _, tag in self.SUITES: + with self.subTest(suite=label): + before = set(self.registry(tag)) + self.make_subclass(f'LibraryStyle_{label}', base) + self.assertEqual(set(self.registry(tag)) - before, set()) + + def test_library_style_subclass_cannot_opt_in_at_all(self) -> None: + """``class X(Base, =...)`` is a :exc:`TypeError`. + + This is the ``final`` substitute the split exists to provide, and it is + the strongest assertion in this module: a library class cannot register + itself *by accident or on purpose*, because the registration keyword + never reaches a hook that understands it. The keyword falls through to + :meth:`object.__init_subclass__`, which takes none. + + Asserting the exception rather than merely "no registry change" is what + distinguishes this from the test above -- a silent no-op would satisfy + that one and still leave the keyword looking supported. + + """ + for label, base, _, keyword, _ in self.SUITES: + with self.subTest(suite=label): + with self.assertRaises(TypeError): + self.make_subclass(f'LibraryOptIn_{label}', base, + **{keyword: f'library_opt_in_{label}'}) + + def test_user_style_subclass_does_not_register_without_the_keyword(self) -> None: + """``class X(Public)`` alone still registers nothing. C.f. #547.""" + for label, _, public, _, tag in self.SUITES: + with self.subTest(suite=label): + before = set(self.registry(tag)) + self.make_subclass(f'UserStyle_{label}', public) + self.assertEqual(set(self.registry(tag)) - before, set()) + + def test_user_style_subclass_registers_when_it_opts_in(self) -> None: + """``class X(Public, =...)`` is the one shape that registers.""" + for label, _, public, keyword, tag in self.SUITES: + with self.subTest(suite=label): + key = f'user_opt_in_{label}' + before = set(self.registry(tag)) + self.make_subclass(f'UserOptIn_{label}', public, **{keyword: key}) + self.assertEqual(set(self.registry(tag)) - before, {key}) + + def test_library_classes_are_not_descendants_of_the_public_class(self) -> None: + """No class shipped under :file:`pcapkit/` subclasses a public class. + + The invariant #514 settled, asserted over the library's own classes so + that a subclass created by another test module cannot affect the result + -- see this module's note on why ``descendants(Public)`` itself is not + the thing to assert. + + """ + for label, base, public in (('protocols', ProtocolBase, Protocol), + ('engines', EngineBase, Engine), + ('reassembly', ReassemblyBase, Reassembly), + ('traceflow', TraceFlowBase, TraceFlow), + ('dumpers', DumperBase, Dumper)): + with self.subTest(suite=label): + found, pending = set(), [base] + while pending: + for sub in pending.pop().__subclasses__(): + if sub not in found: + found.add(sub) + pending.append(sub) + + offenders = sorted( + f'{cls.__module__}.{cls.__qualname__}' for cls in found + if cls is not public + and issubclass(cls, public) + and (cls.__module__ == 'pcapkit' + or cls.__module__.startswith('pcapkit.')) + ) + self.assertEqual( + offenders, [], + f'a library class subclasses {public.__name__} and so ' + f'auto-registers; library classes inherit ' + f'{base.__name__}. C.f. #514.' + ) + + +if __name__ == '__main__': + unittest.main()