c domain: Generate node_id for objects in the right way

4 files changed, 124 insertions, 36 deletions

diff --git a/CHANGES b/CHANGES
index aaf2ac743..70119aa96 100644
--- a/CHANGES
+++ b/CHANGES
@@ -23,7 +23,7 @@ Incompatible changes
 * Due to the scoping changes for :rst:dir:`productionlist` some uses of
   :rst:role:`token` must be modified to include the scope which was previously
   ignored.
-* #6903: Internal data structure of Python, reST and standard domains have
+* #6903: Internal data structure of C, Python, reST and standard domains have
   changed.  The node_id is added to the index of objects and modules.  Now they
   contains a pair of docname and node_id for cross reference.
 * #7210: js domain: Non intended behavior is removed such as ``parseInt_`` links
diff --git a/sphinx/domains/c.py b/sphinx/domains/c.py
index 539a52e59..22d6ea82f 100644
--- a/sphinx/domains/c.py
+++ b/sphinx/domains/c.py
@@ -27,7 +27,7 @@ from sphinx.locale import _, __
 from sphinx.roles import XRefRole
 from sphinx.util import logging
 from sphinx.util.docfields import Field, TypedField
-from sphinx.util.nodes import make_refnode
+from sphinx.util.nodes import make_id, make_refnode
 
 
 logger = logging.getLogger(__name__)
@@ -197,21 +197,23 @@ class CObject(ObjectDescription):
             return ''
 
     def add_target_and_index(self, name: str, sig: str, signode: desc_signature) -> None:
-        # for C API items we add a prefix since names are usually not qualified
-        # by a module name and so easily clash with e.g. section titles
-        targetname = 'c.' + name
-        if targetname not in self.state.document.ids:
-            signode['names'].append(targetname)
-            signode['ids'].append(targetname)
-            self.state.document.note_explicit_target(signode)
+        node_id = make_id(self.env, self.state.document, 'c', name)
+        signode['ids'].append(node_id)
 
-            domain = cast(CDomain, self.env.get_domain('c'))
-            domain.note_object(name, self.objtype)
+        # Assign old styled node_id not to break old hyperlinks (if possible)
+        # Note: Will be removed in Sphinx-5.0 (RemovedInSphinx50Warning)
+        old_node_id = self.make_old_id(name)
+        if old_node_id not in self.state.document.ids and old_node_id not in signode['ids']:
+            signode['ids'].append(old_node_id)
+
+        self.state.document.note_explicit_target(signode)
+
+        domain = cast(CDomain, self.env.get_domain('c'))
+        domain.note_object(name, self.objtype, node_id)
 
         indextext = self.get_index_text(name)
         if indextext:
-            self.indexnode['entries'].append(('single', indextext,
-                                              targetname, '', None))
+            self.indexnode['entries'].append(('single', indextext, node_id, '', None))
 
     def before_content(self) -> None:
         self.typename_set = False
@@ -224,6 +226,14 @@ class CObject(ObjectDescription):
         if self.typename_set:
             self.env.ref_context.pop('c:type', None)
 
+    def make_old_id(self, name: str) -> str:
+        """Generate old styled node_id for C objects.
+
+        .. note:: Old Styled node_id was used until Sphinx-3.0.
+                  This will be removed in Sphinx-5.0.
+        """
+        return 'c.' + name
+
 
 class CXRefRole(XRefRole):
     def process_link(self, env: BuildEnvironment, refnode: Element,
@@ -267,31 +277,31 @@ class CDomain(Domain):
         'type':   CXRefRole(),
     }
     initial_data = {
-        'objects': {},  # fullname -> docname, objtype
-    }  # type: Dict[str, Dict[str, Tuple[str, Any]]]
+        'objects': {},  # fullname -> docname, node_id, objtype
+    }  # type: Dict[str, Dict[str, Tuple[str, str, str]]]
 
     @property
