From 20847254d05994af9baf51e0bd0959c1cd2582b1 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Mon, 14 Aug 2023 08:07:09 +0200 Subject: [PATCH] Migrate to pull based configuration retrieval Fixes https://github.com/elixir-lsp/elixir-ls/issues/961 Fixes https://github.com/elixir-lsp/elixir-ls/issues/921 --- apps/language_server/lib/language_server.ex | 3 +- .../lib/language_server/json_rpc.ex | 17 ++- .../lib/language_server/server.ex | 136 +++++++++++++----- apps/language_server/test/server_test.exs | 39 ++++- guides/initialization.md | 2 +- 5 files changed, 158 insertions(+), 39 deletions(-) diff --git a/apps/language_server/lib/language_server.ex b/apps/language_server/lib/language_server.ex index ad45bc8cf..62e30062f 100644 --- a/apps/language_server/lib/language_server.ex +++ b/apps/language_server/lib/language_server.ex @@ -29,7 +29,8 @@ defmodule ElixirLS.LanguageServer do @impl Application def stop(_state) do - if not Application.get_env(:language_server, :restart, false) and ElixirLS.Utils.WireProtocol.io_intercepted?() do + if not Application.get_env(:language_server, :restart, false) and + ElixirLS.Utils.WireProtocol.io_intercepted?() do LanguageServer.JsonRpc.show_message( :error, "ElixirLS has crashed. See Output panel." diff --git a/apps/language_server/lib/language_server/json_rpc.ex b/apps/language_server/lib/language_server/json_rpc.ex index 072195076..405edaf34 100644 --- a/apps/language_server/lib/language_server/json_rpc.ex +++ b/apps/language_server/lib/language_server/json_rpc.ex @@ -90,11 +90,13 @@ defmodule ElixirLS.LanguageServer.JsonRpc do notify("window/logMessage", %{type: message_type_code(type), message: to_string(message)}) end - def register_capability_request(server \\ __MODULE__, method, options) do + def register_capability_request(server \\ __MODULE__, server_instance_id, method, options) do + id = server_instance_id <> method <> JasonV.encode!(options) + send_request(server, "client/registerCapability", %{ "registrations" => [ %{ - "id" => :crypto.hash(:sha, method) |> Base.encode16(), + "id" => :crypto.hash(:sha, id) |> Base.encode16(), "method" => method, "registerOptions" => options } @@ -102,6 +104,17 @@ defmodule ElixirLS.LanguageServer.JsonRpc do }) end + def get_configuration_request(server \\ __MODULE__, scope_uri, section) do + send_request(server, "workspace/configuration", %{ + "items" => [ + %{ + "scopeUri" => scope_uri, + "section" => section + } + ] + }) + end + def show_message_request(server \\ __MODULE__, type, message, actions) do send_request(server, "window/showMessageRequest", %{ "type" => message_type_code(type), diff --git a/apps/language_server/lib/language_server/server.ex b/apps/language_server/lib/language_server/server.ex index c2c8b6ba7..a96bc6836 100644 --- a/apps/language_server/lib/language_server/server.ex +++ b/apps/language_server/lib/language_server/server.ex @@ -61,7 +61,6 @@ defmodule ElixirLS.LanguageServer.Server do # Tracks source files that are currently open in the editor source_files: %{}, awaiting_contracts: [], - supports_dynamic: false, mix_project?: false, mix_env: nil, mix_target: nil, @@ -91,6 +90,8 @@ defmodule ElixirLS.LanguageServer.Server do ".sface" ] + @default_config_timeout 3 + ## Client API def start_link(name \\ nil) do @@ -235,21 +236,18 @@ defmodule ElixirLS.LanguageServer.Server do end @impl GenServer - def handle_info(:default_config, state = %__MODULE__{}) do - state = - case state do - %{settings: nil} -> - Logger.warning( - "Did not receive workspace/didChangeConfiguration notification after 5 seconds. " <> - "Using default settings." - ) - - set_settings(state, %{}) + def handle_info(:default_config, state = %__MODULE__{settings: nil}) do + Logger.warning( + "Did not receive workspace/didChangeConfiguration notification after #{@default_config_timeout} seconds. " <> + "The server will use default config." + ) - _ -> - state - end + state = set_settings(state, %{}) + {:noreply, state} + end + def handle_info(:default_config, state = %__MODULE__{}) do + # we got workspace/didChangeConfiguration in time, nothing to do here {:noreply, state} end @@ -293,11 +291,67 @@ defmodule ElixirLS.LanguageServer.Server do ## Helpers defp handle_notification(notification("initialized"), state = %__MODULE__{}) do - # If we don't receive workspace/didChangeConfiguration for 5 seconds, use default settings - Process.send_after(self(), :default_config, 5000) + # according to https://github.com/microsoft/language-server-protocol/issues/567#issuecomment-1060605611 + # this is the best point to pull configuration - if state.supports_dynamic do - add_watched_extensions(state, @default_watched_extensions) + supports_get_configuration = + get_in(state.client_capabilities, [ + "workspace", + "configuration" + ]) + + state = + if supports_get_configuration do + response = JsonRpc.get_configuration_request(state.root_uri, "elixirLS") + + case response do + {:ok, [result]} when is_map(result) -> + Logger.info( + "Received client configuration via workspace/configuration\n#{inspect(result)}" + ) + + set_settings(state, result) + + other -> + Logger.error("Cannot get client configuration: #{inspect(other)}") + state + end + else + Logger.info("Client does not support workspace/configuration request") + state + end + + unless state.settings do + # We still don't have the configuration. Let's wait for workspace/didChangeConfiguration + Process.send_after(self(), :default_config, @default_config_timeout * 1000) + end + + supports_dynamic_configuration_change_registration = + get_in(state.client_capabilities, [ + "workspace", + "didChangeConfiguration", + "dynamicRegistration" + ]) + + if supports_dynamic_configuration_change_registration do + Logger.info("Registering for workspace/didChangeConfiguration notifications") + listen_for_configuration_changes(state.server_instance_id) + else + Logger.info("Client does not support workspace/didChangeConfiguration dynamic registration") + end + + supports_dynamic_file_watcher_registration = + get_in(state.client_capabilities, [ + "workspace", + "didChangeWatchedFiles", + "dynamicRegistration" + ]) + + if supports_dynamic_file_watcher_registration do + Logger.info("Registering for workspace/didChangeWatchedFiles notifications") + add_watched_extensions(state.server_instance_id, @default_watched_extensions) + else + Logger.info("Client does not support workspace/didChangeWatchedFiles dynamic registration") end state @@ -321,6 +375,10 @@ defmodule ElixirLS.LanguageServer.Server do # the `projectDir` or `mixEnv` settings. If the settings don't match the format expected, leave # settings unchanged or set default settings if this is the first request. defp handle_notification(did_change_configuration(changed_settings), state = %__MODULE__{}) do + Logger.info( + "Received client configuration via workspace/didChangeConfiguration:\n#{inspect(changed_settings)}" + ) + prev_settings = state.settings || %{} new_settings = @@ -569,19 +627,10 @@ defmodule ElixirLS.LanguageServer.Server do state end - # Explicitly request file watchers from the client if supported - supports_dynamic = - get_in(client_capabilities, [ - "textDocument", - "codeAction", - "dynamicRegistration" - ]) - state = %{ state | client_capabilities: client_capabilities, - server_instance_id: server_instance_id, - supports_dynamic: supports_dynamic + server_instance_id: server_instance_id } {:ok, @@ -1161,7 +1210,8 @@ defmodule ElixirLS.LanguageServer.Server do |> set_mix_target(mix_target) |> set_project_dir(project_dir) |> set_dialyzer_enabled(enable_dialyzer) - |> add_watched_extensions(additional_watched_extensions) + + add_watched_extensions(state.server_instance_id, additional_watched_extensions) maybe_rebuild(state) state = create_gitignore(state) @@ -1173,25 +1223,42 @@ defmodule ElixirLS.LanguageServer.Server do trigger_build(%{state | settings: settings}) end - defp add_watched_extensions(state = %__MODULE__{}, []) do - state + defp add_watched_extensions(_server_instance_id, []) do + :ok end - defp add_watched_extensions(state = %__MODULE__{}, exts) when is_list(exts) do + defp add_watched_extensions(server_instance_id, exts) when is_list(exts) do case JsonRpc.register_capability_request( + server_instance_id, "workspace/didChangeWatchedFiles", %{ "watchers" => Enum.map(exts, &%{"globPattern" => "**/*" <> &1}) } ) do {:ok, nil} -> - :ok + Logger.info("client/registerCapability succeeded") other -> Logger.error("client/registerCapability returned: #{inspect(other)}") end + end - state + defp listen_for_configuration_changes(server_instance_id) do + # the schema is not documented in official LSP docs + # using https://github.com/microsoft/vscode-languageserver-node/blob/7792b0b21c994cc9bebc3117eeb652a22e2d9e1f/protocol/src/common/protocol.ts#L1504C18-L1504C59 + case JsonRpc.register_capability_request( + server_instance_id, + "workspace/didChangeConfiguration", + %{ + "section" => "elixirLS" + } + ) do + {:ok, nil} -> + Logger.info("client/registerCapability succeeded") + + other -> + Logger.error("client/registerCapability returned: #{inspect(other)}") + end end defp set_dialyzer_enabled(state = %__MODULE__{}, enable_dialyzer) do @@ -1221,6 +1288,7 @@ defmodule ElixirLS.LanguageServer.Server do :warning, "Environment variables change detected. ElixirLS will restart" ) + # sleep so the client has time to show the message Process.sleep(5000) ElixirLS.LanguageServer.restart() diff --git a/apps/language_server/test/server_test.exs b/apps/language_server/test/server_test.exs index 9f0b089b3..b4ce48e94 100644 --- a/apps/language_server/test/server_test.exs +++ b/apps/language_server/test/server_test.exs @@ -1,5 +1,5 @@ defmodule ElixirLS.LanguageServer.ServerTest do - alias ElixirLS.LanguageServer.{Server, Protocol, SourceFile, Tracer, Build} + alias ElixirLS.LanguageServer.{Server, Protocol, SourceFile, Tracer, Build, JsonRpc} alias ElixirLS.Utils.PacketCapture alias ElixirLS.LanguageServer.Test.FixtureHelpers alias ElixirLS.LanguageServer.Test.ServerTestHelpers @@ -59,6 +59,43 @@ defmodule ElixirLS.LanguageServer.ServerTest do assert_receive(%{"id" => 1, "result" => %{"capabilities" => %{}}}, 1000) end + test "gets configuration after initialized notification if client supports it", %{ + server: server + } do + Server.receive_packet( + server, + initialize_req(1, root_uri(), %{ + "workspace" => %{ + "configuration" => true + } + }) + ) + + assert_receive(%{"id" => 1, "result" => %{"capabilities" => %{}}}, 1000) + Server.receive_packet(server, notification("initialized")) + uri = root_uri() + + assert_receive( + %{ + "id" => 1, + "method" => "workspace/configuration", + "params" => %{"items" => [%{"scopeUri" => ^uri, "section" => "elixirLS"}]} + }, + 1000 + ) + + JsonRpc.receive_packet( + response(1, [ + %{ + "mixEnv" => "dev", + "autoBuild" => false + } + ]) + ) + + assert :sys.get_state(server).mix_env == "dev" + end + test "Execute commands should include the server instance id", %{server: server} do # If a command does not include the server instance id then it will cause # vscode-elixir-ls to fail to start up on multi-root workspaces. diff --git a/guides/initialization.md b/guides/initialization.md index 4257026dc..a92ce4d24 100644 --- a/guides/initialization.md +++ b/guides/initialization.md @@ -81,7 +81,7 @@ Upon receiving the [initialize request](https://microsoft.github.io/language-ser The delayed message is important because some clients might send a message `workspace/didChangeConfiguration`. If that happens, it will start the build with a different configuration (for example, a different MIX_ENV). -If a notification `workspace/didChangeConfiguration` or the delayed message is handled, it updates the server settings and triggers builders/analyzers and so on. +If a notification `workspace/didChangeConfiguration` is not received in 3s the server will try to get configuration via `workspace/configuration` request. If that request is not supported a default configuration is used. Finally, the server triggers builders/analyzers. ## Starts building/analyzing the project