👌 Handle external references pointing to object types (sphinx-doc#12133)

This commit fixes the issue of `objects.inv` denoting object names, whilst the `external` role only allows for role names. 

As an example, take the `objects.inv` for the sphinx documentation, which contains:

```
py:function
    compile                  : usage/domains/python.html#compile
```

A user might understandably expect that they could reference this using `` :external:py:function:`compile` ``, but actually this would previously error with:

```
WARNING: role for external cross-reference not found: py:function
```

this is because, `function` is the object type, yet `external` expects the related role name `func`.

It should not be necessary for the user to know about this distinction,
so in this commit, we add logic, to first look if the name relates to a role name (as previous, to not be back-breaking) but, if not, then also look if the name relates to an object that has a known role and, if so, use that.

Co-authored-by: Bénédikt Tran <[email protected]>

Author: chrisjsewell

CHANGES.rst

@@ -22,6 +22,9 @@ Deprecated
 Features added
 --------------
 
+* #12133: Allow ``external`` roles to reference object types
+  (rather than role names). Patch by Chris Sewell.
+
 * #12131: Added :confval:`show_warning_types` configuration option.
   Patch by Chris Sewell.
 

doc/usage/extensions/intersphinx.rst

@@ -218,6 +218,7 @@ The Intersphinx extension provides the following role.
      e.g., ``:external:py:class:`zipfile.ZipFile```, or
    - ``:external:reftype:`target```,
      e.g., ``:external:doc:`installation```.
+     With this shorthand, the domain is assumed to be ``std``.
 
    If you would like to constrain the lookup to a specific external project,
    then the key of the project, as specified in :confval:`intersphinx_mapping`,

sphinx/ext/intersphinx.py

@@ -552,13 +552,17 @@ def run(self) -> tuple[list[Node], list[system_message]]:
         return result, messages
 
     def get_inventory_and_name_suffix(self, name: str) -> tuple[str | None, str]:
+        """Extract an inventory name (if any) and ``domain+name`` suffix from a role *name*.
+        and the domain+name suffix.
+
+        The role name is expected to be of one of the following forms:
+
+        - ``external+inv:name`` -- explicit inventory and name, any domain.
+        - ``external+inv:domain:name`` -- explicit inventory, domain and name.
+        - ``external:name`` -- any inventory and domain, explicit name.
+        - ``external:domain:name`` -- any inventory, explicit domain and name.
+        """
         assert name.startswith('external'), name
-        # either we have an explicit inventory name, i.e,
-        # :external+inv:role:        or
-        # :external+inv:domain:role:
-        # or we look in all inventories, i.e.,
-        # :external:role:            or
-        # :external:domain:role:
         suffix = name[9:]
         if name[8] == '+':
             inv_name, suffix = suffix.split(':', 1)
@@ -570,34 +574,56 @@ def get_inventory_and_name_suffix(self, name: str) -> tuple[str | None, str]:
             raise ValueError(msg)
 
     def get_role_name(self, name: str) -> tuple[str, str] | None:
+        """Find (if any) the corresponding ``(domain, role name)`` for *name*.
+
+        The *name* can be either a role name (e.g., ``py:function`` or ``function``)
+        given as ``domain:role`` or ``role``, or its corresponding object name
+        (in this case, ``py:func`` or ``func``) given as ``domain:objname`` or ``objname``.
+
+        If no domain is given, or the object/role name is not found for the requested domain,
+        the 'std' domain is used.
+        """
         names = name.split(':')
         if len(names) == 1:
-            # role
             default_domain = self.env.temp_data.get('default_domain')
             domain = default_domain.name if default_domain else None
-            role = names[0]
+            name = names[0]
         elif len(names) == 2:
-            # domain:role:
             domain = names[0]
-            role = names[1]
+            name = names[1]
         else:
             return None
 
-        if domain and self.is_existent_role(domain, role):
+        if domain and (role := self.get_role_name_from_domain(domain, name)):
             return (domain, role)
-        elif self.is_existent_role('std', role):
+        elif (role := self.get_role_name_from_domain('std', name)):
             return ('std', role)
         else:
             return None
 
-    def is_existent_role(self, domain_name: str, role_name: str) -> bool:
+    def is_existent_role(self, domain_name: str, role_or_obj_name: str) -> bool:
+        """Check if the given role or object exists in the given domain."""
+        return self.get_role_name_from_domain(domain_name, role_or_obj_name) is not None
+
+    def get_role_name_from_domain(self, domain_name: str, role_or_obj_name: str) -> str | None:
+        """Check if the given role or object exists in the given domain,
+        and return the related role name if it exists, otherwise return None.
+        """
         try:
             domain = self.env.get_domain(domain_name)
-            return role_name in domain.roles
         except ExtensionError:
-            return False
+            return None
+        if role_or_obj_name in domain.roles:
+            return role_or_obj_name
+        if (
+            (role_name := domain.role_for_objtype(role_or_obj_name))
+            and role_name in domain.roles
+        ):
+            return role_name
+        return None
 
     def invoke_role(self, role: tuple[str, str]) -> tuple[list[Node], list[system_message]]:
+        """Invoke the role described by a ``(domain, role name)`` pair."""
         domain = self.env.get_domain(role[0])
         if domain:
             role_func = domain.role(role[1])

tests/roots/test-ext-intersphinx-role/index.rst

@@ -35,7 +35,7 @@
 
 
 - a function with explicit inventory:
-  :external+inv:c:func:`CFunc`
+  :external+inv:c:func:`CFunc` or :external+inv:c:function:`CFunc`
 - a class with explicit non-existing inventory, which also has upper-case in name:
   :external+invNope:cpp:class:`foo::Bar`
 

tests/test_extensions/test_ext_intersphinx.py

@@ -18,6 +18,7 @@
     normalize_intersphinx_mapping,
 )
 from sphinx.ext.intersphinx import setup as intersphinx_setup
