refactor(corekit,utilities): detect get_annotations, UnionType and dict reversal by version, not by probe (#1709)

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

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)

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)]
 

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

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:

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()