-    def objects(self) -> Dict[str, Tuple[str, str]]:
-        return self.data.setdefault('objects', {})  # fullname -> docname, objtype
+    def objects(self) -> Dict[str, Tuple[str, str, str]]:
+        return self.data.setdefault('objects', {})  # fullname -> docname, node_id, objtype
 
-    def note_object(self, name: str, objtype: str, location: Any = None) -> None:
+    def note_object(self, name: str, objtype: str, node_id: str, location: Any = None) -> None:
         if name in self.objects:
             docname = self.objects[name][0]
             logger.warning(__('duplicate C object description of %s, '
                               'other instance in %s, use :noindex: for one of them'),
                            name, docname, location=location)
-        self.objects[name] = (self.env.docname, objtype)
+        self.objects[name] = (self.env.docname, node_id, objtype)
 
     def clear_doc(self, docname: str) -> None:
-        for fullname, (fn, _l) in list(self.objects.items()):
+        for fullname, (fn, node_id, _l) in list(self.objects.items()):
             if fn == docname:
                 del self.objects[fullname]
 
     def merge_domaindata(self, docnames: List[str], otherdata: Dict) -> None:
         # XXX check duplicates
-        for fullname, (fn, objtype) in otherdata['objects'].items():
+        for fullname, (fn, node_id, objtype) in otherdata['objects'].items():
             if fn in docnames:
