From 1c44168918444b51032eda581687875858bd87b7 Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Mon, 25 May 2020 12:29:01 -1000 Subject: [PATCH 1/4] Change workspace symbols to only look at modules in the current application This improves speed considerably since many more modules are not processed. If the old behavior is preferred by some users we could investigate making it configurable. However, I think that this behavior is in line with LSP expectation: > The workspace symbol request is sent from the client to the server to list project-wide symbols matching the query string. from: https://microsoft.github.io/language-server-protocol/specification#workspace_symbol --- .../providers/workspace_symbols.ex | 60 +++++++------------ 1 file changed, 23 insertions(+), 37 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..b14821a5c 100644 --- a/apps/language_server/lib/language_server/providers/workspace_symbols.ex +++ b/apps/language_server/lib/language_server/providers/workspace_symbols.ex @@ -128,14 +128,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do ) do JsonRpc.log_message(:info, "[ElixirLS WorkspaceSymbols] Indexing...") - module_paths = - :code.all_loaded() - |> process_chunked(fn chunk -> - for {module, beam_file} <- chunk, - path = find_module_path(module, beam_file), - path != nil, - do: {module, path} - end) + module_paths = module_paths() JsonRpc.log_message(:info, "[ElixirLS WorkspaceSymbols] Module discovery complete") @@ -155,14 +148,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do ) do JsonRpc.log_message(:info, "[ElixirLS WorkspaceSymbols] Updating index...") - module_paths = - :code.all_loaded() - |> process_chunked(fn chunk -> - for {module, beam_file} <- chunk, - path = find_module_path(module, beam_file), - SourceFile.path_to_uri(path) in modified_uris, - do: {module, path} - end) + module_paths = module_paths() JsonRpc.log_message( :info, @@ -247,28 +233,14 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do end end - defp find_module_path(module, beam_file) do - file = - with true <- Code.ensure_loaded?(module), - path when not is_nil(path) <- module.module_info(:compile)[:source], - path_binary = List.to_string(path), - true <- File.exists?(path_binary) do - path_binary - else - _ -> nil - end - - if file do - file + defp find_module_path(module) do + with true <- Code.ensure_loaded?(module), + path when not is_nil(path) <- module.module_info(:compile)[:source], + path_binary = List.to_string(path), + true <- File.exists?(path_binary) do + path_binary else - with beam_file when not is_nil(beam_file) <- - ErlangSourceFile.get_beam_file(module, beam_file), - erl_file = ErlangSourceFile.beam_file_to_erl_file(beam_file), - true <- File.exists?(erl_file) do - erl_file - else - _ -> nil - end + _ -> nil end end @@ -483,6 +455,20 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do end) end + defp module_paths do + app = Keyword.fetch!(Mix.Project.config(), :app) + + Application.load(app) + + Application.spec(app, :modules) + |> Enum.flat_map(fn app_module -> + case find_module_path(app_module) do + nil -> [] + path -> [{app_module, path}] + end + end) + end + @spec build_result(key_t, symbol_t, String.t(), nil | non_neg_integer) :: symbol_information_t defp build_result(key, symbol, path, line) do %{ From 70719c8f6ba01667af0c7acd497866b98aa0d942 Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Sat, 30 May 2020 14:18:54 -1000 Subject: [PATCH 2/4] Umbrella-friendly version --- .../providers/workspace_symbols.ex | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 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 b14821a5c..bf7825675 100644 --- a/apps/language_server/lib/language_server/providers/workspace_symbols.ex +++ b/apps/language_server/lib/language_server/providers/workspace_symbols.ex @@ -456,19 +456,30 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do end defp module_paths do - app = Keyword.fetch!(Mix.Project.config(), :app) + app_names = app_names() - Application.load(app) + Enum.each(app_names, &Application.load/1) - Application.spec(app, :modules) - |> Enum.flat_map(fn app_module -> - case find_module_path(app_module) do - nil -> [] - path -> [{app_module, path}] - end + app_names + |> Enum.map(fn app_name -> + Application.spec(app_name, :modules) + |> Enum.flat_map(fn app_module -> + case find_module_path(app_module) do + nil -> [] + path -> [{app_module, path}] + end + end) end) end + defp app_names do + if Mix.Project.umbrella?() do + Mix.Project.apps_paths() |> Map.keys() + else + Keyword.fetch!(Mix.Project.config(), :app) + end + end + @spec build_result(key_t, symbol_t, String.t(), nil | non_neg_integer) :: symbol_information_t defp build_result(key, symbol, path, line) do %{ From db9d9c0ae80c4ab8ca250496087e3e4fd0c88f54 Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Sat, 30 May 2020 14:40:23 -1000 Subject: [PATCH 3/4] Fix: needs to be single-level of lists --- .../lib/language_server/providers/workspace_symbols.ex | 4 ++-- 1 file changed, 2 insertions(+), 2 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 9663fe62f..4af819721 100644 --- a/apps/language_server/lib/language_server/providers/workspace_symbols.ex +++ b/apps/language_server/lib/language_server/providers/workspace_symbols.ex @@ -461,7 +461,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do Enum.each(app_names, &Application.load/1) app_names - |> Enum.map(fn app_name -> + |> Enum.flat_map(fn app_name -> Application.spec(app_name, :modules) |> Enum.flat_map(fn app_module -> case find_module_path(app_module) do @@ -476,7 +476,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbols do if Mix.Project.umbrella?() do Mix.Project.apps_paths() |> Map.keys() else - Keyword.fetch!(Mix.Project.config(), :app) + [Keyword.fetch!(Mix.Project.config(), :app)] end end From bc8f1e6be5a36e0a3d295d94dd56f11e0c8e50e0 Mon Sep 17 00:00:00 2001 From: Jason Axelson Date: Sat, 30 May 2020 16:28:36 -1000 Subject: [PATCH 4/4] Fix tests --- .../test/providers/workspace_symbols_test.exs | 161 +++++++++++------- 1 file changed, 98 insertions(+), 63 deletions(-) diff --git a/apps/language_server/test/providers/workspace_symbols_test.exs b/apps/language_server/test/providers/workspace_symbols_test.exs index e3100646c..967b512fa 100644 --- a/apps/language_server/test/providers/workspace_symbols_test.exs +++ b/apps/language_server/test/providers/workspace_symbols_test.exs @@ -56,74 +56,91 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do }, name: "ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols" } - ]} = WorkspaceSymbols.symbols("ElixirLS.LanguageServer.Fixtures.", @server_name) + ]} = + WorkspaceSymbols.symbols( + "ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols", + @server_name + ) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") - assert {:ok, - [ - %{ - kind: 2, - location: %{ - range: %{end: %{character: 0, line: 1}, start: %{character: 0, line: 0}}, - uri: uri - }, - name: "ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols" - } - ]} = WorkspaceSymbols.symbols("work", @server_name) + assert {:ok, results} = WorkspaceSymbols.symbols("work", @server_name) + + assert %{ + kind: 2, + location: %{ + range: %{end: %{character: 0, line: 1}, start: %{character: 0, line: 0}}, + uri: uri + }, + name: "ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols" + } = matches(results, "ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols") assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") end test "returns functions" do - assert { - :ok, - [ - %{ - kind: 12, - location: %{ - range: %{end: %{character: 0, line: 1}, start: %{character: 0, line: 0}}, - uri: uri - }, - name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.module_info/1" - }, - %{ - kind: 12, - location: %{ - range: %{end: %{character: 0, line: 1}, start: %{character: 0, line: 0}} - }, - name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.module_info/0" - }, - %{ - kind: 12, - location: %{ - range: %{end: %{character: 0, line: 1}, start: %{character: 0, line: 0}} - }, - name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.behaviour_info/1" - }, - %{ - kind: 12, - location: %{ - range: %{end: %{character: 0, line: 3}, start: %{character: 0, line: 2}} - }, - name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_macro/1" - }, - %{ - kind: 12, - location: %{ - range: %{end: %{character: 0, line: 2}, start: %{character: 0, line: 1}} - }, - name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_function/1" - }, - %{ - kind: 12, - location: %{ - range: %{end: %{character: 0, line: 1}, start: %{character: 0, line: 0}} - }, - name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.__info__/1" - } - ] - } = WorkspaceSymbols.symbols("f ElixirLS.LanguageServer.Fixtures.", @server_name) + query = "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols" + assert {:ok, results} = WorkspaceSymbols.symbols(query, @server_name) + + assert %{ + kind: 12, + location: %{ + range: %{end: %{character: 0, line: 1}, start: %{character: 0, line: 0}}, + uri: uri + }, + name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.module_info/1" + } = + matches(results, "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.module_info/1") + + assert %{ + kind: 12, + location: %{ + range: %{end: %{character: 0, line: 1}, start: %{character: 0, line: 0}} + }, + name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.module_info/0" + } = + matches(results, "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.module_info/0") + + assert %{ + kind: 12, + location: %{ + range: %{end: %{character: 0, line: 1}, start: %{character: 0, line: 0}} + }, + name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.behaviour_info/1" + } = + matches( + results, + "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.behaviour_info/1" + ) + + assert %{ + kind: 12, + location: %{ + range: %{end: %{character: 0, line: 3}, start: %{character: 0, line: 2}} + }, + name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_macro/1" + } = + matches(results, "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_macro/1") + + assert %{ + kind: 12, + location: %{ + range: %{end: %{character: 0, line: 2}, start: %{character: 0, line: 1}} + }, + name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_function/1" + } = + matches( + results, + "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_function/1" + ) + + assert %{ + kind: 12, + location: %{ + range: %{end: %{character: 0, line: 1}, start: %{character: 0, line: 0}} + }, + name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.__info__/1" + } = matches(results, "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.__info__/1") assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") @@ -137,7 +154,7 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do }, name: "f ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_function/1" } - ]} = WorkspaceSymbols.symbols("f fun", @server_name) + ]} = WorkspaceSymbols.symbols("f some_fun", @server_name) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") end @@ -162,7 +179,11 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do name: "t ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_opaque_type/0" } ] - } = WorkspaceSymbols.symbols("t ElixirLS.LanguageServer.Fixtures.", @server_name) + } = + WorkspaceSymbols.symbols( + "t ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.", + @server_name + ) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") @@ -203,7 +224,11 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do name: "c ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.some_macrocallback/1" } ] - } = WorkspaceSymbols.symbols("c ElixirLS.LanguageServer.Fixtures.", @server_name) + } = + WorkspaceSymbols.symbols( + "c ElixirLS.LanguageServer.Fixtures.WorkspaceSymbols.", + @server_name + ) assert uri |> String.ends_with?("test/support/fixtures/workspace_symbols.ex") @@ -230,4 +255,14 @@ defmodule ElixirLS.LanguageServer.Providers.WorkspaceSymbolsTest do wait_until_indexed(pid) end end + + defp matches(results, name) do + assert [matching_result] = + Enum.filter( + results, + &(&1.name == name) + ) + + matching_result + end end