From fa4bf23388db22f807fcc5da1b5bdf3603ea32ea Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Sun, 25 Oct 2026 03:32:04 -0400 Subject: [PATCH] fix(corekit): export SchemaField, SwitchField, PortEnumField and nested_packet_context (#2174) fields.misc.__all__ omitted SchemaField and nested_packet_context, fields.strings.__all__ omitted BytesField (dropped by 19b98fc91 while the package kept exporting it), and the fields package, pcapkit.corekit and pcapkit.all omitted SwitchField and PortEnumField, so import * dropped documented field classes. DeferredPacket.to_dict was annotated as dict, widening Info.to_dict's OrderedMultiDict. Add the field classes to every __all__ that re-lists fields, and nested_packet_context to misc.__all__ only: like packet_byteorder and parse_ip_address it is a helper reached by its module path, not a field. Annotate to_dict as OrderedMultiDict[str, Any]. Test: tests/corekit/test_fields_all_exports_2174_unit.py checks each field submodule's __all__ against the package's in both directions, with written-down exclusions, and the to_dict annotation; it fails on origin/main. --- pcapkit/all.py | 4 +- pcapkit/corekit/__init__.py | 4 +- pcapkit/corekit/fields/__init__.py | 10 +- pcapkit/corekit/fields/misc.py | 3 +- pcapkit/corekit/fields/strings.py | 1 + pcapkit/corekit/packet.py | 4 +- .../test_fields_all_exports_2174_unit.py | 132 ++++++++++++++++++ 7 files changed, 147 insertions(+), 11 deletions(-) create mode 100644 tests/corekit/test_fields_all_exports_2174_unit.py diff --git a/pcapkit/all.py b/pcapkit/all.py index 7ebc733b98..2fe64341d0 100644 --- a/pcapkit/all.py +++ b/pcapkit/all.py @@ -58,11 +58,11 @@ 'VersionInfo', # Version 'NumberField', 'Int32Field', 'UInt32Field', # numeric protocol fields 'Int16Field', 'UInt16Field', 'Int64Field', 'UInt64Field', - 'Int8Field', 'UInt8Field', 'EnumField', + 'Int8Field', 'UInt8Field', 'EnumField', 'PortEnumField', 'BytesField', 'StringField', 'BitField', # text protocol fields 'PaddingField', 'ConditionalField', 'PayloadField', 'SchemaField', # misc protocol fields - 'ForwardMatchField', 'NoValueField', + 'SwitchField', 'ForwardMatchField', 'NoValueField', 'IPv4AddressField', 'IPv6AddressField', # IP address protocol fields 'IPv4InterfaceField', 'IPv6InterfaceField', 'ListField', 'OptionField', # container protocol fields diff --git a/pcapkit/corekit/__init__.py b/pcapkit/corekit/__init__.py index c9058a5646..7d3f380532 100644 --- a/pcapkit/corekit/__init__.py +++ b/pcapkit/corekit/__init__.py @@ -42,14 +42,14 @@ class :class:`~pcapkit.corekit.infoclass.Info`, 'Int16Field', 'UInt16Field', 'Int64Field', 'UInt64Field', 'Int8Field', 'UInt8Field', - 'EnumField', + 'EnumField', 'PortEnumField', 'BytesField', 'StringField', 'BitField', 'PaddingField', - 'ConditionalField', 'PayloadField', 'SchemaField', + 'ConditionalField', 'PayloadField', 'SchemaField', 'SwitchField', 'ForwardMatchField', 'NoValueField', 'ListField', 'OptionField', diff --git a/pcapkit/corekit/fields/__init__.py b/pcapkit/corekit/fields/__init__.py index 92225e15d8..6f4ab908bb 100644 --- a/pcapkit/corekit/fields/__init__.py +++ b/pcapkit/corekit/fields/__init__.py @@ -16,10 +16,10 @@ from pcapkit.corekit.fields.ipaddress import (IPv4AddressField, IPv4InterfaceField, IPv6AddressField, IPv6InterfaceField) from pcapkit.corekit.fields.misc import (ConditionalField, ForwardMatchField, NoValueField, - PayloadField, SchemaField) + PayloadField, SchemaField, SwitchField) from pcapkit.corekit.fields.numbers import (EnumField, Int8Field, Int16Field, Int32Field, - Int64Field, NumberField, UInt8Field, UInt16Field, - UInt32Field, UInt64Field) + Int64Field, NumberField, PortEnumField, UInt8Field, + UInt16Field, UInt32Field, UInt64Field) from pcapkit.corekit.fields.strings import BitField, BytesField, PaddingField, StringField __all__ = [ @@ -28,14 +28,14 @@ 'Int16Field', 'UInt16Field', 'Int64Field', 'UInt64Field', 'Int8Field', 'UInt8Field', - 'EnumField', + 'EnumField', 'PortEnumField', 'BytesField', 'StringField', 'BitField', 'PaddingField', - 'ConditionalField', 'PayloadField', 'SchemaField', + 'ConditionalField', 'PayloadField', 'SchemaField', 'SwitchField', 'ForwardMatchField', 'NoValueField', 'ListField', 'OptionField', diff --git a/pcapkit/corekit/fields/misc.py b/pcapkit/corekit/fields/misc.py index b92a5c0e31..b5bff36784 100644 --- a/pcapkit/corekit/fields/misc.py +++ b/pcapkit/corekit/fields/misc.py @@ -11,9 +11,10 @@ from pcapkit.utilities.warnings import RegistryWarning, warn __all__ = [ - 'ConditionalField', 'PayloadField', + 'ConditionalField', 'PayloadField', 'SchemaField', 'SwitchField', 'ForwardMatchField', 'NoValueField', + 'nested_packet_context', ] if TYPE_CHECKING: diff --git a/pcapkit/corekit/fields/strings.py b/pcapkit/corekit/fields/strings.py index b7f6eee396..a3ee132b1e 100644 --- a/pcapkit/corekit/fields/strings.py +++ b/pcapkit/corekit/fields/strings.py @@ -12,6 +12,7 @@ __all__ = [ '_TextField', + 'BytesField', 'StringField', 'DecodedString', 'BitField', diff --git a/pcapkit/corekit/packet.py b/pcapkit/corekit/packet.py index 0c508d936d..e5de1f7f2e 100644 --- a/pcapkit/corekit/packet.py +++ b/pcapkit/corekit/packet.py @@ -44,6 +44,8 @@ if TYPE_CHECKING: from typing import Any, Callable, ClassVar, Type + from pcapkit.corekit.multidict import OrderedMultiDict + __all__ = ['DeferredPacket'] @@ -168,7 +170,7 @@ def __repr__(self) -> 'str': self.__analyse__() return super().__repr__() - def to_dict(self) -> 'dict[str, Any]': + def to_dict(self) -> 'OrderedMultiDict[str, Any]': """Convert the instance into :obj:`dict`. Returns: diff --git a/tests/corekit/test_fields_all_exports_2174_unit.py b/tests/corekit/test_fields_all_exports_2174_unit.py new file mode 100644 index 0000000000..0f1ff0c573 --- /dev/null +++ b/tests/corekit/test_fields_all_exports_2174_unit.py @@ -0,0 +1,132 @@ +# -*- coding: utf-8 -*- +"""The field modules' :attr:`__all__` lists keep in step with one another. + +GitHub issue #2174. :mod:`pcapkit.corekit.fields.misc` left +:class:`~pcapkit.corekit.fields.misc.SchemaField` and +:func:`~pcapkit.corekit.fields.misc.nested_packet_context` out of its +:attr:`__all__`, :mod:`pcapkit.corekit.fields.strings` left out +:class:`~pcapkit.corekit.fields.strings.BytesField`, and +:mod:`pcapkit.corekit.fields`, :mod:`pcapkit.corekit` and +:mod:`pcapkit.all` all left out +:class:`~pcapkit.corekit.fields.misc.SwitchField` and +:class:`~pcapkit.corekit.fields.numbers.PortEnumField`, so ``import *`` from any +of them dropped documented field classes. +:mod:`tests.project.test_all_exports_unit` checks :mod:`pcapkit.all` against +:mod:`pcapkit.corekit`, but nothing checked the field package against its own +submodules, nor :mod:`pcapkit.corekit` against the field package it re-lists by +hand. + +The same issue found :meth:`DeferredPacket.to_dict +` annotated as returning a +:obj:`dict`, widening the +:class:`~pcapkit.corekit.multidict.OrderedMultiDict` of the +:meth:`Info.to_dict ` it overrides. +""" + +import importlib +import pkgutil +import unittest + +import pcapkit.corekit +import pcapkit.corekit.fields +from pcapkit.corekit.fields import misc +from pcapkit.corekit.infoclass import Info +from pcapkit.corekit.multidict import OrderedMultiDict +from pcapkit.corekit.packet import DeferredPacket + +#: Test tier (#1958): checks the names the field modules export, a question about +#: the tree rather than about the interpreter. +__tier__ = 'LATEST' + +#: Names a field submodule exports and :mod:`pcapkit.corekit.fields` deliberately +#: does not, keyed by the submodule. The package's :attr:`__all__` lists concrete +#: field classes only, which :mod:`pcapkit.corekit` and :mod:`pcapkit.all` then +#: re-export: +#: +#: * ``Field`` and ``_TextField`` are abstract bases, kept out of ``import *`` as +#: every package's base class is (see +#: :data:`tests.project.test_public_api.DELIBERATE_NON_EXPORTS`). +#: * ``packet_byteorder``, ``byteorder_callback``, ``parse_ip_address`` and +#: ``nested_packet_context`` are helpers for writing a field or its callbacks, +#: reached by their module path. ``nested_packet_context`` in particular builds +#: the packet context a nested schema's callbacks receive; star-exporting it +#: would drop a generic name into the caller's namespace (:issue:`2174`). +#: * ``DecodedString`` is the value type :class:`~pcapkit.corekit.fields.strings.StringField` +#: returns, not a field. +FIELDS_DELIBERATE_NON_EXPORTS = { + 'pcapkit.corekit.fields.byteorder': ('byteorder_callback', 'packet_byteorder'), + 'pcapkit.corekit.fields.field': ('Field',), + 'pcapkit.corekit.fields.ipaddress': ('parse_ip_address',), + 'pcapkit.corekit.fields.misc': ('nested_packet_context',), + 'pcapkit.corekit.fields.strings': ('DecodedString', '_TextField'), +} + + +def submodules() -> 'list[str]': + """Return the dotted names of the :mod:`pcapkit.corekit.fields` submodules.""" + return [info.name for info in pkgutil.iter_modules(pcapkit.corekit.fields.__path__, + 'pcapkit.corekit.fields.')] + + +class FieldsExportsTests(unittest.TestCase): + """Every field a submodule exports reaches ``import *`` of the packages above it.""" + + def test_submodules_are_found(self) -> None: + self.assertIn('pcapkit.corekit.fields.misc', submodules()) + self.assertIn('pcapkit.corekit.fields.numbers', submodules()) + + def test_every_submodule_export_is_listed_or_excluded(self) -> None: + listed = set(pcapkit.corekit.fields.__all__) + for name in submodules(): + with self.subTest(module=name): + exported = set(importlib.import_module(name).__all__) + excluded = set(FIELDS_DELIBERATE_NON_EXPORTS.get(name, ())) + self.assertEqual(sorted(exported - listed - excluded), []) + + def test_exclusions_are_not_stale(self) -> None: + listed = set(pcapkit.corekit.fields.__all__) + for name, names in FIELDS_DELIBERATE_NON_EXPORTS.items(): + with self.subTest(module=name): + self.assertIn(name, submodules()) + exported = set(importlib.import_module(name).__all__) + self.assertEqual(sorted(set(names) - exported), []) + self.assertEqual(sorted(set(names) & listed), []) + + def test_each_field_is_exported_by_its_own_module(self) -> None: + # The converse of the check above: a field the package exports is + # also in the __all__ of the module defining it. strings.__all__ had + # dropped BytesField, and misc.__all__ SchemaField (#2174). + for name in pcapkit.corekit.fields.__all__: + home = getattr(pcapkit.corekit.fields, name).__module__ + with self.subTest(name=name, module=home): + self.assertIn(name, importlib.import_module(home).__all__) + + def test_corekit_relists_every_field(self) -> None: + self.assertEqual(sorted(set(pcapkit.corekit.fields.__all__) - set(pcapkit.corekit.__all__)), []) + + def test_misc_exports_what_it_documents(self) -> None: + for name in ('SchemaField', 'SwitchField', 'nested_packet_context'): + with self.subTest(name=name): + self.assertIn(name, misc.__all__) + + def test_star_import_reaches_the_new_names(self) -> None: + for package in ('pcapkit.corekit.fields', 'pcapkit.corekit', 'pcapkit.all'): + namespace = {} # type: dict[str, object] + exec(f'from {package} import *', namespace) # pylint: disable=exec-used + for name in ('SchemaField', 'SwitchField', 'PortEnumField'): + with self.subTest(package=package, name=name): + self.assertIn(name, namespace) + + +class DeferredPacketToDictTests(unittest.TestCase): + """``DeferredPacket.to_dict`` is annotated as what it returns.""" + + def test_annotation_matches_the_override(self) -> None: + mine = DeferredPacket.to_dict.__annotations__['return'] + base = Info.to_dict.__annotations__['return'] + self.assertEqual(mine.split('[', 1)[0], OrderedMultiDict.__name__) + self.assertEqual(mine.split('[', 1)[0], base.split('[', 1)[0]) + + +if __name__ == '__main__': + unittest.main()