diff --git a/docs/source/pcapkit/utilities/index.rst b/docs/source/pcapkit/utilities/index.rst index e99b09e5b5..6195bce664 100644 --- a/docs/source/pcapkit/utilities/index.rst +++ b/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 ` its modules share, -:mod:`lazy imports ` for a package's exports, and -wrappers around third-party libraries. It imports nothing from :mod:`pcapkit` +:mod:`lazy imports ` for a package's exports, an +:mod:`abstract-class check ` 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 ===================== diff --git a/docs/source/pcapkit/utilities/inspect.rst b/docs/source/pcapkit/utilities/inspect.rst new file mode 100644 index 0000000000..f2f38593c7 --- /dev/null +++ b/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__ +` 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 diff --git a/pcapkit/protocols/protocol.py b/pcapkit/protocols/protocol.py index 836ce7f7f5..4119275398 100644 --- a/pcapkit/protocols/protocol.py +++ b/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 ` 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) diff --git a/pcapkit/utilities/__init__.py b/pcapkit/utilities/__init__.py index b996e6af84..c5df17bbcb 100644 --- a/pcapkit/utilities/__init__.py +++ b/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 diff --git a/pcapkit/utilities/inspect.py b/pcapkit/utilities/inspect.py new file mode 100644 index 0000000000..7ac8ccf50b --- /dev/null +++ b/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__', ())) diff --git a/tests/protocols/test_protocol_registry_2763_unit.py b/tests/protocols/test_protocol_registry_2763_unit.py index 70421a5b38..0bbf7bc345 100644 --- a/tests/protocols/test_protocol_registry_2763_unit.py +++ b/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) diff --git a/tests/utilities/test_inspect_isabstract_2784_unit.py b/tests/utilities/test_inspect_isabstract_2784_unit.py new file mode 100644 index 0000000000..4a7b5958a7 --- /dev/null +++ b/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()