fix(corekit): let _py_abc cache its ABC lookups on sentinel types (#1960)

docs/source/contributing/conventions/sentinel-convention.rst

@@ -162,7 +162,10 @@ Where the sentinels differ, the differences are **needs, not inconsistencies**:
    name, as ``None.x = 1`` does. ``__slots__`` is empty, so there is no ``__dict__`` to
    reach round them either. The metaclass does the same for the type, raising
    :exc:`TypeError` as ``type(None).x = 1`` does, so neither ``_name`` nor the cached
-   instance can be swapped out. It also refuses to let a concrete sentinel type be
+   instance can be swapped out. It lets through only ``_abc_negative_cache`` and
+   ``_abc_negative_cache_version``, which ``_py_abc``, the fallback ``abc`` uses
+   without the C ``_abc`` module, rebinds during an :func:`isinstance` check
+   (:issue:`1960`). It also refuses to let a concrete sentinel type be
    subclassed, by a ``class`` statement or by ``register``, as ``type(None)`` cannot be;
    :class:`~pcapkit.corekit.sentinels.Sentinel` itself stays subclassable, being abstract.
    The owner asked for this in review of the change for :issue:`1842`: a sentinel's repr

pcapkit/corekit/sentinels.py

@@ -57,6 +57,17 @@
 #: every attribute write.
 _FROZEN = '_frozen'
 
+#: frozenset[str]: The cache names ``_py_abc``, the pure-Python fallback
+#: ``abc`` uses where the C ``_abc`` module is missing, rebinds on a class
+#: while an :func:`isinstance` or :func:`issubclass` check runs against it
+#: (:issue:`1960`). A frozen sentinel type still lets these through, and only
+#: where its own :attr:`~object.__dict__` already holds the name, which only
+#: that fallback's ``ABCMeta.__new__`` puts there. Each can only make
+#: a check answer "no" or look again, never "yes". ``_abc_registry`` and
+#: ``_abc_cache`` are left out, being set only while the type is built, and
+#: replacing either could make a check answer "yes".
+_PY_ABC_CACHE = frozenset({'_abc_negative_cache', '_abc_negative_cache_version'})
+
 
 def _not_a_base(cls: 'type') -> 'TypeError':
     """The error deriving from ``cls`` raises, worded as for ``type(None)``."""
@@ -84,7 +95,10 @@ class SentinelMeta(abc.ABCMeta):
     * Every sentinel type is immutable once its ``class`` statement has run.
       Setting or deleting any attribute on it raises :exc:`TypeError`, as
       ``type(None).x = 1`` does, so ``_name``, ``_instance`` and the methods
-      every sentinel inherits stay as they were defined.
+      every sentinel inherits stay as they were defined. The one exception is
+      ``_abc_negative_cache`` and ``_abc_negative_cache_version``, which
+      ``_py_abc``, the fallback ``abc`` uses without the C ``_abc``
+      module, rebinds during an :func:`isinstance` check (:issue:`1960`).
 
     A concrete sentinel type also gets ``__final__`` set to :obj:`True` here.
     :func:`~pcapkit.utilities.compat.final` would record the same, but a class
@@ -139,10 +153,12 @@ def __setattr__(cls, name: 'str', value: 'Any') -> 'None':
             value: Attribute value.
 
         Raises:
-            TypeError: Once the ``class`` statement of ``cls`` has run.
+            TypeError: Once the ``class`` statement of ``cls`` has run, unless
+                ``name`` is one of the ``_py_abc`` caches in ``_PY_ABC_CACHE``
+                that ``cls`` already holds.
 
         """
-        if cls.__dict__.get(_FROZEN):
+        if cls.__dict__.get(_FROZEN) and not (name in _PY_ABC_CACHE and name in cls.__dict__):
             raise TypeError(f'cannot set {name!r} attribute of immutable type {cls.__name__!r}')
         super().__setattr__(name, value)
 

tests/corekit/test_sentinel_py_abc_1960_unit.py

@@ -0,0 +1,198 @@
+# -*- coding: utf-8 -*-
+"""GitHub issue #1960: a sentinel type answers :func:`isinstance` under ``_py_abc``.
+
+Where the C ``_abc`` module is missing, as on GraalPy, :mod:`abc` falls back to
+``_py_abc``, whose ``__subclasscheck__`` rebinds ``_abc_negative_cache`` and
+``_abc_negative_cache_version`` on the class once any ABC has registered a
+virtual subclass since the class was built. :class:`~pcapkit.corekit.sentinels.SentinelMeta`
+refused every write to a built sentinel type, so ``isinstance(1, NullType)``
+raised :exc:`TypeError` there. It now lets those two names through, and no
+other.
+
+:class:`PyABCFallbackTests` runs the checks in a child interpreter that really
+takes the fallback. It starts with ``-S``, so that no module but :mod:`io` has
+imported :mod:`abc` yet, drops both from :data:`sys.modules`, blocks ``_abc``
+and imports them again. The child reports whether :class:`abc.ABCMeta` is
+``_py_abc``'s and whether ``_collections_abc`` was already loaded, rather than
+trusting the block. :class:`CModuleTests` pins that, under the C module, the
+names let through for ``_py_abc`` are still refused.
+
+"""
+
+from __future__ import annotations
+
+import _py_abc
+import abc
+import importlib
+import json
+import os
+import subprocess  # nosec B404
+import sys
+import textwrap
+import unittest
+
+from tests._support import reimport_once_per_class
+
+#: Repository root, put first on the child interpreter's path so that it
+#: imports this tree rather than whatever :mod:`pcapkit` is installed.
+REPO_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+
+#: Every sentinel type, the abstract base first, and the module-level name of
+#: each concrete one's instance.
+TYPES = (
+    ('Sentinel', None),
+    ('NullType', 'NULL'),
+    ('NoValueType', 'NO_VALUE'),
+    ('NoDefaultType', 'NO_DEFAULT'),
+    ('AbsentType', 'ABSENT'),
+)
+
+#: Run in the child. It prints one JSON object: whether the fallback was taken,
+#: and for each sentinel type the outcome of every probe, as the ``repr`` of
+#: what it returned or the name of the exception it raised.
+CHILD = textwrap.dedent('''
+    import sys
+
+    preloaded = sorted(name for name in ('abc', 'io', '_collections_abc') if name in sys.modules)
+    for name in ('abc', 'io'):
+        sys.modules.pop(name, None)
+    sys.modules['_abc'] = None
+
+    import abc
+    import io  # noqa: F401  # rebuilt on the fallback before anything uses it
+    import json
+    import _py_abc
+
+    import pcapkit
+    from pcapkit.corekit import sentinels
+
+    class Probe(abc.ABC):
+        """An ABC whose registrations invalidate every ``_py_abc`` negative cache."""
+
+    def bump():
+        """Register a fresh virtual subclass, so the next check rebinds the cache."""
+        Probe.register(type('Virtual', (), {}))
+
+    def outcome(probe):
+        try:
+            return repr(probe())
+        except Exception as error:
+            return type(error).__name__
+
+    report = {
+        'preloaded': preloaded,
+        'fallback': abc.ABCMeta is _py_abc.ABCMeta,
+        'metaclass_base': sentinels.SentinelMeta.__mro__[1] is _py_abc.ABCMeta,
+        'file': pcapkit.__file__,
+        'types': {},
+    }
+    for name, instance_name in json.loads(sys.argv[1]):
+        type_ = getattr(sentinels, name)
+        instance = None if instance_name is None else getattr(sentinels, instance_name)
+        probes = report['types'][name] = {}
+        bump()
+        probes['isinstance(1)'] = outcome(lambda: isinstance(1, type_))
+        bump()
+        probes['issubclass(int)'] = outcome(lambda: issubclass(int, type_))
+        bump()
+        probes['isinstance(instance)'] = outcome(lambda: isinstance(instance, type_))
+        probes['isinstance(instance, Sentinel)'] = outcome(
+            lambda: isinstance(instance, sentinels.Sentinel))
+        probes['register(int)'] = outcome(lambda: type_.register(int))
+        for attr in ('x', '_name', '_instance', '_abc_cache', '_abc_registry'):
+            probes['set ' + attr] = outcome(lambda: setattr(type_, attr, None))
+        for attr in ('x', '_abc_negative_cache', '_abc_negative_cache_version'):
+            probes['delete ' + attr] = outcome(lambda: delattr(type_, attr))
+        if instance is not None:
+            probes['_name'] = outcome(lambda: type_.__dict__['_name'])
+    print(json.dumps(report))
+''')
+
+
+def _expected(instance_name: 'str | None') -> 'dict[str, str]':
+    """The outcome of every probe :data:`CHILD` runs against a sentinel type.
+
+    Args:
+        instance_name: Module-level name of the type's one instance, or
+            :obj:`None` for the abstract base.
+
+    """
+    concrete = instance_name is not None
+    probes = {
+        'isinstance(1)': 'False',
+        'issubclass(int)': 'False',
+        'isinstance(instance)': repr(concrete),
+        'isinstance(instance, Sentinel)': repr(concrete),
+        # Only a concrete type refuses; the abstract base registers ``int``,
+        # after every check against it has run.
+        'register(int)': 'TypeError' if concrete else repr(int),
+    }
+    if concrete:
+        probes['_name'] = repr(instance_name)
+    for attr in ('x', '_name', '_instance', '_abc_cache', '_abc_registry'):
+        probes['set ' + attr] = 'TypeError'
+    for attr in ('x', '_abc_negative_cache', '_abc_negative_cache_version'):
+        probes['delete ' + attr] = 'TypeError'
+    return probes
+
+
+class PyABCFallbackTests(unittest.TestCase):
+    """Every sentinel type behaves under ``_py_abc`` as it does under ``_abc``."""
+
+    report = {}  # type: dict
+
+    @classmethod
+    def setUpClass(cls) -> 'None':
+        path = [REPO_ROOT] + [entry for entry in sys.path if entry]
+        env = dict(os.environ, PCAPKIT_DEVMODE='0', PYTHONSAFEPATH='1',
+                   PYTHONPATH=os.pathsep.join(path))
+        proc = subprocess.run([sys.executable, '-S', '-c', CHILD, json.dumps(TYPES)],  # nosec B603
+                              cwd=REPO_ROOT, env=env, capture_output=True, text=True,
+                              timeout=120, check=False)
+        if proc.returncode != 0:
+            raise AssertionError(f'child interpreter failed:\n{proc.stderr}')
+        cls.report = json.loads(proc.stdout.strip().splitlines()[-1])
+
+    def test_the_child_really_takes_the_fallback(self) -> 'None':
+        """:mod:`abc` is ``_py_abc``'s, and nothing else had imported it at startup."""
+        self.assertTrue(self.report['fallback'], 'abc.ABCMeta is not _py_abc.ABCMeta')
+        self.assertTrue(self.report['metaclass_base'], 'SentinelMeta is not built on _py_abc')
+        self.assertNotIn('_collections_abc', self.report['preloaded'])
+        self.assertTrue(self.report['file'].startswith(REPO_ROOT), self.report['file'])
+
+    def test_every_probe_behaves_as_under_the_c_module(self) -> 'None':
+        """Each check answers as it should, and every other write is still refused."""
+        for name, instance_name in TYPES:
+            expected = _expected(instance_name)
+            actual = self.report['types'][name]
+            with self.subTest(type=name):
+                self.assertEqual(sorted(actual), sorted(expected))
+            for probe, outcome in expected.items():
+                with self.subTest(type=name, probe=probe):
+                    self.assertEqual(actual[probe], outcome)
+
+
+class CModuleTests(unittest.TestCase):
+    """Under the C ``_abc`` module, the names let through for ``_py_abc`` stay refused."""
+
+    def setUp(self) -> 'None':
+        reimport_once_per_class(self)
+
+    def test_the_py_abc_cache_names_are_refused_where_the_type_lacks_them(self) -> 'None':
+        """Neither ``_abc_negative_cache`` nor its version can be set on a type that lacks it."""
+        sentinels = importlib.import_module('pcapkit.corekit.sentinels')
+        if abc.ABCMeta is _py_abc.ABCMeta:
+            self.skipTest('abc is the _py_abc fallback here; PyABCFallbackTests covers it')
+        for name, _ in TYPES:
+            type_ = getattr(sentinels, name)
+            for attr in ('_abc_negative_cache', '_abc_negative_cache_version'):
+                with self.subTest(type=name, attr=attr):
+                    self.assertNotIn(attr, type_.__dict__)
+                    message = f'{attr!r} attribute of immutable type {name!r}'
+                    with self.assertRaisesRegex(TypeError, message):
+                        setattr(type_, attr, None)
+                    self.assertNotIn(attr, type_.__dict__)
+
+
+if __name__ == '__main__':
+    unittest.main()