+from sphinx.util.console import strip_colors
 
 from tests.test_util.test_util_inventory import inventory_v2, inventory_v2_not_having_version
 from tests.utils import http_server
@@ -551,30 +552,32 @@ def test_intersphinx_role(app, warning):
 
     app.build()
     content = (app.outdir / 'index.html').read_text(encoding='utf8')
-    wStr = warning.getvalue()
+    warnings = strip_colors(warning.getvalue()).splitlines()
+    index_path = app.srcdir / 'index.rst'
+    assert warnings == [
+        f'{index_path}:21: WARNING: role for external cross-reference not found: py:nope',
+        f'{index_path}:28: WARNING: role for external cross-reference not found: nope',
+        f'{index_path}:39: WARNING: inventory for external cross-reference not found: invNope',
+        f'{index_path}:9: WARNING: external py:mod reference target not found: module3',
+        f'{index_path}:14: WARNING: external py:mod reference target not found: module10',
+        f'{index_path}:19: WARNING: external py:meth reference target not found: inv:Foo.bar',
+    ]
 
     html = '<a class="reference external" href="https://example.org/{}" title="(in foo v2.0)">'
     assert html.format('foo.html#module-module1') in content
     assert html.format('foo.html#module-module2') in content
-    assert "WARNING: external py:mod reference target not found: module3" in wStr
-    assert "WARNING: external py:mod reference target not found: module10" in wStr
 
     assert html.format('sub/foo.html#module1.func') in content
-    assert "WARNING: external py:meth reference target not found: inv:Foo.bar" in wStr
-
-    assert "WARNING: role for external cross-reference not found: py:nope" in wStr
 
     # default domain
     assert html.format('index.html#std_uint8_t') in content
-    assert "WARNING: role for external cross-reference not found: nope" in wStr
 
     # std roles without domain prefix
     assert html.format('docname.html') in content
     assert html.format('index.html#cmdoption-ls-l') in content
 
     # explicit inventory
     assert html.format('cfunc.html#CFunc') in content
-    assert "WARNING: inventory for external cross-reference not found: invNope" in wStr
 
     # explicit title
     assert html.format('index.html#foons') in content