From e01f381d39c901906508ab19a32a8d3e115be475 Mon Sep 17 00:00:00 2001 From: Samuel Heldak Date: Thu, 9 Mar 2023 12:12:34 +0100 Subject: [PATCH 1/6] add replacing unknown remote function to code actions --- .../code_mod/replace_remote_function.ex | 65 ++++++ .../code_action/replace_remote_function.ex | 147 +++++++++++++ .../code_action/replace_with_underscore.ex | 31 ++- .../provider/handlers/code_action.ex | 20 +- .../replace_remote_function_test.exs | 208 ++++++++++++++++++ .../replace_with_underscore_test.exs | 67 +++--- 6 files changed, 489 insertions(+), 49 deletions(-) create mode 100644 apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex create mode 100644 apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex create mode 100644 apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs diff --git a/apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex b/apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex new file mode 100644 index 000000000..67f303840 --- /dev/null +++ b/apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex @@ -0,0 +1,65 @@ +defmodule ElixirLS.LanguageServer.Experimental.CodeMod.ReplaceRemoteFunction do + alias ElixirLS.LanguageServer.Experimental.CodeMod.Ast + alias ElixirLS.LanguageServer.Experimental.CodeMod.Diff + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit + + @spec text_edits(String.t(), Ast.t(), [atom()], atom(), atom()) :: + {:ok, [TextEdit.t()]} | :error + def text_edits(original_text, ast, module, name, suggestion) do + with {:ok, transformed} <- apply_transforms(original_text, ast, module, name, suggestion) do + {:ok, Diff.diff(original_text, transformed)} + end + end + + defp apply_transforms(line_text, quoted_ast, module, name, suggestion) do + leading_indent = leading_indent(line_text) + + updated_ast = + Macro.postwalk(quoted_ast, fn + {:., meta1, [{:__aliases__, meta2, ^module}, ^name]} -> + {:., meta1, [{:__aliases__, meta2, module}, suggestion]} + + other -> + other + end) + + if updated_ast != quoted_ast do + updated_ast + |> Ast.to_string() + # We're dealing with a single error on a single line. + # If the line doesn't compile (like it has a do with no end), ElixirSense + # adds additional lines do documents with errors, so take the first line, as it's + # the properly transformed source + |> fetch_line(0) + |> case do + {:ok, text} -> + {:ok, "#{leading_indent}#{text}"} + + error -> + error + end + else + :error + end + end + + @indent_regex ~r/^\s+/ + defp leading_indent(line_text) do + case Regex.scan(@indent_regex, line_text) do + [indent] -> indent + _ -> "" + end + end + + defp fetch_line(message, line_number) do + line = + message + |> String.split(["\r\n", "\r", "\n"]) + |> Enum.at(line_number) + + case line do + nil -> :error + other -> {:ok, other} + end + end +end diff --git a/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex new file mode 100644 index 000000000..c40749171 --- /dev/null +++ b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex @@ -0,0 +1,147 @@ +defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemoteFunction do + @moduledoc """ + Code actions that replace unknown remote function with ones suggested by the warning message + """ + + alias ElixirLS.LanguageServer.Experimental.CodeMod + alias ElixirLS.LanguageServer.Experimental.CodeMod.Ast + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeAction, as: CodeActionResult + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Diagnostic + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.WorkspaceEdit + alias ElixirLS.LanguageServer.Experimental.SourceFile + + @pattern ~r/(.*)\/(.*) is undefined or private. .*:\n(.*)/s + + @spec pattern() :: Regex.t() + def pattern, do: @pattern + + @spec apply(SourceFile.t(), Diagnostic.t()) :: [CodeActionResult.t()] + def apply(source_file, diagnostic) do + with {:ok, module, name} <- extract_function(diagnostic.message), + {:ok, suggestions} <- extract_suggestions(diagnostic.message), + one_based_line = extract_line(diagnostic), + {:ok, replies} <- + build_code_actions(source_file, one_based_line, module, name, suggestions) do + replies + else + _ -> + [] + end + end + + defp extract_function(message) do + case Regex.scan(@pattern, message) do + [[_, full_name, _, _]] -> + {module, name} = separate_module_from_name(full_name) + {:ok, module, name} + + _ -> + :error + end + end + + defp separate_module_from_name(full_name) do + {name, module} = + full_name + |> String.split(".") + |> Enum.map(&String.to_atom/1) + |> List.pop_at(-1) + + {module, name} + end + + @suggestion_pattern ~r/\* .*\/[\d]+/ + defp extract_suggestions(message) do + case Regex.scan(@pattern, message) do + [[_, _, arity, suggestions_string]] -> + suggestions = + @suggestion_pattern + |> Regex.scan(suggestions_string) + |> Enum.flat_map(fn [suggestion] -> + case String.split(suggestion, [" ", "/"]) do + ["*", name, ^arity] -> [String.to_atom(name)] + _ -> [] + end + end) + + {:ok, suggestions} + + _ -> + :error + end + end + + defp extract_line(%Diagnostic{} = diagnostic) do + diagnostic.range.start.line + end + + defp build_code_actions(%SourceFile{} = source_file, one_based_line, module, name, suggestions) do + with {:ok, line_text} <- SourceFile.fetch_text_at(source_file, one_based_line), + {:ok, line_ast} <- Ast.from(line_text), + {:ok, edits_per_suggestion} <- + text_edits_per_suggestion(line_text, line_ast, module, name, suggestions) do + case edits_per_suggestion do + [] -> + :error + + [_ | _] -> + edits_per_suggestion = + Enum.map(edits_per_suggestion, fn {text_edits, suggestion} -> + text_edits = Enum.map(text_edits, &update_line(&1, one_based_line)) + {text_edits, suggestion} + end) + + replies = + Enum.map(edits_per_suggestion, fn {text_edits, function_name} -> + CodeActionResult.new( + title: construct_title(module, function_name), + kind: :quick_fix, + edit: WorkspaceEdit.new(changes: %{source_file.uri => text_edits}) + ) + end) + + {:ok, replies} + end + end + end + + defp text_edits_per_suggestion(line_text, line_ast, module, name, suggestions) do + Enum.reduce(suggestions, {:ok, []}, fn + suggestion, {:ok, edits_per_suggestions} -> + case CodeMod.ReplaceRemoteFunction.text_edits( + line_text, + line_ast, + module, + name, + suggestion + ) do + {:ok, []} -> {:ok, edits_per_suggestions} + {:ok, text_edits} -> {:ok, [{text_edits, suggestion} | edits_per_suggestions]} + :error -> :error + end + + _suggestion, :error -> + :error + end) + end + + defp update_line(%TextEdit{} = text_edit, line_number) do + text_edit + |> put_in([:range, :start, :line], line_number - 1) + |> put_in([:range, :end, :line], line_number - 1) + end + + defp construct_title(module_list, function_name) do + module_string = + module_list + |> Enum.map(fn module -> + module + |> Atom.to_string() + |> String.trim_leading("Elixir.") + end) + |> Enum.join(".") + + "Replace function with #{module_string}.#{function_name}" + end +end diff --git a/apps/language_server/lib/language_server/experimental/provider/code_action/replace_with_underscore.ex b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_with_underscore.ex index 1abe91a82..2e31bfa21 100644 --- a/apps/language_server/lib/language_server/experimental/provider/code_action/replace_with_underscore.ex +++ b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_with_underscore.ex @@ -5,28 +5,26 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn alias ElixirLS.LanguageServer.Experimental.CodeMod alias ElixirLS.LanguageServer.Experimental.CodeMod.Ast - alias ElixirLS.LanguageServer.Experimental.Protocol.Requests.CodeAction alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeAction, as: CodeActionResult alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Diagnostic alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit alias ElixirLS.LanguageServer.Experimental.Protocol.Types.WorkspaceEdit alias ElixirLS.LanguageServer.Experimental.SourceFile - @spec apply(CodeAction.t()) :: [CodeActionResult.t()] - def apply(%CodeAction{} = code_action) do - source_file = code_action.source_file - diagnostics = get_in(code_action, [:context, :diagnostics]) || [] + @pattern ~r/variable "([^"]+)" is unused/ - diagnostics - |> Enum.flat_map(fn %Diagnostic{} = diagnostic -> - with {:ok, variable_name, one_based_line} <- extract_variable_and_line(diagnostic), - {:ok, reply} <- build_code_action(source_file, one_based_line, variable_name) do - [reply] - else - _ -> - [] - end - end) + @spec pattern() :: Regex.t() + def pattern, do: @pattern + + @spec apply(SourceFile.t(), Diagnostic.t()) :: [CodeActionResult.t()] + def apply(source_file, diagnostic) do + with {:ok, variable_name, one_based_line} <- extract_variable_and_line(diagnostic), + {:ok, reply} <- build_code_action(source_file, one_based_line, variable_name) do + [reply] + else + _ -> + [] + end end defp build_code_action(%SourceFile{} = source_file, one_based_line, variable_name) do @@ -66,9 +64,8 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn end end - @variable_re ~r/variable "([^"]+)" is unused/ defp extract_variable_name(message) do - case Regex.scan(@variable_re, message) do + case Regex.scan(@pattern, message) do [[_, variable_name]] -> {:ok, String.to_atom(variable_name)} diff --git a/apps/language_server/lib/language_server/experimental/provider/handlers/code_action.ex b/apps/language_server/lib/language_server/experimental/provider/handlers/code_action.ex index c915eb10a..77220490f 100644 --- a/apps/language_server/lib/language_server/experimental/provider/handlers/code_action.ex +++ b/apps/language_server/lib/language_server/experimental/provider/handlers/code_action.ex @@ -1,14 +1,30 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.Handlers.CodeAction do + alias ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemoteFunction alias ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUnderscore alias ElixirLS.LanguageServer.Experimental.Provider.Env alias ElixirLS.LanguageServer.Experimental.Protocol.Requests alias ElixirLS.LanguageServer.Experimental.Protocol.Responses - alias ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUnderscore require Logger def handle(%Requests.CodeAction{} = request, %Env{}) do - code_actions = ReplaceWithUnderscore.apply(request) + source_file = request.source_file + diagnostics = get_in(request, [:context, :diagnostics]) || [] + + code_actions = + Enum.flat_map(diagnostics, fn %{message: message} = diagnostic -> + cond do + String.match?(message, ReplaceRemoteFunction.pattern()) -> + ReplaceRemoteFunction.apply(source_file, diagnostic) + + String.match?(message, ReplaceWithUnderscore.pattern()) -> + ReplaceWithUnderscore.apply(source_file, diagnostic) + + true -> + [] + end + end) + reply = Responses.CodeAction.new(request.id, code_actions) {:reply, reply} diff --git a/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs new file mode 100644 index 000000000..6b0c85d9b --- /dev/null +++ b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs @@ -0,0 +1,208 @@ +defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemoteFunctionTest do + alias ElixirLS.LanguageServer.Experimental.CodeMod.Diff + alias ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemoteFunction + alias ElixirLS.LanguageServer.Experimental.Protocol.Requests + alias ElixirLS.LanguageServer.Experimental.Protocol.Requests.CodeAction + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeAction, as: CodeActionReply + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeActionContext + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Diagnostic + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Range + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit + alias ElixirLS.LanguageServer.Experimental.SourceFile + alias ElixirLS.LanguageServer.Fixtures.LspProtocol + alias ElixirLS.LanguageServer.SourceFile.Path, as: SourceFilePath + + import LspProtocol + + use ExUnit.Case + + setup do + {:ok, _} = start_supervised(SourceFile.Store) + :ok + end + + defp diagnostic_message(arity) do + """ + Enum.counts/#{arity} is undefined or private. Did you mean: + + * concat/1 + * concat/2 + * count/1 + * count/2 + """ + end + + defp code_action(file_body, file_path, line, opts \\ []) do + trimmed_body = String.trim(file_body, "\n") + + file_uri = SourceFilePath.to_uri(file_path) + SourceFile.Store.open(file_uri, trimmed_body, 0) + + {:ok, range} = + build(Range, + start: [line: line, character: 0], + end: [line: line, character: 0] + ) + + message = + Keyword.get_lazy(opts, :diagnostic_message, fn -> + diagnostic_message(1) + end) + + diagnostic = Diagnostic.new(range: range, message: message) + {:ok, context} = build(CodeActionContext, diagnostics: [diagnostic]) + + {:ok, action} = + build(CodeAction, + text_document: [uri: file_uri], + range: range, + context: context + ) + + {:ok, action} = Requests.to_elixir(action) + {file_uri, action} + end + + defp assert_expected_text_edits(file_uri, action, expected_name, line) do + assert %CodeActionReply{edit: %{changes: %{^file_uri => edits}}} = action + + expected_edits = Diff.diff("counts", expected_name) + + assert edits + |> Enum.zip(expected_edits) + |> Enum.all?(fn {%TextEdit{new_text: new_text}, %TextEdit{new_text: expected_new_text}} -> + new_text == expected_new_text + end) + + assert Enum.all?(edits, fn edit -> edit.range.start.line == line end) + assert Enum.all?(edits, fn edit -> edit.range.end.line == line end) + end + + test "produces no actions if the function is not found" do + assert {_, action} = code_action("Enum.count([1, 2])", "/project/file.ex", 0) + + source_file = action.source_file + diagnostics = get_in(action, [:context, :diagnostics]) + + Enum.each(diagnostics, fn diagnostic -> + assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) + end) + end + + test "produces no actions if the line is empty" do + {_, action} = code_action("", "/project/file.ex", 0) + + source_file = action.source_file + diagnostics = get_in(action, [:context, :diagnostics]) + + Enum.each(diagnostics, fn diagnostic -> + assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) + end) + end + + test "produces no results if the diagnostic message doesn't fit the format" do + assert {_, action} = + code_action("", "/project/file.ex", 0, diagnostic_message: "This isn't cool") + + source_file = action.source_file + diagnostics = get_in(action, [:context, :diagnostics]) + + Enum.each(diagnostics, fn diagnostic -> + assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) + end) + end + + test "produces no results for buggy source code" do + {_, action} = + ~S[ + 1 + 2~/3 ; 4ab( + ] + |> code_action("/project/file.ex", 0) + + source_file = action.source_file + diagnostics = get_in(action, [:context, :diagnostics]) + + Enum.each(diagnostics, fn diagnostic -> + assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) + end) + end + + test "applied to an isolated function" do + {file_uri, code_action} = + ~S[ + Enum.counts(a) + ] + |> code_action("/project/file.ex", 0) + + source_file = code_action.source_file + [diagnostic] = get_in(code_action, [:context, :diagnostics]) + + assert [to_count_action, to_concat_action] = + ReplaceRemoteFunction.apply(source_file, diagnostic) + + assert_expected_text_edits(file_uri, to_count_action, "count", 0) + assert_expected_text_edits(file_uri, to_concat_action, "concat", 0) + end + + test "works for a function assigned to a variable" do + {file_uri, code_action} = + ~S[ + var = &Enum.counts/1 + ] + |> code_action("/project/file.ex", 0) + + source_file = code_action.source_file + [diagnostic] = get_in(code_action, [:context, :diagnostics]) + + assert [to_count_action, to_concat_action] = + ReplaceRemoteFunction.apply(source_file, diagnostic) + + assert_expected_text_edits(file_uri, to_count_action, "count", 0) + assert_expected_text_edits(file_uri, to_concat_action, "concat", 0) + end + + test "works with multiple lines" do + {file_uri, code_action} = ~S[ + defmodule MyModule do + def my_func(a) do + Enum.counts(a) + end + end + ] |> code_action("/project/file.ex", 2) + + source_file = code_action.source_file + [diagnostic] = get_in(code_action, [:context, :diagnostics]) + + assert [to_count_action, to_concat_action] = + ReplaceRemoteFunction.apply(source_file, diagnostic) + + assert_expected_text_edits(file_uri, to_count_action, "count", 2) + assert_expected_text_edits(file_uri, to_concat_action, "concat", 2) + end + + test "proposed functions need to match function arity" do + {_, code_action} = + ~S[ + Enum.counts(a) + ] + |> code_action("/project/file.ex", 0, diagnostic_message: diagnostic_message(3)) + + source_file = code_action.source_file + [diagnostic] = get_in(code_action, [:context, :diagnostics]) + + assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) + end + + test "does not replace variables" do + {_, code_action} = + ~S[ + counts + 42 + ] + |> code_action("/project/file.ex", 0) + + source_file = code_action.source_file + [diagnostic] = get_in(code_action, [:context, :diagnostics]) + + assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) + end +end diff --git a/apps/language_server/test/experimental/provider/code_action/replace_with_underscore_test.exs b/apps/language_server/test/experimental/provider/code_action/replace_with_underscore_test.exs index baa1195f0..6c176d426 100644 --- a/apps/language_server/test/experimental/provider/code_action/replace_with_underscore_test.exs +++ b/apps/language_server/test/experimental/provider/code_action/replace_with_underscore_test.exs @@ -11,7 +11,6 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn alias ElixirLS.LanguageServer.SourceFile.Path, as: SourceFilePath import LspProtocol - import ReplaceWithUnderscore use ExUnit.Case use Patch @@ -70,12 +69,24 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn test "produces no actions if the name or variable is not found" do assert {_, action} = code_action("other_var = 6", "/project/file.ex", 1, "not_found") - assert [] = apply(action) + + source_file = action.source_file + diagnostics = get_in(action, [:context, :diagnostics]) + + Enum.each(diagnostics, fn diagnostic -> + assert [] = ReplaceWithUnderscore.apply(source_file, diagnostic) + end) end test "produces no actions if the line is empty" do {_, action} = code_action("", "/project/file.ex", 1, "a") - assert [] = apply(action) + + source_file = action.source_file + diagnostics = get_in(action, [:context, :diagnostics]) + + Enum.each(diagnostics, fn diagnostic -> + assert [] = ReplaceWithUnderscore.apply(source_file, diagnostic) + end) end test "produces no results if the diagnostic message doesn't fit the format" do @@ -84,7 +95,12 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn diagnostic_message: "This isn't cool" ) - assert [] = apply(action) + source_file = action.source_file + diagnostics = get_in(action, [:context, :diagnostics]) + + Enum.each(diagnostics, fn diagnostic -> + assert [] = ReplaceWithUnderscore.apply(source_file, diagnostic) + end) end test "produces no results for buggy source code" do @@ -94,31 +110,12 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn ] |> code_action("/project/file.ex", 0, "unused") - assert [] = apply(action) - end - - test "handles nil context" do - assert {_, action} = code_action("other_var = 6", "/project/file.ex", 1, "not_found") - - action = put_in(action, [:context], nil) + source_file = action.source_file + diagnostics = get_in(action, [:context, :diagnostics]) - assert [] = apply(action) - end - - test "handles nil diagnostics" do - assert {_, action} = code_action("other_var = 6", "/project/file.ex", 1, "not_found") - - action = put_in(action, [:context, :diagnostics], nil) - - assert [] = apply(action) - end - - test "handles empty diagnostics" do - assert {_, action} = code_action("other_var = 6", "/project/file.ex", 1, "not_found") - - action = put_in(action, [:context, :diagnostics], []) - - assert [] = apply(action) + Enum.each(diagnostics, fn diagnostic -> + assert [] = ReplaceWithUnderscore.apply(source_file, diagnostic) + end) end test "applied to an unadorned param" do @@ -128,7 +125,12 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn ] |> code_action("/project/file.ex", 0, "a") - assert [%CodeActionReply{edit: %{changes: %{^file_uri => [edit]}}}] = apply(code_action) + source_file = code_action.source_file + [diagnostic] = get_in(code_action, [:context, :diagnostics]) + + assert [%CodeActionReply{edit: %{changes: %{^file_uri => [edit]}}}] = + ReplaceWithUnderscore.apply(source_file, diagnostic) + assert edit.new_text == "_" end @@ -140,7 +142,12 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn end ] |> code_action("/project/file.ex", 1, "a") - assert [%CodeActionReply{edit: %{changes: %{^file_uri => [edit]}}}] = apply(code_action) + source_file = code_action.source_file + [diagnostic] = get_in(code_action, [:context, :diagnostics]) + + assert [%CodeActionReply{edit: %{changes: %{^file_uri => [edit]}}}] = + ReplaceWithUnderscore.apply(source_file, diagnostic) + assert edit.new_text == "_" assert edit.range.start.line == 1 assert edit.range.end.line == 1 From df73277a9e7120a549a6c5c17cf0ffac45e548f3 Mon Sep 17 00:00:00 2001 From: Samuel Heldak Date: Thu, 9 Mar 2023 12:31:37 +0100 Subject: [PATCH 2/6] fix aliases --- .../provider/code_action/replace_remote_function.ex | 4 ++-- .../code_action/replace_remote_function_test.exs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex index c40749171..63cf98701 100644 --- a/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex +++ b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex @@ -8,7 +8,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeAction, as: CodeActionResult alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Diagnostic alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit - alias ElixirLS.LanguageServer.Experimental.Protocol.Types.WorkspaceEdit + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Workspace alias ElixirLS.LanguageServer.Experimental.SourceFile @pattern ~r/(.*)\/(.*) is undefined or private. .*:\n(.*)/s @@ -97,7 +97,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote CodeActionResult.new( title: construct_title(module, function_name), kind: :quick_fix, - edit: WorkspaceEdit.new(changes: %{source_file.uri => text_edits}) + edit: Workspace.Edit.new(changes: %{source_file.uri => text_edits}) ) end) diff --git a/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs index 6b0c85d9b..ee6a101f3 100644 --- a/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs +++ b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs @@ -1,13 +1,13 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemoteFunctionTest do alias ElixirLS.LanguageServer.Experimental.CodeMod.Diff - alias ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemoteFunction alias ElixirLS.LanguageServer.Experimental.Protocol.Requests - alias ElixirLS.LanguageServer.Experimental.Protocol.Requests.CodeAction + alias ElixirLS.LanguageServer.Experimental.Protocol.Requests.CodeAction, as: CodeActionRequest + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeAction alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeAction, as: CodeActionReply - alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeActionContext alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Diagnostic alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Range alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit + alias ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemoteFunction alias ElixirLS.LanguageServer.Experimental.SourceFile alias ElixirLS.LanguageServer.Fixtures.LspProtocol alias ElixirLS.LanguageServer.SourceFile.Path, as: SourceFilePath @@ -50,10 +50,10 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end) diagnostic = Diagnostic.new(range: range, message: message) - {:ok, context} = build(CodeActionContext, diagnostics: [diagnostic]) + {:ok, context} = build(CodeAction.Context, diagnostics: [diagnostic]) {:ok, action} = - build(CodeAction, + build(CodeActionRequest, text_document: [uri: file_uri], range: range, context: context From 3b5d778054377c15f58253687226d6071da0c506 Mon Sep 17 00:00:00 2001 From: Samuel Heldak Date: Thu, 16 Mar 2023 14:09:44 +0100 Subject: [PATCH 3/6] adjust to review --- .../code_mod/replace_remote_function.ex | 36 ++--- .../code_mod/replace_with_underscore.ex | 25 +--- .../experimental/code_mod/text.ex | 21 +++ .../code_action/replace_remote_function.ex | 139 +++++++++--------- .../code_action/replace_with_underscore.ex | 31 ++-- .../provider/handlers/code_action.ex | 18 +-- .../replace_remote_function_test.exs | 84 +++++------ .../replace_with_underscore_test.exs | 67 ++++----- 8 files changed, 186 insertions(+), 235 deletions(-) create mode 100644 apps/language_server/lib/language_server/experimental/code_mod/text.ex diff --git a/apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex b/apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex index 67f303840..2e3f64f60 100644 --- a/apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex +++ b/apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex @@ -1,23 +1,25 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.ReplaceRemoteFunction do alias ElixirLS.LanguageServer.Experimental.CodeMod.Ast alias ElixirLS.LanguageServer.Experimental.CodeMod.Diff + alias ElixirLS.LanguageServer.Experimental.CodeMod.Text alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit @spec text_edits(String.t(), Ast.t(), [atom()], atom(), atom()) :: {:ok, [TextEdit.t()]} | :error - def text_edits(original_text, ast, module, name, suggestion) do - with {:ok, transformed} <- apply_transforms(original_text, ast, module, name, suggestion) do + def text_edits(original_text, ast, module_aliases, name, suggestion) do + with {:ok, transformed} <- + apply_transforms(original_text, ast, module_aliases, name, suggestion) do {:ok, Diff.diff(original_text, transformed)} end end - defp apply_transforms(line_text, quoted_ast, module, name, suggestion) do - leading_indent = leading_indent(line_text) + defp apply_transforms(line_text, quoted_ast, module_aliases, name, suggestion) do + leading_indent = Text.leading_indent(line_text) updated_ast = Macro.postwalk(quoted_ast, fn - {:., meta1, [{:__aliases__, meta2, ^module}, ^name]} -> - {:., meta1, [{:__aliases__, meta2, module}, suggestion]} + {:., function_meta, [{:__aliases__, module_meta, ^module_aliases}, ^name]} -> + {:., function_meta, [{:__aliases__, module_meta, module_aliases}, suggestion]} other -> other @@ -30,7 +32,7 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.ReplaceRemoteFunction do # If the line doesn't compile (like it has a do with no end), ElixirSense # adds additional lines do documents with errors, so take the first line, as it's # the properly transformed source - |> fetch_line(0) + |> Text.fetch_line(0) |> case do {:ok, text} -> {:ok, "#{leading_indent}#{text}"} @@ -42,24 +44,4 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.ReplaceRemoteFunction do :error end end - - @indent_regex ~r/^\s+/ - defp leading_indent(line_text) do - case Regex.scan(@indent_regex, line_text) do - [indent] -> indent - _ -> "" - end - end - - defp fetch_line(message, line_number) do - line = - message - |> String.split(["\r\n", "\r", "\n"]) - |> Enum.at(line_number) - - case line do - nil -> :error - other -> {:ok, other} - end - end end diff --git a/apps/language_server/lib/language_server/experimental/code_mod/replace_with_underscore.ex b/apps/language_server/lib/language_server/experimental/code_mod/replace_with_underscore.ex index bbb84bad7..dfa3679a4 100644 --- a/apps/language_server/lib/language_server/experimental/code_mod/replace_with_underscore.ex +++ b/apps/language_server/lib/language_server/experimental/code_mod/replace_with_underscore.ex @@ -1,6 +1,7 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.ReplaceWithUnderscore do alias ElixirLS.LanguageServer.Experimental.CodeMod.Ast alias ElixirLS.LanguageServer.Experimental.CodeMod.Diff + alias ElixirLS.LanguageServer.Experimental.CodeMod.Text alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit @spec text_edits(String.t(), Ast.t(), String.t() | atom) :: {:ok, [TextEdit.t()]} | :error @@ -28,7 +29,7 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.ReplaceWithUnderscore do defp apply_transform(line_text, quoted_ast, unused_variable_name) do underscored_variable_name = :"_#{unused_variable_name}" - leading_indent = leading_indent(line_text) + leading_indent = Text.leading_indent(line_text) Macro.postwalk(quoted_ast, fn {^unused_variable_name, meta, context} -> @@ -42,7 +43,7 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.ReplaceWithUnderscore do # If the line doesn't compile (like it has a do with no end), ElixirSense # adds additional lines do documents with errors, so take the first line, as it's # the properly transformed source - |> fetch_line(0) + |> Text.fetch_line(0) |> case do {:ok, text} -> {:ok, "#{leading_indent}#{text}"} @@ -51,24 +52,4 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.ReplaceWithUnderscore do error end end - - @indent_regex ~r/^\s+/ - defp leading_indent(line_text) do - case Regex.scan(@indent_regex, line_text) do - [indent] -> indent - _ -> "" - end - end - - defp fetch_line(message, line_number) do - line = - message - |> String.split(["\r\n", "\r", "\n"]) - |> Enum.at(line_number) - - case line do - nil -> :error - other -> {:ok, other} - end - end end diff --git a/apps/language_server/lib/language_server/experimental/code_mod/text.ex b/apps/language_server/lib/language_server/experimental/code_mod/text.ex new file mode 100644 index 000000000..d8d203df5 --- /dev/null +++ b/apps/language_server/lib/language_server/experimental/code_mod/text.ex @@ -0,0 +1,21 @@ +defmodule ElixirLS.LanguageServer.Experimental.CodeMod.Text do + @indent_regex ~r/^\s+/ + def leading_indent(line_text) do + case Regex.scan(@indent_regex, line_text) do + [indent] -> indent + _ -> "" + end + end + + def fetch_line(message, line_number) do + line = + message + |> String.split(["\r\n", "\r", "\n"]) + |> Enum.at(line_number) + + case line do + nil -> :error + other -> {:ok, other} + end + end +end diff --git a/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex index 63cf98701..f3e451298 100644 --- a/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex +++ b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex @@ -5,36 +5,40 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote alias ElixirLS.LanguageServer.Experimental.CodeMod alias ElixirLS.LanguageServer.Experimental.CodeMod.Ast + alias ElixirLS.LanguageServer.Experimental.Protocol.Requests.CodeAction alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeAction, as: CodeActionResult alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Diagnostic alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Workspace alias ElixirLS.LanguageServer.Experimental.SourceFile - @pattern ~r/(.*)\/(.*) is undefined or private. .*:\n(.*)/s + @function_re ~r/(.*)\/(.*) is undefined or private. .*:\n(.*)/s - @spec pattern() :: Regex.t() - def pattern, do: @pattern + @spec apply(CodeAction.t()) :: [CodeActionResult.t()] + def apply(%CodeAction{} = code_action) do + source_file = code_action.source_file + diagnostics = get_in(code_action, [:context, :diagnostics]) || [] - @spec apply(SourceFile.t(), Diagnostic.t()) :: [CodeActionResult.t()] - def apply(source_file, diagnostic) do - with {:ok, module, name} <- extract_function(diagnostic.message), - {:ok, suggestions} <- extract_suggestions(diagnostic.message), - one_based_line = extract_line(diagnostic), - {:ok, replies} <- - build_code_actions(source_file, one_based_line, module, name, suggestions) do - replies - else - _ -> - [] - end + diagnostics + |> Enum.flat_map(fn %Diagnostic{} = diagnostic -> + one_based_line = extract_start_line(diagnostic) + suggestions = extract_suggestions(diagnostic.message) + + with {:ok, module_aliases, name} <- extract_function(diagnostic.message), + {:ok, replies} <- + build_code_actions(source_file, one_based_line, module_aliases, name, suggestions) do + replies + else + _ -> [] + end + end) end defp extract_function(message) do - case Regex.scan(@pattern, message) do + case Regex.scan(@function_re, message) do [[_, full_name, _, _]] -> - {module, name} = separate_module_from_name(full_name) - {:ok, module, name} + {module_aliases, name} = separate_module_from_name(full_name) + {:ok, module_aliases, name} _ -> :error @@ -42,60 +46,59 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end defp separate_module_from_name(full_name) do - {name, module} = + {name, module_aliases} = full_name |> String.split(".") |> Enum.map(&String.to_atom/1) |> List.pop_at(-1) - {module, name} + {module_aliases, name} end - @suggestion_pattern ~r/\* .*\/[\d]+/ + @suggestion_re ~r/\* .*\/[\d]+/ defp extract_suggestions(message) do - case Regex.scan(@pattern, message) do + case Regex.scan(@function_re, message) do [[_, _, arity, suggestions_string]] -> - suggestions = - @suggestion_pattern - |> Regex.scan(suggestions_string) - |> Enum.flat_map(fn [suggestion] -> - case String.split(suggestion, [" ", "/"]) do - ["*", name, ^arity] -> [String.to_atom(name)] - _ -> [] - end - end) - - {:ok, suggestions} + @suggestion_re + |> Regex.scan(suggestions_string) + |> Enum.flat_map(fn [suggestion] -> + case String.split(suggestion, [" ", "/"]) do + ["*", name, ^arity] -> [String.to_atom(name)] + _ -> [] + end + end) _ -> - :error + [] end end - defp extract_line(%Diagnostic{} = diagnostic) do + defp extract_start_line(%Diagnostic{} = diagnostic) do diagnostic.range.start.line end - defp build_code_actions(%SourceFile{} = source_file, one_based_line, module, name, suggestions) do + defp build_code_actions( + %SourceFile{} = source_file, + one_based_line, + module_aliases, + name, + suggestions + ) do with {:ok, line_text} <- SourceFile.fetch_text_at(source_file, one_based_line), {:ok, line_ast} <- Ast.from(line_text), {:ok, edits_per_suggestion} <- - text_edits_per_suggestion(line_text, line_ast, module, name, suggestions) do + text_edits_per_suggestion(line_text, line_ast, module_aliases, name, suggestions) do case edits_per_suggestion do [] -> :error [_ | _] -> - edits_per_suggestion = + replies = Enum.map(edits_per_suggestion, fn {text_edits, suggestion} -> text_edits = Enum.map(text_edits, &update_line(&1, one_based_line)) - {text_edits, suggestion} - end) - replies = - Enum.map(edits_per_suggestion, fn {text_edits, function_name} -> CodeActionResult.new( - title: construct_title(module, function_name), + title: construct_title(module_aliases, suggestion), kind: :quick_fix, edit: Workspace.Edit.new(changes: %{source_file.uri => text_edits}) ) @@ -106,24 +109,25 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end end - defp text_edits_per_suggestion(line_text, line_ast, module, name, suggestions) do - Enum.reduce(suggestions, {:ok, []}, fn - suggestion, {:ok, edits_per_suggestions} -> - case CodeMod.ReplaceRemoteFunction.text_edits( - line_text, - line_ast, - module, - name, - suggestion - ) do - {:ok, []} -> {:ok, edits_per_suggestions} - {:ok, text_edits} -> {:ok, [{text_edits, suggestion} | edits_per_suggestions]} - :error -> :error - end - - _suggestion, :error -> - :error + defp text_edits_per_suggestion(line_text, line_ast, module_aliases, name, suggestions) do + suggestions + |> Enum.reduce_while([], fn suggestion, acc -> + case CodeMod.ReplaceRemoteFunction.text_edits( + line_text, + line_ast, + module_aliases, + name, + suggestion + ) do + {:ok, []} -> {:cont, acc} + {:ok, edits} -> {:cont, [{edits, suggestion} | acc]} + :error -> {:halt, :error} + end end) + |> case do + :error -> :error + edits -> {:ok, edits} + end end defp update_line(%TextEdit{} = text_edit, line_number) do @@ -132,16 +136,9 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote |> put_in([:range, :end, :line], line_number - 1) end - defp construct_title(module_list, function_name) do - module_string = - module_list - |> Enum.map(fn module -> - module - |> Atom.to_string() - |> String.trim_leading("Elixir.") - end) - |> Enum.join(".") - - "Replace function with #{module_string}.#{function_name}" + defp construct_title(module_aliases, suggestion) do + module_string = Enum.map_join(module_aliases, ".", &Atom.to_string/1) + + "Replace with #{module_string}.#{suggestion}" end end diff --git a/apps/language_server/lib/language_server/experimental/provider/code_action/replace_with_underscore.ex b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_with_underscore.ex index 49323ed70..ceaa10f60 100644 --- a/apps/language_server/lib/language_server/experimental/provider/code_action/replace_with_underscore.ex +++ b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_with_underscore.ex @@ -5,26 +5,28 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn alias ElixirLS.LanguageServer.Experimental.CodeMod alias ElixirLS.LanguageServer.Experimental.CodeMod.Ast + alias ElixirLS.LanguageServer.Experimental.Protocol.Requests.CodeAction alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeAction, as: CodeActionResult alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Diagnostic alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Workspace alias ElixirLS.LanguageServer.Experimental.SourceFile - @pattern ~r/variable "([^"]+)" is unused/ + @spec apply(CodeAction.t()) :: [CodeActionResult.t()] + def apply(%CodeAction{} = code_action) do + source_file = code_action.source_file + diagnostics = get_in(code_action, [:context, :diagnostics]) || [] - @spec pattern() :: Regex.t() - def pattern, do: @pattern - - @spec apply(SourceFile.t(), Diagnostic.t()) :: [CodeActionResult.t()] - def apply(source_file, diagnostic) do - with {:ok, variable_name, one_based_line} <- extract_variable_and_line(diagnostic), - {:ok, reply} <- build_code_action(source_file, one_based_line, variable_name) do - [reply] - else - _ -> - [] - end + diagnostics + |> Enum.flat_map(fn %Diagnostic{} = diagnostic -> + with {:ok, variable_name, one_based_line} <- extract_variable_and_line(diagnostic), + {:ok, reply} <- build_code_action(source_file, one_based_line, variable_name) do + [reply] + else + _ -> + [] + end + end) end defp build_code_action(%SourceFile{} = source_file, one_based_line, variable_name) do @@ -64,8 +66,9 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn end end + @variable_re ~r/variable "([^"]+)" is unused/ defp extract_variable_name(message) do - case Regex.scan(@pattern, message) do + case Regex.scan(@variable_re, message) do [[_, variable_name]] -> {:ok, String.to_atom(variable_name)} diff --git a/apps/language_server/lib/language_server/experimental/provider/handlers/code_action.ex b/apps/language_server/lib/language_server/experimental/provider/handlers/code_action.ex index 77220490f..3d899eccc 100644 --- a/apps/language_server/lib/language_server/experimental/provider/handlers/code_action.ex +++ b/apps/language_server/lib/language_server/experimental/provider/handlers/code_action.ex @@ -7,23 +7,11 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.Handlers.CodeAction do require Logger - def handle(%Requests.CodeAction{} = request, %Env{}) do - source_file = request.source_file - diagnostics = get_in(request, [:context, :diagnostics]) || [] + @code_actions [ReplaceRemoteFunction, ReplaceWithUnderscore] + def handle(%Requests.CodeAction{} = request, %Env{}) do code_actions = - Enum.flat_map(diagnostics, fn %{message: message} = diagnostic -> - cond do - String.match?(message, ReplaceRemoteFunction.pattern()) -> - ReplaceRemoteFunction.apply(source_file, diagnostic) - - String.match?(message, ReplaceWithUnderscore.pattern()) -> - ReplaceWithUnderscore.apply(source_file, diagnostic) - - true -> - [] - end - end) + Enum.flat_map(@code_actions, fn code_action_module -> code_action_module.apply(request) end) reply = Responses.CodeAction.new(request.id, code_actions) diff --git a/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs index ee6a101f3..00d4640dc 100644 --- a/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs +++ b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs @@ -13,8 +13,10 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote alias ElixirLS.LanguageServer.SourceFile.Path, as: SourceFilePath import LspProtocol + import ReplaceRemoteFunction use ExUnit.Case + use Patch setup do {:ok, _} = start_supervised(SourceFile.Store) @@ -80,36 +82,19 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote test "produces no actions if the function is not found" do assert {_, action} = code_action("Enum.count([1, 2])", "/project/file.ex", 0) - - source_file = action.source_file - diagnostics = get_in(action, [:context, :diagnostics]) - - Enum.each(diagnostics, fn diagnostic -> - assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) - end) + assert [] = apply(action) end test "produces no actions if the line is empty" do {_, action} = code_action("", "/project/file.ex", 0) - - source_file = action.source_file - diagnostics = get_in(action, [:context, :diagnostics]) - - Enum.each(diagnostics, fn diagnostic -> - assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) - end) + assert [] = apply(action) end test "produces no results if the diagnostic message doesn't fit the format" do assert {_, action} = code_action("", "/project/file.ex", 0, diagnostic_message: "This isn't cool") - source_file = action.source_file - diagnostics = get_in(action, [:context, :diagnostics]) - - Enum.each(diagnostics, fn diagnostic -> - assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) - end) + assert [] = apply(action) end test "produces no results for buggy source code" do @@ -119,12 +104,31 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote ] |> code_action("/project/file.ex", 0) - source_file = action.source_file - diagnostics = get_in(action, [:context, :diagnostics]) + assert [] = apply(action) + end - Enum.each(diagnostics, fn diagnostic -> - assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) - end) + test "handles nil context" do + assert {_, action} = code_action("other_var = 6", "/project/file.ex", 0) + + action = put_in(action, [:context], nil) + + assert [] = apply(action) + end + + test "handles nil diagnostics" do + assert {_, action} = code_action("other_var = 6", "/project/file.ex", 0) + + action = put_in(action, [:context, :diagnostics], nil) + + assert [] = apply(action) + end + + test "handles empty diagnostics" do + assert {_, action} = code_action("other_var = 6", "/project/file.ex", 0) + + action = put_in(action, [:context, :diagnostics], []) + + assert [] = apply(action) end test "applied to an isolated function" do @@ -134,11 +138,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote ] |> code_action("/project/file.ex", 0) - source_file = code_action.source_file - [diagnostic] = get_in(code_action, [:context, :diagnostics]) - - assert [to_count_action, to_concat_action] = - ReplaceRemoteFunction.apply(source_file, diagnostic) + assert [to_count_action, to_concat_action] = apply(code_action) assert_expected_text_edits(file_uri, to_count_action, "count", 0) assert_expected_text_edits(file_uri, to_concat_action, "concat", 0) @@ -151,11 +151,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote ] |> code_action("/project/file.ex", 0) - source_file = code_action.source_file - [diagnostic] = get_in(code_action, [:context, :diagnostics]) - - assert [to_count_action, to_concat_action] = - ReplaceRemoteFunction.apply(source_file, diagnostic) + assert [to_count_action, to_concat_action] = apply(code_action) assert_expected_text_edits(file_uri, to_count_action, "count", 0) assert_expected_text_edits(file_uri, to_concat_action, "concat", 0) @@ -170,27 +166,20 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end ] |> code_action("/project/file.ex", 2) - source_file = code_action.source_file - [diagnostic] = get_in(code_action, [:context, :diagnostics]) - - assert [to_count_action, to_concat_action] = - ReplaceRemoteFunction.apply(source_file, diagnostic) + assert [to_count_action, to_concat_action] = apply(code_action) assert_expected_text_edits(file_uri, to_count_action, "count", 2) assert_expected_text_edits(file_uri, to_concat_action, "concat", 2) end - test "proposed functions need to match function arity" do + test "proposed functions need to match the replaced function arity" do {_, code_action} = ~S[ Enum.counts(a) ] |> code_action("/project/file.ex", 0, diagnostic_message: diagnostic_message(3)) - source_file = code_action.source_file - [diagnostic] = get_in(code_action, [:context, :diagnostics]) - - assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) + assert [] = apply(code_action) end test "does not replace variables" do @@ -200,9 +189,6 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote ] |> code_action("/project/file.ex", 0) - source_file = code_action.source_file - [diagnostic] = get_in(code_action, [:context, :diagnostics]) - - assert [] = ReplaceRemoteFunction.apply(source_file, diagnostic) + assert [] = apply(code_action) end end diff --git a/apps/language_server/test/experimental/provider/code_action/replace_with_underscore_test.exs b/apps/language_server/test/experimental/provider/code_action/replace_with_underscore_test.exs index d73e4b6b5..3b6cf6f13 100644 --- a/apps/language_server/test/experimental/provider/code_action/replace_with_underscore_test.exs +++ b/apps/language_server/test/experimental/provider/code_action/replace_with_underscore_test.exs @@ -12,6 +12,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn alias ElixirLS.LanguageServer.SourceFile.Path, as: SourceFilePath import LspProtocol + import ReplaceWithUnderscore use ExUnit.Case use Patch @@ -70,24 +71,12 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn test "produces no actions if the name or variable is not found" do assert {_, action} = code_action("other_var = 6", "/project/file.ex", 1, "not_found") - - source_file = action.source_file - diagnostics = get_in(action, [:context, :diagnostics]) - - Enum.each(diagnostics, fn diagnostic -> - assert [] = ReplaceWithUnderscore.apply(source_file, diagnostic) - end) + assert [] = apply(action) end test "produces no actions if the line is empty" do {_, action} = code_action("", "/project/file.ex", 1, "a") - - source_file = action.source_file - diagnostics = get_in(action, [:context, :diagnostics]) - - Enum.each(diagnostics, fn diagnostic -> - assert [] = ReplaceWithUnderscore.apply(source_file, diagnostic) - end) + assert [] = apply(action) end test "produces no results if the diagnostic message doesn't fit the format" do @@ -96,12 +85,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn diagnostic_message: "This isn't cool" ) - source_file = action.source_file - diagnostics = get_in(action, [:context, :diagnostics]) - - Enum.each(diagnostics, fn diagnostic -> - assert [] = ReplaceWithUnderscore.apply(source_file, diagnostic) - end) + assert [] = apply(action) end test "produces no results for buggy source code" do @@ -111,12 +95,31 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn ] |> code_action("/project/file.ex", 0, "unused") - source_file = action.source_file - diagnostics = get_in(action, [:context, :diagnostics]) + assert [] = apply(action) + end + + test "handles nil context" do + assert {_, action} = code_action("other_var = 6", "/project/file.ex", 1, "not_found") + + action = put_in(action, [:context], nil) - Enum.each(diagnostics, fn diagnostic -> - assert [] = ReplaceWithUnderscore.apply(source_file, diagnostic) - end) + assert [] = apply(action) + end + + test "handles nil diagnostics" do + assert {_, action} = code_action("other_var = 6", "/project/file.ex", 1, "not_found") + + action = put_in(action, [:context, :diagnostics], nil) + + assert [] = apply(action) + end + + test "handles empty diagnostics" do + assert {_, action} = code_action("other_var = 6", "/project/file.ex", 1, "not_found") + + action = put_in(action, [:context, :diagnostics], []) + + assert [] = apply(action) end test "applied to an unadorned param" do @@ -126,12 +129,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn ] |> code_action("/project/file.ex", 0, "a") - source_file = code_action.source_file - [diagnostic] = get_in(code_action, [:context, :diagnostics]) - - assert [%CodeActionReply{edit: %{changes: %{^file_uri => [edit]}}}] = - ReplaceWithUnderscore.apply(source_file, diagnostic) - + assert [%CodeActionReply{edit: %{changes: %{^file_uri => [edit]}}}] = apply(code_action) assert edit.new_text == "_" end @@ -143,12 +141,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceWithUn end ] |> code_action("/project/file.ex", 1, "a") - source_file = code_action.source_file - [diagnostic] = get_in(code_action, [:context, :diagnostics]) - - assert [%CodeActionReply{edit: %{changes: %{^file_uri => [edit]}}}] = - ReplaceWithUnderscore.apply(source_file, diagnostic) - + assert [%CodeActionReply{edit: %{changes: %{^file_uri => [edit]}}}] = apply(code_action) assert edit.new_text == "_" assert edit.range.start.line == 1 assert edit.range.end.line == 1 From 9a01a18840ad920d11c3c20cbfd5fef0d60e9286 Mon Sep 17 00:00:00 2001 From: Samuel Heldak Date: Thu, 16 Mar 2023 16:32:29 +0100 Subject: [PATCH 4/6] fix replacing for aliased modules --- .../code_mod/replace_remote_function.ex | 16 +++-- .../code_action/replace_remote_function.ex | 58 +++++++++++++---- .../replace_remote_function_test.exs | 63 +++++++++++++++++++ 3 files changed, 118 insertions(+), 19 deletions(-) diff --git a/apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex b/apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex index 2e3f64f60..bfc99a37b 100644 --- a/apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex +++ b/apps/language_server/lib/language_server/experimental/code_mod/replace_remote_function.ex @@ -4,22 +4,26 @@ defmodule ElixirLS.LanguageServer.Experimental.CodeMod.ReplaceRemoteFunction do alias ElixirLS.LanguageServer.Experimental.CodeMod.Text alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit - @spec text_edits(String.t(), Ast.t(), [atom()], atom(), atom()) :: + @spec text_edits(String.t(), Ast.t(), [[atom()]], atom(), atom()) :: {:ok, [TextEdit.t()]} | :error - def text_edits(original_text, ast, module_aliases, name, suggestion) do + def text_edits(original_text, ast, possible_aliases, name, suggestion) do with {:ok, transformed} <- - apply_transforms(original_text, ast, module_aliases, name, suggestion) do + apply_transforms(original_text, ast, possible_aliases, name, suggestion) do {:ok, Diff.diff(original_text, transformed)} end end - defp apply_transforms(line_text, quoted_ast, module_aliases, name, suggestion) do + defp apply_transforms(line_text, quoted_ast, possible_aliases, name, suggestion) do leading_indent = Text.leading_indent(line_text) updated_ast = Macro.postwalk(quoted_ast, fn - {:., function_meta, [{:__aliases__, module_meta, ^module_aliases}, ^name]} -> - {:., function_meta, [{:__aliases__, module_meta, module_aliases}, suggestion]} + {:., function_meta, [{:__aliases__, module_meta, module_alias}, ^name]} -> + if module_alias in possible_aliases do + {:., function_meta, [{:__aliases__, module_meta, module_alias}, suggestion]} + else + {:., function_meta, [{:__aliases__, module_meta, module_alias}, name]} + end other -> other diff --git a/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex index f3e451298..9c2be3fbb 100644 --- a/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex +++ b/apps/language_server/lib/language_server/experimental/provider/code_action/replace_remote_function.ex @@ -11,6 +11,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Workspace alias ElixirLS.LanguageServer.Experimental.SourceFile + alias ElixirSense.Core.Parser @function_re ~r/(.*)\/(.*) is undefined or private. .*:\n(.*)/s @@ -24,9 +25,9 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote one_based_line = extract_start_line(diagnostic) suggestions = extract_suggestions(diagnostic.message) - with {:ok, module_aliases, name} <- extract_function(diagnostic.message), + with {:ok, module_alias, name} <- extract_function(diagnostic.message), {:ok, replies} <- - build_code_actions(source_file, one_based_line, module_aliases, name, suggestions) do + build_code_actions(source_file, one_based_line, module_alias, name, suggestions) do replies else _ -> [] @@ -37,8 +38,8 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote defp extract_function(message) do case Regex.scan(@function_re, message) do [[_, full_name, _, _]] -> - {module_aliases, name} = separate_module_from_name(full_name) - {:ok, module_aliases, name} + {module_alias, name} = separate_module_from_name(full_name) + {:ok, module_alias, name} _ -> :error @@ -46,13 +47,13 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end defp separate_module_from_name(full_name) do - {name, module_aliases} = + {name, module_alias} = full_name |> String.split(".") |> Enum.map(&String.to_atom/1) |> List.pop_at(-1) - {module_aliases, name} + {module_alias, name} end @suggestion_re ~r/\* .*\/[\d]+/ @@ -80,14 +81,16 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote defp build_code_actions( %SourceFile{} = source_file, one_based_line, - module_aliases, + module_alias, name, suggestions ) do with {:ok, line_text} <- SourceFile.fetch_text_at(source_file, one_based_line), {:ok, line_ast} <- Ast.from(line_text), + {:ok, possible_aliases} <- + fetch_possible_aliases(source_file, one_based_line, module_alias), {:ok, edits_per_suggestion} <- - text_edits_per_suggestion(line_text, line_ast, module_aliases, name, suggestions) do + text_edits_per_suggestion(line_text, line_ast, possible_aliases, name, suggestions) do case edits_per_suggestion do [] -> :error @@ -98,7 +101,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote text_edits = Enum.map(text_edits, &update_line(&1, one_based_line)) CodeActionResult.new( - title: construct_title(module_aliases, suggestion), + title: construct_title(module_alias, suggestion), kind: :quick_fix, edit: Workspace.Edit.new(changes: %{source_file.uri => text_edits}) ) @@ -109,13 +112,42 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end end - defp text_edits_per_suggestion(line_text, line_ast, module_aliases, name, suggestions) do + # Extracted `ElixirSense.Core.State.Env` contains all reachable aliases as a list of tuples + # `{alias, aliased}`. If `aliased` is a prefix of `module_alias`, the function to be replaced + # may use the corresponding `alias`. + defp fetch_possible_aliases(source_file, one_based_line, module_alias) do + metadata = + source_file + |> SourceFile.to_string() + |> Parser.parse_string(true, true, one_based_line) + + case metadata.lines_to_env[one_based_line] do + %ElixirSense.Core.State.Env{aliases: aliases} -> + possible_aliases = + Enum.flat_map(aliases, fn {_alias, aliased} -> + aliased = aliased |> Module.split() |> Enum.map(&String.to_atom/1) + + if aliased == Enum.take(module_alias, length(aliased)) do + [Enum.drop(module_alias, length(aliased) - 1)] + else + [] + end + end) + + {:ok, [module_alias | possible_aliases]} + + _ -> + :error + end + end + + defp text_edits_per_suggestion(line_text, line_ast, possible_aliases, name, suggestions) do suggestions |> Enum.reduce_while([], fn suggestion, acc -> case CodeMod.ReplaceRemoteFunction.text_edits( line_text, line_ast, - module_aliases, + possible_aliases, name, suggestion ) do @@ -136,8 +168,8 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote |> put_in([:range, :end, :line], line_number - 1) end - defp construct_title(module_aliases, suggestion) do - module_string = Enum.map_join(module_aliases, ".", &Atom.to_string/1) + defp construct_title(module_alias, suggestion) do + module_string = Enum.map_join(module_alias, ".", &Atom.to_string/1) "Replace with #{module_string}.#{suggestion}" end diff --git a/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs index 00d4640dc..7a9e3c5f6 100644 --- a/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs +++ b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs @@ -6,6 +6,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeAction, as: CodeActionReply alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Diagnostic alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Range + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Position alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit alias ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemoteFunction alias ElixirLS.LanguageServer.Experimental.SourceFile @@ -191,4 +192,66 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote assert [] = apply(code_action) end + + test "works with aliased modules" do + diagnostic_message = """ + Example.A.B.my_fun/1 is undefined or private. Did you mean: + + * my_func/1 + """ + + code = ~S[ + defmodule Example do + defmodule A.B do + def my_func(a), do: a + end + + defmodule C do + def my_fun(a), do: a + end + + defmodule D do + alias Example.A + alias Example.A.B + alias Example.C + def bar() do + A.B.my_fun(42) + C.my_fun(42) + B.my_fun(42) + end + end + end + ] + + # A.B.my_fun(42) + {file_uri, code_action} = + code_action(code, "/project/file.ex", 14, diagnostic_message: diagnostic_message) + + assert [%CodeActionReply{edit: %{changes: %{^file_uri => edits}}}] = apply(code_action) + + assert [ + %TextEdit{ + new_text: "c", + range: %Range{ + end: %Position{character: 24, line: 14}, + start: %Position{character: 24, line: 14} + } + } + ] = edits + + # B.my_fun(42) + {file_uri, code_action} = + code_action(code, "/project/file.ex", 15, diagnostic_message: diagnostic_message) + + assert [%CodeActionReply{edit: %{changes: %{^file_uri => edits}}}] = apply(code_action) + + assert [ + %TextEdit{ + new_text: "c", + range: %Range{ + end: %Position{character: 37, line: 15}, + start: %Position{character: 37, line: 15} + } + } + ] = edits + end end From 90cb566b551b92f7f1f3144f77575e9fd9d0df13 Mon Sep 17 00:00:00 2001 From: Samuel Heldak Date: Thu, 23 Mar 2023 13:18:55 +0100 Subject: [PATCH 5/6] update tests --- .../experimental/source_file.ex | 9 + .../replace_remote_function_test.exs | 247 +++++++++++------- 2 files changed, 165 insertions(+), 91 deletions(-) diff --git a/apps/language_server/lib/language_server/experimental/source_file.ex b/apps/language_server/lib/language_server/experimental/source_file.ex index 3edd38949..f86156a3a 100644 --- a/apps/language_server/lib/language_server/experimental/source_file.ex +++ b/apps/language_server/lib/language_server/experimental/source_file.ex @@ -5,6 +5,7 @@ defmodule ElixirLS.LanguageServer.Experimental.SourceFile do alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextDocument.ContentChangeEvent.TextDocumentContentChangeEvent1, as: ReplaceContentChangeEvent + alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit alias ElixirLS.LanguageServer.Experimental.SourceFile.Conversions alias ElixirLS.LanguageServer.Experimental.SourceFile.Document alias ElixirLS.LanguageServer.Experimental.SourceFile.Line @@ -162,6 +163,14 @@ defmodule ElixirLS.LanguageServer.Experimental.SourceFile do end end + defp apply_change(%__MODULE__{} = source, %TextEdit{} = change) do + with {:ok, ex_range} <- Conversions.to_elixir(change.range, source) do + apply_change(source, ex_range, change.new_text) + else + _ -> {:error, {:invalid_range, change.range}} + end + end + defp apply_change( %__MODULE__{} = source, %{ diff --git a/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs index 7a9e3c5f6..63ef53d9d 100644 --- a/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs +++ b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs @@ -1,15 +1,13 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemoteFunctionTest do - alias ElixirLS.LanguageServer.Experimental.CodeMod.Diff alias ElixirLS.LanguageServer.Experimental.Protocol.Requests alias ElixirLS.LanguageServer.Experimental.Protocol.Requests.CodeAction, as: CodeActionRequest alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeAction alias ElixirLS.LanguageServer.Experimental.Protocol.Types.CodeAction, as: CodeActionReply alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Diagnostic alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Range - alias ElixirLS.LanguageServer.Experimental.Protocol.Types.Position - alias ElixirLS.LanguageServer.Experimental.Protocol.Types.TextEdit alias ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemoteFunction alias ElixirLS.LanguageServer.Experimental.SourceFile + alias ElixirLS.LanguageServer.Experimental.SourceFile.Document alias ElixirLS.LanguageServer.Fixtures.LspProtocol alias ElixirLS.LanguageServer.SourceFile.Path, as: SourceFilePath @@ -36,10 +34,8 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end defp code_action(file_body, file_path, line, opts \\ []) do - trimmed_body = String.trim(file_body, "\n") - file_uri = SourceFilePath.to_uri(file_path) - SourceFile.Store.open(file_uri, trimmed_body, 0) + SourceFile.Store.open(file_uri, file_body, 0) {:ok, range} = build(Range, @@ -63,43 +59,45 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote ) {:ok, action} = Requests.to_elixir(action) - {file_uri, action} + + {file_uri, file_body, action} end - defp assert_expected_text_edits(file_uri, action, expected_name, line) do - assert %CodeActionReply{edit: %{changes: %{^file_uri => edits}}} = action + defp apply_selected_action({file_uri, file_body, code_action}, index) do + action = + code_action + |> apply() + |> Enum.at(index) - expected_edits = Diff.diff("counts", expected_name) + assert %CodeActionReply{edit: %{changes: %{^file_uri => edits}}} = action - assert edits - |> Enum.zip(expected_edits) - |> Enum.all?(fn {%TextEdit{new_text: new_text}, %TextEdit{new_text: expected_new_text}} -> - new_text == expected_new_text - end) + {:ok, %SourceFile{document: document}} = + file_uri + |> SourceFile.new(file_body, 0) + |> SourceFile.apply_content_changes(1, edits) - assert Enum.all?(edits, fn edit -> edit.range.start.line == line end) - assert Enum.all?(edits, fn edit -> edit.range.end.line == line end) + document end test "produces no actions if the function is not found" do - assert {_, action} = code_action("Enum.count([1, 2])", "/project/file.ex", 0) + assert {_, _, action} = code_action("Enum.count([1, 2])", "/project/file.ex", 0) assert [] = apply(action) end test "produces no actions if the line is empty" do - {_, action} = code_action("", "/project/file.ex", 0) + {_, _, action} = code_action("", "/project/file.ex", 0) assert [] = apply(action) end test "produces no results if the diagnostic message doesn't fit the format" do - assert {_, action} = + assert {_, _, action} = code_action("", "/project/file.ex", 0, diagnostic_message: "This isn't cool") assert [] = apply(action) end test "produces no results for buggy source code" do - {_, action} = + {_, _, action} = ~S[ 1 + 2~/3 ; 4ab( ] @@ -109,7 +107,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end test "handles nil context" do - assert {_, action} = code_action("other_var = 6", "/project/file.ex", 0) + assert {_, _, action} = code_action("other_var = 6", "/project/file.ex", 0) action = put_in(action, [:context], nil) @@ -117,7 +115,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end test "handles nil diagnostics" do - assert {_, action} = code_action("other_var = 6", "/project/file.ex", 0) + assert {_, _, action} = code_action("other_var = 6", "/project/file.ex", 0) action = put_in(action, [:context, :diagnostics], nil) @@ -125,7 +123,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end test "handles empty diagnostics" do - assert {_, action} = code_action("other_var = 6", "/project/file.ex", 0) + assert {_, _, action} = code_action("other_var = 6", "/project/file.ex", 0) action = put_in(action, [:context, :diagnostics], []) @@ -133,48 +131,91 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end test "applied to an isolated function" do - {file_uri, code_action} = - ~S[ - Enum.counts(a) - ] - |> code_action("/project/file.ex", 0) - - assert [to_count_action, to_concat_action] = apply(code_action) - - assert_expected_text_edits(file_uri, to_count_action, "count", 0) - assert_expected_text_edits(file_uri, to_concat_action, "concat", 0) + actual_code = ~S[ + Enum.counts(a) + ] + + expected_doc = ~S[ + Enum.count(a) + ] |> Document.new() + + assert expected_doc == + actual_code + |> code_action("/project/file.ex", 1) + |> apply_selected_action(0) + + expected_doc = ~S[ + Enum.concat(a) + ] |> Document.new() + + assert expected_doc == + actual_code + |> code_action("/project/file.ex", 1) + |> apply_selected_action(1) end test "works for a function assigned to a variable" do - {file_uri, code_action} = - ~S[ - var = &Enum.counts/1 - ] - |> code_action("/project/file.ex", 0) + actual_code = ~S[ + var = &Enum.counts/1 + ] + + expected_doc = ~S[ + var = &Enum.count/1 + ] |> Document.new() + + assert expected_doc == + actual_code + |> code_action("/project/file.ex", 1) + |> apply_selected_action(0) - assert [to_count_action, to_concat_action] = apply(code_action) + expected_doc = ~S[ + var = &Enum.concat/1 + ] |> Document.new() - assert_expected_text_edits(file_uri, to_count_action, "count", 0) - assert_expected_text_edits(file_uri, to_concat_action, "concat", 0) + assert expected_doc == + actual_code + |> code_action("/project/file.ex", 1) + |> apply_selected_action(1) end test "works with multiple lines" do - {file_uri, code_action} = ~S[ + actual_code = ~S[ defmodule MyModule do def my_func(a) do Enum.counts(a) end end - ] |> code_action("/project/file.ex", 2) + ] - assert [to_count_action, to_concat_action] = apply(code_action) + expected_doc = ~S[ + defmodule MyModule do + def my_func(a) do + Enum.count(a) + end + end + ] |> Document.new() + + assert expected_doc == + actual_code + |> code_action("/project/file.ex", 3) + |> apply_selected_action(0) - assert_expected_text_edits(file_uri, to_count_action, "count", 2) - assert_expected_text_edits(file_uri, to_concat_action, "concat", 2) + expected_doc = ~S[ + defmodule MyModule do + def my_func(a) do + Enum.concat(a) + end + end + ] |> Document.new() + + assert expected_doc == + actual_code + |> code_action("/project/file.ex", 3) + |> apply_selected_action(1) end test "proposed functions need to match the replaced function arity" do - {_, code_action} = + {_, _, code_action} = ~S[ Enum.counts(a) ] @@ -184,7 +225,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote end test "does not replace variables" do - {_, code_action} = + {_, _, code_action} = ~S[ counts + 42 ] @@ -200,58 +241,82 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote * my_func/1 """ - code = ~S[ - defmodule Example do - defmodule A.B do - def my_func(a), do: a - end + actual_code = ~S[ + defmodule Example do + defmodule A.B do + def my_func(a), do: a + end - defmodule C do - def my_fun(a), do: a - end + defmodule C do + def my_fun(a), do: a + end - defmodule D do - alias Example.A - alias Example.A.B - alias Example.C - def bar() do - A.B.my_fun(42) - C.my_fun(42) + B.my_fun(42) - end + defmodule D do + alias Example.A + alias Example.A.B + alias Example.C + def bar() do + A.B.my_fun(42) + C.my_fun(42) + B.my_fun(42) end end + end ] # A.B.my_fun(42) - {file_uri, code_action} = - code_action(code, "/project/file.ex", 14, diagnostic_message: diagnostic_message) + expected_doc = ~S[ + defmodule Example do + defmodule A.B do + def my_func(a), do: a + end + + defmodule C do + def my_fun(a), do: a + end - assert [%CodeActionReply{edit: %{changes: %{^file_uri => edits}}}] = apply(code_action) + defmodule D do + alias Example.A + alias Example.A.B + alias Example.C + def bar() do + A.B.my_func(42) + C.my_fun(42) + B.my_fun(42) + end + end + end + ] |> Document.new() - assert [ - %TextEdit{ - new_text: "c", - range: %Range{ - end: %Position{character: 24, line: 14}, - start: %Position{character: 24, line: 14} - } - } - ] = edits + assert expected_doc == + actual_code + |> code_action("/project/file.ex", 15, diagnostic_message: diagnostic_message) + |> apply_selected_action(0) # B.my_fun(42) - {file_uri, code_action} = - code_action(code, "/project/file.ex", 15, diagnostic_message: diagnostic_message) - - assert [%CodeActionReply{edit: %{changes: %{^file_uri => edits}}}] = apply(code_action) - - assert [ - %TextEdit{ - new_text: "c", - range: %Range{ - end: %Position{character: 37, line: 15}, - start: %Position{character: 37, line: 15} - } - } - ] = edits + expected_doc = ~S[ + defmodule Example do + defmodule A.B do + def my_func(a), do: a + end + + defmodule C do + def my_fun(a), do: a + end + + defmodule D do + alias Example.A + alias Example.A.B + alias Example.C + def bar() do + A.B.my_fun(42) + C.my_fun(42) + B.my_func(42) + end + end + end + ] |> Document.new() + + assert expected_doc == + actual_code + |> code_action("/project/file.ex", 16, diagnostic_message: diagnostic_message) + |> apply_selected_action(0) end end From b92e07a7919c8b5ddcbd6cfc95d5a07516ea5a9b Mon Sep 17 00:00:00 2001 From: Samuel Heldak Date: Thu, 23 Mar 2023 17:26:09 +0100 Subject: [PATCH 6/6] format code --- .../provider/code_action/replace_remote_function_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs index 63ef53d9d..e637b10cc 100644 --- a/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs +++ b/apps/language_server/test/experimental/provider/code_action/replace_remote_function_test.exs @@ -59,7 +59,7 @@ defmodule ElixirLS.LanguageServer.Experimental.Provider.CodeAction.ReplaceRemote ) {:ok, action} = Requests.to_elixir(action) - + {file_uri, file_body, action} end