Skip to content

Commit 3ca2c54

Browse files
committed
Fix: Errors generated from invalid xrefs should fail build
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.
1 parent d5f513b commit 3ca2c54

6 files changed

Lines changed: 72 additions & 11 deletions

File tree

sphinx_js/directives.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
from docutils.parsers.rst import Directive
2020
from docutils.parsers.rst import Parser as RstParser
2121
from docutils.parsers.rst.directives import flag
22-
from docutils.utils import new_document
2322
from sphinx import addnodes
2423
from sphinx.addnodes import desc_signature
2524
from sphinx.application import Sphinx
@@ -44,6 +43,7 @@
4443
AutoModuleRenderer,
4544
AutoSummaryRenderer,
4645
Renderer,
46+
new_document_from_parent,
4747
)
4848

4949

@@ -81,8 +81,15 @@ def sphinx_js_type_role( # type: ignore[no-untyped-def]
8181
result in <span class="sphinx_js-type"> </span>
8282
"""
8383
unescaped = unescape(text)
84-
doc = new_document("", inliner.document.settings)
85-
RstParser().parse(unescaped, doc)
84+
parent_doc = inliner.document
85+
source = parent_doc.get("source", "")
86+
# Get line number stored by new_document_from_parent in rst_nodes if we can
87+
# find it, otherwise use lineno of directive.
88+
line = getattr(parent_doc, "sphinx_js_source_line", None) or lineno
89+
doc = new_document_from_parent(source, parent_doc)
90+
# Prepend newlines so errors report correct line number
91+
padded = "\n" * (line - 1) + unescaped
92+
RstParser().parse(padded, doc)
8693
n = nodes.inline(text)
8794
n["classes"].append("sphinx_js-type")
8895
n += doc.children[0].children

sphinx_js/renderers.py

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from docutils.parsers.rst import Directive
1010
from docutils.parsers.rst import Parser as RstParser
1111
from docutils.statemachine import StringList
12-
from docutils.utils import new_document
1312
from jinja2 import Environment, PackageLoader
1413
from sphinx import addnodes
1514
from sphinx import version_info as sphinx_version_info
@@ -52,6 +51,19 @@
5251
logger = logging.getLogger(__name__)
5352

5453

54+
def new_document_from_parent(
55+
source_path: str, parent_doc: nodes.document, line: int | None = None
56+
) -> nodes.document:
57+
"""Create a new document that inherits the parent's settings and reporter."""
58+
settings = parent_doc.settings
59+
reporter = parent_doc.reporter
60+
doc = nodes.document(settings, reporter, source=source_path)
61+
doc.note_source(source_path, -1)
62+
# Store line number for sphinx_js_type_role to use
63+
doc.sphinx_js_source_line = line
64+
return doc
65+
66+
5567
def sort_attributes_first_then_by_path(obj: TopLevel) -> Any:
5668
"""Return a sort key for IR objects."""
5769
match obj:
@@ -325,13 +337,12 @@ def rst_nodes(self) -> list[Node]:
325337
)
326338

327339
# Parse the RST into docutils nodes with a fresh doc, and return
328-
# them.
329-
#
330-
# Not sure if passing the settings from the "real" doc is the right
331-
# thing to do here:
332-
doc = new_document(
333-
f"{obj.filename}:{obj.path}({obj.line})",
334-
settings=self._directive.state.document.settings,
340+
# them. Use the directive's source location for error messages.
341+
source, line = self._directive.state_machine.get_source_and_line(
342+
self._directive.lineno
343+
)
344+
doc = new_document_from_parent(
345+
source or "", self._directive.state.document, line
335346
)
336347
RstParser().parse(rst, doc)
337348
return doc.children
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
extensions = ["sphinx_js"]
2+
3+
js_language = "typescript"
4+
js_source_path = ["../main.ts"]
5+
root_for_relative_js_paths = "../"
6+
7+
suppress_warnings = ["config.cache"]
8+
9+
10+
def ts_type_xref_formatter(config, xref):
11+
"""Always return an invalid :js:None: role to test error propagation."""
12+
return f":js:None:`{xref.name}`"
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
An extra line so we can test whether the error points to the right line.
2+
Another extra line.
3+
4+
.. js:autoattribute:: thing
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
/** A simple variable to document. */
2+
export let thing: number;
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
"""
2+
RST errors from ts_type_xref_formatter should cause build failures.
3+
4+
The test conf.py uses a formatter that always returns :js:None:`...`, which is
5+
an invalid RST role. This should cause the build to fail.
6+
7+
We also test that the error message points to the right location.
8+
"""
9+
10+
import io
11+
from contextlib import redirect_stderr
12+
from pathlib import Path
13+
14+
from sphinx.cmd.build import main as sphinx_main
15+
16+
17+
def test_build_fails_with_invalid_role(tmp_path: Path):
18+
"""Build must fail when ts_type_xref_formatter emits an invalid RST role."""
19+
docs_dir = str(Path(__file__).parent / "source" / "docs")
20+
stderr = io.StringIO()
21+
with redirect_stderr(stderr):
22+
result = sphinx_main([docs_dir, "-b", "text", "-W", "-E", str(tmp_path)])
23+
output = stderr.getvalue()
24+
assert result != 0, "Expected build failure due to invalid :js:None: role"
25+
assert "index.rst:4" in output, f"Expected error at index.rst:4, got: {output}"

0 commit comments

Comments
 (0)