Intersphinx, refactoring

Also, when a reference is unresolved, don't strip the inventory prefix.

4 files changed, 184 insertions, 98 deletions

diff --git a/CHANGES b/CHANGES
index 414fe0be1..ffb1cdc47 100644
--- a/CHANGES
+++ b/CHANGES
@@ -85,6 +85,8 @@ Bugs fixed
 * #9733: Fix for logging handler flushing warnings in the middle of the docs
   build
 * #9656: Fix warnings without subtype being incorrectly suppressed
+* Intersphinx, for unresolved references with an explicit inventory,
+  e.g., ``proj:myFunc``, leave the inventory prefix in the unresolved text.
 
 Testing
 --------
diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py
index 4795d1ae2..848a12eb4 100644
--- a/sphinx/ext/intersphinx.py
+++ b/sphinx/ext/intersphinx.py
@@ -29,11 +29,11 @@ import posixpath
 import sys
 import time
 from os import path
-from typing import IO, Any, Dict, List, Tuple
+from typing import IO, Any, Dict, List, Optional, Tuple
 from urllib.parse import urlsplit, urlunsplit
 
 from docutils import nodes
-from docutils.nodes import TextElement
+from docutils.nodes import Element, TextElement
 from docutils.utils import relative_path
 
 import sphinx
@@ -41,11 +41,12 @@ from sphinx.addnodes import pending_xref
 from sphinx.application import Sphinx
 from sphinx.builders.html import INVENTORY_FILENAME
 from sphinx.config import Config
+from sphinx.domains import Domain
 from sphinx.environment import BuildEnvironment
 from sphinx.locale import _, __
 from sphinx.util import logging, requests
 from sphinx.util.inventory import InventoryFile
-from sphinx.util.typing import Inventory
+from sphinx.util.typing import Inventory, InventoryInner
 
 logger = logging.getLogger(__name__)
 
@@ -258,105 +259,187 @@ def load_mappings(app: Sphinx) -> None:
                 inventories.main_inventory.setdefault(type, {}).update(objects)
 
 
-def missing_reference(app: Sphinx, env: BuildEnvironment, node: pending_xref,
-                      contnode: TextElement) -> nodes.reference:
-    """Attempt to resolve a missing reference via intersphinx references."""
-    target = node['reftarget']
-    inventories = InventoryAdapter(env)
-    objtypes: List[str] = None
-    if node['reftype'] == 'any':
-        # we search anything!
-        objtypes = ['%s:%s' % (domain.name, objtype)
-                    for domain in env.domains.values()
-                    for objtype in domain.object_types]
-        domain = None
+def _create_element_from_result(domain: Domain, inv_name: Optional[str],
+                                data: InventoryInner,
+                                node: pending_xref, contnode: TextElement) -> Element:
+    proj, version, uri, dispname = data
+    if '://' not in uri and node.get('refdoc'):
+        # get correct path in case of subdirectories
+        uri = path.join(relative_path(node['refdoc'], '.'), uri)
+    if version:
+        reftitle = _('(in %s v%s)') % (proj, version)
+    else:
+        reftitle = _('(in %s)') % (proj,)
+    newnode = nodes.reference('', '', internal=False, refuri=uri, reftitle=reftitle)
+    if node.get('refexplicit'):
+        # use whatever title was given
+        newnode.append(contnode)
+    elif dispname == '-' or \
+            (domain.name == 'std' and node['reftype'] == 'keyword'):
+        # use whatever title was given, but strip prefix
+        title = contnode.astext()
+        if inv_name is not None and title.startswith(inv_name + ':'):
+            newnode.append(contnode.__class__(title[len(inv_name) + 1:],
+                                              title[len(inv_name) + 1:]))
+        else:
+            newnode.append(contnode)
+    else:
+        # else use the given display name (used for :ref:)
+        newnode.append(contnode.__class__(dispname, dispname))
+    return newnode
+
+
+def _resolve_reference_in_domain_by_target(
+        inv_name: Optional[str], inventory: Inventory,
+        domain: Domain, objtypes: List[str],
+        target: str,
+        node: pending_xref, contnode: TextElement) -> Optional[Element]:
+    for objtype in objtypes:
+        if objtype not in inventory:
+            # Continue if there's nothing of this kind in the inventory
+            continue
+
+        if target in inventory[objtype]:
+            # Case sensitive match, use it
+            data = inventory[objtype][target]
+        elif objtype == 'std:term':
+            # Check for potential case insensitive matches for terms only
+            target_lower = target.lower()
+            insensitive_matches = list(filter(lambda k: k.lower() == target_lower,
+                                              inventory[objtype].keys()))
+            if insensitive_matches:
+                data = inventory[objtype][insensitive_matches[0]]
+            else:
+                # No case insensitive match either, continue to the next candidate
+                continue
+        else:
+            # Could reach here if we're not a term but have a case insensitive match.
+            # This is a fix for terms specifically, but potentially should apply to
+            # other types.
+            continue
+        return _create_element_from_result(domain, inv_name, data, node, contnode)
+    return None
+
+
+def _resolve_reference_in_domain(inv_name: Optional[str], inventory: Inventory,
+                                 domain: Domain, objtypes: List[str],
+                                 node: pending_xref, contnode: TextElement
+                                 ) -> Optional[Element]:
+    # we adjust the object types for backwards compatibility
+    if domain.name == 'std' and 'cmdoption' in objtypes:
+        # until Sphinx-1.6, cmdoptions are stored as std:option
+        objtypes.append('option')
+    if domain.name == 'py' and 'attribute' in objtypes:
+        # Since Sphinx-2.1, properties are stored as py:method
+        objtypes.append('method')
+
+    # the inventory contains domain:type as objtype
+    objtypes = ["{}:{}".format(domain.name, t) for t in objtypes]
+
+    # without qualification
+    res = _resolve_reference_in_domain_by_target(inv_name, inventory, domain, objtypes,
+                                                 node['reftarget'], node, contnode)
+    if res is not None:
+        return res
+
+    # try with qualification of the current scope instead
+    full_qualified_name = domain.get_full_qualified_name(node)
+    if full_qualified_name is None:
+        return None
+    return _resolve_reference_in_domain_by_target(inv_name, inventory, domain, objtypes,
+                                                  full_qualified_name, node, contnode)
+
+
+def _resolve_reference(env: BuildEnvironment, inv_name: Optional[str], inventory: Inventory,
+                       node: pending_xref, contnode: TextElement) -> Optional[Element]:
+    # figure out which object types we should look for
+    typ = node['reftype']
+    if typ == 'any':
+        for domain_name, domain in env.domains.items():
+            objtypes = list(domain.object_types)
+            res = _resolve_reference_in_domain(inv_name, inventory,
+                                               domain, objtypes,
+                                               node, contnode)
+            if res is not None:
+                return res
+        return None
     else:
