From cfb909b529358d1cb796d5117428f273cec1a130 Mon Sep 17 00:00:00 2001 From: Jarry Shaw Date: Wed, 14 Oct 2026 11:17:40 -0400 Subject: [PATCH] refactor(corekit,utilities): detect get_annotations, UnionType and dict reversal by version, not by probe (#1709) - compat re-exports get_annotations behind a sys.version_info gate: inspect.get_annotations from 3.10, and below it the reader infoclass had, a class's own __dict__['__annotations__'], uncopied - compat re-exports UnionType: types.UnionType from 3.10, None below it - infoclass and protocol take both from compat in place of their getattr(inspect/types, ...) probes; _own_annotations is gone - MultiInfo and OrderedMultiInfo bind __reversed__ behind sys.version_info >= (3, 8) instead of hasattr(dict, '__reversed__') - _make_index_calls reads an ast call's end_lineno behind sys.version_info >= (3, 8) instead of getattr(node, 'end_lineno', ...) - a project test scans pcapkit/ for getattr/hasattr by literal name of a module or builtin class, or of end_lineno/end_col_offset on any object; on main it reports exactly these six Tests: test_compat, test_infoclass, the probe scan, from_dict, MultiInfo, protocol payload and make() selections pass on 3.8, 3.9, 3.10 and 3.14; 492 Info signatures and nested types byte-identical to main. Closes #1709 --- docs/source/pcapkit/utilities/index.rst | 4 + pcapkit/corekit/infoclass.py | 53 ++----- pcapkit/protocols/protocol.py | 19 +-- pcapkit/utilities/compat.py | 29 +++- tests/corekit/test_infoclass.py | 24 ++- .../test_stdlib_feature_probes_1709_unit.py | 141 ++++++++++++++++++ tests/protocols/test_protocol_make_unit.py | 13 ++ tests/utilities/test_compat.py | 48 ++++++ 8 files changed, 272 insertions(+), 59 deletions(-) create mode 100644 tests/project/test_stdlib_feature_probes_1709_unit.py diff --git a/docs/source/pcapkit/utilities/index.rst b/docs/source/pcapkit/utilities/index.rst index 7d22240b4d..3b5e141990 100644 --- a/docs/source/pcapkit/utilities/index.rst +++ b/docs/source/pcapkit/utilities/index.rst @@ -62,3 +62,7 @@ following objects and functions: - Python 3.10 * - :data:`threading.__excepthook__`, as ``default_threading_excepthook`` - Python 3.10 + * - :func:`inspect.get_annotations`, of a class + - Python 3.10 + * - :class:`types.UnionType`, or :data:`None` below it + - Python 3.10 diff --git a/pcapkit/corekit/infoclass.py b/pcapkit/corekit/infoclass.py index 5e71a61cda..379d249a57 100644 --- a/pcapkit/corekit/infoclass.py +++ b/pcapkit/corekit/infoclass.py @@ -17,7 +17,6 @@ import collections.abc import contextlib import enum -import inspect import itertools import sys import types @@ -26,7 +25,7 @@ from pcapkit.corekit.enum import EnumLookup from pcapkit.corekit.multidict import MultiDict, OrderedMultiDict, iter_multi_items -from pcapkit.utilities.compat import Mapping, final +from pcapkit.utilities.compat import Mapping, UnionType, final, get_annotations from pcapkit.utilities.exceptions import InfoError, UnsupportedCall, stacklevel from pcapkit.utilities.warnings import InfoWarning, warn @@ -70,33 +69,6 @@ class FinalisedState(EnumLookup, enum.IntEnum): FINAL = enum.auto() -#: :func:`inspect.get_annotations`, new in Python 3.10, or :data:`None` below it. -_get_annotations = getattr(inspect, 'get_annotations', None) - - -def _own_annotations(cls: 'type') -> 'dict[str, Any]': - """Return the annotations ``cls`` declares itself, never those of a base. - - Below Python 3.10 (bpo-43901), ``cls.__annotations__`` of a class whose body - declares none is its nearest base's, so a field-less :class:`Info` - subclass read :class:`Info`'s own ``__finalised__``, ``__additional__`` and - ``__excluded__`` as its fields (:issue:`1692`). There, a class's own - annotations are in its ``__dict__``; from 3.10 on, they are read with - :func:`inspect.get_annotations`, as from 3.14 they are evaluated lazily and - are not in ``__dict__`` at all. - - Args: - cls: Class to read. - - Returns: - The annotations ``cls``'s own body declares, which may be empty. - - """ - if _get_annotations is None: - return cls.__dict__.get('__annotations__', {}) - return _get_annotations(cls) - - def info_final(cls: 'ST', *, _finalised: 'bool' = True) -> 'ST': """Finalise info class. @@ -195,9 +167,13 @@ def info_final(cls: 'ST', *, _finalised: 'bool' = True) -> 'ST': # NOTE: We iterate in reversed order to ensure that the type # annotations of the superclasses are considered first. Each class - # contributes only the annotations it declares itself (see - # :func:`_own_annotations`), on every Python version. - for key in reversed(_own_annotations(cls_)): + # contributes only the annotations it declares itself, on every + # Python version: below 3.10 (bpo-43901), ``__annotations__`` of a + # class declaring none is its nearest base's, so a field-less + # subclass read Info's ``__finalised__``, ``__additional__`` and + # ``__excluded__`` as its fields (:issue:`1692`). See + # :func:`~pcapkit.utilities.compat.get_annotations`. + for key in reversed(get_annotations(cls_)): # NOTE: We skip duplicated annotations to avoid duplicate # argument in function definition. if key in args_: @@ -899,7 +875,7 @@ class MultiInfo(_MultiInfo[KT, VT], MultiDict[KT, VT]): __iter__ = MultiDict.__iter__ # type: ignore[assignment] __len__ = MultiDict.__len__ __contains__ = MultiDict.__contains__ - if hasattr(dict, '__reversed__'): # pragma: no branch -- Python 3.8 and up + if sys.version_info >= (3, 8): # pragma: no branch -- dict reversal is Python 3.8 and up __reversed__ = MultiDict.__reversed__ __hash__ = None # type: ignore[assignment] __repr__ = MultiDict.__repr__ @@ -990,7 +966,7 @@ class OrderedMultiInfo(_MultiInfo[KT, VT], OrderedMultiDict[KT, VT]): __iter__ = OrderedMultiDict.__iter__ # type: ignore[assignment] __len__ = OrderedMultiDict.__len__ __contains__ = OrderedMultiDict.__contains__ - if hasattr(dict, '__reversed__'): # pragma: no branch -- Python 3.8 and up + if sys.version_info >= (3, 8): # pragma: no branch -- dict reversal is Python 3.8 and up __reversed__ = OrderedMultiDict.__reversed__ __eq__ = OrderedMultiDict.__eq__ __ne__ = _ne @@ -1060,11 +1036,6 @@ def _rebuild(info_cls: 'Type[Info]', value: 'Any') -> 'Any': #: Name of the per-class attribute :func:`_nested_types` caches its answer in. _NESTED_CACHE = '__nested_types__' -#: :pep:`604` union type, i.e. the origin of ``int | None``. It is absent below -#: Python 3.10, where only :data:`typing.Optional` spells a union, and is the -#: same object as :data:`typing.Union` from 3.14 on. -_UNION_TYPE = getattr(types, 'UnionType', None) - #: Type of :data:`None`, i.e. the member ``Optional`` adds to a union. _NONE_TYPE = type(None) @@ -1105,7 +1076,7 @@ def _as_info_type(hint: 'Any') -> 'Optional[Type[Info]]': # NOTE: a union of exactly one non-``None`` member is ``Optional`` of # that member, so it unwraps to it; anything else is not an ``Info``. args = [arg for arg in getattr(hint, '__args__', ()) if arg is not _NONE_TYPE] - is_union = origin is typing.Union or (_UNION_TYPE is not None and origin is _UNION_TYPE) + is_union = origin is typing.Union or (UnionType is not None and origin is UnionType) hint = args[0] if is_union and len(args) == 1 else None if not isinstance(hint, type): @@ -1191,7 +1162,7 @@ def _nested_types(cls: 'Type[Info]') -> 'dict[str, Type[Info]]': module = sys.modules.get(cls_.__module__) globalns = dict(vars(module)) if module is not None else {} localns = dict(vars(cls_)) - for (key, ann) in _own_annotations(cls_).items(): + for (key, ann) in get_annotations(cls_).items(): if key in seen: continue seen.add(key) diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index a600d6a5bb..3bc58837b9 100644 --- a/pcapkit/protocols/protocol.py +++ b/pcapkit/protocols/protocol.py @@ -26,6 +26,7 @@ import shutil import string import struct +import sys import textwrap import types import urllib.parse @@ -46,7 +47,7 @@ from pcapkit.protocols.schema.misc.raw import Raw as Schema_Raw from pcapkit.protocols.schema.schema import Schema from pcapkit.utilities.chardet import detect -from pcapkit.utilities.compat import cached_property +from pcapkit.utilities.compat import cached_property, get_annotations from pcapkit.utilities.decorators import beholder, seekset from pcapkit.utilities.exceptions import (BaseError, ProtocolError, ProtocolNotFound, ProtocolNotImplemented, RegistryError, StructError, @@ -93,8 +94,12 @@ def _make_index_calls(func: 'Callable[..., Any]') -> 'tuple[tuple[int, int, str] tree = ast.parse(textwrap.dedent(''.join(lines))) except (OSError, TypeError, SyntaxError): return () + # NOTE: ``end_lineno`` is new in Python 3.8, where ``ast.parse`` always sets + # it on a call; below that a call's last line is not known, only its first. return tuple( - (node.lineno + start - 1, getattr(node, 'end_lineno', node.lineno) + start - 1, node.args[0].id) + (node.lineno + start - 1, + (cast('int', node.end_lineno) if sys.version_info >= (3, 8) else node.lineno) + start - 1, + node.args[0].id) for node in ast.walk(tree) if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute) and node.func.attr == '_make_index' and node.args and isinstance(node.args[0], ast.Name) @@ -2168,17 +2173,9 @@ def _lookup_nested(data: 'Data') -> 'list[str]': The keys, in the order of ``data``. """ - # NOTE: :func:`inspect.get_annotations` is new in Python 3.10; below it - # a class's own annotations are in its ``__dict__``, where from 3.14 on - # they are evaluated lazily and may not be. - get_annotations = getattr(inspect, 'get_annotations', None) - declared = set() # type: set[str] for klass in type(data).mro(): - if get_annotations is None: - declared.update(klass.__dict__.get('__annotations__', {})) - else: - declared.update(get_annotations(klass)) + declared.update(get_annotations(klass)) return [key for key in data if key not in declared and isinstance(data[key], collections.abc.Mapping)] diff --git a/pcapkit/utilities/compat.py b/pcapkit/utilities/compat.py index 607673a14b..64c2b73cc6 100644 --- a/pcapkit/utilities/compat.py +++ b/pcapkit/utilities/compat.py @@ -8,7 +8,7 @@ __all__ = [ # functions - 'final', 'localcontext', 'show_flag_values', 'default_threading_excepthook', 'unquote', + 'final', 'localcontext', 'show_flag_values', 'default_threading_excepthook', 'unquote', 'get_annotations', # exceptions 'ModuleNotFoundError', @@ -21,7 +21,7 @@ 'pathlib', # typing - 'Mapping', 'Tuple', 'List', 'Dict', 'TypeAlias', + 'Mapping', 'Tuple', 'List', 'Dict', 'TypeAlias', 'UnionType', ] if sys.version_info < (3, 6): @@ -270,3 +270,28 @@ def show_flag_values(value: 'IntFlag') -> 'list[int]': from _thread import _excepthook as default_threading_excepthook else: from threading import __excepthook__ as default_threading_excepthook # novermin + +# A class's own annotations, never a base's (issue #1692). Below 3.10 +# (bpo-43901), ``cls.__annotations__`` of a class whose body declares none is +# its nearest base's, so the fallback reads the class's own ``__dict__``, where +# its body put them, and returns that mapping itself rather than a copy. +# :func:`inspect.get_annotations` reads the same place for a class on 3.10 to +# 3.13; from 3.14 a body without ``from __future__ import annotations`` leaves +# them out of ``__dict__``, so it is the only reader that sees them. Only what +# this library asks of it is backported: a class, without ``globals``, +# ``locals`` or ``eval_str``. +if sys.version_info < (3, 10): + def get_annotations(obj: 'type') -> 'dict[str, Any]': + """Return the annotations a class's own body declares, which may be empty.""" + return obj.__dict__.get('__annotations__', {}) +else: + from inspect import get_annotations # novermin + +# The :pep:`604` union type, i.e. the origin of ``int | None``, which is the +# same object as :data:`typing.Union` from 3.14. Below 3.10 only +# :data:`typing.Union` spells a union, so the name is :data:`None` there rather +# than a failed import, as ``default_threading_excepthook`` is below 3.8. +if sys.version_info < (3, 10): + UnionType = None +else: + from types import UnionType # novermin diff --git a/tests/corekit/test_infoclass.py b/tests/corekit/test_infoclass.py index ed86032224..ae7807dd40 100644 --- a/tests/corekit/test_infoclass.py +++ b/tests/corekit/test_infoclass.py @@ -186,14 +186,28 @@ class BareChild(Bare): info = cls({'a': 1}, b=2) self.assertEqual(list(info.items()), [('a', 1), ('b', 2)]) + def test_annotations_are_read_through_the_compat_shim(self) -> None: + """GitHub issue #1709: one version-gated reader, from :mod:`pcapkit.utilities.compat`.""" + from pcapkit.corekit import infoclass + from pcapkit.protocols import protocol + from pcapkit.utilities import compat + + self.assertIs(infoclass.get_annotations, compat.get_annotations) + self.assertIs(protocol.get_annotations, compat.get_annotations) + def test_the_pre_3_10_reader_gives_the_same_signatures(self) -> None: - # NOTE: ``_get_annotations`` set to ``None`` selects the reader Python - # 3.6-3.9 use, so every leg checks that branch. It is sound on 3.14 too - # because this module's ``from __future__ import annotations`` stores a - # class's own annotations in its ``__dict__`` there as well. + # NOTE: a stand-in for ``compat.get_annotations`` below 3.10 -- the + # class's own ``__dict__`` entry, as ``tests.utilities.test_compat`` pins + # the real fallback to be -- so every leg checks that branch. It is sound + # on 3.14 too because this module's ``from __future__ import + # annotations`` stores a class's own annotations in its ``__dict__`` + # there as well. from pcapkit.corekit import infoclass - with mock.patch.object(infoclass, '_get_annotations', None): + def pre_3_10_reader(obj: type) -> dict: + return obj.__dict__.get('__annotations__', {}) + + with mock.patch.object(infoclass, 'get_annotations', pre_3_10_reader): self.assertEqual(self._signatures(), self.EXPECTED) def test_from_dict_rebuilds_a_field_inherited_by_a_field_less_subclass(self) -> None: diff --git a/tests/project/test_stdlib_feature_probes_1709_unit.py b/tests/project/test_stdlib_feature_probes_1709_unit.py new file mode 100644 index 0000000000..7be9ae9f17 --- /dev/null +++ b/tests/project/test_stdlib_feature_probes_1709_unit.py @@ -0,0 +1,141 @@ +# -*- coding: utf-8 -*- +"""The package detects interpreter features by version, never by probing for them. + +The owner's ruling on GitHub pull request #1703, applied by issue #1709: where +:mod:`pcapkit` needs a name or a behaviour that only some supported interpreters +have, it asks :data:`sys.version_info`, in :mod:`pcapkit.utilities.compat` when +the answer is a name to re-export, rather than probing with :func:`getattr` or +:func:`hasattr`. Issue #1709 found six probes, each with a string literal naming +the attribute. Five ask a module or builtin class: + +* ``getattr(inspect, 'get_annotations', None)``, in + :file:`pcapkit/corekit/infoclass.py` and :file:`pcapkit/protocols/protocol.py`; +* ``getattr(types, 'UnionType', None)``, in :file:`pcapkit/corekit/infoclass.py`; +* ``hasattr(dict, '__reversed__')``, twice in :file:`pcapkit/corekit/infoclass.py`. + +The sixth asks an instance, so nothing about its receiver marks it: it is a +version probe because of the attribute it names. + +* ``getattr(node, 'end_lineno', node.lineno)``, in + :file:`pcapkit/protocols/protocol.py` -- :mod:`ast` nodes carry it from 3.8. + +This scans every module of the package for both shapes: a literal attribute +probe of a module the file imports by name or of a builtin class, and a literal +probe of any object for an attribute in :data:`VERSIONED`. A computed attribute +name, as in ``getattr(hashlib, digest)``, is a lookup rather than a question +about the interpreter, and is not matched. Run against the tree before issue +#1709, it reports exactly those six and the two :data:`EXEMPT` entries. + +This module is unit-tier: it reads source text and never imports the package. + +""" +from __future__ import annotations + +import ast +import builtins +import unittest +from typing import TYPE_CHECKING + +from tests._tiers import ROOT + +if TYPE_CHECKING: + from typing import Iterator + +#: Package directory that is scanned. +PACKAGE = ROOT / 'pcapkit' + +#: ``(path, call)`` of each match that is not a feature probe, with the reason. +EXEMPT = { + # A third-party package's own version string, read for a warning message. + ('pcapkit/foundation/engines/dpkt.py', "getattr(dpkt, '__version__', '(unknown version)')"), + # Every interpreter has the attribute; it is None when there is no console. + ('pcapkit/vendor/default.py', "getattr(sys, 'stdin', None)"), +} + +#: Attributes that only some supported interpreters define on their objects, and +#: the version that added them. A literal probe for one is a version probe on any +#: receiver, which is why these are matched by name alone. +VERSIONED = { + 'end_lineno': (3, 8), # ast node end positions + 'end_col_offset': (3, 8), +} + + +def probes(source: str) -> 'Iterator[tuple[int, str]]': + """Yield ``(lineno, call)`` for each attribute probe that asks about the interpreter. + + That is a literal probe of a module or builtin class, or a literal probe of + any object for an attribute in :data:`VERSIONED`. + + Imports are collected from the whole file, function bodies included, since a + probe of a module imported inside a function is a probe all the same. + + Args: + source: Module source text. + + Yields: + The line number and the source text of each matching call. + + """ + tree = ast.parse(source) + modules = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + modules.add(alias.asname or alias.name.split('.')[0]) + + for node in ast.walk(tree): + if not (isinstance(node, ast.Call) and isinstance(node.func, ast.Name) + and node.func.id in ('getattr', 'hasattr') and len(node.args) >= 2): + continue + target, attr = node.args[:2] + if not (isinstance(attr, ast.Constant) and isinstance(attr.value, str)): + continue + if attr.value in VERSIONED or (isinstance(target, ast.Name) and ( + target.id in modules or isinstance(getattr(builtins, target.id, None), type))): + yield node.lineno, ast.get_source_segment(source, node) + + +class StdlibFeatureProbeTests(unittest.TestCase): + def test_no_module_detects_a_feature_by_probing_for_it(self) -> None: + found = [] + for path in sorted(PACKAGE.rglob('*.py')): + relative = path.relative_to(ROOT).as_posix() + for lineno, call in probes(path.read_text(encoding='utf-8')): + if (relative, call) not in EXEMPT: + found.append(f'{relative}:{lineno}: {call}') + self.assertEqual( + found, [], + 'gate these on sys.version_info instead, re-exported from ' + 'pcapkit/utilities/compat.py where the result is a name (#1703, #1709)') + + def test_every_exemption_still_matches(self) -> None: + """An exemption whose call has gone is a stale entry, not a pass.""" + for relative, call in sorted(EXEMPT): + with self.subTest(path=relative): + source = (ROOT / relative).read_text(encoding='utf-8') + self.assertIn(call, [found for (_, found) in probes(source)]) + + def test_the_issue_1709_shapes_are_matched(self) -> None: + """The scan sees each shape issue #1709 removed, so a pass is not vacuous.""" + source = ( + 'import hashlib\n' + 'import inspect\n' + 'import types\n' + "get_annotations = getattr(inspect, 'get_annotations', None)\n" + "union = getattr(types, 'UnionType', None)\n" + 'class Info(dict):\n' + " if hasattr(dict, '__reversed__'):\n" + ' pass\n' + 'def digest(name):\n' + ' return getattr(hashlib, name)\n' + 'def last_line(node):\n' + " return getattr(node, 'end_lineno', node.lineno)\n" + 'def payload(frame):\n' + " return getattr(frame, 'padding', b'')\n" + ) + self.assertEqual(sorted(lineno for (lineno, _) in probes(source)), [4, 5, 7, 12]) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/protocols/test_protocol_make_unit.py b/tests/protocols/test_protocol_make_unit.py index 7c6159d554..0997b10901 100644 --- a/tests/protocols/test_protocol_make_unit.py +++ b/tests/protocols/test_protocol_make_unit.py @@ -23,6 +23,7 @@ """ import importlib +import sys import unittest import unittest.mock from typing import TYPE_CHECKING @@ -233,6 +234,18 @@ def test_source_unavailable_locates_nothing(self) -> None: # a builtin has no Python source self.assertEqual(module._make_index_calls(len), ()) + def test_call_spans_to_its_last_line(self) -> None: + """A call split over lines ends on its last one, from Python 3.8 (#1709).""" + module = importlib.import_module('pcapkit.protocols.protocol') + + def make_split(self: 'Any', a: 'Any', a_namespace: 'Any' = None) -> 'Any': + return self._make_index(a, + namespace=a_namespace) + + first = make_split.__code__.co_firstlineno + 1 + last = first + 1 if sys.version_info >= (3, 8) else first + self.assertEqual(module._make_index_calls(make_split), ((first, last, 'a'),)) + if __name__ == '__main__': unittest.main() diff --git a/tests/utilities/test_compat.py b/tests/utilities/test_compat.py index 4281711e5d..7aeef0550c 100644 --- a/tests/utilities/test_compat.py +++ b/tests/utilities/test_compat.py @@ -54,9 +54,11 @@ import decimal import enum import importlib +import inspect import subprocess import sys import textwrap +import types import unittest import urllib.parse from typing import Callable @@ -345,6 +347,41 @@ def test_unquote_decodes_bytes_as_python39_does(self) -> None: self.assertIsNot(self.compat.unquote, urllib.parse.unquote) self.assert_unquotes_as_python39(self.compat.unquote) + def test_get_annotations_and_union_type_are_the_stdlib_ones_from_3_10(self) -> None: + """GitHub issue #1709: both are chosen by version, not by probing for them.""" + if sys.version_info >= (3, 10): + self.assertIs(self.compat.get_annotations, inspect.get_annotations) + self.assertIs(self.compat.UnionType, types.UnionType) + else: + self.assertEqual(self.compat.get_annotations.__module__, self.compat.__name__) + self.assertIsNone(self.compat.UnionType) + self.assert_reads_own_annotations(self.compat.get_annotations) + + def assert_reads_own_annotations(self, get_annotations: 'Callable[[type], dict]') -> None: + """``get_annotations`` gives each class only what its own body declares. + + ``Bare`` declares nothing, so below 3.10 its ``__annotations__`` is + ``Base``'s (bpo-43901, issue #1692); the answer is still empty. + + """ + class Base: + a: int + + class Bare(Base): + pass + + class Left(Base): + b: int + + class Diamond(Bare, Left): + d: int + + self.assertEqual(get_annotations(Base), {'a': 'int'}) + self.assertEqual(get_annotations(Bare), {}) + self.assertEqual(get_annotations(Left), {'b': 'int'}) + self.assertEqual(get_annotations(Diamond), {'d': 'int'}) + self.assertEqual(get_annotations(object), {}) + def test_show_flag_values_returns_individual_bits(self) -> None: class DemoFlag(enum.IntFlag): READ = 1 @@ -440,6 +477,17 @@ def test_python35_fallback_implementations(self) -> None: self.assertTrue(issubclass(compat.ModuleNotFoundError, ImportError)) self.assertIsNone(compat.default_threading_excepthook) + self.assertIsNone(compat.UnionType) + + # The reader infoclass and protocol used below 3.10 (#1692, #1709): the + # class's own ``__dict__`` entry, returned as it is rather than copied. + self.assertEqual(compat.get_annotations.__module__, compat.__name__) + self.assert_reads_own_annotations(compat.get_annotations) + + class Annotated: + a: int + + self.assertIs(compat.get_annotations(Annotated), Annotated.__dict__['__annotations__']) class CollectionLike: def __len__(self): return 0