py domain: Generate node_id for objects in the right way

CHANGES

@@ -31,8 +31,10 @@ Incompatible changes
   node_id for cross reference
 * #7229: rst domain: Non intended behavior is removed such as ``numref_`` links
   to ``.. rst:role:: numref``
-* #6903: py domain: Internal data structure has changed. Now modules have
-  node_id for cross reference
+* #6903: py domain: Internal data structure has changed. Both objects and
+  modules have node_id for cross reference
+* #6903: py domain: Non intended behavior is removed such as ``say_hello_``
+  links to ``.. py:function:: say_hello()``
 
 Deprecated
 ----------

sphinx/domains/python.py

@@ -358,19 +358,22 @@ def add_target_and_index(self, name_cls: Tuple[str, str], sig: str,
                              signode: desc_signature) -> None:
         modname = self.options.get('module', self.env.ref_context.get('py:module'))
         fullname = (modname + '.' if modname else '') + name_cls[0]
-        # note target
-        if fullname not in self.state.document.ids:
-            signode['names'].append(fullname)
+        node_id = make_id(self.env, self.state.document, modname or '', name_cls[0])
+        signode['ids'].append(node_id)
+
+        # Assign old styled node_id(fullname) not to break old hyperlinks (if possible)
+        # Note: Will removed in Sphinx-5.0  (RemovedInSphinx50Warning)
+        if node_id != fullname and fullname not in self.state.document.ids:
             signode['ids'].append(fullname)
-            self.state.document.note_explicit_target(signode)
 
-            domain = cast(PythonDomain, self.env.get_domain('py'))
-            domain.note_object(fullname, self.objtype, location=signode)
+        self.state.document.note_explicit_target(signode)
+
+        domain = cast(PythonDomain, self.env.get_domain('py'))
+        domain.note_object(fullname, self.objtype, node_id, location=signode)
 
         indextext = self.get_index_text(modname, name_cls)
         if indextext:
-            self.indexnode['entries'].append(('single', indextext,
-                                              fullname, '', None))
+            self.indexnode['entries'].append(('single', indextext, node_id, '', None))
 
     def before_content(self) -> None:
         """Handle object nesting before content
@@ -805,7 +808,7 @@ def run(self) -> List[Node]:
                                self.options.get('synopsis', ''),
                                self.options.get('platform', ''),
                                'deprecated' in self.options)
-            domain.note_object(modname, 'module', location=target)
+            domain.note_object(modname, 'module', node_id, location=target)
 
             # the platform and synopsis aren't printed; in fact, they are only
             # used in the modindex currently
@@ -1008,10 +1011,10 @@ class PythonDomain(Domain):
     ]
 
     @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:
         """Note a python object for cross reference.
 
         .. versionadded:: 2.1
