fix(protocols): decide auto-registration's abstractness without the interpreter's abstract flag (#2784)

docs/source/pcapkit/utilities/index.rst

@@ -9,8 +9,9 @@ built with: user-refined exceptions and warnings, logging, the tunable limits
 of :mod:`~pcapkit.utilities.config`, a compatibility layer, timestamp
 conversion, pickle reductions, the
 :mod:`context variables <pcapkit.utilities.contextvars>` its modules share,
-:mod:`lazy imports <pcapkit.utilities.importlib>` for a package's exports, and
-wrappers around third-party libraries. It imports nothing from :mod:`pcapkit`
+:mod:`lazy imports <pcapkit.utilities.importlib>` for a package's exports, an
+:mod:`abstract-class check <pcapkit.utilities.inspect>` that reads no interpreter
+flag, and wrappers around third-party libraries. It imports nothing from :mod:`pcapkit`
 outside itself; see :ref:`utilities-or-corekit`.
 
 .. toctree::
@@ -25,6 +26,7 @@ outside itself; see :ref:`utilities-or-corekit`.
    pickle
    contextvars
    importlib
+   inspect
 
 Version Compatibility
 =====================

docs/source/pcapkit/utilities/inspect.rst

@@ -0,0 +1,17 @@
+Abstract Classes
+================
+
+.. module:: pcapkit.utilities.inspect
+
+:mod:`pcapkit.utilities.inspect` holds :func:`isabstract`, which tells an abstract
+class from a concrete one without the interpreter's abstract flag, so the answer is the
+same on every interpreter, during :meth:`~object.__init_subclass__` and after the class
+is made (:issue:`2784`). :meth:`Protocol.__init_subclass__
+<pcapkit.protocols.protocol.Protocol.__init_subclass__>` decides with it whether a
+class names itself in the by-name registry. It is a helper that behaves the same
+everywhere rather than a version gate, so it has a module of its own rather than a
+place in :mod:`~pcapkit.utilities.compat`; see :ref:`utilities-or-corekit`.
+
+.. versionadded:: 1.6.0
+
+.. autofunction:: pcapkit.utilities.inspect.isabstract

pcapkit/protocols/protocol.py

@@ -60,6 +60,7 @@
 from pcapkit.utilities.exceptions import (BaseError, ProtocolError, ProtocolNotFound,
                                           ProtocolNotImplemented, RegistryError, StructError,
                                           UnsupportedCall, stacklevel)
+from pcapkit.utilities.inspect import isabstract
 from pcapkit.utilities.warnings import (ProtocolWarning, RegistryWarning, UnknownFieldWarning,
                                         hold_warnings, warn)
 
@@ -3286,11 +3287,11 @@ def __init_subclass__(cls, /, schema: 'Optional[Type[_ST]]' = None,
         under its upper-cased class name, once
         :meth:`ProtocolBase.__init_subclass__` has accepted its ``code``,
         whereas ``code``'s next-layer dispatch registration is opt-in. An
-        abstract subclass, as :func:`inspect.isabstract` reports it, is not
-        named, and nor is a class deriving :class:`ProtocolBase` without this
-        class, as every built-in protocol does. A name already holding another
-        entry keeps it, and the clash is reported as a
-        :exc:`~pcapkit.utilities.warnings.RegistryWarning`; only
+        abstract subclass, as :func:`~pcapkit.utilities.inspect.isabstract`
+        reports it, is not named, and nor is a class deriving
+        :class:`ProtocolBase` without this class, as every built-in protocol
+        does. A name already holding another entry keeps it, and the clash is
+        reported as a :exc:`~pcapkit.utilities.warnings.RegistryWarning`; only
         :meth:`ProtocolBase.register_protocol
         <pcapkit.protocols.protocol.ProtocolBase.register_protocol>` replaces an
         entry. A class meeting the :class:`~pcapkit.corekit.module.ModuleDescriptor`
@@ -3314,7 +3315,9 @@ def __init_subclass__(cls, /, schema: 'Optional[Type[_ST]]' = None,
         # once its ``code`` is accepted, so a refused one names nothing, and
         # keeps off a name already held. A class meeting the descriptor naming
         # its own module and name leaves it, to be resolved on lookup.
-        if not inspect.isabstract(cls):
+        # #2784: not inspect.isabstract, which reads __flags__: GraalPy freezes them
+        # at that read, so the abstract flag ABCMeta sets next would never show.
+        if not isabstract(cls):
             name = cls.__name__.upper()
             incumbent = ProtocolBase.__registry__.setdefault(name, cls)
             if incumbent is not cls and not (isinstance(incumbent, ModuleDescriptor)

pcapkit/utilities/__init__.py

@@ -11,7 +11,8 @@
 :mod:`~pcapkit.utilities.config`, a compatibility layer,
 timestamp conversion, pickle reductions, the context
 variables its modules share, lazy imports for a package's
-exports, and wrappers around third-party libraries. It imports nothing from :mod:`pcapkit`
+exports, an abstract-class check that reads no interpreter
+flag, and wrappers around third-party libraries. It imports nothing from :mod:`pcapkit`
 outside itself (:issue:`1712`).
 
 .. versionremoved:: 1.5.0

pcapkit/utilities/inspect.py

@@ -0,0 +1,55 @@
+# -*- coding: utf-8 -*-
+"""Abstract Classes
+======================
+
+.. module:: pcapkit.utilities.inspect
+
+:mod:`pcapkit.utilities.inspect` holds :func:`isabstract`, which tells an abstract
+class from a concrete one without the interpreter's abstract flag, so the answer is
+the same on every interpreter, during :meth:`~object.__init_subclass__` and after the
+class is made (:issue:`2784`).
+
+.. versionadded:: 1.6.0
+
+"""
+import abc
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from typing import Any
+
+__all__ = ['isabstract']
+
+
+# GraalPy freezes a class's ``__flags__`` at the first read, so setting
+# ``__abstractmethods__`` afterwards never raises the abstract flag there (#2784,
+# measured on GraalPy 25.4.4). :func:`inspect.isabstract` reads the flag first, so
+# called from ``__init_subclass__``, before :class:`abc.ABCMeta` has set
+# ``__abstractmethods__``, it leaves an abstract class reported concrete from then
+# on. This reads no flag; it derives the answer as :class:`abc.ABCMeta` does.
+def isabstract(cls: 'Any') -> 'bool':
+    """Whether ``cls`` is an abstract class, without its ``__flags__``.
+
+    A class made by :class:`abc.ABCMeta` is abstract when its own namespace holds
+    an abstract method, or when a name one of its bases lists in
+    ``__abstractmethods__`` still resolves to an abstract method on it. That is
+    how :class:`abc.ABCMeta` computes ``__abstractmethods__``, so the answer
+    agrees with it after the class is made, and is already the same while
+    :meth:`~object.__init_subclass__` runs, before :class:`abc.ABCMeta` has set
+    it. :func:`inspect.isabstract` takes the same route there, but reads the
+    interpreter's abstract flag first.
+
+    Args:
+        cls: Object to check. Anything other than a class made by
+            :class:`abc.ABCMeta` is not abstract.
+
+    Returns:
+        Whether ``cls`` is abstract.
+
+    """
+    if not isinstance(cls, abc.ABCMeta):
+        return False
+    if any(getattr(value, '__isabstractmethod__', False) for value in vars(cls).values()):
+        return True
+    return any(getattr(getattr(cls, name, None), '__isabstractmethod__', False)
+               for base in cls.__bases__ for name in getattr(base, '__abstractmethods__', ()))

tests/protocols/test_protocol_registry_2763_unit.py

@@ -11,10 +11,11 @@
   and class, and no entry names anything else.
 * :class:`AutoRegistrationTests` pins what defining a subclass does: a concrete
   :class:`~pcapkit.protocols.protocol.Protocol` subclass names itself, an abstract
-  one does not, nor does a class deriving ``ProtocolBase`` alone, as every built-in
-  does; a clash keeps the entry and warns, a class meeting its own descriptor is
-  silent, and only ``register_protocol`` replaces an entry, putting a built-in's
-  descriptor back as shipped.
+  one does not, and keeps its abstract flag where ``__flags__`` freezes at the first
+  read, as on GraalPy (#2784), nor does a class deriving ``ProtocolBase`` alone, as
+  every built-in does; a clash keeps the entry and warns, a class meeting its own
+  descriptor is silent, and only ``register_protocol`` replaces an entry, putting a
+  built-in's descriptor back as shipped.
 * :class:`GetterTests` pins the lookup and the removal of the module dict.
 * :class:`RegistryViewTests` pins ``AnyProtocol.registry``, the read-only view of the
   entries as stored.
@@ -190,14 +191,28 @@ def test_an_abstract_subclass_is_skipped(self) -> 'None':
         from pcapkit.protocols.protocol import Protocol
 
         bare = type('W2763Bare', (Protocol,), {'__module__': __name__})
-        self.assertTrue(inspect.isabstract(bare))
+        self.assertTrue(bare.__abstractmethods__)
         self.assertNotIn('W2763BARE', self.registry)
 
         # A concrete subclass of an abstract one is still named.
         member = _concrete('W2763Member', base=bare)
-        self.assertFalse(inspect.isabstract(member))
+        self.assertFalse(member.__abstractmethods__)
         self.assertIs(self.registry['W2763MEMBER'], member)
 
+    def test_an_abstract_subclass_keeps_its_abstract_flag(self) -> 'None':
+        """#2784: the hook reads no ``__flags__``, which GraalPy freezes at the first read."""
+        from pcapkit.protocols.protocol import Protocol
+        from tests.utilities.test_inspect_isabstract_2784_unit import frozen_flags
+
+        bare = frozen_flags(type(Protocol))('W2784Bare', (Protocol,), {'__module__': __name__})
+        self.assertTrue(bare.__abstractmethods__)
+        self.assertNotIn('W2784BARE', self.registry)
+        self.assertTrue(inspect.isabstract(bare))
+
+        member = _concrete('W2784Member', base=bare)
+        self.assertFalse(member.__abstractmethods__)
+        self.assertIs(self.registry['W2784MEMBER'], member)
+
     def test_a_clash_keeps_the_entry_and_warns(self) -> 'None':
         shipped = self.registry['IPV4']
         builtin = self.ProtocolBase.get_protocol('IPV4')
@@ -231,7 +246,7 @@ def test_a_class_meeting_its_own_descriptor_is_silent(self) -> 'None':
         with warnings.catch_warnings(record=True) as caught:
             warnings.simplefilter('always')
             redefined = _concrete('NGAP', module=NGAP.__module__)
-        self.assertFalse(inspect.isabstract(redefined))
+        self.assertFalse(redefined.__abstractmethods__)
         self.assertEqual(_registry_warnings(caught), [])
         self.assertIs(self.registry['NGAP'], shipped)
         self.assertIs(self.ProtocolBase.get_protocol('NGAP'), NGAP)

tests/utilities/test_inspect_isabstract_2784_unit.py

@@ -0,0 +1,143 @@
+# -*- coding: utf-8 -*-
+"""#2784: :func:`pcapkit.utilities.inspect.isabstract` reads no abstract flag.
+
+GraalPy freezes a class's ``__flags__`` at the first read, so a class whose flags are
+read before :class:`abc.ABCMeta` sets ``__abstractmethods__`` never shows the abstract
+flag there. :func:`inspect.isabstract` reads them first, so called from
+``__init_subclass__`` it leaves an abstract class reported concrete from then on.
+:func:`isabstract` derives the answer as :class:`abc.ABCMeta` does instead.
+
+GraalPy is not installed here, so :func:`frozen_flags` stands in for it: a metaclass
+whose ``__flags__`` keeps the value of its first read.
+:mod:`tests.protocols.test_protocol_registry_2763_unit` uses it for the
+auto-registration hook. The oracle is ``__abstractmethods__``, which
+:class:`abc.ABCMeta` sets on every interpreter.
+
+"""
+
+import abc
+import inspect
+import unittest
+import weakref
+from typing import TYPE_CHECKING
+
+from tests._support import reimport_once_per_class
+
+if TYPE_CHECKING:
+    from typing import Any
+
+#: Test tier (#1958): exercises ABC machinery, which differs by interpreter.
+__tier__ = 'ALL'
+
+
+def frozen_flags(meta: 'type') -> 'Any':
+    """A subclass of the metaclass ``meta`` whose ``__flags__`` keeps the value of its first read.
+
+    That is what GraalPy 25.4.4 does with every class (#2784).
+
+    """
+    read = type.__dict__['__flags__'].__get__
+    frozen = weakref.WeakKeyDictionary()  # type: weakref.WeakKeyDictionary[type, int]
+
+    class FrozenFlags(meta):  # type: ignore[misc,valid-type]
+        @property
+        def __flags__(cls) -> 'int':  # type: ignore[override]
+            if cls not in frozen:
+                frozen[cls] = read(cls)
+            return frozen[cls]
+
+    return FrozenFlags
+
+
+class IsAbstractTests(unittest.TestCase):
+    """:func:`~pcapkit.utilities.inspect.isabstract` against ``__abstractmethods__``."""
+
+    def setUp(self) -> 'None':
+        reimport_once_per_class(self)
+        from pcapkit.utilities.inspect import isabstract
+
+        self.isabstract = isabstract
+
+    def test_the_answer_is_the_same_during_init_subclass_and_after(self) -> 'None':
+        isabstract, during = self.isabstract, {}  # type: Any, dict[str, bool]
+
+        class Base(abc.ABC):
+            def __init_subclass__(cls, **kwargs: 'Any') -> 'None':
+                super().__init_subclass__(**kwargs)
+                during[cls.__name__] = isabstract(cls)
+
+            @abc.abstractmethod
+            def f(self) -> 'None': ...
+
+            @abc.abstractmethod
+            def g(self) -> 'None': ...
+
+        class Inherits(Base):
+            pass
+
+        class OverridesOne(Base):
+            def f(self) -> 'None': ...
+
+        class OverridesBoth(OverridesOne):
+            g = None  # type: ignore[assignment]
+
+        class DeclaresItsOwn(OverridesBoth):
+            @property
+            @abc.abstractmethod
+            def h(self) -> 'None': ...
+
+        for klass, expected in ((Inherits, True), (OverridesOne, True),
+                                (OverridesBoth, False), (DeclaresItsOwn, True)):
+            with self.subTest(cls=klass.__name__):
+                self.assertEqual(bool(klass.__abstractmethods__), expected)
+                self.assertIs(during[klass.__name__], expected)
+                self.assertIs(isabstract(klass), expected)
+
+    def test_a_frozen_flag_leaves_the_answer(self) -> 'None':
+        """Where the flags are read during ``__init_subclass__``, as GraalPy freezes them."""
+        class Base(metaclass=frozen_flags(abc.ABCMeta)):
+            def __init_subclass__(cls, **kwargs: 'Any') -> 'None':
+                super().__init_subclass__(**kwargs)
+                cls.__flags__  # pylint: disable=pointless-statement
+
+            @abc.abstractmethod
+            def f(self) -> 'None': ...
+
+        class Inherits(Base):
+            pass
+
+        self.assertTrue(Inherits.__abstractmethods__)
+        self.assertFalse(inspect.isabstract(Inherits))  # the stand-in misreports it, as GraalPy does
+        self.assertTrue(self.isabstract(Inherits))
+
+    def test_no_flag_is_read(self) -> 'None':
+        class Unreadable(abc.ABCMeta):
+            @property
+            def __flags__(cls) -> 'int':  # type: ignore[override]
+                raise AssertionError('__flags__ was read')
+
+        class Base(metaclass=Unreadable):
+            @abc.abstractmethod
+            def f(self) -> 'None': ...
+
+        class Concrete(Base):
+            def f(self) -> 'None': ...
+
+        self.assertTrue(self.isabstract(Base))
+        self.assertFalse(self.isabstract(Concrete))
+
+    def test_a_class_with_nothing_abstract_or_not_from_abcmeta_is_concrete(self) -> 'None':
+        class Plain:
+            @abc.abstractmethod
+            def f(self) -> 'None': ...
+
+        class Concrete(abc.ABC):
+            pass
+
+        for value in (Plain, Concrete, Concrete(), abc.ABC, abc.abstractmethod, None):
+            with self.subTest(value=value):
+                self.assertFalse(self.isabstract(value))
+
+
+if __name__ == '__main__':
+    unittest.main()