Fix C/C++ domain support for :node-id: and :symbol-ids:

Previously, these were not properly taken into account by parameter
objects.

sphinx_immaterial/apidoc/cpp/parameter_objects.py

@@ -1,5 +1,5 @@
 import re
-from typing import cast, Optional, Dict, List, Tuple, Iterator, Type
+from typing import cast, Optional, Dict, List, Tuple, Iterator, Type, Union
 
 import docutils.nodes
 import docutils.parsers.rst.states
@@ -90,15 +90,6 @@ def _monkey_patch_cpp_parameter_fields(doc_field_types):
 OBJECT_PRIORITY_DEFAULT = 1
 OBJECT_PRIORITY_UNIMPORTANT = 2
 
-PARAMETER_OBJECT_TYPES = (
-    "functionParam",
-    "macroParam",
-    "templateParam",
-    "templateTypeParam",
-    "templateTemplateParam",
-    "templateNonTypeParam",
-)
-
 
 def get_precise_template_parameter_object_type(
     object_type: str, symbol: Optional[sphinx.domains.cpp.Symbol]
@@ -146,12 +137,29 @@ def _monkey_patch_cpp_add_precise_template_parameter_object_types():
         )
     )
 
+
+def _monkey_patch_domain_get_objects(
+    domain_class: Union[
+        Type[sphinx.domains.cpp.CPPDomain], Type[sphinx.domains.c.CDomain]
+    ],
+):
+    """Monkey patches `get_objects` to better handle parameter objects.
+
+    Parameter objects are assigned `OBJECT_PRIORITY_UNIMPORTANT`.
+
+    Also adds support for overridden symbol anchors.
+    """
+
     def get_objects(
-        self: sphinx.domains.cpp.CPPDomain,
+        self: Union[sphinx.domains.cpp.CPPDomain, sphinx.domains.c.CDomain],
     ) -> Iterator[Tuple[str, str, str, str, str, int]]:
         rootSymbol = self.data["root_symbol"]
         for symbol in rootSymbol.get_all_symbols():
-            if symbol.declaration is None:
+            if (
+                symbol.declaration is None
+                or getattr(symbol.declaration, symbol_ids.AST_ID_OVERRIDE_ATTR, None)
+                is None
+            ):
                 continue
             assert symbol.docname
             last_resolved_symbol.set_symbol(symbol)
@@ -163,13 +171,13 @@ def get_objects(
             )
             docname = symbol.docname
             anchor = symbol_ids.get_symbol_anchor(symbol)
-            if objectType in PARAMETER_OBJECT_TYPES:
+            if objectType in symbol_ids.PARAMETER_OBJECT_TYPES:
                 priority = OBJECT_PRIORITY_UNIMPORTANT
             else:
                 priority = OBJECT_PRIORITY_DEFAULT
             yield (name, dispname, objectType, docname, anchor, priority)
 
