Merge pull request #7236 from tk0miya/refactor_py_domain2

py domain: Generate node_id for objects and modules in the right way

CHANGES

@@ -31,6 +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. 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
 ----------

doc/extdev/appapi.rst

@@ -45,7 +45,6 @@ package.
 
 .. automethod:: Sphinx.add_enumerable_node(node, figtype, title_getter=None, \*\*kwds)
 
-.. method:: Sphinx.add_directive(name, func, content, arguments, \*\*options)
 .. automethod:: Sphinx.add_directive(name, directiveclass)
 
 .. automethod:: Sphinx.add_role(name, role)
@@ -54,7 +53,6 @@ package.
 
 .. automethod:: Sphinx.add_domain(domain)
 
-.. method:: Sphinx.add_directive_to_domain(domain, name, func, content, arguments, \*\*options)
 .. automethod:: Sphinx.add_directive_to_domain(domain, name, directiveclass)
 
 .. automethod:: Sphinx.add_role_to_domain(domain, name, role)
@@ -107,6 +105,7 @@ Emitting events
 ---------------
 
 .. class:: Sphinx
+   :noindex:
 
    .. automethod:: emit(event, \*arguments)
 

doc/templating.rst

@@ -227,6 +227,7 @@ them to generate links or output multiply used elements.
    documents.
 
 .. function:: pathto(file, 1)
+   :noindex:
 
    Return the path to a *file* which is a filename relative to the root of the
    generated output.  Use this to refer to static files.
@@ -413,10 +414,6 @@ are in HTML form), these variables are also available:
    nonempty if the :confval:`html_copy_source` value is ``True``.
    This has empty value on creating automatically-generated files.
 
-.. data:: title
-
-   The page title.
-
 .. data:: toc
 
    The local table of contents for the current page, rendered as HTML bullet

doc/usage/extensions/autosummary.rst

@@ -273,6 +273,7 @@ Additionally, the following filters are available
    replaces the builtin Jinja `escape filter`_ that does html-escaping.
 
 .. function:: underline(s, line='=')
+   :noindex:
 
    Add a title underline to a piece of text.
 

sphinx/domains/python.py

@@ -32,7 +32,7 @@
 from sphinx.util.docfields import Field, GroupedField, TypedField
 from sphinx.util.docutils import SphinxDirective
 from sphinx.util.inspect import signature_from_str
-from sphinx.util.nodes import make_refnode
+from sphinx.util.nodes import make_id, make_refnode
 from sphinx.util.typing import TextlikeNode
 
 if False:
@@ -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
@@ -788,24 +791,43 @@ def run(self) -> List[Node]:
         ret = []  # type: List[Node]
         if not noindex:
             # note module to the domain
+            node_id = make_id(self.env, self.state.document, 'module', modname)
+            target = nodes.target('', '', ids=[node_id], ismod=True)
+            self.set_source_info(target)
+
+            # Assign old styled node_id not to break old hyperlinks (if possible)
+            # Note: Will removed in Sphinx-5.0  (RemovedInSphinx50Warning)
+            old_node_id = self.make_old_id(modname)
+            if node_id != old_node_id and old_node_id not in self.state.document.ids:
+                target['ids'].append(old_node_id)
+
+            self.state.document.note_explicit_target(target)
+
             domain.note_module(modname,
+                               node_id,
                                self.options.get('synopsis', ''),
                                self.options.get('platform', ''),
                                'deprecated' in self.options)
-            domain.note_object(modname, 'module', location=(self.env.docname, self.lineno))
+            domain.note_object(modname, 'module', node_id, location=target)
 
-            targetnode = nodes.target('', '', ids=['module-' + modname],
-                                      ismod=True)
-            self.state.document.note_explicit_target(targetnode)
             # the platform and synopsis aren't printed; in fact, they are only
             # used in the modindex currently
-            ret.append(targetnode)
+            ret.append(target)
             indextext = _('%s (module)') % modname
-            inode = addnodes.index(entries=[('single', indextext,
-                                             'module-' + modname, '', None)])
+            inode = addnodes.index(entries=[('single', indextext, node_id, '', None)])
             ret.append(inode)
         return ret
 
+    def make_old_id(self, name: str) -> str:
+        """Generate old styled node_id.
+
+        Old styled node_id is incompatible with docutils' node_id.
+        It can contain dots and hyphens.
+
+        .. note:: Old styled node_id was mainly used until Sphinx-3.0.
+        """
+        return 'module-%s' % name
+
 
 class PyCurrentModule(SphinxDirective):
     """
@@ -888,7 +910,7 @@ def generate(self, docnames: Iterable[str] = None
         # sort out collapsable modules
         prev_modname = ''
         num_toplevels = 0
-        for modname, (docname, synopsis, platforms, deprecated) in modules:
+        for modname, (docname, node_id, synopsis, platforms, deprecated) in modules:
             if docnames and docname not in docnames:
                 continue
 
@@ -925,8 +947,7 @@ def generate(self, docnames: Iterable[str] = None
 
             qualifier = _('Deprecated') if deprecated else ''
             entries.append(IndexEntry(stripped + modname, subtype, docname,
-                                      'module-' + stripped + modname, platforms,
-                                      qualifier, synopsis))
+                                      node_id, platforms, qualifier, synopsis))
             prev_modname = modname
 
         # apply heuristics when to collapse modindex at page load:
@@ -990,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
@@ -1003,39 +1024,40 @@ 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, bool]]:
-        return self.data.setdefault('modules', {})  # modname -> docname, synopsis, platform, deprecated  # NOQA
+    def modules(self) -> Dict[str, Tuple[str, str, str, str, bool]]:
+        return self.data.setdefault('modules', {})  # modname -> docname, node_id, synopsis, platform, deprecated  # NOQA
 
-    def note_module(self, name: str, synopsis: str, platform: str, deprecated: bool) -> None:
+    def note_module(self, name: str, node_id: str, synopsis: str,
+                    platform: str, deprecated: bool) -> None:
         """Note a python module for cross reference.
 
         .. versionadded:: 2.1
         """
-        self.modules[name] = (self.env.docname, synopsis, platform, deprecated)
+        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, _y) in list(self.modules.items()):
+        for modname, (fn, _x, _x, _x, _y) in list(self.modules.items()):
             if fn == docname:
                 del self.modules[modname]
 
     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.
         """
@@ -1046,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:
@@ -1057,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:
@@ -1118,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
@@ -1133,36 +1155,35 @@ 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
 
     def _make_module_refnode(self, builder: Builder, fromdocname: str, name: str,
                              contnode: Node) -> Element:
         # get additional info for modules
-        docname, synopsis, platform, deprecated = self.modules[name]
+        docname, node_id, synopsis, platform, deprecated = self.modules[name]
         title = name
         if synopsis:
             title += ': ' + synopsis
         if deprecated:
             title += _(' (deprecated)')
         if platform:
             title += ' (' + platform + ')'
-        return make_refnode(builder, fromdocname, docname,
-                            'module-' + name, contnode, title)
+        return make_refnode(builder, fromdocname, docname, node_id, contnode, title)
 
     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], 'module-' + modname, 0)
-        for refname, (docname, type) in self.objects.items():
+            yield (modname, modname, 'module', info[0], info[1], 0)
+        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')
@@ -1182,7 +1203,7 @@ def setup(app: Sphinx) -> Dict[str, Any]:
 
     return {
         'version': 'builtin',
-        'env_version': 1,
+        'env_version': 2,
         'parallel_read_safe': True,
         'parallel_write_safe': True,
     }

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']", ''),