Scala: give Metals the build roots rather than the repository root - #1767
Open
merlinorg wants to merge 2 commits into
Open
Scala: give Metals the build roots rather than the repository root#1767merlinorg wants to merge 2 commits into
merlinorg wants to merge 2 commits into
Conversation
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.
- 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1766.
Root cause
Metals serves one build per workspace folder —
MetalsLanguageServer.initializemakes aFolderper entry of theworkspaceFoldersit is given andWorkspaceFolders.initServicescreates aProjectMetalsLspServicefor each. Serena sends exactly one: the repository root, fromLanguageServerConfig.workspace_folders(default["."]) viaDefaultInitializeParamsBuilder, whichScalaLanguageServerdoes not override.Where the builds live below that root, Metals falls back to its own search,
BuildTools.searchForBuildTool— one level down,collectFirst. That is what keeps a single build in a subdirectory working, and it is exactly why a second one cannot: every build but the first is left with no build target and answered by the fallback presentation compiler, sofind_referencing_symbols/request_referencescome back empty with nothing in the logs at Serena's level to say why.ls_workspace_foldersis a real workaround for a pure-Scala repository, and #1766 says so. It runs out in a polyglot one: it is a single project-level list shared by every language server and it also sets what SolidLSP indexes, so once another server needs the repository root as its folder there is no value that suits both.Fix
ScalaLanguageServerdetects the build roots under the repository root and passes them all as workspace folders, one Metals service per build. Detection follows the markersBuildToolsprobes for — sbt (including aproject/build.propertiesnaming ansbt.versionwith nobuild.sbt), mill, gradle, maven, scala-cli, Bazel, Deder, and a.bsp/.bloopholding a connection file. It stops descending at a build root, skips build output and dependency directories, follows symlinks as Metals does, and is bounded by depth.Where the repository root is itself a build root it returns immediately and nothing changes for existing single-root users.
Two settings under
ls_specific_settings.scalafor where the heuristic guesses wrong:project_roots(an explicit list, relative to the repository root) andproject_root_scan_depth(default 3), both validated in the manner of the existingon_stale_lock.ls_additional_workspace_foldersis preserved and appended — those folders can lie outside the repository, so detection could never recover them.This uses the
_create_initialize_params_builderextension point from #1631 as intended; it is the first override of it. The shape is #1444's_find_mix_exsfor Elixir, generalised from "first match, one level" to "all of them, bounded depth".Behaviour change worth knowing
For a repository whose builds are below its root, Metals now writes
.metals/and.bloop/into each build directory instead of the repository root, and those users get a one-time re-import. The H2 stale-lock check follows, running per build root rather than only at the repository root. Single-root repositories are untouched.Testing
33 unit tests in
test/solidlsp/scala/test_scala_build_roots.pycovering detection, the two settings, and the workspace folders that actually reachinitialize. None of them need Metals, so they run in CI rather than joining the end-to-end suite #839 is about.End-to-end, against the reproduction in #1766 — before,
betareturns nothing for the full four minutes; after:ruff check,ruff format --check,ty check src/serena src/solidlspandty check test --exclude test/resourcesare clean.Checklist
CONTRIBUTING.mdregarding the scope of PRs.CHANGELOG.md, which concisely describes the change.Prepared with the help of Claude Code; I have reviewed and tested the change and will handle review myself.