sphinx-agent: Implement a fallback html_theme

The sphinx agent can now handle the case where the requested
`html_theme` is not available in the environment by suppressing the
raised error, overriding the value of `html_theme` and attempting to
run `Sphinx.__init__` again.

lib/esbonio/changes/916.enhancement.md

@@ -0,0 +1 @@
+When asking for a `html_theme` that is not available in the current environment, the server will now fallback to Sphinx's `alabaster` theme and report the error as a diagnostic

lib/esbonio/esbonio/sphinx_agent/app.py

@@ -6,6 +6,7 @@
 import typing
 
 from sphinx.application import Sphinx as _Sphinx
+from sphinx.errors import ThemeError
 from sphinx.util import console
 from sphinx.util import logging as sphinx_logging_module
 from sphinx.util.logging import NAMESPACE as SPHINX_LOG_NAMESPACE
@@ -19,6 +20,9 @@
     from typing import Any
     from typing import Literal
 
+    from docutils.nodes import Element
+    from docutils.parsers.rst import Directive
+
     RoleDefinition = tuple[str, Any, list[types.Role.TargetProvider]]
 
 sphinx_logger = logging.getLogger(SPHINX_LOG_NAMESPACE)
@@ -138,12 +142,22 @@ def __init__(self, *args, **kwargs):
         # Override sphinx's usual logging setup function
         sphinx_logging_module.setup = setup_logging  # type: ignore
 
-        super().__init__(*args, **kwargs)
+        # `try_run_init` may call `__init__` more than once, this could lead to spamming
+        # the user with warning messages, so we will suppress these messages if the
+        # retry counter has been set.
+        self._esbonio_retry_count = 0
+        try_run_init(self, super().__init__, *args, **kwargs)
 
     def add_role(self, name: str, role: Any, override: bool = False):
-        super().add_role(name, role, override)
+        super().add_role(name, role, override or self._esbonio_retry_count > 0)
         self.esbonio.add_role(name, role)
 
+    def add_directive(self, name: str, cls: type[Directive], override: bool = False):
+        super().add_directive(name, cls, override or self._esbonio_retry_count > 0)
+
+    def add_node(self, node: type[Element], override: bool = False, **kwargs):
+        super().add_node(node, override or self._esbonio_retry_count > 0, **kwargs)
+
     def setup_extension(self, extname: str):
         """Override Sphinx's implementation of `setup_extension`
 
@@ -188,6 +202,106 @@ def _report_missing_extension(self, extname: str, exc: Exception):
         self.esbonio.diagnostics.setdefault(uri, set()).add(diagnostic)
 
 
+def try_run_init(app: Sphinx, init_fn, *args, **kwargs):
+    """Try and run Sphinx's ``__init__`` function.
+
+    There are occasions where Sphinx will try and throw an error that is recoverable
+    e.g. a missing theme. In these situations we want to suppress the error, record a
+    diagnostic, and try again - which is what this function will do.
+
+    Some errors however, are not recoverable in which case we will allow the error to
+    proceed as normal.
+
+    Parameters
+    ----------
+    app
+       The application instance we are trying to initialize
+
+    init_fn
+       The application's `__init__` method, as returned by ``super().__init__``
+
+    args
+       Positional arguments to ``__init__``
+
+    retries
+       Max number of retries, a fallback in case we end up creating infinite recursion
+
+    kwargs
+       Keyword arguments to ``__init__``
+    """
+
+    if app._esbonio_retry_count >= 100:
+        raise RuntimeError("Unable to initialize Sphinx: max retries exceeded")
+
+    try:
+        init_fn(*args, **kwargs)
+    except ThemeError as exc:
+        # Fallback to the default theme.
+        kwargs.setdefault("confoverrides", {})["html_theme"] = "alabaster"
+        kwargs["confoverrides"]["html_theme_options"] = {}
+
+        app._esbonio_retry_count += 1
+        report_theme_error(app, exc)
+        try_run_init(app, init_fn, *args, **kwargs)
+    except Exception:
+        logger.exception("Unable to initialize Sphinx")
+        raise
+
+
+def report_theme_error(app: Sphinx, exc: ThemeError):
+    """Attempt to convert the given theme error into a useful diagnostic.
+
+    Parameters
+    ----------
+    app
+       The Sphinx object being initialized
+
+    exc
+       The error instance
+    """
+
+    if (config := app.esbonio.config_ast) is None:
+        return
+
+    if (range_ := find_html_theme_declaration(config)) is None:
+        return
+
+    diagnostic = types.Diagnostic(
+        range=range_,
+        message=f"{exc}",
+        severity=types.DiagnosticSeverity.Error,
+    )
+
+    uri = app.esbonio.config_uri
+    logger.debug("Adding diagnostic %s: %s", uri, diagnostic)
+    app.esbonio.diagnostics.setdefault(uri, set()).add(diagnostic)
+
+
+def find_html_theme_declaration(mod: ast.Module) -> types.Range | None:
+    """Attempt to find the location in the user's conf.py file where the ``html_theme``
+    was declared."""
+
+    for node in mod.body:
+        if not isinstance(node, ast.Assign):
+            continue
+
+        if len(targets := node.targets) != 1:
+            continue
+
+        if not isinstance(name := targets[0], ast.Name):
+            continue
+
+        if name.id == "html_theme":
+            break
+
+    else:
+        # Nothing found, abort
+        logger.debug("Unable to find 'html_theme' node")
+        return None
+
+    return ast_node_to_range(node)
+
+
 def find_extension_declaration(mod: ast.Module, extname: str) -> types.Range | None:
     """Attempt to find the location in the user's conf.py file where the given
     ``extname`` was declared.
