From f92fdb901b41ef5c1ef520afb8c9ce3967ac1ce7 Mon Sep 17 00:00:00 2001 From: merlin Date: Mon, 27 Jul 2026 13:04:52 +0000 Subject: [PATCH 1/2] scala: give Metals the build roots, not the repository root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Metals serves one build per workspace folder, and Serena sends only the repository root. Where the builds live below the root, Metals falls back to its own search (BuildTools.searchForBuildTool), which looks one level down and takes the *first* match — so in a monorepo every build but that one is served with no build target, and cross-file references silently come back empty. Detect the build roots instead and send them all, one Metals service each. `ls_specific_settings.scala.project_roots` names them explicitly where the detection guesses wrong, `project_root_scan_depth` bounds the search. The configured `ls_workspace_folders` are not usable for this: they are about what SolidLSP indexes and are shared by every language server of a project, so in a polyglot monorepo no single value suits both Metals and, say, tsserver. Where the repository root is itself a build root, nothing changes. --- CHANGELOG.md | 5 + .../scala_setup_guide_for_serena.md | 15 ++ .../language_servers/scala_language_server.py | 151 +++++++++++++++++- test/solidlsp/scala/test_scala_build_roots.py | 71 ++++++++ 4 files changed, 238 insertions(+), 4 deletions(-) create mode 100644 test/solidlsp/scala/test_scala_build_roots.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d5e7e093b7..68758e5d15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,11 @@ Status of the `main` branch. Changes prior to the next official version change w - Language servers and their dependency providers now go through the `subprocess_run` helper instead of calling `subprocess.run` directly (e.g. for installation processes), so all such subprocesses get `stdin=DEVNULL` and can no longer interfere with the stdio MCP connection #1748 + - `scala`: Fix: in a repository whose builds live below its root, Metals was given only the repository + root as a workspace folder, and its own one-level search takes just the first build it finds — so in + a monorepo all but one build were served with no build target, silently returning no cross-file + references. The build roots are now detected and passed as workspace folders, one Metals service per + build; `ls_specific_settings.scala.project_roots` and `project_root_scan_depth` override the detection * Hooks: - Add `serena-hooks --client=grok`, including Grok-native PreToolUse allow/deny output. diff --git a/docs/03-special-guides/scala_setup_guide_for_serena.md b/docs/03-special-guides/scala_setup_guide_for_serena.md index 1e4bebca55..9bb21fd294 100644 --- a/docs/03-special-guides/scala_setup_guide_for_serena.md +++ b/docs/03-special-guides/scala_setup_guide_for_serena.md @@ -63,6 +63,21 @@ These instructions cover the setup for projects that use sbt as the build tool, Notes: - Ensure you completed the manual or auto‑import steps so that the build is compiled and indexed; otherwise, code navigation and references may be incomplete until the first successful compile. +--- +## Monorepos: builds below the repository root + +Metals serves one build per workspace folder, so what it needs is the build roots, not the repository root. Serena detects them: if the repository root is not itself a build root (no `build.sbt`, `build.mill`, `pom.xml`, `.bsp/`, …), it searches up to three levels below for directories that are, and passes those to Metals — one Metals service per build. + +Override the detection where it guesses wrong: + +```yaml +# ~/.serena/serena_config.yml or .serena/project.yml +ls_specific_settings: + scala: + project_roots: ["backend", "tooling/plugin"] # relative to the repository root + project_root_scan_depth: 3 # only applies when project_roots is unset +``` + --- ## Running Multiple Metals Instances diff --git a/src/solidlsp/language_servers/scala_language_server.py b/src/solidlsp/language_servers/scala_language_server.py index 2a7d633054..2683fd9438 100644 --- a/src/solidlsp/language_servers/scala_language_server.py +++ b/src/solidlsp/language_servers/scala_language_server.py @@ -10,6 +10,7 @@ from overrides import override +from solidlsp.initialize_params import DefaultInitializeParamsBuilder, InitializeParamsBuilder from solidlsp.ls import SolidLanguageServer from solidlsp.ls_config import LanguageServerConfig from solidlsp.ls_utils import PlatformUtils @@ -28,6 +29,30 @@ DEFAULT_CLIENT_NAME = "Serena" DEFAULT_ON_STALE_LOCK = "auto-clean" DEFAULT_LOG_MULTI_INSTANCE_NOTICE = True +DEFAULT_PROJECT_ROOT_SCAN_DEPTH = 3 + +# Files whose presence marks a directory as the root of a build Metals can import. +# Mirrors the per-build-tool probes in Metals' `BuildTools` (scala/meta/internal/builds/BuildTools.scala). +BUILD_ROOT_MARKER_FILES = ( + "build.gradle", + "build.gradle.kts", + "build.mill", + "build.mill.scala", + "build.mill.yaml", + "build.sbt", + "build.sc", + "pom.xml", + "project.scala", + "settings.gradle", + "settings.gradle.kts", +) + +# Directories whose presence marks a directory as an already-configured Metals project. +BUILD_ROOT_MARKER_DIRS = (".bloop", ".bsp") + +# Directories never worth descending into when scanning for build roots: build output, dependencies, +# and the two directories (`project`, `src`) that belong to a build we would already have recognised. +BUILD_ROOT_SCAN_SKIP_DIRS = frozenset({"node_modules", "out", "project", "src", "target", "venv"}) class StaleLockMode(Enum): @@ -43,6 +68,79 @@ class StaleLockMode(Enum): """Raise an error and refuse to start.""" +def _is_build_root(path: str) -> bool: + """ + Whether `path` is the root of a build that Metals can import. + """ + if any(os.path.isfile(os.path.join(path, name)) for name in BUILD_ROOT_MARKER_FILES): + return True + if any(os.path.isdir(os.path.join(path, name)) for name in BUILD_ROOT_MARKER_DIRS): + return True + # sbt allows the build to be defined entirely under project/, with no build.sbt + build_properties = os.path.join(path, "project", "build.properties") + if os.path.isfile(build_properties): + try: + with open(build_properties, encoding="utf-8", errors="replace") as f: + return any(line.lstrip().startswith("sbt.version") for line in f) + except OSError: + return False + return False + + +def find_build_roots(repository_root_path: str, max_depth: int = DEFAULT_PROJECT_ROOT_SCAN_DEPTH) -> list[str]: + """ + Find the roots of the builds contained in the given repository. + + Metals serves one build per workspace folder, so a repository holding several builds + (or a single build below its root) must name those directories rather than the repository root. + + :param repository_root_path: the repository root + :param max_depth: how many directory levels below the repository root to search; + the search does not descend into a directory that is itself a build root + :return: the absolute paths of the build roots found, or `[repository_root_path]` if there are none + (which leaves Metals' own behaviour unchanged) + """ + if _is_build_root(repository_root_path): + return [repository_root_path] + + roots: list[str] = [] + + def scan(directory: str, depth: int) -> None: + if depth > max_depth: + return + try: + entries = sorted(os.scandir(directory), key=lambda e: e.name) + except OSError: + return + for entry in entries: + if not entry.is_dir(follow_symlinks=False) or entry.name.startswith(".") or entry.name in BUILD_ROOT_SCAN_SKIP_DIRS: + continue + if _is_build_root(entry.path): + roots.append(entry.path) + else: + scan(entry.path, depth + 1) + + scan(repository_root_path, 1) + return roots or [repository_root_path] + + +class ScalaInitializeParamsBuilder(DefaultInitializeParamsBuilder): + """ + Sends the repository's build roots as the workspace folders, so that Metals creates one + service per build (see `MetalsLanguageServer.initialize`), rather than the configured + workspace folders, which are about what SolidLSP indexes and are shared across languages. + """ + + def __init__(self, ls: SolidLanguageServer, build_roots: list[str]): + super().__init__(ls) + self._build_roots = build_roots + + @override + def _apply_updates(self) -> None: + super()._apply_updates() + self._set("workspaceFolders", [self._create_workspace_folder_entry(path) for path in self._build_roots]) + + def _get_scala_settings(solidlsp_settings: SolidLSPSettings) -> dict[str, object]: """ Extract Scala-specific settings with defaults applied. @@ -52,6 +150,8 @@ def _get_scala_settings(solidlsp_settings: SolidLSPSettings) -> dict[str, object - client_name: str - on_stale_lock: StaleLockMode - log_multi_instance_notice: bool + - project_roots: list[str] | None + - project_root_scan_depth: int """ from solidlsp.ls_config import LanguageServerId @@ -60,6 +160,8 @@ def _get_scala_settings(solidlsp_settings: SolidLSPSettings) -> dict[str, object "client_name": DEFAULT_CLIENT_NAME, "on_stale_lock": StaleLockMode.AUTO_CLEAN, "log_multi_instance_notice": DEFAULT_LOG_MULTI_INSTANCE_NOTICE, + "project_roots": None, + "project_root_scan_depth": DEFAULT_PROJECT_ROOT_SCAN_DEPTH, } if not solidlsp_settings.ls_specific_settings: @@ -80,6 +182,8 @@ def _get_scala_settings(solidlsp_settings: SolidLSPSettings) -> dict[str, object "client_name": scala_settings.get("client_name", DEFAULT_CLIENT_NAME), "on_stale_lock": on_stale_lock, "log_multi_instance_notice": scala_settings.get("log_multi_instance_notice", DEFAULT_LOG_MULTI_INSTANCE_NOTICE), + "project_roots": scala_settings.get("project_roots"), + "project_root_scan_depth": scala_settings.get("project_root_scan_depth", DEFAULT_PROJECT_ROOT_SCAN_DEPTH), } @@ -100,6 +204,16 @@ class ScalaLanguageServer(SolidLanguageServer): metals_version: '1.6.4' # Client identifier sent to Metals (default: DEFAULT_CLIENT_NAME) client_name: 'Serena' + # Build roots to serve, relative to the repository root; when unset, they are + # auto-detected (see find_build_roots) + project_roots: ['backend', 'tooling/plugin'] + # How many levels below the repository root auto-detection searches + project_root_scan_depth: 3 + + Monorepo support: + Metals serves one build per workspace folder, so the build roots — not the repository + root — are what it must be given. They are detected automatically; `project_roots` + overrides the detection where it guesses wrong. Multi-instance support: Metals uses H2 AUTO_SERVER mode (enabled by default) to support multiple @@ -113,8 +227,12 @@ def __init__(self, config: LanguageServerConfig, repository_root_path: str, soli Creates a ScalaLanguageServer instance. This class is not meant to be instantiated directly. Use LanguageServer.create() instead. """ + self._build_roots = self._resolve_build_roots(repository_root_path, solidlsp_settings) + log.info(f"Metals will be given these build roots as workspace folders: {self._build_roots}") + # Check for stale locks before setting up dependencies (fail-fast) - self._check_metals_db_status(repository_root_path, solidlsp_settings) + for build_root in self._build_roots: + self._check_metals_db_status(build_root, solidlsp_settings) scala_lsp_executable_path = self._setup_runtime_dependencies(config, solidlsp_settings) super().__init__( @@ -125,9 +243,34 @@ def __init__(self, config: LanguageServerConfig, repository_root_path: str, soli solidlsp_settings, ) - def _check_metals_db_status(self, repository_root_path: str, solidlsp_settings: SolidLSPSettings) -> None: + @staticmethod + def _resolve_build_roots(repository_root_path: str, solidlsp_settings: SolidLSPSettings) -> list[str]: + """ + Determine the build roots to serve, from the `project_roots` setting if given and by + detection otherwise. + """ + settings = _get_scala_settings(solidlsp_settings) + configured_roots: list[str] | None = settings["project_roots"] # type: ignore[assignment] + if configured_roots is None: + scan_depth: int = settings["project_root_scan_depth"] # type: ignore[assignment] + return find_build_roots(repository_root_path, scan_depth) + + roots = [] + for root in configured_roots: + abs_root = os.path.abspath(os.path.join(repository_root_path, root)) + if os.path.isdir(abs_root): + roots.append(abs_root) + else: + log.warning(f"Configured Scala project root does not exist, skipping: {abs_root}") + return roots or [repository_root_path] + + @override + def _create_initialize_params_builder(self) -> InitializeParamsBuilder: + return ScalaInitializeParamsBuilder(self, self._build_roots) + + def _check_metals_db_status(self, build_root_path: str, solidlsp_settings: SolidLSPSettings) -> None: """ - Check the Metals H2 database status and handle stale locks. + Check the Metals H2 database status of one build root and handle stale locks. This method is called before setting up runtime dependencies to fail-fast if there's a stale lock that the user has configured to fail on. @@ -141,7 +284,7 @@ def _check_metals_db_status(self, repository_root_path: str, solidlsp_settings: cleanup_stale_lock, ) - project_path = Path(repository_root_path) + project_path = Path(build_root_path) status, lock_info = check_metals_db_status(project_path) # Get settings using the shared helper function diff --git a/test/solidlsp/scala/test_scala_build_roots.py b/test/solidlsp/scala/test_scala_build_roots.py new file mode 100644 index 0000000000..b4da0fa697 --- /dev/null +++ b/test/solidlsp/scala/test_scala_build_roots.py @@ -0,0 +1,71 @@ +""" +Unit tests for the detection of Scala build roots, which Metals is given as workspace folders. +""" + +from pathlib import Path + +import pytest + +from solidlsp.language_servers.scala_language_server import find_build_roots + + +def make_sbt_build(path: Path) -> Path: + path.mkdir(parents=True, exist_ok=True) + (path / "build.sbt").write_text('ThisBuild / scalaVersion := "3.3.6"\n') + (path / "src" / "main" / "scala").mkdir(parents=True) + return path + + +@pytest.mark.scala +class TestFindBuildRoots: + def test_repository_root_is_the_build_root(self, tmp_path: Path) -> None: + make_sbt_build(tmp_path) + assert find_build_roots(str(tmp_path)) == [str(tmp_path)] + + def test_single_build_below_the_root(self, tmp_path: Path) -> None: + make_sbt_build(tmp_path / "backend") + assert find_build_roots(str(tmp_path)) == [str(tmp_path / "backend")] + + def test_several_builds_below_the_root(self, tmp_path: Path) -> None: + make_sbt_build(tmp_path / "backend") + make_sbt_build(tmp_path / "tooling") + assert find_build_roots(str(tmp_path)) == [str(tmp_path / "backend"), str(tmp_path / "tooling")] + + def test_nested_build(self, tmp_path: Path) -> None: + make_sbt_build(tmp_path / "scala" / "backend") + assert find_build_roots(str(tmp_path)) == [str(tmp_path / "scala" / "backend")] + + def test_scan_depth_is_bounded(self, tmp_path: Path) -> None: + make_sbt_build(tmp_path / "a" / "b" / "c") + assert find_build_roots(str(tmp_path), max_depth=2) == [str(tmp_path)] + assert find_build_roots(str(tmp_path), max_depth=3) == [str(tmp_path / "a" / "b" / "c")] + + def test_does_not_descend_into_a_build_root(self, tmp_path: Path) -> None: + """A subproject of an sbt build is not a build root of its own.""" + make_sbt_build(tmp_path / "backend") + make_sbt_build(tmp_path / "backend" / "module") + assert find_build_roots(str(tmp_path)) == [str(tmp_path / "backend")] + + def test_falls_back_to_the_repository_root(self, tmp_path: Path) -> None: + """With nothing to find, Metals' own behaviour is left unchanged.""" + (tmp_path / "docs").mkdir() + assert find_build_roots(str(tmp_path)) == [str(tmp_path)] + + def test_hidden_and_uninteresting_directories_are_skipped(self, tmp_path: Path) -> None: + make_sbt_build(tmp_path / ".git" / "backend") + make_sbt_build(tmp_path / "node_modules" / "backend") + assert find_build_roots(str(tmp_path)) == [str(tmp_path)] + + def test_bsp_directory_marks_a_build_root(self, tmp_path: Path) -> None: + (tmp_path / "backend" / ".bsp").mkdir(parents=True) + assert find_build_roots(str(tmp_path)) == [str(tmp_path / "backend")] + + def test_sbt_build_defined_only_under_project(self, tmp_path: Path) -> None: + (tmp_path / "backend" / "project").mkdir(parents=True) + (tmp_path / "backend" / "project" / "build.properties").write_text("sbt.version=1.11.7\n") + assert find_build_roots(str(tmp_path)) == [str(tmp_path / "backend")] + + def test_build_properties_without_a_version_is_not_a_build_root(self, tmp_path: Path) -> None: + (tmp_path / "backend" / "project").mkdir(parents=True) + (tmp_path / "backend" / "project" / "build.properties").write_text("# nothing to see here\n") + assert find_build_roots(str(tmp_path)) == [str(tmp_path)] From 87fd946b0e1d341be72ab9033c149cd405a58225 Mon Sep 17 00:00:00 2001 From: merlin Date: Mon, 27 Jul 2026 14:06:35 +0000 Subject: [PATCH 2/2] scala: harden the build-root detection after review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - keep `ls_additional_workspace_folders`, which may name folders outside the repository and so can never be detected; build the folder list ourselves rather than letting the default builder compute one we then discard - require a JSON file inside `.bsp`/`.bloop` before treating it as a build root, as Metals does (`BuildTools.hasJsonFile`) — an empty leftover was both claiming a root and hiding the real builds beneath it - validate `project_roots` and `project_root_scan_depth` in the manner of `on_stale_lock`; a null depth used to raise from the constructor and a bare string was iterated character by character - probe the skipped directories themselves, only refusing to descend below them, and follow symlinks as Metals does, guarding against cycles - add the Bazel, mill-wrapper and Deder markers; note in the comment that the list is deliberately partial - fall back to detection, not the repository root, when an explicitly configured root exists on paper but not on disk --- CHANGELOG.md | 3 +- .../language_servers/scala_language_server.py | 96 ++++++++++++--- test/solidlsp/scala/test_scala_build_roots.py | 115 +++++++++++++++++- 3 files changed, 194 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68758e5d15..f3e0cca6be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,8 @@ Status of the `main` branch. Changes prior to the next official version change w root as a workspace folder, and its own one-level search takes just the first build it finds — so in a monorepo all but one build were served with no build target, silently returning no cross-file references. The build roots are now detected and passed as workspace folders, one Metals service per - build; `ls_specific_settings.scala.project_roots` and `project_root_scan_depth` override the detection + build; `ls_specific_settings.scala.project_roots` and `project_root_scan_depth` override the + detection #1766 * Hooks: - Add `serena-hooks --client=grok`, including Grok-native PreToolUse allow/deny output. diff --git a/src/solidlsp/language_servers/scala_language_server.py b/src/solidlsp/language_servers/scala_language_server.py index 2683fd9438..3e0f89e23c 100644 --- a/src/solidlsp/language_servers/scala_language_server.py +++ b/src/solidlsp/language_servers/scala_language_server.py @@ -31,9 +31,14 @@ DEFAULT_LOG_MULTI_INSTANCE_NOTICE = True DEFAULT_PROJECT_ROOT_SCAN_DEPTH = 3 -# Files whose presence marks a directory as the root of a build Metals can import. -# Mirrors the per-build-tool probes in Metals' `BuildTools` (scala/meta/internal/builds/BuildTools.scala). +# Files whose presence marks a directory as the root of a build Metals can import, following the +# per-build-tool probes in Metals' `BuildTools` (scala/meta/internal/builds/BuildTools.scala) and +# `BazelBuildTool.workspaceSupportsBsp`. Deliberately partial: the probes that need to read a file's +# contents (scala-cli's BSP scope) are left out, since missing a build root only returns us to the +# previous behaviour, whereas a false positive would hide the real builds beneath it. BUILD_ROOT_MARKER_FILES = ( + "MODULE.bazel", + "WORKSPACE", "build.gradle", "build.gradle.kts", "build.mill", @@ -41,17 +46,22 @@ "build.mill.yaml", "build.sbt", "build.sc", + "deder.pkl", + "mill", + "mill.bat", "pom.xml", "project.scala", "settings.gradle", "settings.gradle.kts", ) -# Directories whose presence marks a directory as an already-configured Metals project. -BUILD_ROOT_MARKER_DIRS = (".bloop", ".bsp") +# Directories which, when they hold a JSON file, mark an already-configured Metals project +# (`BuildTools.hasJsonFile`). An empty one is a leftover, not a build. +BUILD_ROOT_MARKER_JSON_DIRS = (".bloop", ".bsp") -# Directories never worth descending into when scanning for build roots: build output, dependencies, -# and the two directories (`project`, `src`) that belong to a build we would already have recognised. +# Directories not worth descending into when scanning for build roots: build output, dependencies, +# and the directories belonging to a build we would have recognised at their parent. They are still +# probed themselves — only the descent below them is skipped. BUILD_ROOT_SCAN_SKIP_DIRS = frozenset({"node_modules", "out", "project", "src", "target", "venv"}) @@ -68,13 +78,20 @@ class StaleLockMode(Enum): """Raise an error and refuse to start.""" +def _contains_json_file(path: str) -> bool: + try: + return any(entry.name.endswith(".json") for entry in os.scandir(path)) + except OSError: + return False + + def _is_build_root(path: str) -> bool: """ Whether `path` is the root of a build that Metals can import. """ if any(os.path.isfile(os.path.join(path, name)) for name in BUILD_ROOT_MARKER_FILES): return True - if any(os.path.isdir(os.path.join(path, name)) for name in BUILD_ROOT_MARKER_DIRS): + if any(_contains_json_file(os.path.join(path, name)) for name in BUILD_ROOT_MARKER_JSON_DIRS): return True # sbt allows the build to be defined entirely under project/, with no build.sbt build_properties = os.path.join(path, "project", "build.properties") @@ -104,20 +121,24 @@ def find_build_roots(repository_root_path: str, max_depth: int = DEFAULT_PROJECT return [repository_root_path] roots: list[str] = [] + visited: set[str] = set() def scan(directory: str, depth: int) -> None: - if depth > max_depth: + # symlinks are followed, as Metals' own search does, so guard against cycles + real_path = os.path.realpath(directory) + if depth > max_depth or real_path in visited: return + visited.add(real_path) try: entries = sorted(os.scandir(directory), key=lambda e: e.name) except OSError: return for entry in entries: - if not entry.is_dir(follow_symlinks=False) or entry.name.startswith(".") or entry.name in BUILD_ROOT_SCAN_SKIP_DIRS: + if entry.name.startswith(".") or not entry.is_dir(): continue if _is_build_root(entry.path): roots.append(entry.path) - else: + elif entry.name not in BUILD_ROOT_SCAN_SKIP_DIRS: scan(entry.path, depth + 1) scan(repository_root_path, 1) @@ -127,18 +148,56 @@ def scan(directory: str, depth: int) -> None: class ScalaInitializeParamsBuilder(DefaultInitializeParamsBuilder): """ Sends the repository's build roots as the workspace folders, so that Metals creates one - service per build (see `MetalsLanguageServer.initialize`), rather than the configured - workspace folders, which are about what SolidLSP indexes and are shared across languages. + service per build (see `MetalsLanguageServer.initialize`), in place of `ls_workspace_folders`, + which is about what SolidLSP indexes and is shared across a project's language servers. + + `ls_additional_workspace_folders` is still honoured: those folders can lie outside the + repository and so could never be detected, which is the whole point of the setting. """ def __init__(self, ls: SolidLanguageServer, build_roots: list[str]): - super().__init__(ls) + super().__init__(ls, set_workspace_folders=False) self._build_roots = build_roots @override def _apply_updates(self) -> None: super()._apply_updates() - self._set("workspaceFolders", [self._create_workspace_folder_entry(path) for path in self._build_roots]) + folders = list(self._build_roots) + for path in self._ls.config.get_absolute_additional_workspace_folders(self._ls.repository_root_path): + if path not in folders: + folders.append(path) + log.info("Workspace folders sent to Metals: %s", folders) + self._set("workspaceFolders", [self._create_workspace_folder_entry(path) for path in folders]) + + +def _parse_project_roots(value: object) -> list[str] | None: + """ + Validate the `project_roots` setting, returning None (i.e. detect them) if it is unusable. + """ + if value is None: + return None + if isinstance(value, str) or not isinstance(value, list) or not all(isinstance(item, str) for item in value): + log.warning(f"Invalid project_roots value {value!r}, expected a list of paths; detecting the build roots instead") + return None + roots: list[str] = [item for item in value if isinstance(item, str)] + if not roots: + log.warning("Empty project_roots; detecting the build roots instead") + return None + return roots + + +def _parse_project_root_scan_depth(value: object) -> int: + """ + Validate the `project_root_scan_depth` setting, falling back to the default if it is unusable. + """ + if value is None: + return DEFAULT_PROJECT_ROOT_SCAN_DEPTH + if isinstance(value, bool) or not isinstance(value, int) or value < 1: + log.warning( + f"Invalid project_root_scan_depth value {value!r}, expected a positive integer; using {DEFAULT_PROJECT_ROOT_SCAN_DEPTH}" + ) + return DEFAULT_PROJECT_ROOT_SCAN_DEPTH + return value def _get_scala_settings(solidlsp_settings: SolidLSPSettings) -> dict[str, object]: @@ -182,8 +241,8 @@ def _get_scala_settings(solidlsp_settings: SolidLSPSettings) -> dict[str, object "client_name": scala_settings.get("client_name", DEFAULT_CLIENT_NAME), "on_stale_lock": on_stale_lock, "log_multi_instance_notice": scala_settings.get("log_multi_instance_notice", DEFAULT_LOG_MULTI_INSTANCE_NOTICE), - "project_roots": scala_settings.get("project_roots"), - "project_root_scan_depth": scala_settings.get("project_root_scan_depth", DEFAULT_PROJECT_ROOT_SCAN_DEPTH), + "project_roots": _parse_project_roots(scala_settings.get("project_roots")), + "project_root_scan_depth": _parse_project_root_scan_depth(scala_settings.get("project_root_scan_depth")), } @@ -262,7 +321,10 @@ def _resolve_build_roots(repository_root_path: str, solidlsp_settings: SolidLSPS roots.append(abs_root) else: log.warning(f"Configured Scala project root does not exist, skipping: {abs_root}") - return roots or [repository_root_path] + if not roots: + log.error("No configured Scala project root exists; detecting the build roots instead") + return find_build_roots(repository_root_path, settings["project_root_scan_depth"]) # type: ignore[arg-type] + return roots @override def _create_initialize_params_builder(self) -> InitializeParamsBuilder: diff --git a/test/solidlsp/scala/test_scala_build_roots.py b/test/solidlsp/scala/test_scala_build_roots.py index b4da0fa697..2977bdb3a1 100644 --- a/test/solidlsp/scala/test_scala_build_roots.py +++ b/test/solidlsp/scala/test_scala_build_roots.py @@ -3,10 +3,22 @@ """ from pathlib import Path +from types import SimpleNamespace +from typing import cast +from urllib.parse import urlparse +from urllib.request import url2pathname import pytest -from solidlsp.language_servers.scala_language_server import find_build_roots +from solidlsp import SolidLanguageServer +from solidlsp.language_servers.scala_language_server import ( + DEFAULT_PROJECT_ROOT_SCAN_DEPTH, + ScalaInitializeParamsBuilder, + ScalaLanguageServer, + find_build_roots, +) +from solidlsp.ls_config import LanguageServerConfig, LanguageServerId +from solidlsp.settings import SolidLSPSettings def make_sbt_build(path: Path) -> Path: @@ -56,8 +68,9 @@ def test_hidden_and_uninteresting_directories_are_skipped(self, tmp_path: Path) make_sbt_build(tmp_path / "node_modules" / "backend") assert find_build_roots(str(tmp_path)) == [str(tmp_path)] - def test_bsp_directory_marks_a_build_root(self, tmp_path: Path) -> None: + def test_bsp_connection_file_marks_a_build_root(self, tmp_path: Path) -> None: (tmp_path / "backend" / ".bsp").mkdir(parents=True) + (tmp_path / "backend" / ".bsp" / "sbt.json").write_text("{}") assert find_build_roots(str(tmp_path)) == [str(tmp_path / "backend")] def test_sbt_build_defined_only_under_project(self, tmp_path: Path) -> None: @@ -69,3 +82,101 @@ def test_build_properties_without_a_version_is_not_a_build_root(self, tmp_path: (tmp_path / "backend" / "project").mkdir(parents=True) (tmp_path / "backend" / "project" / "build.properties").write_text("# nothing to see here\n") assert find_build_roots(str(tmp_path)) == [str(tmp_path)] + + def test_an_empty_bsp_directory_is_not_a_build_root(self, tmp_path: Path) -> None: + """Metals requires a connection file in there, not merely the directory.""" + (tmp_path / "backend" / ".bsp").mkdir(parents=True) + make_sbt_build(tmp_path / "backend" / "module") + assert find_build_roots(str(tmp_path)) == [str(tmp_path / "backend" / "module")] + + def test_a_skipped_directory_is_still_probed(self, tmp_path: Path) -> None: + """`src` and friends are not descended into, but may themselves be a build root.""" + make_sbt_build(tmp_path / "src") + assert find_build_roots(str(tmp_path)) == [str(tmp_path / "src")] + + def test_a_skipped_directory_is_not_descended_into(self, tmp_path: Path) -> None: + make_sbt_build(tmp_path / "node_modules" / "backend") + assert find_build_roots(str(tmp_path)) == [str(tmp_path)] + + def test_bazel_workspace_marker(self, tmp_path: Path) -> None: + (tmp_path / "backend").mkdir() + (tmp_path / "backend" / "MODULE.bazel").write_text("") + assert find_build_roots(str(tmp_path)) == [str(tmp_path / "backend")] + + def test_symlinked_build_is_found_without_looping(self, tmp_path: Path) -> None: + make_sbt_build(tmp_path / "elsewhere" / "backend") + (tmp_path / "repo").mkdir() + (tmp_path / "repo" / "link").symlink_to(tmp_path / "elsewhere", target_is_directory=True) + (tmp_path / "repo" / "loop").symlink_to(tmp_path / "repo", target_is_directory=True) + assert find_build_roots(str(tmp_path / "repo")) == [str(tmp_path / "repo" / "link" / "backend")] + + +@pytest.mark.scala +class TestResolveBuildRoots: + """The `project_roots` / `project_root_scan_depth` settings, which override the detection.""" + + @staticmethod + def resolve(root: Path, **scala_settings) -> list[str]: + settings = SolidLSPSettings(ls_specific_settings={LanguageServerId.SCALA: scala_settings}) + return ScalaLanguageServer._resolve_build_roots(str(root), settings) + + def test_detection_is_used_when_unset(self, tmp_path: Path) -> None: + make_sbt_build(tmp_path / "backend") + assert self.resolve(tmp_path) == [str(tmp_path / "backend")] + + def test_configured_roots_are_resolved_against_the_repository_root(self, tmp_path: Path) -> None: + (tmp_path / "a" / "b").mkdir(parents=True) + assert self.resolve(tmp_path, project_roots=["a/b"]) == [str(tmp_path / "a" / "b")] + + def test_configured_roots_need_not_look_like_builds(self, tmp_path: Path) -> None: + """The setting exists precisely for where the detection is wrong.""" + (tmp_path / "odd").mkdir() + assert self.resolve(tmp_path, project_roots=["odd"]) == [str(tmp_path / "odd")] + + def test_a_missing_configured_root_is_skipped(self, tmp_path: Path) -> None: + (tmp_path / "here").mkdir() + assert self.resolve(tmp_path, project_roots=["here", "gone"]) == [str(tmp_path / "here")] + + def test_detection_takes_over_when_no_configured_root_exists(self, tmp_path: Path) -> None: + make_sbt_build(tmp_path / "backend") + assert self.resolve(tmp_path, project_roots=["gone"]) == [str(tmp_path / "backend")] + + @pytest.mark.parametrize("bad", ["backend", 42, [1, 2], []]) + def test_a_malformed_project_roots_falls_back_to_detection(self, tmp_path: Path, bad: object) -> None: + make_sbt_build(tmp_path / "backend") + assert self.resolve(tmp_path, project_roots=bad) == [str(tmp_path / "backend")] + + @pytest.mark.parametrize("bad", [None, "3", 0, -1, True]) + def test_a_malformed_scan_depth_falls_back_to_the_default(self, tmp_path: Path, bad: object) -> None: + make_sbt_build(tmp_path / "a" / "b" / "c") + assert self.resolve(tmp_path, project_root_scan_depth=bad) == [str(tmp_path / "a" / "b" / "c")] + assert DEFAULT_PROJECT_ROOT_SCAN_DEPTH == 3 + + def test_scan_depth_is_honoured(self, tmp_path: Path) -> None: + make_sbt_build(tmp_path / "a" / "b" / "c") + assert self.resolve(tmp_path, project_root_scan_depth=2) == [str(tmp_path)] + + +@pytest.mark.scala +class TestScalaInitializeParamsBuilder: + @staticmethod + def workspace_folder_paths(tmp_path: Path, build_roots: list[str], additional: list[str]) -> list[str]: + ls = SimpleNamespace( + repository_root_path=str(tmp_path), + custom_settings={}, + config=LanguageServerConfig(ls_id=LanguageServerId.SCALA, additional_workspace_folders=additional), + ) + params = ScalaInitializeParamsBuilder(cast(SolidLanguageServer, ls), build_roots).build() + folders = params["workspaceFolders"] or [] + return [url2pathname(urlparse(folder["uri"]).path) for folder in folders] + + def test_the_build_roots_become_the_workspace_folders(self, tmp_path: Path) -> None: + roots = [str(tmp_path / "alpha"), str(tmp_path / "beta")] + assert self.workspace_folder_paths(tmp_path, roots, additional=[]) == roots + + def test_additional_workspace_folders_are_kept(self, tmp_path: Path) -> None: + """They can lie outside the repository, so detection could never recover them.""" + outside = tmp_path.parent / "shared-lib" + outside.mkdir(exist_ok=True) + roots = [str(tmp_path / "alpha")] + assert self.workspace_folder_paths(tmp_path, roots, additional=[str(outside)]) == [*roots, str(outside)]