From ec49d92e258ec91bde31ecd55098df02f097d55b Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Mon, 25 May 2020 15:05:45 -1000 Subject: [PATCH 1/4] WorkspaceSymbols index after successful build Previously WorkspaceSymbols depended on a side-effect of running Dialyzer: https://github.com/elixir-lsp/elixir-ls/pull/110#discussion_r376827177 Now that the Build step loads all modules (added in #227) we need to change the WorkspaceSymbols index to be rebuilt after a successful compile. --- apps/language_server/lib/language_server/server.ex | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/language_server/lib/language_server/server.ex b/apps/language_server/lib/language_server/server.ex index 734ada13a..9656a0737 100644 --- a/apps/language_server/lib/language_server/server.ex +++ b/apps/language_server/lib/language_server/server.ex @@ -183,6 +183,10 @@ defmodule ElixirLS.LanguageServer.Server do _ -> handle_build_result(:error, [Build.exception_to_diagnostic(reason)], state) end + if reason == :normal do + WorkspaceSymbols.notify_build_complete() + end + state = if state.needs_build?, do: trigger_build(state), else: state {:noreply, state} end @@ -669,8 +673,6 @@ defmodule ElixirLS.LanguageServer.Server do GenServer.reply(from, contracts) end - WorkspaceSymbols.notify_build_complete() - %{state | analysis_ready?: true, awaiting_contracts: dirty} else state From 4e75b892844cb730446fcd591a736a8e43467585 Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Mon, 25 May 2020 15:34:49 -1000 Subject: [PATCH 2/4] Fix tests Allow WorkspaceSymbol to have it's own instance of the GenServer for the test by being parameterized by the server's registered name --- .../providers/workspace_symbols.ex | 24 ++++++------- .../test/providers/workspace_symbols_test.exs | 34 ++++++++----------- 2 files changed, 26 insertions(+), 32 deletions(-) diff --git a/apps/language_server/lib/language_server/providers/workspace_symbols.ex b/apps/language_server/lib/language_server/providers/workspace_symbols.ex index 7d8c759ce..d51cf29b5 100644 --- a/apps/language_server/lib/language_server/providers/workspace_symbols.ex +++ b/apps/language_server/lib/language_server/providers/workspace_symbols.ex @@ -53,36 +53,36 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do ## Client API @spec symbols(String.t()) :: {:ok, [symbol_information_t]} - def symbols(query) do + def symbols(query, server \\ __MODULE__) do results = case query do "f " <> fun_query -> - query(:functions, fun_query) + query(:functions, fun_query, server) "t " <> type_query -> - query(:types, type_query) + query(:types, type_query, server) "c " <> callback_query -> - query(:callbacks, callback_query) + query(:callbacks, callback_query, server) module_query -> - query(:modules, module_query) + query(:modules, module_query, server) end {:ok, results} end def start_link(opts) do - GenServer.start_link(__MODULE__, :ok, opts |> Keyword.put(:name, __MODULE__)) + GenServer.start_link(__MODULE__, :ok, opts |> Keyword.put_new(:name, __MODULE__)) end - def notify_build_complete do - GenServer.cast(__MODULE__, :build_complete) + def notify_build_complete(server \\ __MODULE__) do + GenServer.cast(server, :build_complete) end @spec notify_uris_modified([String.t()]) :: :ok - def notify_uris_modified(uris) do - GenServer.cast(__MODULE__, {:uris_modified, uris}) + def notify_uris_modified(uris, server \\ __MODULE__) do + GenServer.cast(server, {:uris_modified, uris}) end ## Server Callbacks @@ -351,13 +351,13 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do |> elem(0) end - defp query(kind, query) do + defp query(kind, query, server) do case String.trim(query) do "" -> [] trimmed -> - GenServer.call(__MODULE__, {:query, kind, trimmed}) + GenServer.call(server, {:query, kind, trimmed}) end end diff --git a/apps/language_server/test/providers/workspace_symbols_test.exs b/apps/language_server/test/providers/workspace_symbols_test.exs index fae63ade9..11d14eee1 100644 --- a/apps/language_server/test/providers/workspace_symbols_test.exs +++ b/apps/language_server/test/providers/workspace_symbols_test.exs @@ -1,13 +1,11 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do - use ExUnit.Case + use ExUnit.Case, async: false alias ElixirLS.LanguageServer.Providers.WorkspaceSymbols + @server_name WorkspaceSymbolsTestServer + setup_all do - pid = - case WorkspaceSymbols.start_link([]) do - {:ok, pid} -> pid - {:error, {:already_started, pid}} -> pid - end + {:ok, pid} = WorkspaceSymbols.start_link(name: @server_name) state = :sys.get_state(pid) @@ -27,19 +25,15 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do } end) - WorkspaceSymbols.notify_build_complete() + WorkspaceSymbols.notify_build_complete(@server_name) wait_until_indexed(pid) - on_exit(fn -> - :sys.replace_state(pid, fn _ -> state end) - end) - {:ok, %{}} end test "empty query" do - assert {:ok, []} == WorkspaceSymbols.symbols("") + assert {:ok, []} == WorkspaceSymbols.symbols("", @server_name) end test "returns modules" do @@ -53,7 +47,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do }, name: "ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols" } - ]} = WorkspaceSymbols.symbols("ElixirLS.LanguageServer.Fixtures.") + ]} = WorkspaceSymbols.symbols("ElixirLS.LanguageServer.Fixtures.", @server_name) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") @@ -67,7 +61,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do }, name: "ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols" } - ]} = WorkspaceSymbols.symbols("work") + ]} = WorkspaceSymbols.symbols("work", @server_name) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") end @@ -120,7 +114,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.__info__/1" } ] - } = WorkspaceSymbols.symbols("f ElixirLS.LanguageServer.Fixtures.") + } = WorkspaceSymbols.symbols("f ElixirLS.LanguageServer.Fixtures.", @server_name) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") @@ -134,7 +128,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do }, name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_function/1" } - ]} = WorkspaceSymbols.symbols("f fun") + ]} = WorkspaceSymbols.symbols("f fun", @server_name) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") end @@ -159,7 +153,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do name: "t ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_opaque_type/0" } ] - } = WorkspaceSymbols.symbols("t ElixirLS.LanguageServer.Fixtures.") + } = WorkspaceSymbols.symbols("t ElixirLS.LanguageServer.Fixtures.", @server_name) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") @@ -175,7 +169,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do name: "t ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_opaque_type/0" } ] - } = WorkspaceSymbols.symbols("t opa") + } = WorkspaceSymbols.symbols("t opa", @server_name) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") end @@ -200,7 +194,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do name: "c ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_macrocallback/1" } ] - } = WorkspaceSymbols.symbols("c ElixirLS.LanguageServer.Fixtures.") + } = WorkspaceSymbols.symbols("c ElixirLS.LanguageServer.Fixtures.", @server_name) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") @@ -214,7 +208,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do }, name: "c ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_macrocallback/1" } - ]} = WorkspaceSymbols.symbols("c macr") + ]} = WorkspaceSymbols.symbols("c macr", @server_name) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") end From 09fc4768b759522ac62e1ab7874af48e5faf38f1 Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Fri, 29 May 2020 07:27:56 -1000 Subject: [PATCH 3/4] Give Server some more time to respond This is probably necessary now because workspace symbols is running earlier --- apps/language_server/test/server_test.exs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/apps/language_server/test/server_test.exs b/apps/language_server/test/server_test.exs index 4681b1714..81d3233d1 100644 --- a/apps/language_server/test/server_test.exs +++ b/apps/language_server/test/server_test.exs @@ -267,14 +267,13 @@ defmodule ElixirLS.LanguageServer.ServerTest do "uri" => ^error_file, "diagnostics" => [ %{ - "message" => - "(CompileError) undefined function does_not_exist" <> - _, + "message" => "(CompileError) undefined function does_not_exist" <> _, "range" => %{"end" => %{"line" => 3}, "start" => %{"line" => 3}}, "severity" => 1 } ] - }) + }), + 300 end) end @@ -288,14 +287,13 @@ defmodule ElixirLS.LanguageServer.ServerTest do "uri" => ^error_file, "diagnostics" => [ %{ - "message" => - "(SyntaxError) syntax error before: ','" <> - _, + "message" => "(SyntaxError) syntax error before: ','" <> _, "range" => %{"end" => %{"line" => 1}, "start" => %{"line" => 1}}, "severity" => 1 } ] - }) + }), + 300 end) end From 8fd61b42f61bd92e2ab6da0d7f7d06387e35af88 Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Fri, 29 May 2020 07:47:12 -1000 Subject: [PATCH 4/4] Bump up the timeout --- apps/language_server/test/server_test.exs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/language_server/test/server_test.exs b/apps/language_server/test/server_test.exs index 81d3233d1..0aa23db6c 100644 --- a/apps/language_server/test/server_test.exs +++ b/apps/language_server/test/server_test.exs @@ -273,7 +273,7 @@ defmodule ElixirLS.LanguageServer.ServerTest do } ] }), - 300 + 1000 end) end @@ -293,7 +293,7 @@ defmodule ElixirLS.LanguageServer.ServerTest do } ] }), - 300 + 1000 end) end