Fix: Errors generated from invalid xrefs should fail build (#300)

Previously, if `ts_type_xref_formatter` generated a bad xref, it would not fail
the build because the error reporter for the subdocument tree was not hooked up
to the top level error reporter. This also fixes the line numbers reported in
the errors.

Author: hoodmane

sphinx_js/directives.py

@@ -19,7 +19,6 @@
 from docutils.parsers.rst import Directive
 from docutils.parsers.rst import Parser as RstParser
 from docutils.parsers.rst.directives import flag
-from docutils.utils import new_document
 from sphinx import addnodes
 from sphinx.addnodes import desc_signature
 from sphinx.application import Sphinx
@@ -44,6 +43,7 @@
     AutoModuleRenderer,
     AutoSummaryRenderer,
     Renderer,
+    new_document_from_parent,
 )
 
 
@@ -81,8 +81,15 @@ def sphinx_js_type_role(  # type: ignore[no-untyped-def]
     result in <span class="sphinx_js-type"> </span>
     """
     unescaped = unescape(text)
-    doc = new_document("", inliner.document.settings)
-    RstParser().parse(unescaped, doc)
+    parent_doc = inliner.document
+    source = parent_doc.get("source", "")
+    # Get line number stored by new_document_from_parent in rst_nodes if we can
+    # find it, otherwise use lineno of directive.
+    line = getattr(parent_doc, "sphinx_js_source_line", None) or lineno
+    doc = new_document_from_parent(source, parent_doc)
+    # Prepend newlines so errors report correct line number
+    padded = "\n" * (line - 1) + unescaped
+    RstParser().parse(padded, doc)
     n = nodes.inline(text)
     n["classes"].append("sphinx_js-type")
     n += doc.children[0].children

sphinx_js/renderers.py

@@ -9,7 +9,6 @@
 from docutils.parsers.rst import Directive
 from docutils.parsers.rst import Parser as RstParser
 from docutils.statemachine import StringList
-from docutils.utils import new_document
 from jinja2 import Environment, PackageLoader
 from sphinx import addnodes
 from sphinx import version_info as sphinx_version_info
@@ -52,6 +51,19 @@
 logger = logging.getLogger(__name__)
 
 
+def new_document_from_parent(
+    source_path: str, parent_doc: nodes.document, line: int | None = None
+) -> nodes.document:
+    """Create a new document that inherits the parent's settings and reporter."""
+    settings = parent_doc.settings
+    reporter = parent_doc.reporter
+    doc = nodes.document(settings, reporter, source=source_path)
+    doc.note_source(source_path, -1)
+    # Store line number for sphinx_js_type_role to use
+    doc.sphinx_js_source_line = line  # type: ignore[attr-defined]
+    return doc
+
+
 def sort_attributes_first_then_by_path(obj: TopLevel) -> Any:
     """Return a sort key for IR objects."""
     match obj:
@@ -325,13 +337,12 @@ def rst_nodes(self) -> list[Node]:
         )
 
         # Parse the RST into docutils nodes with a fresh doc, and return
-        # them.
-        #
-        # Not sure if passing the settings from the "real" doc is the right
-        # thing to do here:
-        doc = new_document(
-            f"{obj.filename}:{obj.path}({obj.line})",
-            settings=self._directive.state.document.settings,
+        # them. Use the directive's source location for error messages.
+        source, line = self._directive.state_machine.get_source_and_line(
+            self._directive.lineno
+        )
+        doc = new_document_from_parent(
+            source or "", self._directive.state.document, line
         )
         RstParser().parse(rst, doc)
         return doc.children

tests/test_build_xref_none/source/docs/conf.py

@@ -0,0 +1,12 @@
+extensions = ["sphinx_js"]
+
+js_language = "typescript"
+js_source_path = ["../main.ts"]
+root_for_relative_js_paths = "../"
+
+suppress_warnings = ["config.cache"]
+
+
+def ts_type_xref_formatter(config, xref):
+    """Always return an invalid :js:None: role to test error propagation."""
+    return f":js:None:`{xref.name}`"

tests/test_build_xref_none/source/docs/index.rst

@@ -0,0 +1,4 @@
+An extra line so we can test whether the error points to the right line.
+Another extra line.
+
+.. js:autoattribute:: thing

tests/test_build_xref_none/source/main.ts

@@ -0,0 +1,2 @@
+/** A simple variable to document. */
+export let thing: number;

tests/test_build_xref_none/test_build_xref_none.py

@@ -0,0 +1,25 @@
+"""
+RST errors from ts_type_xref_formatter should cause build failures.
+
+The test conf.py uses a formatter that always returns :js:None:`...`, which is
+an invalid RST role. This should cause the build to fail.
+
+We also test that the error message points to the right location.
+"""
+
+import io
+from contextlib import redirect_stderr
+from pathlib import Path
+
+from sphinx.cmd.build import main as sphinx_main
+
+
+def test_build_fails_with_invalid_role(tmp_path: Path):
+    """Build must fail when ts_type_xref_formatter emits an invalid RST role."""
+    docs_dir = str(Path(__file__).parent / "source" / "docs")
+    stderr = io.StringIO()
+    with redirect_stderr(stderr):
+        result = sphinx_main([docs_dir, "-b", "text", "-W", "-E", str(tmp_path)])
+    output = stderr.getvalue()
+    assert result != 0, "Expected build failure due to invalid :js:None: role"
+    assert "index.rst:4" in output, f"Expected error at index.rst:4, got: {output}"