-        domain = node.get('refdomain')
-        if not domain:
+        domain_name = node.get('refdomain')
+        if not domain_name:
             # only objects in domains are in the inventory
             return None
-        objtypes = env.get_domain(domain).objtypes_for_role(node['reftype'])
+        domain = env.get_domain(domain_name)
+        objtypes = domain.objtypes_for_role(typ)
         if not objtypes:
             return None
-        objtypes = ['%s:%s' % (domain, objtype) for objtype in objtypes]
-    if 'std:cmdoption' in objtypes:
-        # until Sphinx-1.6, cmdoptions are stored as std:option
-        objtypes.append('std:option')
-    if 'py:attribute' in objtypes:
-        # Since Sphinx-2.1, properties are stored as py:method
-        objtypes.append('py:method')
-
-    to_try = [(inventories.main_inventory, target)]
-    if domain:
-        full_qualified_name = env.get_domain(domain).get_full_qualified_name(node)
-        if full_qualified_name:
-            to_try.append((inventories.main_inventory, full_qualified_name))
-    in_set = None
-    if ':' in target:
-        # first part may be the foreign doc set name
-        setname, newtarget = target.split(':', 1)
-        if setname in inventories.named_inventory:
-            in_set = setname
-            to_try.append((inventories.named_inventory[setname], newtarget))
-            if domain:
-                node['reftarget'] = newtarget
-                full_qualified_name = env.get_domain(domain).get_full_qualified_name(node)
-                if full_qualified_name:
-                    to_try.append((inventories.named_inventory[setname], full_qualified_name))
-    for inventory, target in to_try:
-        for objtype in objtypes:
-            if objtype not in inventory:
-                # Continue if there's nothing of this kind in the inventory
-                continue
-            if target in inventory[objtype]:
-                # Case sensitive match, use it
-                proj, version, uri, dispname = inventory[objtype][target]
-            elif objtype == 'std:term':
-                # Check for potential case insensitive matches for terms only
-                target_lower = target.lower()
-                insensitive_matches = list(filter(lambda k: k.lower() == target_lower,
-                                                  inventory[objtype].keys()))
-                if insensitive_matches:
-                    proj, version, uri, dispname = inventory[objtype][insensitive_matches[0]]
-                else:
-                    # No case insensitive match either, continue to the next candidate
-                    continue
-            else:
-                # Could reach here if we're not a term but have a case insensitive match.
-                # This is a fix for terms specifically, but potentially should apply to
-                # other types.
-                continue
+        return _resolve_reference_in_domain(inv_name, inventory,
+                                            domain, objtypes,
+                                            node, contnode)
 