@@ -1021,7 +1024,7 @@ def note_object(self, name: str, objtype: str, location: Any = None) -> None:
             logger.warning(__('duplicate 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)
 
     @property
     def modules(self) -> Dict[str, Tuple[str, str, str, str, bool]]:
@@ -1036,7 +1039,7 @@ def note_module(self, name: str, node_id: str, synopsis: str,
         self.modules[name] = (self.env.docname, node_id, synopsis, platform, deprecated)
 
     def clear_doc(self, docname: str) -> None:
-        for fullname, (fn, _l) in list(self.objects.items()):
+        for fullname, (fn, _x, _x) in list(self.objects.items()):
             if fn == docname:
                 del self.objects[fullname]
         for modname, (fn, _x, _x, _x, _y) in list(self.modules.items()):
@@ -1045,16 +1048,16 @@ def clear_doc(self, docname: str) -> None:
 
     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.objects[fullname] = (fn, objtype)
+                self.objects[fullname] = (fn, node_id, objtype)
         for modname, data in otherdata['modules'].items():
             if data[0] in docnames:
                 self.modules[modname] = data
 
     def find_obj(self, env: BuildEnvironment, modname: str, classname: str,
                  name: str, type: str, searchmode: int = 0
-                 ) -> List[Tuple[str, Tuple[str, str]]]:
+                 ) -> List[Tuple[str, Tuple[str, str, str]]]:
         """Find a Python object for "name", perhaps using the given module
         and/or classname.  Returns a list of (name, object entry) tuples.
         """
@@ -1065,7 +1068,7 @@ def find_obj(self, env: BuildEnvironment, modname: str, classname: str,
         if not name:
             return []
 
-        matches = []  # type: List[Tuple[str, Tuple[str, str]]]
+        matches = []  # type: List[Tuple[str, Tuple[str, str, str]]]
 
         newname = None
         if searchmode == 1:
@@ -1076,20 +1079,20 @@ def find_obj(self, env: BuildEnvironment, modname: str, classname: str,
             if objtypes is not None:
                 if modname and classname:
                     fullname = modname + '.' + classname + '.' + name
-                    if fullname in self.objects and self.objects[fullname][1] in objtypes:
+                    if fullname in self.objects and self.objects[fullname][2] in objtypes:
                         newname = fullname
                 if not newname:
                     if modname and modname + '.' + name in self.objects and \
-                       self.objects[modname + '.' + name][1] in objtypes:
+                       self.objects[modname + '.' + name][2] in objtypes:
                         newname = modname + '.' + name
-                    elif name in self.objects and self.objects[name][1] in objtypes:
+                    elif name in self.objects and self.objects[name][2] in objtypes:
                         newname = name
                     else:
                         # "fuzzy" searching mode
                         searchname = '.' + name
                         matches = [(oname, self.objects[oname]) for oname in self.objects
                                    if oname.endswith(searchname) and
-                                   self.objects[oname][1] in objtypes]
+                                   self.objects[oname][2] in objtypes]
         else:
             # NOTE: searching for exact match, object type is not considered
             if name in self.objects:
@@ -1137,10 +1140,10 @@ def resolve_xref(self, env: BuildEnvironment, fromdocname: str, builder: Builder
                            type='ref', subtype='python', location=node)
         name, obj = matches[0]
 
-        if obj[1] == 'module':
+        if obj[2] == 'module':
             return self._make_module_refnode(builder, fromdocname, name, contnode)
         else:
-            return make_refnode(builder, fromdocname, obj[0], name, contnode, name)
+            return make_refnode(builder, fromdocname, obj[0], obj[1], contnode, name)
 
     def resolve_any_xref(self, env: BuildEnvironment, fromdocname: str, builder: Builder,
                          target: str, node: pending_xref, contnode: Element
@@ -1152,13 +1155,13 @@ def resolve_any_xref(self, env: BuildEnvironment, fromdocname: str, builder: Bui
         # always search in "refspecific" mode with the :any: role
         matches = self.find_obj(env, modname, clsname, target, None, 1)
         for name, obj in matches:
-            if obj[1] == 'module':
+            if obj[2] == 'module':
                 results.append(('py:mod',
                                 self._make_module_refnode(builder, fromdocname,
                                                           name, contnode)))
             else:
-                results.append(('py:' + self.role_for_objtype(obj[1]),
-                                make_refnode(builder, fromdocname, obj[0], name,
+                results.append(('py:' + self.role_for_objtype(obj[2]),
+                                make_refnode(builder, fromdocname, obj[0], obj[1],
                                              contnode, name)))
         return results
 
@@ -1178,9 +1181,9 @@ def _make_module_refnode(self, builder: Builder, fromdocname: str, name: str,
     def get_objects(self) -> Iterator[Tuple[str, str, str, str, str, int]]:
         for modname, info in self.modules.items():
             yield (modname, modname, 'module', info[0], info[1], 0)
-        for refname, (docname, type) in self.objects.items():
+        for refname, (docname, node_id, type) in self.objects.items():
             if type != 'module':  # modules are already handled
-                yield (refname, refname, type, docname, refname, 1)
+                yield (refname, refname, type, docname, node_id, 1)
 
     def get_full_qualified_name(self, node: Element) -> str:
         modname = node.get('py:module')

tests/test_build_html.py

@@ -176,8 +176,8 @@ def test_html4_output(app, status, warning):
          r'-|      |-'),
     ],
     'autodoc.html': [
-        (".//dl[@class='py class']/dt[@id='autodoc_target.Class']", ''),
-        (".//dl[@class='py function']/dt[@id='autodoc_target.function']/em", r'\*\*kwds'),
+        (".//dl[@class='py class']/dt[@id='autodoc-target-class']", ''),
+        (".//dl[@class='py function']/dt[@id='autodoc-target-function']/em", r'\*\*kwds'),
         (".//dd/p", r'Return spam\.'),
     ],
     'extapi.html': [
@@ -262,7 +262,7 @@ def test_html4_output(app, status, warning):
         (".//p", 'Always present'),
         # tests for ``any`` role
         (".//a[@href='#with']/span", 'headings'),
-        (".//a[@href='objects.html#func_without_body']/code/span", 'objects'),
+        (".//a[@href='objects.html#func-without-body']/code/span", 'objects'),
         # tests for numeric labels
         (".//a[@href='#id1'][@class='reference internal']/span", 'Testing various markup'),
         # tests for smartypants
@@ -274,18 +274,18 @@ def test_html4_output(app, status, warning):
         (".//p", 'Il dit : « C’est “super” ! »'),
     ],
     'objects.html': [
-        (".//dt[@id='mod.Cls.meth1']", ''),
-        (".//dt[@id='errmod.Error']", ''),
+        (".//dt[@id='mod-cls-meth1']", ''),
+        (".//dt[@id='errmod-error']", ''),
         (".//dt/code", r'long\(parameter,\s* list\)'),
         (".//dt/code", 'another one'),
-        (".//a[@href='#mod.Cls'][@class='reference internal']", ''),
+        (".//a[@href='#mod-cls'][@class='reference internal']", ''),
         (".//dl[@class='std userdesc']", ''),
         (".//dt[@id='userdesc-myobj']", ''),
         (".//a[@href='#userdesc-myobj'][@class='reference internal']", ''),
         # docfields
-        (".//a[@class='reference internal'][@href='#TimeInt']/em", 'TimeInt'),
-        (".//a[@class='reference internal'][@href='#Time']", 'Time'),
-        (".//a[@class='reference internal'][@href='#errmod.Error']/strong", 'Error'),
+        (".//a[@class='reference internal'][@href='#timeint']/em", 'TimeInt'),
+        (".//a[@class='reference internal'][@href='#time']", 'Time'),
+        (".//a[@class='reference internal'][@href='#errmod-error']/strong", 'Error'),
         # C references
         (".//span[@class='pre']", 'CFunction()'),
         (".//a[@href='#c.Sphinx_DoSomething']", ''),