👌 Improve [](#target) resolution warnings (#716)

Previously, if `[](#target)` matched nothing locally, but multiple items in intersphinx inventories, then no warning would be logged, and the first match silently used.
Resolution of intersphinx now proceeds (if no local matches found) the same as for `[](inv:#target)`, such that a `myst.iref_ambiguous` warning is emitted in this case (the first match is then still used).

Warnings coming from resolution of `[](#target)` can also now be "individually" turned off using https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-nitpick_ignore, e.g. `nitpick_ignore = [("myst", "target")]`

Author: chrisjsewell

myst_parser/mdit_to_docutils/base.py

@@ -1065,8 +1065,7 @@ def render_inventory_link(self, token: SyntaxTreeNode) -> None:
 
         # create the docutils node
         match = matches[0]
-        ref_node = nodes.reference()
-        ref_node["internal"] = False
+        ref_node = nodes.reference("", "", internal=False)
         ref_node["inv_match"] = inventory.filter_string(
             match.inv, match.domain, match.otype, match.name
         )

myst_parser/mdit_to_docutils/sphinx_.py

@@ -83,13 +83,13 @@ def render_internal_link(self, token: SyntaxTreeNode) -> None:
                 text = ""
             else:
                 wrap_node = addnodes.download_reference(
-                    refdomain=None, reftarget=path_dest, refwarn=False, **kwargs
+                    refdomain=None, reftarget=path_dest, **kwargs
                 )
                 classes = ["xref", "download", "myst"]
                 text = destination if not token.children else ""
         else:
             wrap_node = addnodes.pending_xref(
-                refdomain=None, reftarget=destination, refwarn=True, **kwargs
+                refdomain=None, reftarget=destination, **kwargs
             )
             classes = ["xref", "myst"]
             text = ""

myst_parser/mdit_to_docutils/transforms.py

@@ -116,7 +116,6 @@ def apply(self, **kwargs: t.Any) -> None:
                     refdomain=None,
                     reftype="myst",
                     reftarget=target,
-                    refwarn=True,
                     refexplicit=bool(refnode.children),
                 )
                 inner_node = nodes.inline(

myst_parser/sphinx_ext/myst_refs.py

@@ -3,33 +3,30 @@
 This is applied to MyST type references only, such as ``[text](target)``,
 and allows for nested syntax
 """
+from __future__ import annotations
+
 import re
-from typing import Any, List, Optional, Tuple, cast
+from typing import Any, cast
 
 from docutils import nodes
 from docutils.nodes import Element, document
+from markdown_it.common.normalize_url import normalizeLink
 from sphinx import addnodes
 from sphinx.addnodes import pending_xref
 from sphinx.domains.std import StandardDomain
 from sphinx.errors import NoUri
-from sphinx.locale import __
+from sphinx.ext.intersphinx import InventoryAdapter
 from sphinx.transforms.post_transforms import ReferencesResolver
 from sphinx.util import docname_join, logging
 from sphinx.util.nodes import clean_astext, make_refnode
 
+from myst_parser import inventory
 from myst_parser._compat import findall
 from myst_parser.warnings_ import MystWarnings
 
 LOGGER = logging.getLogger(__name__)
 
 
-def log_warning(msg: str, subtype: MystWarnings, **kwargs: Any):
-    """Log a warning, with a myst type and specific subtype."""
-    LOGGER.warning(
-        msg + f" [myst.{subtype.value}]", type="myst", subtype=subtype.value, **kwargs
-    )
-
-
 class MystReferenceResolver(ReferencesResolver):
     """Resolves cross-references on doctrees.
 
@@ -38,6 +35,41 @@ class MystReferenceResolver(ReferencesResolver):
 
     default_priority = 9  # higher priority than ReferencesResolver (10)
 
+    def log_warning(
+        self, target: None | str, msg: str, subtype: MystWarnings, **kwargs: Any
+    ):
+        """Log a warning, with a myst type and specific subtype."""
+
+        # MyST references are warned about by default (the same as the `any` role)
+        # However, warnings can also be ignored by adding ("myst", target)
+        # nitpick_ignore/nitpick_ignore_regex lists
+        # https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-nitpicky
+        if (
+            target
+            and self.config.nitpick_ignore
+            and ("myst", target) in self.config.nitpick_ignore
+        ):
+            return
+        if (
+            target
+            and self.config.nitpick_ignore_regex
+            and any(
+                (
+                    re.fullmatch(ignore_type, "myst")
+                    and re.fullmatch(ignore_target, target)
+                )
+                for ignore_type, ignore_target in self.config.nitpick_ignore_regex
+            )
+        ):
+            return
+
+        LOGGER.warning(
+            msg + f" [myst.{subtype.value}]",
+            type="myst",
+            subtype=subtype.value,
+            **kwargs,
+        )
+
     def run(self, **kwargs: Any) -> None:
         self.document: document
         for node in findall(self.document)(addnodes.pending_xref):
@@ -48,41 +80,45 @@ def run(self, **kwargs: Any) -> None:
                 self.resolve_myst_ref_doc(node)
                 continue
 
-            contnode = cast(nodes.TextElement, node[0].deepcopy())
             newnode = None
-
+            contnode = cast(nodes.TextElement, node[0].deepcopy())
             target = node["reftarget"]
             refdoc = node.get("refdoc", self.env.docname)
+            search_domains: None | list[str] = self.env.config.myst_ref_domains
 
+            # try to resolve the reference within the local project,
+            # this asks all domains to resolve the reference,
+            # return None if no domain could resolve the reference
+            # or returns the first result, and logs a warning if
+            # multiple domains resolved the reference
             try:
-                newnode = self.resolve_myst_ref_any(refdoc, node, contnode)
-                if newnode is None:
-                    # no new node found? try the missing-reference event
-                    # but first we change the the reftype to 'any'
-                    # this means it is picked up by extensions like intersphinx
-                    node["reftype"] = "any"
-                    try:
-                        newnode = self.app.emit_firstresult(
-                            "missing-reference",
-                            self.env,
-                            node,
-                            contnode,
-                            allowed_exceptions=(NoUri,),
-                        )
-                    finally:
-                        node["reftype"] = "myst"
-                    if newnode is None:
-                        # still not found? warn if node wishes to be warned about or
-                        # we are in nit-picky mode
-                        self._warn_missing_reference(target, node)
+                newnode = self.resolve_myst_ref_any(
+                    refdoc, node, contnode, search_domains
+                )
             except NoUri:
                 newnode = contnode
+            if newnode is None:
+                # If no local domain could resolve the reference, try to
+                # resolve it as an inter-sphinx reference
+                newnode = self._resolve_myst_ref_intersphinx(
+                    node, contnode, target, search_domains
+                )
+            if newnode is None:
+                # if still not resolved, log a warning,
+                self.log_warning(
+                    target,
+                    f"'myst' cross-reference target not found: {target!r}",
+                    MystWarnings.XREF_MISSING,
+                    location=node,
+                )
 
+            # if the target could not be found, then default to using an external link
             if not newnode:
                 newnode = nodes.reference()
-                newnode["refid"] = target
+                newnode["refid"] = normalizeLink(target)
                 newnode.append(node[0].deepcopy())
 
+            # ensure the output node has some content
             if (
                 len(newnode.children) == 1
                 and isinstance(newnode[0], nodes.inline)
@@ -94,46 +130,17 @@ def run(self, **kwargs: Any) -> None:
 
             node.replace_self(newnode)
 
-    def _warn_missing_reference(self, target: str, node: pending_xref) -> None:
-        """Warn about a missing reference."""
-        dtype = "myst"
-        if not node.get("refwarn"):
-            return
-        if (
-            self.config.nitpicky
-            and self.config.nitpick_ignore
-            and (dtype, target) in self.config.nitpick_ignore
-        ):
-            return
-        if (
-            self.config.nitpicky
-            and self.config.nitpick_ignore_regex
-            and any(
-                (
-                    re.fullmatch(ignore_type, dtype)
-                    and re.fullmatch(ignore_target, target)
-                )
-                for ignore_type, ignore_target in self.config.nitpick_ignore_regex
-            )
-        ):
-            return
-
-        log_warning(
-            f"'myst' cross-reference target not found: {target!r}",
-            MystWarnings.XREF_MISSING,
-            location=node,
-        )
-
     def resolve_myst_ref_doc(self, node: pending_xref):
         """Resolve a reference, from a markdown link, to another document,
         optionally with a target id within that document.
         """
         from_docname = node.get("refdoc", self.env.docname)
         ref_docname: str = node["reftarget"]
-        ref_id: Optional[str] = node["reftargetid"]
+        ref_id: str | None = node["reftargetid"]
 
         if ref_docname not in self.env.all_docs:
-            log_warning(
+            self.log_warning(
+                ref_docname,
                 f"Unknown source document {ref_docname!r}",
                 MystWarnings.XREF_MISSING,
                 location=node,
@@ -148,7 +155,8 @@ def resolve_myst_ref_doc(self, node: pending_xref):
         if ref_id:
             slug_to_section = self.env.metadata[ref_docname].get("myst_slugs", {})
             if ref_id not in slug_to_section:
-                log_warning(
+                self.log_warning(
+                    ref_id,
                     f"local id not found in doc {ref_docname!r}: {ref_id!r}",
                     MystWarnings.XREF_MISSING,
                     location=node,
@@ -176,9 +184,13 @@ def resolve_myst_ref_doc(self, node: pending_xref):
         node.replace_self(ref_node)
 
     def resolve_myst_ref_any(
-        self, refdoc: str, node: pending_xref, contnode: Element
-    ) -> Element:
-        """Resolve reference generated by the "myst" role; ``[text](reference)``.
+        self,
+        refdoc: str,
+        node: pending_xref,
+        contnode: Element,
+        only_domains: None | list[str],
+    ) -> Element | None:
+        """Resolve reference generated by the "myst" role; ``[text](#reference)``.
 
         This builds on the sphinx ``any`` role to also resolve:
 
@@ -189,7 +201,7 @@ def resolve_myst_ref_any(
 
         """
         target: str = node["reftarget"]
-        results: List[Tuple[str, Element]] = []
+        results: list[tuple[str, Element]] = []
 
         # resolve standard references
         res = self._resolve_ref_nested(node, refdoc)
@@ -201,13 +213,10 @@ def resolve_myst_ref_any(
         if res:
             results.append(("std:doc", res))
 
-        # get allowed domains for referencing
-        ref_domains = self.env.config.myst_ref_domains
-
         assert self.app.builder
 
         # next resolve for any other standard reference objects
-        if ref_domains is None or "std" in ref_domains:
+        if only_domains is None or "std" in only_domains:
             stddomain = cast(StandardDomain, self.env.get_domain("std"))
             for objtype in stddomain.object_types:
                 key = (objtype, target)
@@ -225,7 +234,7 @@ def resolve_myst_ref_any(
         for domain in self.env.domains.values():
             if domain.name == "std":
                 continue  # we did this one already
-            if ref_domains is not None and domain.name not in ref_domains:
+            if only_domains is not None and domain.name not in only_domains:
                 continue
             try:
                 results.extend(
@@ -237,7 +246,8 @@ def resolve_myst_ref_any(
                 # the domain doesn't yet support the new interface
                 # we have to manually collect possible references (SLOW)
                 if not (getattr(domain, "__module__", "").startswith("sphinx.")):
-                    log_warning(
+                    self.log_warning(
+                        None,
                         f"Domain '{domain.__module__}::{domain.name}' has not "
                         "implemented a `resolve_any_xref` method",
                         MystWarnings.LEGACY_DOMAIN,
@@ -260,11 +270,10 @@ def stringify(name, node):
                 return f":{name}:`{reftitle}`"
 
             candidates = " or ".join(stringify(name, role) for name, role in results)
-            log_warning(
-                __(
-                    f"more than one target found for 'myst' cross-reference {target}: "
-                    f"could be {candidates}"
-                ),
+            self.log_warning(
+                target,
+                f"more than one target found for 'myst' cross-reference {target}: "
+                f"could be {candidates}",
                 MystWarnings.XREF_AMBIGUOUS,
                 location=node,
             )
@@ -283,7 +292,7 @@ def stringify(name, node):
 
     def _resolve_ref_nested(
         self, node: pending_xref, fromdocname: str, target=None
-    ) -> Optional[Element]:
+    ) -> Element | None:
         """This is the same as ``sphinx.domains.std._resolve_ref_xref``,
         but allows for nested syntax, rather than converting the inner node to raw text.
         """
@@ -311,7 +320,7 @@ def _resolve_ref_nested(
 
     def _resolve_doc_nested(
         self, node: pending_xref, fromdocname: str
-    ) -> Optional[Element]:
+    ) -> Element | None:
         """This is the same as ``sphinx.domains.std._resolve_doc_xref``,
         but allows for nested syntax, rather than converting the inner node to raw text.
 
@@ -332,3 +341,58 @@ def _resolve_doc_nested(
 
         assert self.app.builder
         return make_refnode(self.app.builder, fromdocname, docname, "", innernode)
+
+    def _resolve_myst_ref_intersphinx(
+        self,
+        node: nodes.Element,
+        contnode: nodes.Element,
+        target: str,
+        only_domains: list[str] | None,
+    ) -> None | nodes.reference:
+        """Resolve a myst reference to an intersphinx inventory."""
+        matches = [
+            m
+            for m in inventory.filter_sphinx_inventories(
+                InventoryAdapter(self.env).named_inventory,
+                targets=target,
+            )
+            if only_domains is None or m.domain in only_domains
+        ]
+        if not matches:
+            return None
+        if len(matches) > 1:
+            # log a warning if there are multiple matches
+            show_num = 3
+            matches_str = ", ".join(
+                [
+                    inventory.filter_string(m.inv, m.domain, m.otype, m.name)
+                    for m in matches[:show_num]
+                ]
+            )
+            if len(matches) > show_num:
+                matches_str += ", ..."
+            self.log_warning(
+                target,
+                f"Multiple matches found for {target!r}: {matches_str}",
+                MystWarnings.IREF_AMBIGUOUS,
+                location=node,
+            )
+        # get the first match and create a reference node
+        match = matches[0]
+        newnode = nodes.reference("", "", internal=False, refuri=match.loc)
+        if "reftitle" in node:
+            newnode["reftitle"] = node["reftitle"]
+        else:
+            newnode["reftitle"] = f"{match.project} {match.version}".strip()
+        if node.get("refexplicit"):
+            newnode.append(contnode)
+        elif match.text:
+            newnode.append(
+                contnode.__class__(match.text, match.text, classes=["iref", "myst"])
+            )
+        else:
+            newnode.append(
+                contnode.__class__(match.name, match.name, classes=["iref", "myst"])
+            )
+
+        return newnode