-                self.data['objects'][fullname] = (fn, objtype)
+                self.data['objects'][fullname] = (fn, node_id, objtype)
 
     def resolve_xref(self, env: BuildEnvironment, fromdocname: str, builder: Builder,
                      typ: str, target: str, node: pending_xref, contnode: Element
@@ -303,9 +313,8 @@ class CDomain(Domain):
             return contnode
         if target not in self.objects:
             return None
-        obj = self.objects[target]
-        return make_refnode(builder, fromdocname, obj[0], 'c.' + target,
-                            contnode, target)
+        docname, node_id, objtype = self.objects[target]
+        return make_refnode(builder, fromdocname, docname, node_id, contnode, target)
 
     def resolve_any_xref(self, env: BuildEnvironment, fromdocname: str, builder: Builder,
                          target: str, node: pending_xref, contnode: Element
@@ -314,14 +323,13 @@ class CDomain(Domain):
         target = target.rstrip(' *')
         if target not in self.objects:
             return []
-        obj = self.objects[target]
-        return [('c:' + self.role_for_objtype(obj[1]),
-                 make_refnode(builder, fromdocname, obj[0], 'c.' + target,
-                              contnode, target))]
+        docname, node_id, objtype = self.objects[target]
+        return [('c:' + self.role_for_objtype(objtype),
+                 make_refnode(builder, fromdocname, docname, node_id, contnode, target))]
 
     def get_objects(self) -> Iterator[Tuple[str, str, str, str, str, int]]:
-        for refname, (docname, type) in list(self.objects.items()):
-            yield (refname, refname, type, docname, 'c.' + refname, 1)
+        for refname, (docname, node_id, objtype) in list(self.objects.items()):
+            yield (refname, refname, objtype, docname, node_id, 1)
 
 
 def setup(app: Sphinx) -> Dict[str, Any]:
diff --git a/tests/test_build_html.py b/tests/test_build_html.py
index 39cb3bf71..779f357e5 100644
--- a/tests/test_build_html.py
+++ b/tests/test_build_html.py
@@ -288,11 +288,11 @@ def test_html4_output(app, status, warning):
         (".//a[@class='reference internal'][@href='#errmod-error']/strong", 'Error'),
         # C references
         (".//span[@class='pre']", 'CFunction()'),
-        (".//a[@href='#c.Sphinx_DoSomething']", ''),
-        (".//a[@href='#c.SphinxStruct.member']", ''),
-        (".//a[@href='#c.SPHINX_USE_PYTHON']", ''),
-        (".//a[@href='#c.SphinxType']", ''),
-        (".//a[@href='#c.sphinx_global']", ''),
+        (".//a[@href='#c-sphinx-dosomething']", ''),
+        (".//a[@href='#c-sphinxstruct-member']", ''),
+        (".//a[@href='#c-sphinx-use-python']", ''),
+        (".//a[@href='#c-sphinxtype']", ''),
+        (".//a[@href='#c-sphinx-global']", ''),
         # test global TOC created by toctree()
         (".//ul[@class='current']/li[@class='toctree-l1 current']/a[@href='#']",
          'Testing object descriptions'),
diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py
new file mode 100644
index 000000000..4e5de2287
--- /dev/null
+++ b/tests/test_domain_c.py
@@ -0,0 +1,80 @@
+"""
+    test_domain_c
+    ~~~~~~~~~~~~~
+
+    Tests the C Domain
+
+    :copyright: Copyright 2007-2020 by the Sphinx team, see AUTHORS.
+    :license: BSD, see LICENSE for details.
+"""
+
+from docutils import nodes
+
+from sphinx import addnodes
+from sphinx.addnodes import (
+    desc, desc_addname, desc_annotation, desc_content, desc_name, desc_optional,
+    desc_parameter, desc_parameterlist, desc_returns, desc_signature, desc_type,
+    pending_xref
+)
+from sphinx.testing import restructuredtext
+from sphinx.testing.util import assert_node
+
+
+def test_cfunction(app):
+    text = (".. c:function:: PyObject* "
+            "PyType_GenericAlloc(PyTypeObject *type, Py_ssize_t nitems)")
+    doctree = restructuredtext.parse(app, text)
+    assert_node(doctree,
+                (addnodes.index,
+                 [desc, ([desc_signature, ([desc_type, ([pending_xref, "PyObject"],
+                                                        "* ")],
+                                           [desc_name, "PyType_GenericAlloc"],
+                                           [desc_parameterlist, (desc_parameter,
+                                                                 desc_parameter)])],
+                         desc_content)]))
+    assert_node(doctree[1], addnodes.desc, desctype="function",
+                domain="c", objtype="function", noindex=False)
+    assert_node(doctree[1][0][2][0],
+                [desc_parameter, ([pending_xref, "PyTypeObject"],
+                                  [nodes.emphasis, "\xa0*type"])])
+    assert_node(doctree[1][0][2][1],
+                [desc_parameter, ([pending_xref, "Py_ssize_t"],
+                                  [nodes.emphasis, "\xa0nitems"])])
+
+    domain = app.env.get_domain('c')
+    entry = domain.objects.get('PyType_GenericAlloc')
+    assert entry == ('index', 'c-pytype-genericalloc', 'function')
+
+
+def test_cmember(app):
+    text = ".. c:member:: PyObject* PyTypeObject.tp_bases"
+    doctree = restructuredtext.parse(app, text)
+    assert_node(doctree,
+                (addnodes.index,
+                 [desc, ([desc_signature, ([desc_type, ([pending_xref, "PyObject"],
+                                                        "* ")],
+                                           [desc_name, "PyTypeObject.tp_bases"])],
+                         desc_content)]))
+    assert_node(doctree[1], addnodes.desc, desctype="member",
+                domain="c", objtype="member", noindex=False)
+
+    domain = app.env.get_domain('c')
+    entry = domain.objects.get('PyTypeObject.tp_bases')
+    assert entry == ('index', 'c-pytypeobject-tp-bases', 'member')
+
+
+def test_cvar(app):
+    text = ".. c:var:: PyObject* PyClass_Type"
+    doctree = restructuredtext.parse(app, text)
+    assert_node(doctree,
+                (addnodes.index,
+                 [desc, ([desc_signature, ([desc_type, ([pending_xref, "PyObject"],
+                                                        "* ")],
+                                           [desc_name, "PyClass_Type"])],
+                         desc_content)]))
+    assert_node(doctree[1], addnodes.desc, desctype="var",
+                domain="c", objtype="var", noindex=False)
+
+    domain = app.env.get_domain('c')
+    entry = domain.objects.get('PyClass_Type')
+    assert entry == ('index', 'c-pyclass-type', 'var')