From b1df4471aed8094639d3edc6b9cab7980f319d21 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Wed, 11 Sep 2024 14:32:59 +0200 Subject: [PATCH] add tests for read and write variables remove no longer needed hacks in completions --- .../providers/completion/suggestion.ex | 27 +--- .../providers/definition/locator.ex | 1 + .../providers/completion/suggestions_test.exs | 39 ++++++ .../providers/definition/locator_test.exs | 126 ++++++++++++++++-- .../test/providers/hover/docs_test.exs | 64 +++++++++ .../providers/references/locator_test.exs | 78 +++++++++++ 6 files changed, 296 insertions(+), 39 deletions(-) diff --git a/apps/language_server/lib/language_server/providers/completion/suggestion.ex b/apps/language_server/lib/language_server/providers/completion/suggestion.ex index fd9302376..b09260d92 100644 --- a/apps/language_server/lib/language_server/providers/completion/suggestion.ex +++ b/apps/language_server/lib/language_server/providers/completion/suggestion.ex @@ -145,32 +145,7 @@ defmodule ElixirLS.LanguageServer.Providers.Completion.Suggestion do {{line, column - String.length(prefix)}, {line, column + String.length(suffix)}} end - env = - Metadata.get_cursor_env(metadata, {line, column}, surround) - |> Metadata.add_scope_vars( - metadata, - {line, column}, - &(to_string(&1.name) != hint) - ) - - # if variable is rebound then in env there are many variables with the same name - # find the one defined closest to cursor - vars = - env.vars - |> Enum.group_by(fn %State.VarInfo{name: name} -> name end) - |> Enum.map(fn {_name, list} -> - list - |> Enum.max_by(fn - %State.VarInfo{positions: [_position]} -> - # variable is being defined - it's not a good candidate - {0, 0} - - %State.VarInfo{positions: positions} -> - Enum.min(positions) - end) - end) - - env = %{env | vars: vars} + env = Metadata.get_cursor_env(metadata, {line, column}, surround) module_store = ModuleStore.build() diff --git a/apps/language_server/lib/language_server/providers/definition/locator.ex b/apps/language_server/lib/language_server/providers/definition/locator.ex index 9959f3c96..9f0c7dcb1 100644 --- a/apps/language_server/lib/language_server/providers/definition/locator.ex +++ b/apps/language_server/lib/language_server/providers/definition/locator.ex @@ -91,6 +91,7 @@ defmodule ElixirLS.LanguageServer.Providers.Definition.Locator do %Location{type: :variable, file: nil, line: definition_line, column: definition_column} else + # find local call find_function_or_module( {nil, variable}, context, diff --git a/apps/language_server/test/providers/completion/suggestions_test.exs b/apps/language_server/test/providers/completion/suggestions_test.exs index 1f6bc5868..224e430f5 100644 --- a/apps/language_server/test/providers/completion/suggestions_test.exs +++ b/apps/language_server/test/providers/completion/suggestions_test.exs @@ -1908,6 +1908,45 @@ defmodule ElixirLS.LanguageServer.Providers.Completion.SuggestionTest do ] end + test "lists write vars in match context" do + buffer = """ + defmodule MyServer do + def my(arg = 1, a), do: :ok + end + """ + + list = + Suggestion.suggestions(buffer, 2, 20) + |> Enum.filter(fn s -> s.type == :variable end) + + assert list == [ + %{name: "arg", type: :variable} + ] + end + + test "does not list write vars" do + buffer = """ + defmodule MyServer do + [arg = 1, a] + a + end + """ + + list = + Suggestion.suggestions(buffer, 2, 14) + |> Enum.filter(fn s -> s.type == :variable end) + + # arg is a write var and is not available for read in the cursor context + assert list == [] + + list = + Suggestion.suggestions(buffer, 3, 4) + |> Enum.filter(fn s -> s.type == :variable end) + + # arg is a read var here + assert list == [%{name: "arg", type: :variable}] + end + test "lists params and vars in cond clauses" do buffer = """ defmodule MyServer do diff --git a/apps/language_server/test/providers/definition/locator_test.exs b/apps/language_server/test/providers/definition/locator_test.exs index 63551d114..e08c5b9ce 100644 --- a/apps/language_server/test/providers/definition/locator_test.exs +++ b/apps/language_server/test/providers/definition/locator_test.exs @@ -921,19 +921,19 @@ defmodule ElixirLS.LanguageServer.Providers.Definition.LocatorTest do end """ - # assert Locator.definition(buffer, 2, 32) == %Location{ - # type: :variable, - # file: nil, - # line: 2, - # column: 14 - # } - - # assert Locator.definition(buffer, 4, 16) == %Location{ - # type: :variable, - # file: nil, - # line: 4, - # column: 7 - # } + assert Locator.definition(buffer, 2, 32) == %Location{ + type: :variable, + file: nil, + line: 2, + column: 14 + } + + assert Locator.definition(buffer, 4, 16) == %Location{ + type: :variable, + file: nil, + line: 4, + column: 7 + } assert Locator.definition(buffer, 7, 32) == %Location{ type: :variable, @@ -1060,6 +1060,106 @@ defmodule ElixirLS.LanguageServer.Providers.Definition.LocatorTest do } end + # TODO not supported in Code.Fragment.surround_context + # test "find definition of &1 capture variable" do + # buffer = """ + # defmodule MyModule do + # def go() do + # abc = 5 + # & [ + # &1, + # abc, + # cde = 1, + # record_env() + # ] + # end + # end + # """ + + # assert Locator.definition(buffer, 4, 8) == %Location{ + # type: :variable, + # file: nil, + # line: 4, + # column: 7 + # } + # end + + test "find definition of write variable on definition" do + buffer = """ + defmodule MyModule do + def go() do + abc = 5 + & [ + &1, + abc, + cde = 1, + record_env() + ] + end + end + """ + + assert Locator.definition(buffer, 7, 8) == %Location{ + type: :variable, + file: nil, + line: 7, + column: 7 + } + end + + test "does not find definition of write variable on read" do + buffer = """ + defmodule MyModule do + def go() do + abc = 5 + & [ + &1, + abc, + cde = 1, + record_env(cde) + ] + end + end + """ + + assert Locator.definition(buffer, 8, 19) == nil + end + + test "find definition of write variable in match context" do + buffer = """ + defmodule MyModule do + def go(asd = 3, asd) do + :ok + end + + def go(asd = 3, [2, asd]) do + :ok + end + end + """ + + assert Locator.definition(buffer, 2, 11) == %Location{ + type: :variable, + file: nil, + line: 2, + column: 10 + } + + assert Locator.definition(buffer, 2, 20) == %Location{ + type: :variable, + file: nil, + line: 2, + column: 10 + } + + assert Locator.definition(buffer, 6, 24) == %Location{ + type: :variable, + file: nil, + line: 6, + column: 10 + } + end + test "find definition of a variable when using pin operator" do buffer = """ defmodule MyModule do diff --git a/apps/language_server/test/providers/hover/docs_test.exs b/apps/language_server/test/providers/hover/docs_test.exs index dae8d3dc0..bd8c360b5 100644 --- a/apps/language_server/test/providers/hover/docs_test.exs +++ b/apps/language_server/test/providers/hover/docs_test.exs @@ -1846,6 +1846,70 @@ defmodule ElixirLS.LanguageServer.Providers.Hover.DocsTest do docs: [%{kind: :variable}] } = Docs.docs(buffer, 8, 21) end + + test "find docs for write variable on definition" do + buffer = """ + defmodule MyModule do + def go() do + abc = 5 + & [ + &1, + abc, + cde = 1, + record_env() + ] + end + end + """ + + assert %{ + docs: [%{kind: :variable}] + } = Docs.docs(buffer, 7, 8) + end + + test "does not find docs for write variable on read" do + buffer = """ + defmodule MyModule do + def go() do + abc = 5 + & [ + &1, + abc, + cde = 1, + record_env(cde) + ] + end + end + """ + + assert Docs.docs(buffer, 8, 19) == nil + end + + test "finds docs for write variable in match context" do + buffer = """ + defmodule MyModule do + def go(asd = 3, asd) do + :ok + end + + def go(asd = 3, [2, asd]) do + :ok + end + end + """ + + assert %{ + docs: [%{kind: :variable}] + } = Docs.docs(buffer, 2, 11) + + assert %{ + docs: [%{kind: :variable}] + } = Docs.docs(buffer, 2, 20) + + assert %{ + docs: [%{kind: :variable}] + } = Docs.docs(buffer, 6, 24) + end end test "find local type in typespec local def elsewhere" do diff --git a/apps/language_server/test/providers/references/locator_test.exs b/apps/language_server/test/providers/references/locator_test.exs index 85ee60eae..2538e51ce 100644 --- a/apps/language_server/test/providers/references/locator_test.exs +++ b/apps/language_server/test/providers/references/locator_test.exs @@ -1574,6 +1574,84 @@ defmodule ElixirLS.LanguageServer.Providers.References.LocatorTest do assert Locator.references(buffer, 8, 21, trace) == expected_references end + test "find references of write variable on definition", %{trace: trace} do + buffer = """ + defmodule MyModule do + def go() do + abc = 5 + & [ + &1, + abc, + cde = 1, + record_env() + ] + end + end + """ + + expected_references = [ + %{uri: nil, range: %{start: %{line: 7, column: 7}, end: %{line: 7, column: 10}}} + ] + + assert Locator.references(buffer, 7, 8, trace) == expected_references + end + + test "does not find references of write variable on read", %{trace: trace} do + buffer = """ + defmodule MyModule do + def go() do + abc = 5 + & [ + &1, + abc, + cde = 1, + record_env(cde) + ] + end + end + """ + + expected_references = [ + %{uri: nil, range: %{start: %{line: 7, column: 7}, end: %{line: 7, column: 10}}} + ] + + # cde in cde = 1 is defined + assert Locator.references(buffer, 7, 8, trace) == expected_references + + # cde in record_env(cde) is undefined + assert Locator.references(buffer, 8, 19, trace) == [] + end + + test "find definition of write variable in match context", %{trace: trace} do + buffer = """ + defmodule MyModule do + def go(asd = 3, asd) do + :ok + end + + def go(asd = 3, [2, asd]) do + :ok + end + end + """ + + expected_references = [ + %{uri: nil, range: %{start: %{line: 2, column: 10}, end: %{line: 2, column: 13}}}, + %{uri: nil, range: %{start: %{line: 2, column: 19}, end: %{line: 2, column: 22}}} + ] + + assert Locator.references(buffer, 2, 11, trace) == expected_references + + assert Locator.references(buffer, 2, 20, trace) == expected_references + + expected_references = [ + %{uri: nil, range: %{start: %{line: 6, column: 10}, end: %{line: 6, column: 13}}}, + %{uri: nil, range: %{start: %{line: 6, column: 23}, end: %{line: 6, column: 26}}} + ] + + assert Locator.references(buffer, 6, 24, trace) == expected_references + end + test "find references of attributes", %{trace: trace} do buffer = """ defmodule MyModule do