Improve SigElementFallbackTransform fallback logic. (#11311)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
This commit is contained in:
Bénédikt Tran 2023-08-15 15:25:25 +02:00 committed by GitHub
parent 65fd5be20d
commit 19018f01b6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 346 additions and 31 deletions

View File

@ -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 </extdev/nodes>` for more
details.
Patch by Bénédikt Tran.
Bugs fixed
----------

View File

@ -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
------------------------------

View File

@ -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

View File

@ -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)

51
tests/test_addnodes.py Normal file
View File

@ -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

View File

@ -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