-    sphinx.domains.cpp.CPPDomain.get_objects = get_objects  # type: ignore[assignment]
+    domain_class.get_objects = get_objects  # type: ignore[assignment]
 
 
 def _add_parameter_links_to_signature(
@@ -248,7 +256,6 @@ def _add_parameter_documentation_ids(
     symbols,
     domain_module,
     starting_id_version,
-    qualify_parameter_ids: bool,
     signodes: List[sphinx.addnodes.desc_signature],
 ) -> None:
     domain = obj_content.parent["domain"]
@@ -323,6 +330,9 @@ def cross_link_single_parameter(
                         cast(docutils.nodes.Element, param_node.parent[-1]),
                         generate_synopses,
                     )
+                first_match = True
+            else:
+                first_match = False
 
             if synopsis:
                 synopses.set_synopsis(param_symbol, synopsis)
@@ -337,8 +347,12 @@ def cross_link_single_parameter(
                 parent_symbol.declaration.get_newest_id() + "-" + param_id_suffix,
             )
 
-            if qualify_parameter_ids:
-                # Generate a separate id for each id version.
+            parent_symbol_anchor = getattr(
+                parent_symbol.declaration, symbol_ids.ANCHOR_ATTR, None
+            )
+
+            if parent_symbol_anchor is None:
+                # Generate a separate anchor for each id version.
                 prev_parent_id = None
                 id_prefixes = []
 
@@ -351,16 +365,22 @@ def cross_link_single_parameter(
                         id_prefixes.append(parent_id + "-")
                     except domain_module.NoOldIdError:
                         continue
-            else:
-                id_prefixes = [""]
-                setattr(
-                    param_symbol.declaration, symbol_ids.ANCHOR_ATTR, param_id_suffix
-                )
 
-            if id_prefixes:
                 for id_prefix in id_prefixes:
                     param_id = id_prefix + param_id_suffix
                     param_node["ids"].append(param_id)
+            else:
+                # Use a single anchor
+                param_id_prefix = (
+                    f"{parent_symbol_anchor}-" if parent_symbol_anchor else ""
+                )
+                setattr(
+                    param_symbol.declaration,
+                    symbol_ids.ANCHOR_ATTR,
+                    param_id_prefix + param_id_suffix,
+                )
+                if first_match:
+                    param_node["ids"].append(param_id_prefix + param_id_suffix)
 
         if object_type is not None:
             if param_options["include_in_toc"]:
@@ -369,9 +389,6 @@ def cross_link_single_parameter(
                     toc_title += f" [{kind}]"
                 param_node["toc_title"] = toc_title
 
-        if not qualify_parameter_ids:
-            param_node["ids"].append(param_id_suffix)
-
         del param_node[:]
 
         new_param_nodes = []
@@ -483,7 +500,6 @@ def _cross_link_parameters(
         symbols=symbols,
         domain_module=getattr(sphinx.domains, domain),
         starting_id_version=_FIRST_PARAMETER_ID_VERSIONS[domain],
-        qualify_parameter_ids=bool(signodes[0]["ids"]),
         signodes=signodes,
     )
 
@@ -547,3 +563,5 @@ def after_content(self: sphinx.directives.ObjectDescription) -> None:
 _monkey_patch_domain_to_cross_link_parameters_and_add_synopses(
     sphinx.domains.c.CObject, domain="c"
 )
+_monkey_patch_domain_get_objects(sphinx.domains.c.CDomain)
+_monkey_patch_domain_get_objects(sphinx.domains.cpp.CPPDomain)

sphinx_immaterial/apidoc/cpp/symbol_ids.py

@@ -1,16 +1,30 @@
 """Adds support for :noindex:, :symbol-ids:, and :node-id: options."""
 
 import json
-from typing import Optional, Union, Type, List
+import re
+from typing import Optional, Union, Type, List, Tuple
 
 import docutils.parsers.rst.directives
 import sphinx.addnodes
 import sphinx.domains.c
 import sphinx.domains.cpp
+from sphinx.domains.c import CDomain
+from sphinx.domains.cpp import CPPDomain
+
+from . import last_resolved_symbol
 
 AST_ID_OVERRIDE_ATTR = "_sphinx_immaterial_id"
 ANCHOR_ATTR = "sphinx_immaterial_anchor"
 
+PARAMETER_OBJECT_TYPES = (
+    "functionParam",
+    "macroParam",
+    "templateParam",
+    "templateTypeParam",
+    "templateTemplateParam",
+    "templateNonTypeParam",
+)
+
 
 def _monkey_patch_override_ast_id(
     ast_declaration_class: Union[
@@ -176,6 +190,63 @@ def run(
     object_class.run = run  # type: ignore[assignment]
 
 
+def _monkey_patch_to_override_symbol_anchor(
+    domain_class: Union[
+        Type[sphinx.domains.cpp.CPPDomain], Type[sphinx.domains.c.CDomain]
+    ],
+):
+    """Patch C/C++ resolve_xref to allow overriding the anchor id."""
+    orig_resolve_xref_inner = domain_class._resolve_xref_inner
+
+    def _resolve_xref_inner(
+        self: Union[CDomain, CPPDomain],
+        env: sphinx.environment.BuildEnvironment,
+        fromdocname: str,
+        builder: sphinx.builders.Builder,
+        typ: str,
+        target: str,
+        node: sphinx.addnodes.pending_xref,
+        contnode: docutils.nodes.Element,
+    ) -> Tuple[Optional[docutils.nodes.Element], Optional[str]]:
+        refnode, objtype = orig_resolve_xref_inner(
+            self,  # type: ignore
+            env,
+            fromdocname,
+            builder,
+            typ,
+            target,
+            node,
+            contnode,
+        )
+        if refnode is None:
+            return refnode, objtype
+
+        assert objtype is not None
+
+        last_symbol = last_resolved_symbol.get_symbol()
+
+        if (
+            objtype in PARAMETER_OBJECT_TYPES
+            and getattr(last_symbol.declaration, AST_ID_OVERRIDE_ATTR, None) is None
+        ):
+            # This is a parameter without an associated `:param:` entry.  It
+            # should just be linked to the parent symbol.
+            anchor = get_symbol_anchor(last_symbol.parent)
+        else:
+            anchor = get_symbol_anchor(last_symbol)
+
+        if "refid" in refnode:
+            refnode["refid"] = anchor
+        else:
+            new_refurl = re.sub("#.*", "", refnode["refuri"])
+            if anchor:
+                new_refurl += f"#{anchor}"
+            refnode["refurl"] = new_refurl
+        return refnode, objtype
+
+    domain_class._resolve_xref_inner = _resolve_xref_inner  # type: ignore
+
+
 _monkey_patch_override_ast_id(sphinx.domains.c.ASTDeclaration)
 _monkey_patch_override_ast_id(sphinx.domains.cpp.ASTDeclaration)
 _monkey_patch_cpp_noindex_option(
@@ -190,3 +261,5 @@ def run(
     env_parent_symbol_key="c:parent_symbol",
     duplicate_symbol_error=sphinx.domains.c._DuplicateSymbolError,
 )
+_monkey_patch_to_override_symbol_anchor(domain_class=sphinx.domains.cpp.CPPDomain)
+_monkey_patch_to_override_symbol_anchor(domain_class=sphinx.domains.c.CDomain)

sphinx_immaterial/apidoc/cpp/synopses.py

@@ -1,3 +1,5 @@
+"""Adds support for synopses to C/C++ domains."""
+
 from typing import Union, Type, Optional, Tuple, Iterator
 
 import docutils.nodes

tests/cpp_domain_test.py

@@ -0,0 +1,101 @@
+"""Tests C++ domain functionality added by this theme."""
+
+import json
+
+import docutils.nodes
+import pytest
+
+pytest_plugins = ("sphinx.testing.fixtures",)
+
+
+def snapshot_references(app, snapshot):
+    doc = app.env.get_and_resolve_doctree("index", app.builder)
+
+    nodes = list(doc.findall(condition=docutils.nodes.reference))
+
+    node_data = [
+        {
+            "text": node.astext(),
+            **{
+                attr: node.get(attr)
+                for attr in ["refid", "refurl", "reftitle"]
+                if attr in node
+            },
+        }
+        for node in nodes
+    ]
+
+    snapshot.assert_match("\n".join(json.dumps(n) for n in node_data), "references.txt")
+
+
+@pytest.mark.parametrize("node_id", [None, "", "abc"])
+def test_parameter_objects(immaterial_make_app, snapshot, node_id: str):
+    """Tests that parameter objects take into account the `node-id` option."""
+
+    attrs = []
+    if node_id is not None:
+        attrs.append(f":node-id: {node_id}")
+    attrs_text = "\n".join(attrs)
+
+    app = immaterial_make_app(
+        files={
+            "index.rst": f"""
+
+.. cpp:function:: void foo(int bar, int baz, int undocumented);
+   {attrs_text}
+
+   Test function.
+
+   :param bar: Bar parameter.
+   :param baz: Baz parameter.
+""",
+        },
+    )
+
+    app.build()
+
+    snapshot_references(app, snapshot)
+
+
+def test_template_parameter_objects(immaterial_make_app, snapshot):
+    """Tests that xrefs to template parameters include template parameter kind
+    in tooltip text."""
+    app = immaterial_make_app(
+        files={
+            "index.rst": """
+
+.. cpp:function:: template <typename T, int N, template<typename> class U>\
+                  void foo();
+
+   Test function.
+
+   :tparam T: T parameter.
+   :tparam N: N parameter.
+""",
+        },
+    )
+
+    app.build()
+
+    snapshot_references(app, snapshot)
+
+
+def test_macro_parameter_objects(immaterial_make_app, snapshot):
+    """Tests that macro parameters work correctly."""
+    app = immaterial_make_app(
+        files={
+            "index.rst": """
+
+.. c:macro:: FOO(a, b, c)
+
+   Test macro.
+
+   :param a: A parameter.
+   :param b: B parameter.
+""",
+        },
+    )
+
+    app.build()
+
+    snapshot_references(app, snapshot)

tests/snapshots/cpp_domain_test/test_macro_parameter_objects/references.txt

@@ -0,0 +1,3 @@
+{"text": "a", "refid": "c.FOO-p-a", "reftitle": "a (C macro parameter) \u2014 A parameter."}
+{"text": "b", "refid": "c.FOO-p-b", "reftitle": "b (C macro parameter) \u2014 B parameter."}
+{"text": "c", "refid": "c.FOO", "reftitle": "c (C macro parameter)"}

tests/snapshots/cpp_domain_test/test_parameter_objects/None/references.txt

@@ -0,0 +1,3 @@
+{"text": "bar", "refid": "_CPPv43fooiii-p-bar", "reftitle": "foo::bar (C++ function parameter) \u2014 Bar parameter."}
+{"text": "baz", "refid": "_CPPv43fooiii-p-baz", "reftitle": "foo::baz (C++ function parameter) \u2014 Baz parameter."}
+{"text": "undocumented", "refid": "_CPPv43fooiii", "reftitle": "foo::undocumented (C++ function parameter)"}

tests/snapshots/cpp_domain_test/test_parameter_objects/abc/references.txt

@@ -0,0 +1,3 @@
+{"text": "bar", "refid": "abc-p-bar", "reftitle": "foo::bar (C++ function parameter) \u2014 Bar parameter."}
+{"text": "baz", "refid": "abc-p-baz", "reftitle": "foo::baz (C++ function parameter) \u2014 Baz parameter."}
+{"text": "undocumented", "refid": "abc", "reftitle": "foo::undocumented (C++ function parameter)"}

tests/snapshots/cpp_domain_test/test_parameter_objects/empty/references.txt

@@ -0,0 +1,3 @@
+{"text": "bar", "refid": "p-bar", "reftitle": "foo::bar (C++ function parameter) \u2014 Bar parameter."}
+{"text": "baz", "refid": "p-baz", "reftitle": "foo::baz (C++ function parameter) \u2014 Baz parameter."}
+{"text": "undocumented", "refid": "", "reftitle": "foo::undocumented (C++ function parameter)"}

tests/snapshots/cpp_domain_test/test_template_parameter_objects/references.txt

@@ -0,0 +1,3 @@
+{"text": "T", "refid": "_CPPv4I0_iI0E0E3foovv-p-T", "reftitle": "foo::T (C++ type template parameter) \u2014 T parameter."}
+{"text": "N", "refid": "_CPPv4I0_iI0E0E3foovv-p-N", "reftitle": "foo::N (C++ non-type template parameter) \u2014 N parameter."}
+{"text": "U", "refid": "_CPPv4I0_iI0E0E3foovv", "reftitle": "foo::U (C++ template template parameter)"}