Merge pull request #7210 from tk0miya/refactor_js_domain2

js domain: Generate node_id in the right way

CHANGES

@@ -23,6 +23,10 @@ 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: js domain: Internal data structure has changed. Both objects and
+  modules have node_id for cross reference
+* #7210: js domain: Non intended behavior is removed such as ``parseInt_`` links
+  to ``.. js:function:: parseInt``
 
 Deprecated
 ----------

sphinx/domains/javascript.py

@@ -28,12 +28,30 @@ from sphinx.roles import XRefRole
 from sphinx.util import logging
 from sphinx.util.docfields import Field, GroupedField, TypedField
 from sphinx.util.docutils import SphinxDirective
-from sphinx.util.nodes import make_refnode
+from sphinx.util.nodes import make_id, make_refnode
 
 
 logger = logging.getLogger(__name__)
 
 
+def make_old_jsmod_id(modname: str) -> str:
+    """Generate old styled node_id for JS modules.
+
+    .. note:: Old Styled node_id was used until Sphinx-3.0.
+              This will be removed in Sphinx-5.0.
+    """
+    return 'module-' + modname
+
+
+def make_old_jsobj_id(fullname: str) -> str:
+    """Generate old styled node_id for JS objects.
+
+    .. note:: Old Styled node_id was used until Sphinx-3.0.
+              This will be removed in Sphinx-5.0.
+    """
+    return fullname.replace('$', '_S_')
+
+
 class JSObject(ObjectDescription):
     """
     Description of a JavaScript object.
@@ -106,19 +124,23 @@ class JSObject(ObjectDescription):
                              signode: desc_signature) -> None:
         mod_name = self.env.ref_context.get('js:module')
         fullname = (mod_name + '.' if mod_name else '') + name_obj[0]
-        if fullname not in self.state.document.ids:
+        node_id = make_id(self.env, self.state.document, '', fullname)
-            signode['names'].append(fullname)
+        signode['ids'].append(node_id)
-            signode['ids'].append(fullname.replace('$', '_S_'))
-            self.state.document.note_explicit_target(signode)
 
-            domain = cast(JavaScriptDomain, self.env.get_domain('js'))
+        # Assign old styled node_id not to break old hyperlinks (if possible)
-            domain.note_object(fullname, self.objtype, location=signode)
+        # Note: Will be removed in Sphinx-5.0 (RemovedInSphinx50Warning)
+        old_node_id = make_old_jsobj_id(fullname)
+        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(JavaScriptDomain, self.env.get_domain('js'))
+        domain.note_object(fullname, self.objtype, node_id, location=signode)
 
         indextext = self.get_index_text(mod_name, name_obj)
         if indextext:
-            self.indexnode['entries'].append(('single', indextext,
+            self.indexnode['entries'].append(('single', indextext, node_id, '', None))
-                                              fullname.replace('$', '_S_'),
-                                              '', None))
 
     def get_index_text(self, objectname: str, name_obj: Tuple[str, str]) -> str:
         name, obj = name_obj
@@ -249,18 +271,25 @@ class JSModule(SphinxDirective):
         if not noindex:
             domain = cast(JavaScriptDomain, self.env.get_domain('js'))
 
-            domain.note_module(mod_name)
+            node_id = make_id(self.env, self.state.document, 'module', mod_name)
+            domain.note_module(mod_name, node_id)
             # Make a duplicate entry in 'objects' to facilitate searching for
             # the module in JavaScriptDomain.find_obj()
-            domain.note_object(mod_name, 'module', location=(self.env.docname, self.lineno))
+            domain.note_object(mod_name, 'module', node_id,
+                               location=(self.env.docname, self.lineno))
 
-            targetnode = nodes.target('', '', ids=['module-' + mod_name],
+            target = nodes.target('', '', ids=[node_id], ismod=True)
-                                      ismod=True)
+
-            self.state.document.note_explicit_target(targetnode)
+            # Assign old styled node_id not to break old hyperlinks (if possible)
-            ret.append(targetnode)
+            # Note: Will be removed in Sphinx-5.0 (RemovedInSphinx50Warning)
+            old_node_id = make_old_jsmod_id(mod_name)
+            if old_node_id not in self.state.document.ids and old_node_id not in target['ids']:
+                target['ids'].append(old_node_id)
+
+            self.state.document.note_explicit_target(target)
+            ret.append(target)
             indextext = _('%s (module)') % mod_name
-            inode = addnodes.index(entries=[('single', indextext,
+            inode = addnodes.index(entries=[('single', indextext, node_id, '', None)])
-                                             'module-' + mod_name, '', None)])
             ret.append(inode)
         return ret
 
@@ -315,47 +344,48 @@ class JavaScriptDomain(Domain):
         'mod':   JSXRefRole(),
     }
     initial_data = {
-        'objects': {},  # fullname -> docname, objtype
+        'objects': {},  # fullname -> docname, node_id, objtype
-        'modules': {},  # modname  -> docname
+        'modules': {},  # modname  -> docname, node_id
     }  # type: Dict[str, Dict[str, Tuple[str, str]]]
 
     @property
-    def objects(self) -> Dict[str, Tuple[str, str]]:
+    def objects(self) -> Dict[str, Tuple[str, str, str]]:
-        return self.data.setdefault('objects', {})  # fullname -> docname, objtype
+        return self.data.setdefault('objects', {})  # fullname -> docname, node_id, objtype
 
-    def note_object(self, fullname: str, objtype: str, location: Any = None) -> None:
+    def note_object(self, fullname: str, objtype: str, node_id: str,
+                    location: Any = None) -> None:
         if fullname in self.objects:
             docname = self.objects[fullname][0]
-            logger.warning(__('duplicate object description of %s, other instance in %s'),
+            logger.warning(__('duplicate %s description of %s, other %s in %s'),
-                           fullname, docname, location=location)
+                           objtype, fullname, objtype, docname, location=location)
-        self.objects[fullname] = (self.env.docname, objtype)
+        self.objects[fullname] = (self.env.docname, node_id, objtype)
 
     @property
-    def modules(self) -> Dict[str, str]:
+    def modules(self) -> Dict[str, Tuple[str, str]]:
-        return self.data.setdefault('modules', {})  # modname -> docname
+        return self.data.setdefault('modules', {})  # modname -> docname, node_id
 
-    def note_module(self, modname: str) -> None:
+    def note_module(self, modname: str, node_id: str) -> None:
-        self.modules[modname] = self.env.docname
+        self.modules[modname] = (self.env.docname, node_id)
 
     def clear_doc(self, docname: str) -> None:
-        for fullname, (pkg_docname, _l) in list(self.objects.items()):
+        for fullname, (pkg_docname, node_id, _l) in list(self.objects.items()):
             if pkg_docname == docname:
                 del self.objects[fullname]
-        for modname, pkg_docname in list(self.modules.items()):
+        for modname, (pkg_docname, node_id) in list(self.modules.items()):
             if pkg_docname == 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 mod_name, pkg_docname in otherdata['modules'].items():
+        for mod_name, (pkg_docname, node_id) in otherdata['modules'].items():
             if pkg_docname in docnames:
-                self.modules[mod_name] = pkg_docname
+                self.modules[mod_name] = (pkg_docname, node_id)
 
     def find_obj(self, env: BuildEnvironment, mod_name: str, prefix: str, name: str,
-                 typ: str, searchorder: int = 0) -> Tuple[str, Tuple[str, str]]:
+                 typ: str, searchorder: int = 0) -> Tuple[str, Tuple[str, str, str]]:
         if name[-2:] == '()':
             name = name[:-2]
 
@@ -387,8 +417,7 @@ class JavaScriptDomain(Domain):
         name, obj = self.find_obj(env, mod_name, prefix, target, typ, searchorder)
         if not obj:
             return None
-        return make_refnode(builder, fromdocname, obj[0],
+        return make_refnode(builder, fromdocname, obj[0], obj[1], contnode, name)
-                            name.replace('$', '_S_'), contnode, name)
 
     def resolve_any_xref(self, env: BuildEnvironment, fromdocname: str, builder: Builder,
                          target: str, node: pending_xref, contnode: Element
@@ -398,13 +427,12 @@ class JavaScriptDomain(Domain):
         name, obj = self.find_obj(env, mod_name, prefix, target, None, 1)
         if not obj:
             return []
-        return [('js:' + self.role_for_objtype(obj[1]),
+        return [('js:' + self.role_for_objtype(obj[2]),
-                 make_refnode(builder, fromdocname, obj[0],
+                 make_refnode(builder, fromdocname, obj[0], obj[1], contnode, name))]
-                              name.replace('$', '_S_'), contnode, name))]
 
     def get_objects(self) -> Iterator[Tuple[str, str, str, str, str, int]]:
-        for refname, (docname, type) in list(self.objects.items()):
+        for refname, (docname, node_id, typ) in list(self.objects.items()):
-            yield refname, refname, type, docname, refname.replace('$', '_S_'), 1
+            yield refname, refname, typ, docname, node_id, 1
 
     def get_full_qualified_name(self, node: Element) -> str:
         modname = node.get('js:module')
@@ -421,7 +449,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,
     }

sphinx/util/nodes.py

@@ -443,7 +443,7 @@ def make_id(env: "BuildEnvironment", document: nodes.document,
     if prefix:
         idformat = prefix + "-%s"
     else:
-        idformat = document.settings.id_prefix + "%s"
+        idformat = (document.settings.id_prefix or "id") + "%s"
 
     # try to generate node_id by *term*
     if prefix and term:
@@ -451,6 +451,10 @@ def make_id(env: "BuildEnvironment", document: nodes.document,
         if node_id == prefix:
             # *term* is not good to generate a node_id.
             node_id = None
+    elif term:
+        node_id = nodes.make_id(term)
+        if node_id == '':
+            node_id = None  # fallback to None
 
     while node_id is None or node_id in document.ids:
         node_id = idformat % env.new_serialno(prefix)

tests/test_domain_js.py

@@ -91,22 +91,22 @@ def test_domain_js_objects(app, status, warning):
     assert 'module_b.submodule' in modules
     assert 'module_b.submodule' in objects
 
-    assert objects['module_a.submodule.ModTopLevel'] == ('module', 'class')
+    assert objects['module_a.submodule.ModTopLevel'][2] == 'class'
-    assert objects['module_a.submodule.ModTopLevel.mod_child_1'] == ('module', 'method')
+    assert objects['module_a.submodule.ModTopLevel.mod_child_1'][2] == 'method'
-    assert objects['module_a.submodule.ModTopLevel.mod_child_2'] == ('module', 'method')
+    assert objects['module_a.submodule.ModTopLevel.mod_child_2'][2] == 'method'
-    assert objects['module_b.submodule.ModTopLevel'] == ('module', 'class')
+    assert objects['module_b.submodule.ModTopLevel'][2] == 'class'
 
-    assert objects['TopLevel'] == ('roles', 'class')
+    assert objects['TopLevel'][2] == 'class'
-    assert objects['top_level'] == ('roles', 'function')
+    assert objects['top_level'][2] == 'function'
-    assert objects['NestedParentA'] == ('roles', 'class')
+    assert objects['NestedParentA'][2] == 'class'
-    assert objects['NestedParentA.child_1'] == ('roles', 'function')
+    assert objects['NestedParentA.child_1'][2] == 'function'
-    assert objects['NestedParentA.any_child'] == ('roles', 'function')
+    assert objects['NestedParentA.any_child'][2] == 'function'
-    assert objects['NestedParentA.NestedChildA'] == ('roles', 'class')
+    assert objects['NestedParentA.NestedChildA'][2] == 'class'
-    assert objects['NestedParentA.NestedChildA.subchild_1'] == ('roles', 'function')
+    assert objects['NestedParentA.NestedChildA.subchild_1'][2] == 'function'
-    assert objects['NestedParentA.NestedChildA.subchild_2'] == ('roles', 'function')
+    assert objects['NestedParentA.NestedChildA.subchild_2'][2] == 'function'
-    assert objects['NestedParentA.child_2'] == ('roles', 'function')
+    assert objects['NestedParentA.child_2'][2] == 'function'
-    assert objects['NestedParentB'] == ('roles', 'class')
+    assert objects['NestedParentB'][2] == 'class'
-    assert objects['NestedParentB.child_1'] == ('roles', 'function')
+    assert objects['NestedParentB.child_1'][2] == 'function'
 
 
 @pytest.mark.sphinx('dummy', testroot='domain-js')
@@ -120,21 +120,28 @@ def test_domain_js_find_obj(app, status, warning):
 
     assert (find_obj(None, None, 'NONEXISTANT', 'class') == (None, None))
     assert (find_obj(None, None, 'NestedParentA', 'class') ==
-            ('NestedParentA', ('roles', 'class')))
+            ('NestedParentA', ('roles', 'nestedparenta', 'class')))
     assert (find_obj(None, None, 'NestedParentA.NestedChildA', 'class') ==
-            ('NestedParentA.NestedChildA', ('roles', 'class')))
+            ('NestedParentA.NestedChildA',
+             ('roles', 'nestedparenta-nestedchilda', 'class')))
     assert (find_obj(None, 'NestedParentA', 'NestedChildA', 'class') ==
-            ('NestedParentA.NestedChildA', ('roles', 'class')))
+            ('NestedParentA.NestedChildA',
+             ('roles', 'nestedparenta-nestedchilda', 'class')))
     assert (find_obj(None, None, 'NestedParentA.NestedChildA.subchild_1', 'func') ==
-            ('NestedParentA.NestedChildA.subchild_1', ('roles', 'function')))
+            ('NestedParentA.NestedChildA.subchild_1',
+             ('roles', 'nestedparenta-nestedchilda-subchild-1', 'function')))
     assert (find_obj(None, 'NestedParentA', 'NestedChildA.subchild_1', 'func') ==
-            ('NestedParentA.NestedChildA.subchild_1', ('roles', 'function')))
+            ('NestedParentA.NestedChildA.subchild_1',
+             ('roles', 'nestedparenta-nestedchilda-subchild-1', 'function')))
     assert (find_obj(None, 'NestedParentA.NestedChildA', 'subchild_1', 'func') ==
-            ('NestedParentA.NestedChildA.subchild_1', ('roles', 'function')))
+            ('NestedParentA.NestedChildA.subchild_1',
+             ('roles', 'nestedparenta-nestedchilda-subchild-1', 'function')))
     assert (find_obj('module_a.submodule', 'ModTopLevel', 'mod_child_2', 'meth') ==
-            ('module_a.submodule.ModTopLevel.mod_child_2', ('module', 'method')))
+            ('module_a.submodule.ModTopLevel.mod_child_2',
+             ('module', 'module-a-submodule-modtoplevel-mod-child-2', 'method')))
     assert (find_obj('module_b.submodule', 'ModTopLevel', 'module_a.submodule', 'mod') ==
-            ('module_a.submodule', ('module', 'module')))
+            ('module_a.submodule',
+             ('module', 'module-module-a-submodule', 'module')))
 
 
 def test_get_full_qualified_name():
@@ -198,7 +205,7 @@ def test_js_class(app):
                                                     [desc_parameterlist, ()])],
                                   [desc_content, ()])]))
     assert_node(doctree[0], addnodes.index,
-                entries=[("single", "Application() (class)", "Application", "", None)])
+                entries=[("single", "Application() (class)", "application", "", None)])
     assert_node(doctree[1], addnodes.desc, domain="js", objtype="class", noindex=False)