From 19018f01b68640375e2682bf6fb1d4c5acadd394 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 15 Aug 2023 15:25:25 +0200 Subject: [PATCH] Improve `SigElementFallbackTransform` fallback logic. (#11311) Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> --- CHANGES | 4 + doc/extdev/nodes.rst | 33 +++ sphinx/addnodes.py | 45 ++-- sphinx/transforms/post_transforms/__init__.py | 17 +- tests/test_addnodes.py | 51 ++++ tests/test_transforms_post_transforms.py | 227 +++++++++++++++++- 6 files changed, 346 insertions(+), 31 deletions(-) create mode 100644 tests/test_addnodes.py diff --git a/CHANGES b/CHANGES index aa8cefcbc..1d1fa0075 100644 --- a/CHANGES +++ b/CHANGES @@ -53,6 +53,10 @@ Features added Patch by Halldor Fannar. * #11570: Use short names when using :pep:`585` built-in generics. Patch by Riccardo Mori. +* #11300: Improve ``SigElementFallbackTransform`` fallback logic and signature + text elements nodes. See :doc:`the documentation ` for more + details. + Patch by Bénédikt Tran. Bugs fixed ---------- diff --git a/doc/extdev/nodes.rst b/doc/extdev/nodes.rst index 5ef669468..7603763fd 100644 --- a/doc/extdev/nodes.rst +++ b/doc/extdev/nodes.rst @@ -35,6 +35,39 @@ and in :py:class:`desc_signature_line` nodes. .. autoclass:: desc_optional .. autoclass:: desc_annotation +Nodes for signature text elements +................................. + +These nodes inherit :py:class:`desc_sig_element` and are generally translated +to ``docutils.nodes.inline`` by :py:class:`!SigElementFallbackTransform`. + +Extensions may create additional ``desc_sig_*``-like nodes but in order for +:py:class:`!SigElementFallbackTransform` to translate them to inline nodes +automatically, they must be added to :py:data:`SIG_ELEMENTS` via the class +keyword argument `_sig_element=True` of :py:class:`desc_sig_element`, e.g.: + + .. code-block:: python + + class desc_custom_sig_node(desc_sig_element, _sig_element=True): ... + +For backwards compatibility, it is still possible to add the nodes directly +using ``SIG_ELEMENTS.add(desc_custom_sig_node)``. + +.. autodata:: SIG_ELEMENTS + :no-value: + +.. autoclass:: desc_sig_element + +.. autoclass:: desc_sig_space +.. autoclass:: desc_sig_name +.. autoclass:: desc_sig_operator +.. autoclass:: desc_sig_punctuation +.. autoclass:: desc_sig_keyword +.. autoclass:: desc_sig_keyword_type +.. autoclass:: desc_sig_literal_number +.. autoclass:: desc_sig_literal_string +.. autoclass:: desc_sig_literal_char + New admonition-like constructs ------------------------------ diff --git a/sphinx/addnodes.py b/sphinx/addnodes.py index ceac01c56..d51d6000b 100644 --- a/sphinx/addnodes.py +++ b/sphinx/addnodes.py @@ -298,9 +298,20 @@ class desc_annotation(nodes.Part, nodes.Inline, nodes.FixedTextElement): # Leaf nodes for markup of text fragments ######################################### +#: A set of classes inheriting :class:`desc_sig_element`. Each node class +#: is expected to be handled by the builder's translator class if the latter +#: does not inherit from SphinxTranslator. +#: +#: This set can be extended manually by third-party extensions or +#: by subclassing :class:`desc_sig_element` and using the class +#: keyword argument `_sig_element=True`. +SIG_ELEMENTS: set[type[desc_sig_element]] = set() + + # Signature text elements, generally translated to node.inline # in SigElementFallbackTransform. -# When adding a new one, add it to SIG_ELEMENTS. +# When adding a new one, add it to SIG_ELEMENTS via the class +# keyword argument `_sig_element=True` (e.g., see `desc_sig_space`). class desc_sig_element(nodes.inline, _desc_classes_injector): """Common parent class of nodes for inline text of a signature.""" @@ -311,11 +322,17 @@ class desc_sig_element(nodes.inline, _desc_classes_injector): super().__init__(rawsource, text, *children, **attributes) self['classes'].extend(self.classes) + def __init_subclass__(cls, *, _sig_element=False, **kwargs): + super().__init_subclass__(**kwargs) + if _sig_element: + # add the class to the SIG_ELEMENTS set if asked + SIG_ELEMENTS.add(cls) + # to not reinvent the wheel, the classes in the following desc_sig classes # are based on those used in Pygments -class desc_sig_space(desc_sig_element): +class desc_sig_space(desc_sig_element, _sig_element=True): """Node for a space in a signature.""" classes = ["w"] @@ -324,54 +341,46 @@ class desc_sig_space(desc_sig_element): super().__init__(rawsource, text, *children, **attributes) -class desc_sig_name(desc_sig_element): +class desc_sig_name(desc_sig_element, _sig_element=True): """Node for an identifier in a signature.""" classes = ["n"] -class desc_sig_operator(desc_sig_element): +class desc_sig_operator(desc_sig_element, _sig_element=True): """Node for an operator in a signature.""" classes = ["o"] -class desc_sig_punctuation(desc_sig_element): +class desc_sig_punctuation(desc_sig_element, _sig_element=True): """Node for punctuation in a signature.""" classes = ["p"] -class desc_sig_keyword(desc_sig_element): +class desc_sig_keyword(desc_sig_element, _sig_element=True): """Node for a general keyword in a signature.""" classes = ["k"] -class desc_sig_keyword_type(desc_sig_element): +class desc_sig_keyword_type(desc_sig_element, _sig_element=True): """Node for a keyword which is a built-in type in a signature.""" classes = ["kt"] -class desc_sig_literal_number(desc_sig_element): +class desc_sig_literal_number(desc_sig_element, _sig_element=True): """Node for a numeric literal in a signature.""" classes = ["m"] -class desc_sig_literal_string(desc_sig_element): +class desc_sig_literal_string(desc_sig_element, _sig_element=True): """Node for a string literal in a signature.""" classes = ["s"] -class desc_sig_literal_char(desc_sig_element): +class desc_sig_literal_char(desc_sig_element, _sig_element=True): """Node for a character literal in a signature.""" classes = ["sc"] -SIG_ELEMENTS = [desc_sig_space, - desc_sig_name, - desc_sig_operator, - desc_sig_punctuation, - desc_sig_keyword, desc_sig_keyword_type, - desc_sig_literal_number, desc_sig_literal_string, desc_sig_literal_char] - - ############################################################### # new admonition-like constructs diff --git a/sphinx/transforms/post_transforms/__init__.py b/sphinx/transforms/post_transforms/__init__.py index 5390aaf06..485f1f1af 100644 --- a/sphinx/transforms/post_transforms/__init__.py +++ b/sphinx/transforms/post_transforms/__init__.py @@ -250,18 +250,27 @@ class SigElementFallbackTransform(SphinxPostTransform): # subclass of SphinxTranslator supports desc_sig_element nodes automatically. return - # for the leaf elements (desc_sig_element), the translator should support _all_ - if not all(has_visitor(translator, node) for node in addnodes.SIG_ELEMENTS): + # for the leaf elements (desc_sig_element), the translator should support _all_, + # unless there exists a generic visit_desc_sig_element default visitor + if (not all(has_visitor(translator, node) for node in addnodes.SIG_ELEMENTS) + and not has_visitor(translator, addnodes.desc_sig_element)): self.fallback(addnodes.desc_sig_element) if not has_visitor(translator, addnodes.desc_inline): self.fallback(addnodes.desc_inline) - def fallback(self, nodeType: Any) -> None: - for node in self.document.findall(nodeType): + def fallback(self, node_type: Any) -> None: + """Translate nodes of type *node_type* to docutils inline nodes. + + The original node type name is stored as a string in a private + ``_sig_node_type`` attribute if the latter did not exist. + """ + for node in self.document.findall(node_type): newnode = nodes.inline() newnode.update_all_atts(node) newnode.extend(node) + # Only set _sig_node_type if not defined by the user + newnode.setdefault('_sig_node_type', node.tagname) node.replace_self(newnode) diff --git a/tests/test_addnodes.py b/tests/test_addnodes.py new file mode 100644 index 000000000..184a6960b --- /dev/null +++ b/tests/test_addnodes.py @@ -0,0 +1,51 @@ +"""Test the non-trivial features in the :mod:`sphinx.addnodes` module.""" + +from __future__ import annotations + +import pytest + +from sphinx import addnodes + + +@pytest.fixture() +def sig_elements() -> set[type[addnodes.desc_sig_element]]: + """Fixture returning the current ``addnodes.SIG_ELEMENTS`` set.""" + original = addnodes.SIG_ELEMENTS.copy() # safe copy of the current nodes + yield {*addnodes.SIG_ELEMENTS} # temporary value to use during tests + addnodes.SIG_ELEMENTS = original # restore the previous value + + +def test_desc_sig_element_nodes(sig_elements): + """Test the registration of ``desc_sig_element`` subclasses.""" + + # expected desc_sig_* node classes (must be declared *after* reloading + # the module since otherwise the objects are not the correct ones) + EXPECTED_SIG_ELEMENTS = { + addnodes.desc_sig_space, + addnodes.desc_sig_name, + addnodes.desc_sig_operator, + addnodes.desc_sig_punctuation, + addnodes.desc_sig_keyword, + addnodes.desc_sig_keyword_type, + addnodes.desc_sig_literal_number, + addnodes.desc_sig_literal_string, + addnodes.desc_sig_literal_char, + } + + assert addnodes.SIG_ELEMENTS == EXPECTED_SIG_ELEMENTS + + # create a built-in custom desc_sig_element (added to SIG_ELEMENTS) + class BuiltInSigElementLikeNode(addnodes.desc_sig_element, _sig_element=True): + pass + + # create a custom desc_sig_element (implicitly not added to SIG_ELEMENTS) + class Custom1SigElementLikeNode(addnodes.desc_sig_element): + pass + + # create a custom desc_sig_element (explicitly not added to SIG_ELEMENTS) + class Custom2SigElementLikeNode(addnodes.desc_sig_element, _sig_element=False): + pass + + assert BuiltInSigElementLikeNode in addnodes.SIG_ELEMENTS + assert Custom1SigElementLikeNode not in addnodes.SIG_ELEMENTS + assert Custom2SigElementLikeNode not in addnodes.SIG_ELEMENTS diff --git a/tests/test_transforms_post_transforms.py b/tests/test_transforms_post_transforms.py index 215e6d14f..b9b612638 100644 --- a/tests/test_transforms_post_transforms.py +++ b/tests/test_transforms_post_transforms.py @@ -1,11 +1,29 @@ """Tests the post_transforms""" +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING + import pytest from docutils import nodes +from sphinx import addnodes +from sphinx.addnodes import SIG_ELEMENTS +from sphinx.testing.util import assert_node +from sphinx.transforms.post_transforms import SigElementFallbackTransform +from sphinx.util.docutils import new_document + +if TYPE_CHECKING: + from typing import Any, NoReturn + + from _pytest.fixtures import SubRequest + + from sphinx.testing.util import SphinxTestApp + @pytest.mark.sphinx('html', testroot='transforms-post_transforms-missing-reference') -def test_nitpicky_warning(app, status, warning): +def test_nitpicky_warning(app, warning): app.build() assert ('index.rst:4: WARNING: py:class reference target ' 'not found: io.StringIO' in warning.getvalue()) @@ -17,12 +35,12 @@ def test_nitpicky_warning(app, status, warning): @pytest.mark.sphinx('html', testroot='transforms-post_transforms-missing-reference', freshenv=True) -def test_missing_reference(app, status, warning): - def missing_reference(app, env, node, contnode): - assert app is app - assert env is app.env - assert node['reftarget'] == 'io.StringIO' - assert contnode.astext() == 'io.StringIO' +def test_missing_reference(app, warning): + def missing_reference(app_, env_, node_, contnode_): + assert app_ is app + assert env_ is app.env + assert node_['reftarget'] == 'io.StringIO' + assert contnode_.astext() == 'io.StringIO' return nodes.inline('', 'missing-reference.StringIO') @@ -37,8 +55,8 @@ def test_missing_reference(app, status, warning): @pytest.mark.sphinx('html', testroot='domain-py-python_use_unqualified_type_names', freshenv=True) -def test_missing_reference_conditional_pending_xref(app, status, warning): - def missing_reference(app, env, node, contnode): +def test_missing_reference_conditional_pending_xref(app, warning): + def missing_reference(_app, _env, _node, contnode): return contnode warning.truncate(0) @@ -57,3 +75,194 @@ def test_keyboard_hyphen_spaces(app): app.build() assert "spanish" in (app.outdir / 'index.html').read_text(encoding='utf8') assert "inquisition" in (app.outdir / 'index.html').read_text(encoding='utf8') + + +class TestSigElementFallbackTransform: + """Integration test for :class:`sphinx.transforms.post_transforms.SigElementFallbackTransform`.""" + # safe copy of the "built-in" desc_sig_* nodes (during the test, instances of such nodes + # will be created sequentially, so we fix a possible order at the beginning using a tuple) + _builtin_sig_elements: tuple[type[addnodes.desc_sig_element], ...] = tuple(SIG_ELEMENTS) + + @pytest.fixture(autouse=True) + def builtin_sig_elements(self) -> tuple[type[addnodes.desc_sig_element], ...]: + """Fixture returning an ordered view on the original value of :data:`!sphinx.addnodes.SIG_ELEMENTS`.""" + return self._builtin_sig_elements + + @pytest.fixture() + def document( + self, app: SphinxTestApp, builtin_sig_elements: tuple[type[addnodes.desc_sig_element], ...], + ) -> nodes.document: + """Fixture returning a new document with built-in ``desc_sig_*`` nodes and a final ``desc_inline`` node.""" + doc = new_document('') + doc.settings.env = app.env + # Nodes that should be supported by a default custom translator class. + # It is important that builtin_sig_elements has a fixed order so that + # the nodes can be deterministically checked. + doc += [node_type('', '') for node_type in builtin_sig_elements] + doc += addnodes.desc_inline('py') + return doc + + @pytest.fixture() + def with_desc_sig_elements(self, value: Any) -> bool: + """Dynamic fixture acting as the identity on booleans.""" + assert isinstance(value, bool) + return value + + @pytest.fixture() + def add_visitor_method_for(self, value: Any) -> list[str]: + """Dynamic fixture acting as the identity on a list of strings.""" + assert isinstance(value, list) + assert all(isinstance(item, str) for item in value) + return value + + @pytest.fixture(autouse=True) + def translator_class(self, request: SubRequest) -> type[nodes.NodeVisitor]: + """Minimal interface fixture similar to SphinxTranslator but orthogonal thereof.""" + logger = logging.getLogger(__name__) + + class BaseCustomTranslatorClass(nodes.NodeVisitor): + """Base class for a custom translator class, orthogonal to ``SphinxTranslator``.""" + + def __init__(self, document, *_a): + super().__init__(document) + # ignore other arguments + + def dispatch_visit(self, node): + for node_class in node.__class__.__mro__: + if method := getattr(self, f'visit_{node_class.__name__}', None): + method(node) + break + else: + logger.info('generic visit: %r', node.__class__.__name__) + super().dispatch_visit(node) + + def unknown_visit(self, node): + logger.warning('unknown visit: %r', node.__class__.__name__) + raise nodes.SkipDeparture # ignore unknown departure + + def visit_document(self, node): + raise nodes.SkipDeparture # ignore departure + + def mark_node(self, node: nodes.Node) -> NoReturn: + logger.info('mark: %r', node.__class__.__name__) + raise nodes.SkipDeparture # ignore departure + + with_desc_sig_elements = request.getfixturevalue('with_desc_sig_elements') + if with_desc_sig_elements: + desc_sig_elements_list = request.getfixturevalue('builtin_sig_elements') + else: + desc_sig_elements_list = [] + add_visitor_method_for = request.getfixturevalue('add_visitor_method_for') + visitor_methods = {f'visit_{tp.__name__}' for tp in desc_sig_elements_list} + visitor_methods.update(f'visit_{name}' for name in add_visitor_method_for) + class_dict = dict.fromkeys(visitor_methods, BaseCustomTranslatorClass.mark_node) + return type('CustomTranslatorClass', (BaseCustomTranslatorClass,), class_dict) # type: ignore[return-value] + + @pytest.mark.parametrize( + 'add_visitor_method_for', + [[], ['desc_inline']], + ids=[ + 'no_explicit_visitor', + 'explicit_desc_inline_visitor', + ], + ) + @pytest.mark.parametrize( + 'with_desc_sig_elements', + [True, False], + ids=[ + 'with_default_visitors_for_desc_sig_elements', + 'without_default_visitors_for_desc_sig_elements', + ], + ) + @pytest.mark.sphinx('dummy') + def test_support_desc_inline( + self, document: nodes.document, with_desc_sig_elements: bool, + add_visitor_method_for: list[str], request: SubRequest, + ) -> None: + document, _, _ = self._exec(request) + # count the number of desc_inline nodes with the extra _sig_node_type field + desc_inline_typename = addnodes.desc_inline.__name__ + visit_desc_inline = desc_inline_typename in add_visitor_method_for + if visit_desc_inline: + assert_node(document[-1], addnodes.desc_inline) + else: + assert_node(document[-1], nodes.inline, _sig_node_type=desc_inline_typename) + + @pytest.mark.parametrize( + 'add_visitor_method_for', + [ + [], # no support + ['desc_sig_space'], # enable desc_sig_space visitor + ['desc_sig_element'], # enable generic visitor + ['desc_sig_space', 'desc_sig_element'], # enable desc_sig_space and generic visitors + ], + ids=[ + 'no_explicit_visitor', + 'explicit_desc_sig_space_visitor', + 'explicit_desc_sig_element_visitor', + 'explicit_desc_sig_space_and_desc_sig_element_visitors', + ], + ) + @pytest.mark.parametrize( + 'with_desc_sig_elements', + [True, False], + ids=[ + 'with_default_visitors_for_desc_sig_elements', + 'without_default_visitors_for_desc_sig_elements', + ], + ) + @pytest.mark.sphinx('dummy') + def test_custom_implementation( + self, + document: nodes.document, + with_desc_sig_elements: bool, + add_visitor_method_for: list[str], + request: SubRequest, + ) -> None: + document, stdout, stderr = self._exec(request) + assert len(self._builtin_sig_elements) == len(document.children[:-1]) == len(stdout[:-1]) + + visit_desc_sig_element = addnodes.desc_sig_element.__name__ in add_visitor_method_for + ignore_sig_element_fallback_transform = visit_desc_sig_element or with_desc_sig_elements + + if ignore_sig_element_fallback_transform: + # desc_sig_element is implemented or desc_sig_* nodes are properly handled (and left untouched) + for node_type, node, mess in zip(self._builtin_sig_elements, document.children[:-1], stdout[:-1]): + assert_node(node, node_type) + assert not node.hasattr('_sig_node_type') + assert mess == f'mark: {node_type.__name__!r}' + else: + # desc_sig_* nodes are converted into inline nodes + for node_type, node, mess in zip(self._builtin_sig_elements, document.children[:-1], stdout[:-1]): + assert_node(node, nodes.inline, _sig_node_type=node_type.__name__) + assert mess == f'generic visit: {nodes.inline.__name__!r}' + + # desc_inline node is never handled and always transformed + assert addnodes.desc_inline.__name__ not in add_visitor_method_for + assert_node(document[-1], nodes.inline, _sig_node_type=addnodes.desc_inline.__name__) + assert stdout[-1] == f'generic visit: {nodes.inline.__name__!r}' + + # nodes.inline are never handled + assert len(stderr) == 1 if ignore_sig_element_fallback_transform else len(document.children) + assert set(stderr) == {f'unknown visit: {nodes.inline.__name__!r}'} + + @staticmethod + def _exec(request: SubRequest) -> tuple[nodes.document, list[str], list[str]]: + caplog = request.getfixturevalue('caplog') + caplog.set_level(logging.INFO, logger=__name__) + + app = request.getfixturevalue('app') + translator_class = request.getfixturevalue('translator_class') + app.set_translator('dummy', translator_class) + # run the post-transform directly [building phase] + # document contains SIG_ELEMENTS nodes followed by a desc_inline node + document = request.getfixturevalue('document') + SigElementFallbackTransform(document).run() + # run the translator [writing phase] + translator = translator_class(document, app.builder) + document.walkabout(translator) + # extract messages + messages = caplog.record_tuples + stdout = [message for _, lvl, message in messages if lvl == logging.INFO] + stderr = [message for _, lvl, message in messages if lvl == logging.WARN] + return document, stdout, stderr