Restore support for nested only nodes in toctrees (#13663)

Author: AA-Turner

sphinx/environment/adapters/toctree.py

@@ -482,56 +482,84 @@ def _toctree_add_classes(node: Element, depth: int, docname: str) -> None:
                     subnode = subnode.parent
 
 
-ET = TypeVar('ET', bound=Element)
+_ET = TypeVar('_ET', bound=Element)
 
 
 def _toctree_copy(
-    node: ET, depth: int, maxdepth: int, collapse: bool, tags: Tags
-) -> ET:
+    node: _ET, depth: int, maxdepth: int, collapse: bool, tags: Tags
+) -> _ET:
     """Utility: Cut and deep-copy a TOC at a specified depth."""
-    keep_bullet_list_sub_nodes = depth <= 1 or (
-        (depth <= maxdepth or maxdepth <= 0) and (not collapse or 'iscurrent' in node)
-    )
+    assert not isinstance(node, addnodes.only)
+    depth = max(depth - 1, 1)
+    copied = _toctree_copy_seq(node, depth, maxdepth, collapse, tags, initial_call=True)
+    assert len(copied) == 1
+    return copied[0]  # type: ignore[return-value]
 
-    copy = node.copy()
-    for subnode in node.children:
-        if isinstance(subnode, addnodes.compact_paragraph | nodes.list_item):
-            # for <p> and <li>, just recurse
-            copy.append(_toctree_copy(subnode, depth, maxdepth, collapse, tags))
-        elif isinstance(subnode, nodes.bullet_list):
-            # for <ul>, copy if the entry is top-level
-            # or, copy if the depth is within bounds and;
-            # collapsing is disabled or the sub-entry's parent is 'current'.
-            # The boolean is constant so is calculated outwith the loop.
-            if keep_bullet_list_sub_nodes:
-                copy.append(_toctree_copy(subnode, depth + 1, maxdepth, collapse, tags))
-        elif isinstance(subnode, addnodes.toctree):
-            # copy sub toctree nodes for later processing
-            copy.append(subnode.copy())
-        elif isinstance(subnode, addnodes.only):
-            # only keep children if the only node matches the tags
-            if _only_node_keep_children(subnode, tags):
-                for child in subnode.children:
-                    copy.append(
-                        _toctree_copy(
-                            child,
-                            depth,
-                            maxdepth,
-                            collapse,
-                            tags,  # type: ignore[type-var]
-                        )
-                    )
-        elif isinstance(subnode, nodes.reference | nodes.title):
-            # deep copy references and captions
-            sub_node_copy = subnode.copy()
-            sub_node_copy.children = [child.deepcopy() for child in subnode.children]
-            for child in sub_node_copy.children:
-                child.parent = sub_node_copy
-            copy.append(sub_node_copy)
-        else:
-            msg = f'Unexpected node type {subnode.__class__.__name__!r}!'
-            raise ValueError(msg)  # NoQA: TRY004
-    return copy
+
+def _toctree_copy_seq(
+    node: Node,
+    depth: int,
+    maxdepth: int,
+    collapse: bool,
+    tags: Tags,
+    *,
+    initial_call: bool = False,
+    is_current: bool = False,
+) -> list[Element]:
+    copy: Element
+    if isinstance(node, addnodes.compact_paragraph | nodes.list_item):
+        # for <p> and <li>, just recurse
+        copy = node.copy()
+        for subnode in node.children:
+            copy += _toctree_copy_seq(  # type: ignore[assignment,operator]
+                subnode, depth, maxdepth, collapse, tags, is_current='iscurrent' in node
+            )
+        return [copy]
+
+    if isinstance(node, nodes.bullet_list):
+        # for <ul>, copy if the entry is top-level
+        # or, copy if the depth is within bounds and;
+        # collapsing is disabled or the sub-entry's parent is 'current'.
+        # The boolean is constant so is calculated outwith the loop.
+        keep_bullet_list_sub_nodes = depth <= 1 or (
+            (depth <= maxdepth or maxdepth <= 0)
+            and (not collapse or is_current or 'iscurrent' in node)
+        )
+        if not keep_bullet_list_sub_nodes and not initial_call:
+            return []
+        depth += 1
+        copy = node.copy()
+        for subnode in node.children:
+            copy += _toctree_copy_seq(
+                subnode, depth, maxdepth, collapse, tags, is_current='iscurrent' in node
+            )
+        return [copy]
+
+    if isinstance(node, addnodes.toctree):
+        # copy sub toctree nodes for later processing
+        return [node.copy()]
+
+    if isinstance(node, addnodes.only):
+        # only keep children if the only node matches the tags
+        if not _only_node_keep_children(node, tags):
+            return []
+        copied: list[Element] = []
+        for subnode in node.children:
+            copied += _toctree_copy_seq(
+                subnode, depth, maxdepth, collapse, tags, is_current='iscurrent' in node
+            )
+        return copied
+
+    if isinstance(node, nodes.reference | nodes.title):
+        # deep copy references and captions
+        sub_node_copy = node.copy()
+        sub_node_copy.children = [child.deepcopy() for child in node.children]
+        for child in sub_node_copy.children:
+            child.parent = sub_node_copy
+        return [sub_node_copy]
+
+    msg = f'Unexpected node type {node.__class__.__name__!r}!'
+    raise ValueError(msg)
 
 
 def _get_toctree_ancestors(

sphinx/util/tags.py

@@ -10,7 +10,7 @@
 from sphinx.deprecation import RemovedInSphinx90Warning
 
 if TYPE_CHECKING:
-    from collections.abc import Iterator, Sequence
+    from collections.abc import Collection, Iterator
     from typing import Literal
 
 _ENV = jinja2.environment.Environment()
@@ -42,7 +42,7 @@ def parse_compare(self) -> jinja2.nodes.Expr:
 
 
 class Tags:
-    def __init__(self, tags: Sequence[str] = ()) -> None:
+    def __init__(self, tags: Collection[str] = ()) -> None:
         self._tags = set(tags or ())
         self._condition_cache: dict[str, bool] = {}
 

tests/roots/test-toctree-only/conf.py

@@ -0,0 +1,26 @@
+test-toctree-only
+=================
+
+.. only:: not nonexistent
+
+   hello world
+
+   .. only:: text or not text
+
+      .. js:data:: test_toctree_only1
+
+      lorem ipsum dolor sit amet...
+
+      .. only:: not lorem
+
+         .. only:: not ipsum
+
+            .. js:data:: test_toctree_only2
+
+            lorem ipsum dolor sit amet...
+
+         after ``only:: not ipsum``
+
+   .. js:data:: test_toctree_only2
+
+we're just normal men; we're just innocent men

tests/roots/test-toctree-only/index.rst

@@ -4,15 +4,21 @@
 
 from typing import TYPE_CHECKING
 
+import docutils
 import pytest
 from docutils import nodes
 from docutils.nodes import bullet_list, list_item, literal, reference, title
 
 from sphinx import addnodes
 from sphinx.addnodes import compact_paragraph, only
 from sphinx.builders.html import StandaloneHTMLBuilder
-from sphinx.environment.adapters.toctree import document_toc, global_toctree_for_doc
+from sphinx.environment.adapters.toctree import (
+    _toctree_copy,
+    document_toc,
+    global_toctree_for_doc,
+)
 from sphinx.testing.util import assert_node
+from sphinx.util.tags import Tags
 
 if TYPE_CHECKING:
     from sphinx.testing.util import SphinxTestApp
@@ -916,3 +922,87 @@ def test_toctree_index(app):
         numbered=0,
         entries=[(None, 'genindex'), (None, 'modindex'), (None, 'search')],
     )