-            if '://' not in uri and node.get('refdoc'):
-                # get correct path in case of subdirectories
-                uri = path.join(relative_path(node['refdoc'], '.'), uri)
-            if version:
-                reftitle = _('(in %s v%s)') % (proj, version)
-            else:
-                reftitle = _('(in %s)') % (proj,)
-            newnode = nodes.reference('', '', internal=False, refuri=uri, reftitle=reftitle)
-            if node.get('refexplicit'):
-                # use whatever title was given
-                newnode.append(contnode)
-            elif dispname == '-' or \
-                    (domain == 'std' and node['reftype'] == 'keyword'):
-                # use whatever title was given, but strip prefix
-                title = contnode.astext()
-                if in_set and title.startswith(in_set + ':'):
-                    newnode.append(contnode.__class__(title[len(in_set) + 1:],
-                                                      title[len(in_set) + 1:]))
-                else:
-                    newnode.append(contnode)
-            else:
-                # else use the given display name (used for :ref:)
-                newnode.append(contnode.__class__(dispname, dispname))
-            return newnode
-    # at least get rid of the ':' in the target if no explicit title given
-    if in_set is not None and not node.get('refexplicit', True):
-        if len(contnode) and isinstance(contnode[0], nodes.Text):
-            contnode[0] = nodes.Text(newtarget, contnode[0].rawsource)
 
-    return None
+def inventory_exists(env: BuildEnvironment, inv_name: str) -> bool:
+    return inv_name in InventoryAdapter(env).named_inventory
+
+
+def resolve_reference_in_inventory(env: BuildEnvironment,
+                                   inv_name: str,
+                                   node: pending_xref,
+                                   contnode: TextElement) -> Optional[Element]:
+    """Attempt to resolve a missing reference via intersphinx references.
+
+    Resolution is tried in the given inventory with the target as is.
+
+    Requires ``inventory_exists(env, inv_name)``.
+    """
+    assert inventory_exists(env, inv_name)
+    return _resolve_reference(env, inv_name, InventoryAdapter(env).named_inventory[inv_name],
+                              node, contnode)
+
+
+def resolve_reference_any_inventory(env: BuildEnvironment,
+                                    node: pending_xref,
+                                    contnode: TextElement) -> Optional[Element]:
+    """Attempt to resolve a missing reference via intersphinx references.
+
+    Resolution is tried with the target as is in any inventory.
+    """
+    return _resolve_reference(env, None, InventoryAdapter(env).main_inventory, node, contnode)
+
+
+def resolve_reference_detect_inventory(env: BuildEnvironment,
+                                       node: pending_xref,
+                                       contnode: TextElement) -> Optional[Element]:
+    """Attempt to resolve a missing reference via intersphinx references.
+
+    Resolution is tried first with the target as is in any inventory.
+    If this does not succeed, then the target is split by the first ``:``,
+    to form ``inv_name:newtarget``. If ``inv_name`` is a named inventory, then resolution
+    is tried in that inventory with the new target.
+    """
+
+    # ordinary direct lookup, use data as is
+    res = resolve_reference_any_inventory(env, node, contnode)
+    if res is not None:
+        return res
+
+    # try splitting the target into 'inv_name:target'
+    target = node['reftarget']
+    if ':' not in target:
+        return None
+    inv_name, newtarget = target.split(':', 1)
+    if not inventory_exists(env, inv_name):
+        return None
+    node['reftarget'] = newtarget
+    res_inv = resolve_reference_in_inventory(env, inv_name, node, contnode)
+    node['reftarget'] = target
+    return res_inv
+
+
+def missing_reference(app: Sphinx, env: BuildEnvironment, node: pending_xref,
+                      contnode: TextElement) -> Optional[Element]:
+    """Attempt to resolve a missing reference via intersphinx references."""
+
+    return resolve_reference_detect_inventory(env, node, contnode)
 
 
 def normalize_intersphinx_mapping(app: Sphinx, config: Config) -> None:
diff --git a/sphinx/util/typing.py b/sphinx/util/typing.py
index a2ab5f931..b7e591a82 100644
--- a/sphinx/util/typing.py
+++ b/sphinx/util/typing.py
@@ -70,7 +70,8 @@ OptionSpec = Dict[str, Callable[[str], Any]]
 TitleGetter = Callable[[nodes.Node], str]
 
 # inventory data on memory
-Inventory = Dict[str, Dict[str, Tuple[str, str, str, str]]]
+InventoryInner = Tuple[str, str, str, str]
+Inventory = Dict[str, Dict[str, InventoryInner]]
 
 
 def get_type_hints(obj: Any, globalns: Dict = None, localns: Dict = None) -> Dict[str, Any]:
diff --git a/tests/test_ext_intersphinx.py b/tests/test_ext_intersphinx.py
index 28b5e63b1..2f18748dd 100644
--- a/tests/test_ext_intersphinx.py
+++ b/tests/test_ext_intersphinx.py
@@ -133,12 +133,12 @@ def test_missing_reference(tempdir, app, status, warning):
                          refexplicit=True)
     assert rn[0].astext() == 'py3k:module2'
 
-    # prefix given, target not found and nonexplicit title: prefix is stripped
+    # prefix given, target not found and nonexplicit title: prefix is not stripped
     node, contnode = fake_node('py', 'mod', 'py3k:unknown', 'py3k:unknown',
                                refexplicit=False)
     rn = missing_reference(app, app.env, node, contnode)
     assert rn is None
-    assert contnode[0].astext() == 'unknown'
+    assert contnode[0].astext() == 'py3k:unknown'
 
     # prefix given, target not found and explicit title: nothing is changed
     node, contnode = fake_node('py', 'mod', 'py3k:unknown', 'py3k:unknown',