@@ -230,15 +344,21 @@ def find_extension_declaration(mod: ast.Module, extname: str) -> types.Range | N
         logger.debug("Unable to find node for extension %r", extname)
         return None
 
+    return ast_node_to_range(element)
+
+
+def ast_node_to_range(node: ast.stmt | ast.expr) -> types.Range:
+    """Convert the given ast node to a range."""
+
     # Finally, try and extract the source location.
-    start_line = element.lineno - 1
-    start_char = element.col_offset
+    start_line = node.lineno - 1
+    start_char = node.col_offset
 
-    if (end_line := (element.end_lineno or 0) - 1) < 0:
+    if (end_line := (node.end_lineno or 0) - 1) < 0:
         end_line = start_line + 1
         end_char: int | None = 0
 
-    elif (end_char := element.end_col_offset) is None:
+    elif (end_char := node.end_col_offset) is None:
         end_line += 1
         end_char = 0
 

lib/esbonio/hatch.toml

@@ -30,7 +30,6 @@ sphinx = ["8"]
 
 [envs.hatch-test.overrides]
 matrix.sphinx.dependencies = [
-    "sphinx-design",
     "myst-parser",
     { value = "sphinx>=6,<7", if = ["6"] },
     { value = "sphinx>=7,<8", if = ["7"] },

lib/esbonio/tests/e2e/conftest.py

@@ -47,10 +47,6 @@ async def client(lsp_client: LanguageClient, uri_for, tmp_path_factory):
                         workspace_uri.fs_path,
                         str(build_dir),
                     ],
-                    "configOverrides": {
-                        "html_theme": "alabaster",
-                        "html_theme_options": {},
-                    },
                     "pythonCommand": [sys.executable],
                 },
             },

lib/esbonio/tests/e2e/test_e2e_diagnostics.py

@@ -81,12 +81,19 @@ async def test_workspace_diagnostic(client: LanguageClient, uri_for):
 
     workspace_uri = uri_for("workspaces", "demo")
     expected = {
-        str(workspace_uri / "rst" / "diagnostics.rst"): {message},
-        str(workspace_uri / "myst" / "diagnostics.md"): {message},
+        str(workspace_uri / "conf.py"): (
+            "no theme named 'furo' found",
+            "Could not import extension sphinx_design (exception: "
+            "No module named 'sphinx_design')",
+        ),
+        str(workspace_uri / "index.rst"): ('Unknown directive type "grid"'),
+        str(workspace_uri / "rst" / "diagnostics.rst"): (message,),
+        str(workspace_uri / "myst" / "diagnostics.md"): (message,),
     }
     assert len(report.items) == len(expected)
     for item in report.items:
