diff --git a/CHANGELOG.md b/CHANGELOG.md index d5e7e093b7..de0fccd17e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,13 @@ 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: Metals asks via `window/showMessageRequest` whether to import a workspace it has not + seen before, and Serena had no handler, so the request failed with `MethodNotFound` and Metals gave + up on the import ("Unexpected error initializing server"). No build server was ever connected and + every cross-file query fell back to the presentation compiler, which sees one file at a time, unless + the project happened to have been imported beforehand by another editor. The three prompts that lead + to a build server are now answered; anything else is dismissed, including the choice between several + build definitions in one workspace. `ls_specific_settings.scala.auto_import_build: false` opts out * Hooks: - Add `serena-hooks --client=grok`, including Grok-native PreToolUse allow/deny output. diff --git a/docs/01-about/020_programming-languages.md b/docs/01-about/020_programming-languages.md index 46d764dbe9..9134633ac9 100644 --- a/docs/01-about/020_programming-languages.md +++ b/docs/01-about/020_programming-languages.md @@ -146,7 +146,7 @@ Some languages require additional installations or setup steps, as noted. * **Rust** (requires [rustup](https://rustup.rs/) - uses rust-analyzer from your toolchain) * **Scala** - (requires some [manual setup](../03-special-guides/scala_setup_guide_for_serena); uses Metals LSP) + (uses Metals LSP, which imports the build on first use — see the [setup guide](../03-special-guides/scala_setup_guide_for_serena)) * **SCSS / Sass / CSS** (experimental; requires Node.js + npm; uses [some-sass-language-server](https://github.com/wkillerud/some-sass) to handle `.scss`, `.sass`, and `.css`) 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..643e42acf8 100644 --- a/docs/03-special-guides/scala_setup_guide_for_serena.md +++ b/docs/03-special-guides/scala_setup_guide_for_serena.md @@ -15,7 +15,16 @@ Install the following on your system and ensure they are available on `PATH`: - Serena uses `cs` if available; if only `coursier` exists, it will attempt to install `cs`. If neither is present, install Coursier first. --- -## Quick Start (Recommended: VS Code + Metals auto‑import) +## Quick Start + +Start Serena in your project root. Metals asks whether to import a workspace it has not seen before, and Serena answers that prompt for it — the build is imported (for sbt, by running `sbt bloopInstall`), `.bloop/` and `.metals/` are created, and cross-file navigation works from there. The first run therefore takes as long as your build takes to load. + +Set `auto_import_build: false` under `ls_specific_settings.scala` to decline instead; you then need to import the build yourself by one of the routes below, or cross-file queries will be served by the fallback presentation compiler and see only one file at a time. + +Serena answers three of Metals' prompts — “Import build”, “Import changes”, “Connect”. Anything else Metals asks is dismissed and logged, including “Multiple build definitions found. Which would you like to use?”, so a workspace holding more than one kind of build (say both an sbt and a Maven definition) still needs importing by one of the routes below. + +--- +## Importing the build yourself (VS Code) 1. Open your Scala project in VS Code. 2. When prompted by Metals, accept “Import build”. Wait until the import and initial compile/indexing finish. @@ -25,7 +34,7 @@ Install the following on your system and ensure they are available on `PATH`: This flow ensures the `.bloop/` and (if applicable) `.metals/` directories are created and your build is known to the build server that Metals uses. --- -## Manual Setup (No VS Code) +## Importing the build yourself (No VS Code) Follow these steps if you prefer a manual setup or you are not using VS Code: diff --git a/src/solidlsp/language_servers/scala_language_server.py b/src/solidlsp/language_servers/scala_language_server.py index 2a7d633054..771c61043d 100644 --- a/src/solidlsp/language_servers/scala_language_server.py +++ b/src/solidlsp/language_servers/scala_language_server.py @@ -28,6 +28,22 @@ DEFAULT_CLIENT_NAME = "Serena" DEFAULT_ON_STALE_LOCK = "auto-clean" DEFAULT_LOG_MULTI_INSTANCE_NOTICE = True +DEFAULT_AUTO_IMPORT_BUILD = True + +# The `window/showMessageRequest` actions Serena answers affirmatively: the ones standing between +# an un-imported workspace and a build server. Everything else is dismissed, since a prompt we do +# not recognise is one whose consequences we cannot judge — `Messages.OldBloopVersionRunning` +# offers to kill a process, `Messages.NewScalaProject` to open a window. "Don't show again" is +# never chosen: Metals persists that dismissal in the project's own state. +# (scala/meta/internal/metals/Messages.scala, at `ImportBuild`, `ImportBuildChanges`, +# `GenerateBspAndConnect`.) +# +# Not answered, deliberately: `Messages.ChooseBuildTool` ("Multiple build definitions found. Which +# would you like to use?"), whose actions are the build tools' own executable names. It precedes +# the import prompt in a workspace holding more than one kind of build, so such a workspace is +# still not imported — but choosing a build tool for the user is a guess of a different order, and +# Metals offers no way to say "whichever you would have picked". +BUILD_IMPORT_PROMPT_ACTIONS = ("Import build", "Import changes", "Connect") class StaleLockMode(Enum): @@ -43,6 +59,28 @@ class StaleLockMode(Enum): """Raise an error and refuse to start.""" +def choose_show_message_request_action(params: dict, auto_import_build: bool = DEFAULT_AUTO_IMPORT_BUILD) -> dict | None: + """ + Choose Serena's answer to a `window/showMessageRequest`, which Metals uses to ask whether to + import the build. + + :param params: the request's `ShowMessageRequestParams` + :param auto_import_build: whether to answer the build-import prompts affirmatively + :return: the action to select, or None to select none — which is what the LSP spec provides + for, and what leaving the request unanswered fails to say + """ + message = params.get("message", "") + actions = params.get("actions") or [] + if auto_import_build: + for action in actions: + if isinstance(action, dict) and action.get("title") in BUILD_IMPORT_PROMPT_ACTIONS: + log.info(f"Metals asked: {message!r}; answering {action['title']!r}") + return action + offered = [action.get("title") for action in actions if isinstance(action, dict)] + log.info(f"Metals asked: {message!r}; dismissing (offered: {offered})") + return None + + def _get_scala_settings(solidlsp_settings: SolidLSPSettings) -> dict[str, object]: """ Extract Scala-specific settings with defaults applied. @@ -52,6 +90,7 @@ def _get_scala_settings(solidlsp_settings: SolidLSPSettings) -> dict[str, object - client_name: str - on_stale_lock: StaleLockMode - log_multi_instance_notice: bool + - auto_import_build: bool """ from solidlsp.ls_config import LanguageServerId @@ -60,6 +99,7 @@ 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, + "auto_import_build": DEFAULT_AUTO_IMPORT_BUILD, } if not solidlsp_settings.ls_specific_settings: @@ -80,6 +120,7 @@ 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), + "auto_import_build": scala_settings.get("auto_import_build", DEFAULT_AUTO_IMPORT_BUILD), } @@ -100,6 +141,14 @@ class ScalaLanguageServer(SolidLanguageServer): metals_version: '1.6.4' # Client identifier sent to Metals (default: DEFAULT_CLIENT_NAME) client_name: 'Serena' + # Answer Metals' build-import prompts affirmatively, which lets it run the project's + # build tool (e.g. sbt bloopInstall). Set false to leave the build un-imported. + auto_import_build: true + + Build import: + Metals asks, via `window/showMessageRequest`, whether to import a workspace it has not + seen before; until that is answered it has no build server and so no build target, and + every cross-file query is served by the fallback presentation compiler. Multi-instance support: Metals uses H2 AUTO_SERVER mode (enabled by default) to support multiple @@ -116,6 +165,8 @@ def __init__(self, config: LanguageServerConfig, repository_root_path: str, soli # Check for stale locks before setting up dependencies (fail-fast) self._check_metals_db_status(repository_root_path, solidlsp_settings) + self._auto_import_build: bool = _get_scala_settings(solidlsp_settings)["auto_import_build"] # type: ignore[assignment] + scala_lsp_executable_path = self._setup_runtime_dependencies(config, solidlsp_settings) super().__init__( config, @@ -300,10 +351,15 @@ def _create_base_initialize_params(self) -> dict: } return initialize_params + def _answer_show_message_request(self, params: dict) -> dict | None: + return choose_show_message_request_action(params, auto_import_build=self._auto_import_build) + def _start_server(self) -> None: """ Starts the Scala Language Server """ + self.server.on_request("window/showMessageRequest", self._answer_show_message_request) + log.info("Starting Scala server process") self.server.start() diff --git a/test/solidlsp/scala/test_scala_show_message_request.py b/test/solidlsp/scala/test_scala_show_message_request.py new file mode 100644 index 0000000000..beb336b977 --- /dev/null +++ b/test/solidlsp/scala/test_scala_show_message_request.py @@ -0,0 +1,94 @@ +""" +Unit tests for Serena's answer to Metals' `window/showMessageRequest` prompts. +""" + +import pytest + +from solidlsp.language_servers.scala_language_server import ( + _get_scala_settings, + choose_show_message_request_action, +) +from solidlsp.ls_config import LanguageServerId +from solidlsp.settings import SolidLSPSettings + +# Metals' own prompts, with the message and action titles as its `Messages` object builds them +# (scala/meta/internal/metals/Messages.scala) and `type` as lsp4j serialises `MessageType.Info`. +IMPORT_BUILD = { + "message": "New sbt workspace detected, would you like to import the build?", + "type": 3, + "actions": [{"title": "Import build"}, {"title": "Not now"}, {"title": "Don't show again"}], +} +IMPORT_CHANGES = { + "message": "sbt build needs to be re-imported", + "type": 3, + "actions": [{"title": "Import changes"}, {"title": "Not now"}, {"title": "Don't show again"}], +} +GENERATE_BSP_AND_CONNECT = { + "message": "New sbt workspace detected, would you like connect to the Bloop build server?", + "type": 3, + "actions": [{"title": "Connect"}, {"title": "Not now"}, {"title": "Don't show again"}], +} +OLD_BLOOP_VERSION_RUNNING = { + "message": "Deprecated Bloop server is still running and is taking up resources, do you want to kill the process?", + "type": 3, + "actions": [{"title": "Yes"}, {"title": "Not now"}], +} +CHOOSE_BUILD_TOOL = { + "message": "Multiple build definitions found. Which would you like to use?", + "type": 3, + "actions": [{"title": "sbt"}, {"title": "mill"}], +} + + +@pytest.mark.scala +class TestChooseShowMessageRequestAction: + @pytest.mark.parametrize("params", [IMPORT_BUILD, IMPORT_CHANGES, GENERATE_BSP_AND_CONNECT]) + def test_build_import_prompts_are_answered_affirmatively(self, params: dict) -> None: + chosen = choose_show_message_request_action(params) + assert chosen is not None + assert chosen["title"] in ("Import build", "Import changes", "Connect") + + @pytest.mark.parametrize("params", [IMPORT_BUILD, IMPORT_CHANGES, GENERATE_BSP_AND_CONNECT]) + def test_auto_import_build_can_be_turned_off(self, params: dict) -> None: + assert choose_show_message_request_action(params, auto_import_build=False) is None + + def test_dont_show_again_is_never_chosen(self) -> None: + """Metals persists that dismissal in the project's own state.""" + params = {"message": "…", "actions": [{"title": "Don't show again"}, {"title": "Not now"}]} + assert choose_show_message_request_action(params) is None + + def test_an_unrecognised_prompt_is_dismissed(self) -> None: + """Answering "Yes" here would kill a process on the user's machine.""" + assert choose_show_message_request_action(OLD_BLOOP_VERSION_RUNNING) is None + + def test_a_prompt_with_no_actions_is_dismissed(self) -> None: + assert choose_show_message_request_action({"message": "just so you know"}) is None + assert choose_show_message_request_action({"message": "just so you know", "actions": None}) is None + + def test_malformed_actions_do_not_raise(self) -> None: + params = {"message": "…", "actions": ["Import build", None, {"title": "Import build"}]} + assert choose_show_message_request_action(params) == {"title": "Import build"} + + def test_choosing_between_build_tools_is_left_alone(self) -> None: + """Naming the build tool for the user is a guess of a different order; see the comment + on BUILD_IMPORT_PROMPT_ACTIONS. A workspace with several build definitions is therefore + still not imported. + """ + assert choose_show_message_request_action(CHOOSE_BUILD_TOOL) is None + + +@pytest.mark.scala +class TestAutoImportBuildSetting: + """`auto_import_build` has to reach the handler from the project configuration.""" + + @staticmethod + def setting(**scala_settings) -> object: + settings = SolidLSPSettings(ls_specific_settings={LanguageServerId.SCALA: scala_settings}) + return _get_scala_settings(settings)["auto_import_build"] + + def test_defaults_to_true(self) -> None: + assert self.setting() is True + assert _get_scala_settings(SolidLSPSettings())["auto_import_build"] is True + + def test_can_be_disabled(self) -> None: + assert self.setting(auto_import_build=False) is False