From e14a611e157c0c4f6b54db5fce4719a51c4b7fc6 Mon Sep 17 00:00:00 2001 From: Nikola Jichev Date: Sat, 20 Apr 2024 01:36:26 +0300 Subject: [PATCH] feat: alias-refactor workspace command (#386) Adds an alias refactor workspace command to Next LS. Your cursor should be at the target module that you wish to alias. It will insert the alias at the of the nearest defmodule definition scoping the refactor only to the current module instead of the whole file. --- lib/next_ls.ex | 18 +- lib/next_ls/commands.ex | 15 + lib/next_ls/commands/alias.ex | 140 ++++++ .../elixir_extension/code_action/require.ex | 24 +- lib/next_ls/helpers/ast_helpers.ex | 24 ++ test/next_ls/alias_test.exs | 91 ++++ test/next_ls/commands/alias_test.exs | 401 ++++++++++++++++++ test/next_ls/helpers/ast_helpers_test.exs | 67 +++ test/next_ls/pipe_test.exs | 4 +- 9 files changed, 758 insertions(+), 26 deletions(-) create mode 100644 lib/next_ls/commands.ex create mode 100644 lib/next_ls/commands/alias.ex create mode 100644 test/next_ls/alias_test.exs create mode 100644 test/next_ls/commands/alias_test.exs diff --git a/lib/next_ls.ex b/lib/next_ls.ex index 4538afe4..6228f69f 100644 --- a/lib/next_ls.ex +++ b/lib/next_ls.ex @@ -154,7 +154,8 @@ defmodule NextLS do execute_command_provider: %GenLSP.Structures.ExecuteCommandOptions{ commands: [ "to-pipe", - "from-pipe" + "from-pipe", + "alias-refactor" ] }, hover_provider: true, @@ -769,6 +770,19 @@ defmodule NextLS do position: position }) + "alias-refactor" -> + [arguments] = params.arguments + + uri = arguments["uri"] + position = arguments["position"] + text = lsp.assigns.documents[uri] + + NextLS.Commands.Alias.run(%{ + uri: uri, + text: text, + position: position + }) + _ -> NextLS.Logger.show_message( lsp.logger, @@ -783,7 +797,7 @@ defmodule NextLS do %WorkspaceEdit{} = edit -> GenLSP.request(lsp, %WorkspaceApplyEdit{ id: System.unique_integer([:positive]), - params: %ApplyWorkspaceEditParams{label: "Pipe", edit: edit} + params: %ApplyWorkspaceEditParams{label: NextLS.Commands.label(command), edit: edit} }) _reply -> diff --git a/lib/next_ls/commands.ex b/lib/next_ls/commands.ex new file mode 100644 index 00000000..f313b637 --- /dev/null +++ b/lib/next_ls/commands.ex @@ -0,0 +1,15 @@ +defmodule NextLS.Commands do + @moduledoc false + + @labels %{ + "from-pipe" => "Inlined pipe", + "to-pipe" => "Extracted to a pipe", + "alias-refactor" => "Refactored with an alias" + } + @doc "Creates a label for the workspace apply struct from the command name" + def label(command) when is_map_key(@labels, command), do: @labels[command] + + def label(command) do + raise ArgumentError, "command #{inspect(command)} not supported" + end +end diff --git a/lib/next_ls/commands/alias.ex b/lib/next_ls/commands/alias.ex new file mode 100644 index 00000000..4a66a454 --- /dev/null +++ b/lib/next_ls/commands/alias.ex @@ -0,0 +1,140 @@ +defmodule NextLS.Commands.Alias do + @moduledoc """ + Refactors a module with fully qualified calls to an alias. + The cursor position should be under the module name that you wish to alias. + """ + import Schematic + + alias GenLSP.Enumerations.ErrorCodes + alias GenLSP.Structures.Position + alias GenLSP.Structures.Range + alias GenLSP.Structures.TextEdit + alias GenLSP.Structures.WorkspaceEdit + alias NextLS.ASTHelpers + alias NextLS.EditHelpers + alias Sourceror.Zipper, as: Z + + @line_length 121 + + defp opts do + map(%{ + position: Position.schematic(), + uri: str(), + text: list(str()) + }) + end + + def run(opts) do + with {:ok, %{text: text, uri: uri, position: position}} <- unify(opts(), Map.new(opts)), + {:ok, ast, comments} = parse(text), + {:ok, defm} <- ASTHelpers.get_surrounding_module(ast, position), + {:ok, {:__aliases__, _, modules}} <- get_node(ast, position) do + range = make_range(defm) + indent = EditHelpers.get_indent(text, range.start.line) + aliased = get_aliased(defm, modules) + + comments = + Enum.filter(comments, fn comment -> + comment.line > range.start.line && comment.line <= range.end.line + end) + + to_algebra_opts = [comments: comments] + doc = Code.quoted_to_algebra(aliased, to_algebra_opts) + formatted = doc |> Inspect.Algebra.format(@line_length) |> IO.iodata_to_binary() + + %WorkspaceEdit{ + changes: %{ + uri => [ + %TextEdit{ + new_text: + EditHelpers.add_indent_to_edit( + formatted, + indent + ), + range: range + } + ] + } + } + else + {:error, message} -> + %GenLSP.ErrorResponse{code: ErrorCodes.parse_error(), message: inspect(message)} + end + end + + defp parse(lines) do + lines + |> Enum.join("\n") + |> Spitfire.parse_with_comments(literal_encoder: &{:ok, {:__block__, &2, [&1]}}) + |> case do + {:error, ast, comments, _errors} -> + {:ok, ast, comments} + + other -> + other + end + end + + defp make_range(original_ast) do + range = Sourceror.get_range(original_ast) + + %Range{ + start: %Position{line: range.start[:line] - 1, character: range.start[:column] - 1}, + end: %Position{line: range.end[:line] - 1, character: range.end[:column] - 1} + } + end + + def get_node(ast, pos) do + pos = [line: pos.line + 1, column: pos.character + 1] + + result = + ast + |> Z.zip() + |> Z.traverse(nil, fn tree, acc -> + node = Z.node(tree) + range = Sourceror.get_range(node) + + if not is_nil(range) and + match?({:__aliases__, _context, _modules}, node) && + Sourceror.compare_positions(range.start, pos) in [:lt, :eq] && + Sourceror.compare_positions(range.end, pos) in [:gt, :eq] do + {tree, node} + else + {tree, acc} + end + end) + + case result do + {_, nil} -> + {:error, "could not find a module to alias at the cursor position"} + + {_, {_t, _m, []}} -> + {:error, "could not find a module to alias at the cursor position"} + + {_, {_t, _m, [_argument | _rest]} = node} -> + {:ok, node} + end + end + + defp get_aliased(defm, modules) do + last = List.last(modules) + + replaced = + Macro.prewalk(defm, fn + {:__aliases__, context, ^modules} -> {:__aliases__, context, [last]} + ast -> ast + end) + + alias_to_add = {:alias, [alias: false], [{:__aliases__, [], modules}]} + + {:defmodule, context, [module, [{do_block, block}]]} = replaced + + case block do + {:__block__, block_context, defs} -> + {:defmodule, context, [module, [{do_block, {:__block__, block_context, [alias_to_add | defs]}}]]} + + {_, _, _} = original -> + {:defmodule, context, [module, [{do_block, {:__block__, [], [alias_to_add, original]}}]]} + end + end +end diff --git a/lib/next_ls/extensions/elixir_extension/code_action/require.ex b/lib/next_ls/extensions/elixir_extension/code_action/require.ex index ea638847..1f5ddcdf 100644 --- a/lib/next_ls/extensions/elixir_extension/code_action/require.ex +++ b/lib/next_ls/extensions/elixir_extension/code_action/require.ex @@ -7,6 +7,7 @@ defmodule NextLS.ElixirExtension.CodeAction.Require do alias GenLSP.Structures.Range alias GenLSP.Structures.TextEdit alias GenLSP.Structures.WorkspaceEdit + alias NextLS.ASTHelpers @one_indentation_level " " @spec new(diagnostic :: Diagnostic.t(), [text :: String.t()], uri :: String.t()) :: [CodeAction.t()] @@ -15,7 +16,7 @@ defmodule NextLS.ElixirExtension.CodeAction.Require do with {:ok, require_module} <- get_edit(diagnostic.message), {:ok, ast} <- parse_ast(text), - {:ok, defm} <- nearest_defmodule(ast, range), + {:ok, defm} <- ASTHelpers.get_surrounding_module(ast, range.start), indentation <- get_indent(text, defm), nearest <- find_nearest_node_for_require(defm), range <- get_edit_range(nearest) do @@ -47,27 +48,6 @@ defmodule NextLS.ElixirExtension.CodeAction.Require do |> Spitfire.parse() end - defp nearest_defmodule(ast, range) do - defmodules = - ast - |> Macro.prewalker() - |> Enum.filter(fn - {:defmodule, _, _} -> true - _ -> false - end) - - if defmodules != [] do - defm = - Enum.min_by(defmodules, fn {_, ctx, _} -> - range.start.character - ctx[:line] + 1 - end) - - {:ok, defm} - else - {:error, "no defmodule definition"} - end - end - @module_name ~r/require\s+([^\s]+)\s+before/ defp get_edit(message) do case Regex.run(@module_name, message) do diff --git a/lib/next_ls/helpers/ast_helpers.ex b/lib/next_ls/helpers/ast_helpers.ex index d3ad83e4..55fca7f6 100644 --- a/lib/next_ls/helpers/ast_helpers.ex +++ b/lib/next_ls/helpers/ast_helpers.ex @@ -1,5 +1,6 @@ defmodule NextLS.ASTHelpers do @moduledoc false + alias GenLSP.Structures.Position alias Sourceror.Zipper defmodule Attributes do @@ -154,6 +155,29 @@ defmodule NextLS.ASTHelpers do end) end + @spec get_surrounding_module(ast :: Macro.t(), position :: Position.t()) :: {:ok, Macro.t()} | {:error, String.t()} + def get_surrounding_module(ast, position) do + defm = + ast + |> Macro.prewalker() + |> Enum.filter(fn node -> match?({:defmodule, _, _}, node) end) + |> Enum.filter(fn {_, ctx, _} -> + position.line + 1 - ctx[:line] >= 0 + end) + |> Enum.min_by( + fn {_, ctx, _} -> + abs(ctx[:line] - 1 - position.line) + end, + fn -> nil end + ) + + if defm do + {:ok, defm} + else + {:error, "no defmodule definition"} + end + end + def find_cursor(ast) do with nil <- ast diff --git a/test/next_ls/alias_test.exs b/test/next_ls/alias_test.exs new file mode 100644 index 00000000..0e150793 --- /dev/null +++ b/test/next_ls/alias_test.exs @@ -0,0 +1,91 @@ +defmodule NextLS.AliasTest do + use ExUnit.Case, async: true + + import GenLSP.Test + import NextLS.Support.Utils + + @moduletag :tmp_dir + @moduletag root_paths: ["my_proj"] + + setup %{tmp_dir: tmp_dir} do + File.mkdir_p!(Path.join(tmp_dir, "my_proj/lib")) + File.write!(Path.join(tmp_dir, "my_proj/mix.exs"), mix_exs()) + + cwd = Path.join(tmp_dir, "my_proj") + + foo_path = Path.join(cwd, "lib/foo.ex") + + foo = """ + defmodule Foo do + def to_list() do + Foo.Bar.to_list(Map.new()) + end + + def to_map() do + Foo.Bar.to_map(List.new()) + end + end + """ + + File.write!(foo_path, foo) + + [foo: foo, foo_path: foo_path] + end + + setup :with_lsp + + setup context do + assert :ok == notify(context.client, %{method: "initialized", jsonrpc: "2.0", params: %{}}) + assert_is_ready(context, "my_proj") + assert_compiled(context, "my_proj") + assert_notification "$/progress", %{"value" => %{"kind" => "end", "message" => "Finished indexing!"}} + + did_open(context.client, context.foo_path, context.foo) + context + end + + test "refactors with alias", %{client: client, foo_path: foo} do + foo_uri = uri(foo) + id = 1 + + request client, %{ + method: "workspace/executeCommand", + id: id, + jsonrpc: "2.0", + params: %{ + command: "alias-refactor", + arguments: [%{uri: foo_uri, position: %{line: 2, character: 8}}] + } + } + + expected_edit = + String.trim(""" + defmodule Foo do + alias Foo.Bar + + def to_list() do + Bar.to_list(Map.new()) + end + + def to_map() do + Bar.to_map(List.new()) + end + end + """) + + assert_request(client, "workspace/applyEdit", 500, fn params -> + assert %{"edit" => edit, "label" => "Refactored with an alias"} = params + + assert %{ + "changes" => %{ + ^foo_uri => [%{"newText" => text, "range" => range}] + } + } = edit + + assert text == expected_edit + + assert range["start"] == %{"character" => 0, "line" => 0} + assert range["end"] == %{"character" => 3, "line" => 8} + end) + end +end diff --git a/test/next_ls/commands/alias_test.exs b/test/next_ls/commands/alias_test.exs new file mode 100644 index 00000000..ce002e2c --- /dev/null +++ b/test/next_ls/commands/alias_test.exs @@ -0,0 +1,401 @@ +defmodule NextLS.Commands.AliasTest do + use ExUnit.Case, async: true + + alias GenLSP.Structures.TextEdit + alias GenLSP.Structures.WorkspaceEdit + alias NextLS.Commands.Alias + + # @parse_error_code -32_700 + + describe "alias-refactor" do + test "works with single calls" do + uri = "my_app.ex" + + text = + String.split( + """ + defmodule MyApp do + def to_list(map) do + Foo.Bar.to_list(map) + end + end + """, + "\n" + ) + + expected_edit = + String.trim(""" + defmodule MyApp do + alias Foo.Bar + + def to_list(map) do + Bar.to_list(map) + end + end + """) + + line = 2 + position = %{"line" => line, "character" => 6} + + assert %WorkspaceEdit{changes: %{^uri => [edit = %TextEdit{range: range}]}} = + Alias.run(%{uri: uri, text: text, position: position}) + + assert edit.new_text == expected_edit + assert range.start.line == 0 + assert range.start.character == 0 + assert range.end.line == 4 + assert range.end.character == 3 + end + + test "works with multiple def calls" do + uri = "my_app.ex" + + text = + String.split( + """ + defmodule MyApp do + def to_list(map) do + Foo.Bar.to_list(map) + end + + def bar, do: :bar + end + """, + "\n" + ) + + expected_edit = + String.trim(""" + defmodule MyApp do + alias Foo.Bar + + def to_list(map) do + Bar.to_list(map) + end + + def bar, do: :bar + end + """) + + line = 2 + position = %{"line" => line, "character" => 6} + + assert %WorkspaceEdit{changes: %{^uri => [edit = %TextEdit{range: range}]}} = + Alias.run(%{uri: uri, text: text, position: position}) + + assert edit.new_text == expected_edit + assert range.start.line == 0 + assert range.start.character == 0 + assert range.end.line == 6 + assert range.end.character == 3 + end + + test "works with aliasing multiple calls" do + uri = "my_app.ex" + + text = + String.split( + """ + defmodule MyApp do + def to_list(map) do + Foo.Bar.to_list(map) + end + + def bar do + Foo.Bar.bar(:foo, :bar) + end + end + """, + "\n" + ) + + expected_edit = + String.trim(""" + defmodule MyApp do + alias Foo.Bar + + def to_list(map) do + Bar.to_list(map) + end + + def bar do + Bar.bar(:foo, :bar) + end + end + """) + + line = 2 + position = %{"line" => line, "character" => 6} + + assert %WorkspaceEdit{changes: %{^uri => [edit = %TextEdit{range: range}]}} = + Alias.run(%{uri: uri, text: text, position: position}) + + assert edit.new_text == expected_edit + assert range.start.line == 0 + assert range.start.character == 0 + assert range.end.line == 8 + assert range.end.character == 3 + end + + test "works with nested modules" do + uri = "my_app.ex" + + text = + String.split( + """ + defmodule MyApp do + defmodule Foo.Baz do + def to_list(map) do + Foo.Bar.to_list(map) + end + + def bar do + Foo.Bar.bar(:foo, :bar) + end + end + + defmodule Quix do + def quix do + Foo.Bar.quix() + end + end + end + """, + "\n" + ) + + expected_edit = + String.trim(""" + defmodule Foo.Baz do + alias Foo.Bar + + def to_list(map) do + Bar.to_list(map) + end + + def bar do + Bar.bar(:foo, :bar) + end + end + """) + + line = 3 + position = %{"line" => line, "character" => 8} + + assert %WorkspaceEdit{changes: %{^uri => [edit = %TextEdit{range: range}]}} = + Alias.run(%{uri: uri, text: text, position: position}) + + assert edit.new_text == expected_edit + assert range.start.line == 1 + assert range.start.character == 2 + assert range.end.line == 9 + assert range.end.character == 5 + end + + test "works with structs" do + uri = "my_app.ex" + + text = + String.split( + """ + defmodule MyApp do + def to_list(map) do + %Foo.Bar{key: map} + end + end + """, + "\n" + ) + + expected_edit = + String.trim(""" + defmodule MyApp do + alias Foo.Bar + + def to_list(map) do + %Bar{key: map} + end + end + """) + + line = 2 + position = %{"line" => line, "character" => 6} + + assert %WorkspaceEdit{changes: %{^uri => [edit = %TextEdit{range: range}]}} = + Alias.run(%{uri: uri, text: text, position: position}) + + assert edit.new_text == expected_edit + assert range.start.line == 0 + assert range.start.character == 0 + assert range.end.line == 4 + assert range.end.character == 3 + end + + test "works with 0 arity functions" do + uri = "my_app.ex" + + text = + String.split( + """ + defmodule MyApp do + def baz() do + Foo.Bar.baz() + end + end + """, + "\n" + ) + + expected_edit = + String.trim(""" + defmodule MyApp do + alias Foo.Bar + + def baz() do + Bar.baz() + end + end + """) + + line = 2 + position = %{"line" => line, "character" => 6} + + assert %WorkspaceEdit{changes: %{^uri => [edit = %TextEdit{range: range}]}} = + Alias.run(%{uri: uri, text: text, position: position}) + + assert edit.new_text == expected_edit + assert range.start.line == 0 + assert range.start.character == 0 + assert range.end.line == 4 + assert range.end.character == 3 + end + + test "works with top level macros" do + uri = "my_app.ex" + + text = + String.split( + """ + defmodule MyApp do + import Foo.Bar + + require Foo.Bar + def baz() do + Foo.Bar.baz() + end + end + """, + "\n" + ) + + expected_edit = + String.trim(""" + defmodule MyApp do + alias Foo.Bar + import Bar + + require Bar + + def baz() do + Bar.baz() + end + end + """) + + line = 5 + position = %{"line" => line, "character" => 6} + + assert %WorkspaceEdit{changes: %{^uri => [edit = %TextEdit{range: range}]}} = + Alias.run(%{uri: uri, text: text, position: position}) + + assert edit.new_text == expected_edit + assert range.start.line == 0 + assert range.start.character == 0 + assert range.end.line == 7 + assert range.end.character == 3 + end + + test "preserves comment metadata" do + uri = "my_app.ex" + + text = + String.split( + """ + defmodule MyApp do + # Comment + defmodule Baz do + # Another comment + def to_list(map) do + # Also a comment + Foo.Bar.to_list(map) + end + end + end + """, + "\n" + ) + + expected_edit = + String.trim(""" + defmodule Baz do + alias Foo.Bar + # Another comment + def to_list(map) do + # Also a comment + Bar.to_list(map) + end + end + """) + + position = %{"line" => 6, "character" => 8} + + assert %WorkspaceEdit{changes: %{^uri => [edit = %TextEdit{range: range}]}} = + Alias.run(%{uri: uri, text: text, position: position}) + + assert edit.new_text == expected_edit + assert range.start.line == 2 + assert range.start.character == 2 + assert range.end.line == 8 + assert range.end.character == 5 + end + + test "preserves newlines" do + uri = "my_app.ex" + + text = + String.split( + """ + defmodule MyApp do + def to_list(map) do + Foo.Bar.to_list(map, "\nfoo") + end + end + """, + "\n" + ) + + expected_edit = + String.trim(""" + defmodule MyApp do + alias Foo.Bar + + def to_list(map) do + Bar.to_list(map, "\\nfoo") + end + end + """) + + line = 2 + position = %{"line" => line, "character" => 6} + + assert %WorkspaceEdit{changes: %{^uri => [edit = %TextEdit{range: range}]}} = + Alias.run(%{uri: uri, text: text, position: position}) + + assert edit.new_text == expected_edit + assert range.start.line == 0 + assert range.start.character == 0 + assert range.end.line == 5 + assert range.end.character == 3 + end + end +end diff --git a/test/next_ls/helpers/ast_helpers_test.exs b/test/next_ls/helpers/ast_helpers_test.exs index ca329d46..c378da58 100644 --- a/test/next_ls/helpers/ast_helpers_test.exs +++ b/test/next_ls/helpers/ast_helpers_test.exs @@ -1,6 +1,7 @@ defmodule NextLS.ASTHelpersTest do use ExUnit.Case, async: true + alias GenLSP.Structures.Position alias NextLS.ASTHelpers alias NextLS.ASTHelpers.Aliases @@ -74,4 +75,70 @@ defmodule NextLS.ASTHelpersTest do assert {{5, 5}, {5, 8}} == Aliases.extract_alias_range(code, {start, stop}, :Four) end end + + describe "get_surrounding_module/2" do + test "finds the nearest defmodule definition in the ast" do + {:ok, ast} = + Spitfire.parse(""" + defmodule Test do + defmodule Foo do + def hello(), do: :foo + end + + defmodule Bar do + def hello(), do: :bar + end + end + """) + + lines = 1..3 + + for line <- lines do + position = %Position{line: line, character: 0} + + assert {:ok, {:defmodule, _, [{:__aliases__, _, [:Foo]} | _]}} = + ASTHelpers.get_surrounding_module(ast, position) + end + + lines = 5..7 + + for line <- lines do + position = %Position{line: line, character: 0} + + assert {:ok, {:defmodule, _, [{:__aliases__, _, [:Bar]} | _]}} = + ASTHelpers.get_surrounding_module(ast, position) + end + + position = %Position{line: 0, character: 0} + assert {:ok, {:defmodule, _, [{:__aliases__, _, [:Test]} | _]}} = ASTHelpers.get_surrounding_module(ast, position) + end + + test "errors out when it can't find a module" do + {:ok, ast} = + Spitfire.parse(""" + def foo, do: :bar + """) + + position = %Position{line: 0, character: 0} + assert {:error, "no defmodule definition"} = ASTHelpers.get_surrounding_module(ast, position) + end + + test "it finds the nearest surrounding module" do + {:ok, ast} = + Spitfire.parse(""" + defmodule Test do + alias Foo + alias Bar + alias Baz + + defmodule Quix do + defstruct [:key] + end + end + """) + + position = %Position{line: 4, character: 0} + assert {:ok, {:defmodule, _, [{:__aliases__, _, [:Test]} | _]}} = ASTHelpers.get_surrounding_module(ast, position) + end + end end diff --git a/test/next_ls/pipe_test.exs b/test/next_ls/pipe_test.exs index 8d7ee811..7d9b71b7 100644 --- a/test/next_ls/pipe_test.exs +++ b/test/next_ls/pipe_test.exs @@ -68,7 +68,7 @@ defmodule NextLS.PipeTest do } assert_request(client, "workspace/applyEdit", 500, fn params -> - assert %{"edit" => edit, "label" => "Pipe"} = params + assert %{"edit" => edit, "label" => "Extracted to a pipe"} = params assert %{ "changes" => %{ @@ -98,7 +98,7 @@ defmodule NextLS.PipeTest do } assert_request(client, "workspace/applyEdit", 500, fn params -> - assert %{"edit" => edit, "label" => "Pipe"} = params + assert %{"edit" => edit, "label" => "Inlined pipe"} = params assert %{ "changes" => %{