+
+
+@pytest.mark.sphinx('dummy', testroot='toctree-only')
+def test_toctree_only(app):
+    # regression test for https://github.com/sphinx-doc/sphinx/issues/13022
+    # we mainly care that this doesn't fail
+
+    if docutils.__version_info__[:2] >= (0, 22):
+        true = '1'
+    else:
+        true = 'True'
+    expected_pformat = f"""\
+<bullet_list>
+  <list_item>
+    <compact_paragraph>
+      <reference anchorname="" internal="{true}" refuri="#">
+        test-toctree-only
+    <bullet_list>
+      <list_item>
+        <compact_paragraph skip_section_number="{true}">
+          <reference anchorname="#test_toctree_only1" internal="{true}" refuri="#test_toctree_only1">
+            <literal>
+              test_toctree_only1
+      <list_item>
+        <compact_paragraph skip_section_number="{true}">
+          <reference anchorname="#test_toctree_only2" internal="{true}" refuri="#test_toctree_only2">
+            <literal>
+              test_toctree_only2
+      <list_item>
+        <compact_paragraph skip_section_number="{true}">
+          <reference anchorname="#id0" internal="{true}" refuri="#id0">
+            <literal>
+              test_toctree_only2
+"""
+    app.build()
+    toc = document_toc(app.env, 'index', app.tags)
+    assert toc.pformat('  ') == expected_pformat
+
+
+def test_toctree_copy_only():
+    # regression test for https://github.com/sphinx-doc/sphinx/issues/13022
+    # ensure ``_toctree_copy()`` properly filters out ``only`` nodes,
+    # including nested nodes.
+    node = nodes.literal('lobster!', 'lobster!')
+    node = nodes.reference('', '', node, anchorname='', internal=True, refuri='index')
+    node = addnodes.only('', node, expr='lobster')
+    node = addnodes.compact_paragraph('', '', node, skip_section_number=True)
+    node = nodes.list_item('', node)
+    node = addnodes.only('', node, expr='not spam')
+    node = addnodes.only('', node, expr='lobster')
+    node = addnodes.only('', node, expr='not ham')
+    node = nodes.bullet_list('', node)
+    # this is a tree of the shape:
+    # <bullet_list>
+    #   <only expr="not ham">
+    #     <only expr="lobster">
+    #       <only expr="not spam">
+    #         <list_item>
+    #           <compact_paragraph skip_section_number="True">
+    #             <only expr="lobster">
+    #               <reference anchorname="" internal="True" refuri="index">
+    #                 <literal>
+    #                   lobster!
+
+    tags = Tags({'lobster'})
+    toc = _toctree_copy(node, 2, 0, False, tags)
+    # the filtered ToC should look like:
+    # <bullet_list>
+    #   <list_item>
+    #     <compact_paragraph skip_section_number="True">
+    #       <reference anchorname="" internal="True" refuri="index">
+    #         <literal>
+    #           lobster!
+
+    # no only nodes should remain
+    assert list(toc.findall(addnodes.only)) == []
+
+    # the tree is preserved
+    assert isinstance(toc, nodes.bullet_list)
+    assert isinstance(toc[0], nodes.list_item)
+    assert isinstance(toc[0][0], addnodes.compact_paragraph)
+    assert isinstance(toc[0][0][0], nodes.reference)
+    assert isinstance(toc[0][0][0][0], nodes.literal)
+    assert toc[0][0][0][0][0] == nodes.Text('lobster!')