-        assert expected[item.uri] == {d.message for d in item.items}
+        for diagnostic in item.items:
+            assert diagnostic.message.startswith(expected[item.uri])
 
     assert len(client.diagnostics) == 0, "Server should not publish diagnostics"
 

lib/esbonio/tests/sphinx-agent/conftest.py

@@ -52,10 +52,6 @@ async def client(request, uri_for, build_dir):
             demo_workspace.fs_path,
             str(build_dir),
         ],
-        config_overrides={
-            "html_theme": "alabaster",
-            "html_theme_options": {},
-        },
     )
     resolved = config.resolve(test_uri, workspace, logger)
     assert resolved is not None

lib/esbonio/tests/sphinx-agent/handlers/test_diagnostics.py

@@ -5,7 +5,6 @@
 
 import pytest
 from pygls.protocol import default_converter
-from sphinx import version_info as sphinx_version
 
 from esbonio.server import Uri
 from esbonio.server.features.project_manager import Project
@@ -26,8 +25,17 @@ def check_diagnostics(
     for k, ex_diags in expected.items():
         actual_diags = [converter.structure(d, types.Diagnostic) for d in actual[k]]
 
-        # Order of results is not important
-        assert set(actual_diags) == set(ex_diags)
+        assert len(ex_diags) == len(actual_diags)
+
+        for actual_diagnostic in actual_diags:
+            # Assumes ranges are unique
+            matches = [e for e in ex_diags if e.range == actual_diagnostic.range]
+            assert len(matches) == 1
+
+            expected_diagnostic = matches[0]
+            assert actual_diagnostic.range == expected_diagnostic.range
+            assert actual_diagnostic.severity == expected_diagnostic.severity
+            assert actual_diagnostic.message.startswith(expected_diagnostic.message)
 
 
 @pytest.mark.asyncio
@@ -36,13 +44,40 @@ async def test_diagnostics(client: SubprocessSphinxClient, project: Project, uri
     that they are correctly reset when fixed."""
     rst_diagnostics_uri = uri_for("workspaces/demo/rst/diagnostics.rst")
     myst_diagnostics_uri = uri_for("workspaces/demo/myst/diagnostics.md")
+    index_uri = uri_for("workspaces/demo/index.rst")
+    conf_uri = uri_for("workspaces/demo/conf.py")
 
-    if sphinx_version[0] >= 8:
-        message = "image file not readable: not-an-image.png [image.not_readable]"
-    else:
-        message = "image file not readable: not-an-image.png"
+    message = "image file not readable: not-an-image.png"
 
     expected = {
+        index_uri: [
+            types.Diagnostic(
+                message='Unknown directive type "grid"',
+                severity=types.DiagnosticSeverity.Error,
+                range=types.Range(
+                    start=types.Position(line=11, character=0),
+                    end=types.Position(line=12, character=0),
+                ),
+            )
+        ],
+        conf_uri: [
+            types.Diagnostic(
+                message="Could not import extension sphinx_design",
+                severity=types.DiagnosticSeverity.Error,
+                range=types.Range(
+                    start=types.Position(line=20, character=4),
+                    end=types.Position(line=20, character=19),
+                ),
+            ),
+            types.Diagnostic(
+                message="no theme named 'furo' found",
+                severity=types.DiagnosticSeverity.Error,
+                range=types.Range(
+                    start=types.Position(line=41, character=0),
+                    end=types.Position(line=41, character=19),
+                ),
+            ),
+        ],
         rst_diagnostics_uri: [
             types.Diagnostic(
                 message=message,
@@ -77,7 +112,10 @@ async def test_diagnostics(client: SubprocessSphinxClient, project: Project, uri
     )
 
     actual = await project.get_diagnostics()
-    check_diagnostics({myst_diagnostics_uri: expected[myst_diagnostics_uri]}, actual)
+
+    fixed_expected = expected.copy()
+    del fixed_expected[rst_diagnostics_uri]
+    check_diagnostics(fixed_expected, actual)
 
     # The original diagnostics should be reported when the issues are re-introduced.
     #