diff --git a/docs/source/conf.py b/docs/source/conf.py index 6ee709c74..2f6ad20a7 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -13,10 +13,10 @@ import os import pkgutil import re +import sys import typing from typing import TYPE_CHECKING -import aenum from docutils import nodes from sphinx import addnodes from sphinx.ext.autodoc.preserve_defaults import DefaultValue, update_default_value @@ -53,7 +53,7 @@ if TYPE_CHECKING: from types import FunctionType, ModuleType - from typing import Any, Dict, List, Optional + from typing import Any, Dict, List, Optional, Set from docutils.nodes import document from sphinx.application import Sphinx from sphinx.config import Config @@ -221,7 +221,7 @@ todo_include_todos = True set_type_checking_flag = True -# NB: ``typehints_formatter`` is set below, beside ``format_aenum_class``. +# NB: ``typehints_formatter`` is set below, beside ``format_class``. typehints_fully_qualified = False always_document_param_types = False typehints_document_rtype = True @@ -437,6 +437,9 @@ def bind_type_checking_names(app: 'Sphinx', env: 'BuildEnvironment', # pylint: it would be discarded, and the library would run those blocks again as reading reached them. + The walk also records each module's classes for :func:`format_class`, through + :func:`record_class_names`. + One consequence is worth knowing before it is met as a mystery: making the annotations resolvable also makes a malformed one **fatal**. A quoted annotation with stray whitespace inside the quotes -- ``rank: ' int'``, the @@ -466,6 +469,7 @@ def bind_type_checking_names(app: 'Sphinx', env: 'BuildEnvironment', # pylint: logger.info('skipped unimportable module %s: %s', info.name, exc) continue bind_guarded_names(app.config.autodoc_mock_imports, module) + record_class_names(module) #: Every name :mod:`builtins` binds -- ``dict``, ``set``, ``type`` and the rest. @@ -586,40 +590,92 @@ def restore_enum_member_docstring(app: 'Sphinx', what: str, name: str, # pylint lines[:] = original -def format_aenum_class(annotation: 'Any', config: 'Config') -> 'Optional[str]': # pylint: disable=unused-argument - """Name a class that :mod:`aenum` defines by its qualified name. +#: The bare names of the classes the ``pcapkit`` modules bind, each with the +#: qualified names of the classes bound under it; :func:`record_class_names` +#: fills it as the build starts. +CLASS_NAMES = {} # type: Dict[str, Set[str]] + + +def record_class_names(module: 'ModuleType') -> None: + """Note the classes one module binds, at runtime or in its ``if TYPE_CHECKING:`` block. + + :func:`format_class` reads the record to tell which bare names more than one + class answers to. + + Args: + module: The module, its guarded names already recorded by + :func:`resolve_type_guarded_imports`. + + """ + for value in (*vars(module).values(), *guarded_names(module).values()): + if isinstance(value, type) and isinstance(getattr(value, '__module__', None), str): + CLASS_NAMES.setdefault(value.__name__, set()).add(f'{value.__module__}.{value.__qualname__}') + + +def exported_name(cls: type) -> str: + """The shortest dotted name a class is importable by, from the package down. + + That is ``aenum.IntEnum`` for the class ``aenum._enum`` defines. + + Args: + cls: The class. - ``typehints_fully_qualified`` is off, so a class renders by its bare name, and - :mod:`aenum` publishes no inventory to link to. A union of an :mod:`aenum` - class and its standard library namesake therefore read ``IntEnum | IntEnum``: - the first linked to :class:`enum.IntEnum`, the second plain text - (:issue:`1858`). Qualifying the :mod:`aenum` one tells them apart. + Returns: + The qualified name. - The name is the one the package exports, ``aenum.IntEnum``, rather than the - ``aenum._enum`` module that defines the class. ``sphinx-autodoc-typehints`` - 3.15 renders ``Bases:`` lines too, so ``NumberField[IntEnum | IntEnum]`` - reads apart there as well. + """ + parts = cls.__module__.split('.') + for end in range(1, len(parts)): + owner = '.'.join(parts[:end]) + if getattr(sys.modules.get(owner), cls.__qualname__, None) is cls: + return f'{owner}.{cls.__qualname__}' + return f'{cls.__module__}.{cls.__qualname__}' + + +def format_class(annotation: 'Any', config: 'Config') -> 'Optional[str]': # pylint: disable=unused-argument + """Name a third-party class by its qualified name when its bare name is ambiguous. + + ``typehints_fully_qualified`` is off, so a class renders by its bare name. + Two classes of one name therefore read the same: ``IntEnum | IntEnum`` for + the standard library's and ``aenum``'s (:issue:`1858`), and + ``Packet | Packet | Packet`` for those of ``scapy``, ``dpkt`` and ``pyshark`` + (:issue:`1918`). A class from another distribution is qualified when + :data:`CLASS_NAMES` records another class of its name, and an ``aenum`` + class always is, since each shadows a standard library one. + ``sphinx-autodoc-typehints`` 3.15 renders ``Bases:`` lines too, so + ``NumberField[IntEnum | IntEnum]`` reads apart there as well. + + A package in :data:`intersphinx_mapping` gets a cross-reference, by the module + defining the class, as its inventory names it. Any other package has no + ``py:`` objects to link to (:issue:`934`), so it gets a literal, by the name + the package exports, ``aenum.IntEnum`` rather than ``aenum._enum.IntEnum`` + (:issue:`1920`). Args: annotation: The type being rendered, or a part of one. config: Sphinx configuration. Returns: - A ``:py:class:`` role naming an :mod:`aenum` class, or :data:`None` to - leave anything else to the default rendering. + The markup naming such a class, or :data:`None` to leave anything else + to the default rendering. """ module = getattr(annotation, '__module__', None) - if not isinstance(annotation, type) or not isinstance(module, str) or module.partition('.')[0] != 'aenum': + if not isinstance(annotation, type) or not isinstance(module, str): + return None + package = module.partition('.')[0] + if package == 'pcapkit' or package in sys.stdlib_module_names: return None - name = annotation.__qualname__ - owner = 'aenum' if getattr(aenum, name, None) is annotation else module - return f':py:class:`{owner}.{name}`' + if package != 'aenum' and len(CLASS_NAMES.get(annotation.__name__, ())) < 2: + return None + if package in intersphinx_mapping: + return f':py:class:`{module}.{annotation.__qualname__}`' + return f'``{exported_name(annotation)}``' # NB: only one formatter can be configured, so any other formatter-based fix has # to share this one. -typehints_formatter = format_aenum_class +typehints_formatter = format_class #: A name followed by an attribute access or a call, in the source text of a default. DOTTED_HEAD = re.compile(r'(? None obj.__dict__['__signature__'] = renamed +def sign_wrapped_function(app: 'Sphinx', obj: 'Any', bound_method: bool) -> None: # pylint: disable=unused-argument + """Move a preserved signature off a ``functools.wraps`` wrapper, onto the function it wraps. + + ``autodoc_preserve_defaults`` stores the signature on the documented object, + which for a decorated method is the wrapper. ``sphinx-autodoc-typehints`` + stops unwrapping at a stated ``__signature__``, so it evaluated the + annotations in the decorator's module. The ``@seekset`` overload of + :meth:`~pcapkit.protocols.protocol.ProtocolBase._read_packet` then rendered + its return as the bare alias ``Data_Packet``, which only the protocol module + binds (:issue:`1917`). + + Only a signature listing what the wrapped function takes is moved, so a + wrapper stating a signature of its own keeps it. Python reads the moved one + through the wrapper, so autodoc renders the same signature. + + Args: + app: Sphinx application. + obj: The function or method whose signature autodoc is about to read. + bound_method: Whether it is read as a bound method. + + """ + wrapper = getattr(obj, '__func__', obj) + wrapped = inspect.unwrap(wrapper) + if wrapped is wrapper or not inspect.isfunction(wrapper) or not inspect.isfunction(wrapped): + return + signature = vars(wrapper).get('__signature__') + if not isinstance(signature, inspect.Signature) or '__signature__' in vars(wrapped): + return + # NB: autodoc rewrites the defaults only, so everything else has to match. + own = inspect.signature(wrapped) + if (signature.return_annotation != own.return_annotation + or [(param.name, param.kind, param.annotation) for param in signature.parameters.values()] + != [(param.name, param.kind, param.annotation) for param in own.parameters.values()]): + return + del wrapper.__signature__ + wrapped.__signature__ = signature + + def passes_through(new: 'FunctionType') -> bool: """Whether a ``__new__`` takes nothing but ``*args`` and ``**kwargs``. @@ -987,6 +1081,8 @@ def setup(app: 'Sphinx') -> None: # NB: after the ``update_defvalue`` that autodoc's legacy documenters connect # at the default 500; the current ones preserve the defaults before emitting. app.connect('autodoc-before-process-signature', name_aliased_defaults, priority=600) + # NB: after ``name_aliased_defaults``, which writes to the wrapper. + app.connect('autodoc-before-process-signature', sign_wrapped_function, priority=700) app.connect('viewcode-follow-imported', follow_builtins_alias) app.add_post_transform(DropTableWrapperIds) app.connect('doctree-read', resolve_builtins_exactly) diff --git a/tests/_dependency_gates.py b/tests/_dependency_gates.py index f2f5f7308..377d35a1a 100644 --- a/tests/_dependency_gates.py +++ b/tests/_dependency_gates.py @@ -649,9 +649,9 @@ class Exclusion(NamedTuple): dark={'test': ('sphinx', 'sphinx_autodoc_typehints'), 'gate': ('sphinx', 'sphinx_autodoc_typehints')}, reason=( - 'The classes it gates (#1775, #1856, #1858-#1863 and the enum-value hook\'s) load ' - 'docs/source/conf.py and drive its hooks with the nodes and events Sphinx ' - 'itself builds, so they need Sphinx and the pinned sphinx-autodoc-typehints. ' + 'The classes it gates (#1775, #1856, #1858-#1863, #1917, #1918 and the enum-value ' + 'hook\'s) load docs/source/conf.py and drive its hooks with the nodes and events ' + 'Sphinx itself builds, so they need Sphinx and the pinned sphinx-autodoc-typehints. ' 'Only deploy-pages.yml installs those, from docs/requirements.txt, and that ' 'workflow builds the documentation rather than running pytest; no workflow ' 'installs the docs extra itself. The same argument declines it here as ' diff --git a/tests/project/test_docs_aenum_formatter_1858_unit.py b/tests/project/test_docs_aenum_formatter_1858_unit.py index 3a60da841..884eea65e 100644 --- a/tests/project/test_docs_aenum_formatter_1858_unit.py +++ b/tests/project/test_docs_aenum_formatter_1858_unit.py @@ -3,9 +3,10 @@ GitHub issue #1858: ``typehints_fully_qualified`` is off and :mod:`aenum` has no inventory, so a parameter annotated ``StdlibIntEnum | AenumIntEnum`` rendered as -``IntEnum | IntEnum``. ``format_aenum_class`` in :file:`docs/source/conf.py`, the -build's ``typehints_formatter``, names an :mod:`aenum` class ``aenum.IntEnum`` -and leaves everything else to the default rendering. +``IntEnum | IntEnum``. ``format_class`` in :file:`docs/source/conf.py`, the +build's ``typehints_formatter``, names an :mod:`aenum` class ``aenum.IntEnum``, +as a literal since there is nothing to link to (#1920), and leaves the classes +of pcapkit and the standard library to the default rendering. The tests call the formatter directly. They need Sphinx and the pinned ``sphinx-autodoc-typehints``, which :file:`conf.py` imports, and skip @@ -31,7 +32,7 @@ @unittest.skipUnless(HAS_SPHINX, SPHINX_SKIP_REASON) class AenumFormatterTests(unittest.TestCase): - """``format_aenum_class`` qualifies :mod:`aenum` classes and nothing else.""" + """``format_class`` qualifies :mod:`aenum` classes, and leaves these others alone.""" conf = None # type: Any @@ -41,19 +42,19 @@ def setUpClass(cls) -> 'None': def format(self, annotation: 'Any') -> 'Any': """The formatter's answer for ``annotation``.""" - return self.conf.format_aenum_class(annotation, None) + return self.conf.format_class(annotation, None) def test_aenum_classes_are_qualified(self) -> 'None': """By the name the package exports, not the ``aenum._enum`` module defining them.""" for cls in (aenum.IntEnum, aenum.StrEnum, aenum.Enum, aenum.IntFlag): with self.subTest(cls=cls): self.assertEqual(cls.__module__, 'aenum._enum') - self.assertEqual(self.format(cls), f':py:class:`aenum.{cls.__name__}`') + self.assertEqual(self.format(cls), f'``aenum.{cls.__name__}``') def test_union_members_render_apart(self) -> 'None': """``StdlibIntEnum | AenumIntEnum``, as ``dumpkit/pcap.py`` annotates ``protocol``.""" answers = [self.format(arg) for arg in typing.get_args(typing.Union[enum.IntEnum, aenum.IntEnum])] - self.assertEqual(answers, [None, ':py:class:`aenum.IntEnum`']) + self.assertEqual(answers, [None, '``aenum.IntEnum``']) def test_other_annotations_are_left_alone(self) -> 'None': """A standard library class, a pcapkit registry built on :mod:`aenum`, and non-classes.""" @@ -64,7 +65,7 @@ def test_other_annotations_are_left_alone(self) -> 'None': def test_conf_configures_it(self) -> 'None': """Only one formatter can be configured, and this is it.""" - self.assertIs(self.conf.typehints_formatter, self.conf.format_aenum_class) + self.assertIs(self.conf.typehints_formatter, self.conf.format_class) if __name__ == '__main__': diff --git a/tests/project/test_docs_class_formatter_1918_unit.py b/tests/project/test_docs_class_formatter_1918_unit.py new file mode 100644 index 000000000..23ad29515 --- /dev/null +++ b/tests/project/test_docs_class_formatter_1918_unit.py @@ -0,0 +1,122 @@ +# -*- coding: utf-8 -*- +"""Pins the docs build's rendering of third-party classes that share a bare name. + +GitHub issue #1918: ``typehints_fully_qualified`` is off, so the frame types of +:class:`~pcapkit.foundation.extraction.Extractor` read +``Packet | Packet | Packet``, for the ``scapy``, ``dpkt`` and ``pyshark`` classes +:mod:`pcapkit.foundation.extraction` imports under ``TYPE_CHECKING``. +``format_class`` in :file:`docs/source/conf.py`, the build's +``typehints_formatter``, qualifies a third-party class when the pcapkit modules +bind another class of its name, which ``record_class_names`` notes as the build +starts. + +GitHub issue #1920: a package without ``py:`` objects in its inventory cannot be +cross-referenced (#934), so such a class is named by a literal, not a role. + +The tests call the hooks directly. They need Sphinx and the pinned +``sphinx-autodoc-typehints``, and skip without them, but run no build. + +""" + +from __future__ import annotations + +import importlib.util +import types +import unittest +from typing import TYPE_CHECKING +from unittest import mock + +from tests.project.test_docs_ambiguous_xrefs_1775_unit import HAS_SPHINX, SPHINX_SKIP_REASON, load_conf + +if TYPE_CHECKING: + from typing import Any + +HAS_SCAPY = importlib.util.find_spec('scapy') is not None +HAS_DPKT = importlib.util.find_spec('dpkt') is not None +HAS_PYSHARK = importlib.util.find_spec('pyshark') is not None + + +@unittest.skipUnless(HAS_SPHINX, SPHINX_SKIP_REASON) +class ClassFormatterTests(unittest.TestCase): + """``format_class`` qualifies a third-party class whose bare name is ambiguous.""" + + conf = None # type: Any + + @classmethod + def setUpClass(cls) -> 'None': + cls.conf = load_conf() + + def setUp(self) -> 'None': + patcher = mock.patch.dict(self.conf.CLASS_NAMES, clear=True) + patcher.start() + self.addCleanup(patcher.stop) + + def record(self, *classes: 'type') -> 'None': + """Record a module binding ``classes``.""" + module = types.ModuleType('pcapkit_test_module') + for at, cls in enumerate(classes): + setattr(module, f'Alias{at}', cls) + self.conf.record_class_names(module) + + @unittest.skipUnless(HAS_SCAPY, 'scapy is not installed') + @unittest.skipUnless(HAS_DPKT, 'dpkt is not installed') + @unittest.skipUnless(HAS_PYSHARK, 'pyshark is not installed') + def test_extraction_packets_render_apart(self) -> 'None': + """From the names the module's ``TYPE_CHECKING`` block binds, as the build records them.""" + # pylint: disable=import-outside-toplevel + from dpkt.dpkt import Packet as DPKTPacket + from pyshark.packet.packet import Packet as PySharkPacket + from scapy.packet import Packet as ScapyPacket + + from pcapkit.foundation import extraction + + self.conf.resolve_type_guarded_imports([], extraction) + self.conf.record_class_names(extraction) + answers = [self.conf.format_class(cls, None) for cls in (ScapyPacket, DPKTPacket, PySharkPacket)] + self.assertEqual(answers, [':py:class:`scapy.packet.Packet`', ':py:class:`dpkt.dpkt.Packet`', + '``pyshark.packet.packet.Packet``']) + + def test_unlinked_package_gets_a_literal(self) -> 'None': + """``aenum`` has no ``py:`` objects to link to; its exported name is shown.""" + import aenum # pylint: disable=import-outside-toplevel + self.assertEqual(self.conf.format_class(aenum.IntFlag, None), '``aenum.IntFlag``') + self.assertNotIn(':py:', self.conf.format_class(aenum.IntFlag, None)) + + def test_unambiguous_class_is_left_alone(self) -> 'None': + """A third-party class no other recorded class shares a name with.""" + import dictdumper # pylint: disable=import-outside-toplevel + self.record(dictdumper.JSON) + self.assertIsNone(self.conf.format_class(dictdumper.JSON, None)) + + def test_pcapkit_and_stdlib_classes_are_left_alone(self) -> 'None': + """Even when they share a name with a third-party class.""" + import enum # pylint: disable=import-outside-toplevel + + import aenum # pylint: disable=import-outside-toplevel + + from pcapkit.protocols.data.protocol import Packet # pylint: disable=import-outside-toplevel + + class Imposter: # pylint: disable=too-few-public-methods + """A third-party class named ``Packet``.""" + + Imposter.__name__ = Imposter.__qualname__ = 'Packet' + Imposter.__module__ = 'thirdparty.packet' + self.record(Packet, Imposter, enum.IntEnum, aenum.IntEnum) + self.assertIsNone(self.conf.format_class(Packet, None)) + self.assertIsNone(self.conf.format_class(enum.IntEnum, None)) + self.assertEqual(self.conf.format_class(Imposter, None), '``thirdparty.packet.Packet``') + + def test_walk_records_every_module(self) -> 'None': + """``bind_type_checking_names`` records each module it walks.""" + from pcapkit.foundation import extraction # pylint: disable=import-outside-toplevel + app = types.SimpleNamespace(config=types.SimpleNamespace(autodoc_mock_imports=[])) + with mock.patch.object(self.conf.pkgutil, 'walk_packages', + return_value=[types.SimpleNamespace(name=extraction.__name__)]), \ + mock.patch.object(self.conf, 'bind_guarded_names'), \ + mock.patch.object(self.conf, 'record_class_names') as record: + self.conf.bind_type_checking_names(app, None, []) + record.assert_called_once_with(extraction) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/project/test_docs_wrapped_overloads_1917_unit.py b/tests/project/test_docs_wrapped_overloads_1917_unit.py new file mode 100644 index 000000000..f42f82e21 --- /dev/null +++ b/tests/project/test_docs_wrapped_overloads_1917_unit.py @@ -0,0 +1,99 @@ +# -*- coding: utf-8 -*- +"""Pins the docs build's reading of a decorated method's overloads in its own module. + +GitHub issue #1917: ``autodoc_preserve_defaults`` stores the signature on the +``@seekset`` wrapper of +:meth:`~pcapkit.protocols.protocol.ProtocolBase._read_packet`, and +``sphinx-autodoc-typehints`` stops unwrapping at a stated signature, so it read +the overloads in :mod:`pcapkit.corekit.decorators`. The third overload's return, +``'Data_Packet'``, is an alias only :mod:`pcapkit.protocols.protocol` binds, and +rendered as that bare word. ``sign_wrapped_function`` in +:file:`docs/source/conf.py` moves the signature onto the wrapped function. + +The tests replay ``autodoc-before-process-signature`` through Sphinx's own +:class:`~sphinx.events.EventManager`, as the #1859 tests do. They need Sphinx and +the pinned ``sphinx-autodoc-typehints``, and skip without them, but run no build. + +""" + +from __future__ import annotations + +import functools +import inspect +import unittest +from typing import TYPE_CHECKING + +from tests.project.test_docs_ambiguous_xrefs_1775_unit import (HAS_SPHINX, SPHINX_SKIP_REASON, connections, + load_conf) +from tests.project.test_docs_class_signature_1860_unit import app_for, emit + +if TYPE_CHECKING: + from typing import Any + +#: The event the hook answers. +EVENT = 'autodoc-before-process-signature' + + +@unittest.skipUnless(HAS_SPHINX, SPHINX_SKIP_REASON) +class WrappedOverloadTests(unittest.TestCase): + """A decorated method's annotations evaluate where they were written.""" + + conf = None # type: Any + + @classmethod + def setUpClass(cls) -> 'None': + cls.conf = load_conf() + + def test_read_packet_overload_resolves_its_alias(self) -> 'None': + """``'Data_Packet'`` evaluates to the class, in the function the library reads.""" + # pylint: disable=import-outside-toplevel + from sphinx.ext.autodoc.preserve_defaults import DefaultValue, update_defvalue + from sphinx_autodoc_typehints._objects import callable_target + from sphinx_autodoc_typehints._resolve.evaluate.expression import resolve_string_annotations + + from pcapkit.protocols.data.protocol import Packet + from pcapkit.protocols.protocol import ProtocolBase + + wrapper = vars(ProtocolBase)['_read_packet'] + wrapped = inspect.unwrap(wrapper) + self.assertIsNot(wrapped, wrapper) + saved = [(function, vars(function).get('__signature__')) for function in (wrapper, wrapped)] + try: + emit(self.conf, app_for(self.conf), EVENT, wrapper, False, library=update_defvalue) + target = callable_target(wrapper) + self.assertIsNotNone(target) + hints = resolve_string_annotations(target[0], {'return': 'Data_Packet'}, {}) # type: ignore[index] + self.assertIs(hints['return'], Packet) + # NB: and the preserved defaults are still the ones autodoc reads. + self.assertIsInstance(inspect.signature(wrapper).parameters['length'].default, DefaultValue) + finally: + for function, signature in saved: + vars(function).pop('__signature__', None) + if signature is not None: + function.__signature__ = signature # type: ignore[attr-defined] + + def test_a_stated_signature_stays(self) -> 'None': + """A wrapper whose signature differs from the wrapped function's keeps it.""" + def wrapped(first: 'int', second: 'int' = 0) -> 'int': + return first + second + + @functools.wraps(wrapped) + def wrapper(*args: 'Any', **kwargs: 'Any') -> 'int': + return wrapped(*args, **kwargs) + + stated = inspect.Signature([inspect.Parameter('first', inspect.Parameter.POSITIONAL_OR_KEYWORD)]) + wrapper.__signature__ = stated # type: ignore[attr-defined] + self.conf.sign_wrapped_function(app_for(self.conf), wrapper, False) + self.assertIs(vars(wrapper)['__signature__'], stated) + self.assertNotIn('__signature__', vars(wrapped)) + + def test_setup_moves_after_the_aliases_are_named(self) -> 'None': + """After ``name_aliased_defaults``, which writes its signature to the wrapper.""" + wired = {handler: (event, priority) for event, handler, priority in connections(self.conf)} + event, priority = wired[self.conf.sign_wrapped_function] + self.assertEqual(event, EVENT) + self.assertGreater(priority, wired[self.conf.name_aliased_defaults][1]) + + +if __name__ == '__main__': + unittest.main()