From 89454809c6e9b8586bdb5fc2289b7788541915ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ton=C4=87i=20Gali=C4=87?= Date: Fri, 11 Feb 2022 11:36:24 +0100 Subject: [PATCH 1/4] POC for "rename" functionality LSPs can offer a rename capability to rename a "symbol" across a workspace. We're not there yet, but this is a starting point. In VS Code it can be used by right clicking a symbol (currently variable or call to local function) and selecting "Rename symbol". A dialog should pop up asking for a new name. Official spec: https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_rename --- .../lib/language_server/protocol.ex | 19 ++ .../lib/language_server/providers/rename.ex | 100 ++++++++ .../lib/language_server/server.ex | 22 ++ .../test/providers/rename_test.exs | 213 ++++++++++++++++++ .../test/support/fixtures/rename_example.exs | 67 ++++++ 5 files changed, 421 insertions(+) create mode 100644 apps/language_server/lib/language_server/providers/rename.ex create mode 100644 apps/language_server/test/providers/rename_test.exs create mode 100644 apps/language_server/test/support/fixtures/rename_example.exs diff --git a/apps/language_server/lib/language_server/protocol.ex b/apps/language_server/lib/language_server/protocol.ex index faf5af976..044f496d0 100644 --- a/apps/language_server/lib/language_server/protocol.ex +++ b/apps/language_server/lib/language_server/protocol.ex @@ -181,6 +181,25 @@ defmodule ElixirLS.LanguageServer.Protocol do end end + defmacro rename_req(id, uri, line, character, new_name) do + quote do + request(unquote(id), "textDocument/rename", %{ + "textDocument" => %{"uri" => unquote(uri)}, + "position" => %{"line" => unquote(line), "character" => unquote(character)}, + "newName" => unquote(new_name) + }) + end + end + + defmacro prepare_rename_req(id, uri, line, character) do + quote do + request(unquote(id), "textDocument/prepareRename", %{ + "textDocument" => %{"uri" => unquote(uri)}, + "position" => %{"line" => unquote(line), "character" => unquote(character)} + }) + end + end + defmacro execute_command_req(id, command, arguments) do quote do request(unquote(id), "workspace/executeCommand", %{ diff --git a/apps/language_server/lib/language_server/providers/rename.ex b/apps/language_server/lib/language_server/providers/rename.ex new file mode 100644 index 000000000..2c31af2bb --- /dev/null +++ b/apps/language_server/lib/language_server/providers/rename.ex @@ -0,0 +1,100 @@ +defmodule ElixirLS.LanguageServer.Providers.Rename do + @moduledoc """ + Provides functionality to rename a symbol inside a workspace + + https://microsoft.github.io/language-server-protocol/specifications/specification-current/#textDocument_rename + """ + + alias ElixirLS.LanguageServer.SourceFile + #import ElixirLS.LanguageServer.Protocol + + def rename(%SourceFile{} = source_file, start_uri, line, character, new_name) do + edits = + with %{context: {context, char_ident}} when context in [:local_or_var, :local_call] <- + Code.Fragment.surround_context(source_file.text, {line, character}), + %ElixirSense.Location{} = definition <- + ElixirSense.definition(source_file.text, line, character), + references <- ElixirSense.references(source_file.text, line, character) do + + + length_old = length(char_ident) + + [ + %{ + uri: start_uri, + range: + adjust_range( + definition.line, + definition.column, + definition.line, + definition.column + length_old + ) + } + | repack_references(references, start_uri) + ] + else + _ -> + [] + end + + changes = + edits + |> Enum.group_by(& &1.uri) + |> Enum.map(fn {uri, edits} -> + %{ + "textDocument" => %{ + "uri" => uri, + "version" => source_file.version + 1 + }, + "edits" => + Enum.map(edits, fn edit -> + %{"range" => edit.range, "newText" => new_name} + end) + } + end) + + {:ok, %{"documentChanges" => changes}} + end + + def prepare(%SourceFile{} = source_file, _uri, line, character) do + result = + with %{ + begin: {start_line, start_col}, + end: {end_line, end_col}, + context: {context, char_ident} + } when context in [:local_or_var, :local_call] <- Code.Fragment.surround_context(source_file.text, {line, character}) do + %{ + range: adjust_range(start_line, start_col, end_line, end_col), + placeholder: to_string(char_ident) + } + else + _ -> + # Not a variable or local call, skipping for now + nil + end + + {:ok, result} + end + + defp repack_references(references, uri) do + for reference <- references do + %{ + uri: uri, + range: %{ + end: %{character: reference.range.end.column - 1, line: reference.range.end.line - 1}, + start: %{ + character: reference.range.start.column - 1, + line: reference.range.start.line - 1 + } + } + } + end + end + + defp adjust_range(start_line, start_character, end_line, end_character) do + %{ + start: %{line: start_line - 1, character: start_character - 1}, + end: %{line: end_line - 1, character: end_character - 1} + } + end +end diff --git a/apps/language_server/lib/language_server/server.ex b/apps/language_server/lib/language_server/server.ex index 0f2d4c09b..4e16c0920 100644 --- a/apps/language_server/lib/language_server/server.ex +++ b/apps/language_server/lib/language_server/server.ex @@ -24,6 +24,7 @@ defmodule ElixirLS.LanguageServer.Server do Definition, Implementation, References, + Rename, Formatting, SignatureHelp, DocumentSymbols, @@ -739,6 +740,26 @@ defmodule ElixirLS.LanguageServer.Server do {:async, fun, state} end + defp handle_request(rename_req(_id, uri, line, character, new_name), state = %__MODULE__{}) do + source_file = get_source_file(state, uri) + + fun = fn -> + Rename.rename(source_file, uri, line + 1, character + 1, new_name) + end + + {:async, fun, state} + end + + defp handle_request(prepare_rename_req(_id, uri, line, character), state = %__MODULE__{}) do + source_file = get_source_file(state, uri) + + fun = fn -> + Rename.prepare(source_file, uri, line + 1, character + 1) + end + + {:async, fun, state} + end + defp handle_request(execute_command_req(_id, command, args) = req, state = %__MODULE__{}) do {:async, fn -> @@ -809,6 +830,7 @@ defmodule ElixirLS.LanguageServer.Server do "workspaceSymbolProvider" => true, "documentOnTypeFormattingProvider" => %{"firstTriggerCharacter" => "\n"}, "codeLensProvider" => %{"resolveProvider" => false}, + "renameProvider" => %{"prepareProvider" => true}, "executeCommandProvider" => %{ "commands" => ExecuteCommand.get_commands(server_instance_id) }, diff --git a/apps/language_server/test/providers/rename_test.exs b/apps/language_server/test/providers/rename_test.exs new file mode 100644 index 000000000..ad3793c5b --- /dev/null +++ b/apps/language_server/test/providers/rename_test.exs @@ -0,0 +1,213 @@ +defmodule ElixirLS.LanguageServer.Providers.RenameTest do + use ExUnit.Case, async: true + + alias ElixirLS.LanguageServer.Providers.Rename + alias ElixirLS.LanguageServer.SourceFile + alias ElixirLS.LanguageServer.Test.FixtureHelpers + # mix cmd --app language_server mix test test/providers/rename_test.exs + + @fake_uri "file:///World/Netherlands/Amsterdam/supercomputer/amazing.ex" + + test "rename blank space" do + text = """ + defmodule MyModule do + def hello() do + IO.inspect("hello world") + end + end + """ + + {line, char} = {2, 1} + + assert {:ok, %{"documentChanges" => []}} = + Rename.rename(%SourceFile{text: text, version: 0}, @fake_uri, line, char, "test") + end + + describe "renaming variable" do + test "a -> test" do + text = """ + defmodule MyModule do + def add(a, b) do + a + b + end + end + """ + + # _a + b + {line, char} = {3, 5} + + assert {:ok, %{"documentChanges" => changes}} = + Rename.rename(%SourceFile{text: text, version: 0}, @fake_uri, line, char, "test") + + assert %{ + "textDocument" => %{ + "uri" => @fake_uri, + "version" => 1 + }, + "edits" => [ + %{ + "range" => %{end: %{character: 11, line: 1}, start: %{character: 10, line: 1}}, + "newText" => "test" + }, + %{ + "range" => %{end: %{character: 5, line: 2}, start: %{character: 4, line: 2}}, + "newText" => "test" + } + ] + } == List.first(changes) + end + + test "nema -> name" do + text = """ + defmodule MyModule do + def hello(nema) do + "Hello " <> nema + end + end + """ + + # "Hello " <> ne_ma + {line, char} = {3, 19} + + assert {:ok, %{"documentChanges" => [changes]}} = + Rename.rename( + %SourceFile{text: text, version: 0}, + @fake_uri, + line, + char, + "name" + ) + + assert %{ + "textDocument" => %{ + "uri" => @fake_uri, + "version" => 1 + }, + "edits" => [ + %{ + "range" => %{end: %{character: 16, line: 1}, start: %{character: 12, line: 1}}, + "newText" => "name" + }, + %{ + "range" => %{end: %{character: 20, line: 2}, start: %{character: 16, line: 2}}, + "newText" => "name" + } + ] + } == changes + end + end + + describe "renaming local function" do + test "create_message -> store_message" do + file_path = FixtureHelpers.get_path("rename_example.exs") + text = File.read!(file_path) + uri = SourceFile.path_to_uri(file_path) + + # |> _create_message + {line, char} = {28, 8} + + assert {:ok, %{"documentChanges" => [changes]}} = + Rename.rename( + %SourceFile{text: text, version: 0}, + uri, + line, + char, + "store_message" + ) + + assert %{ + "textDocument" => %{ + "uri" => uri, + "version" => 1 + }, + "edits" => [ + %{ + "newText" => "store_message", + "range" => %{end: %{character: 21, line: 43}, start: %{character: 7, line: 43}} + }, + %{ + "newText" => "store_message", + "range" => %{end: %{character: 21, line: 27}, start: %{character: 7, line: 27}} + } + ] + } == changes + end + end + + describe "not yet (fully) supported/working renaming cases" do + test "rename started with cursor at function definition" do + file_path = FixtureHelpers.get_path("rename_example.exs") + text = File.read!(file_path) + uri = SourceFile.path_to_uri(file_path) + + # defp _handle_error({:ok, message}) + {line, char} = {4, 8} + + assert {:ok, %{"documentChanges" => changes}} = + Rename.rename( + %SourceFile{text: text, version: 0}, + uri, + line, + char, + "handle_errors" + ) + + refute %{ + "textDocument" => %{ + "uri" => uri, + "version" => 1 + }, + "edits" => [ + %{ + "newText" => "handle_errors", + "range" => %{end: %{character: 19, line: 37}, start: %{character: 7, line: 37}} + }, + %{ + "newText" => "handle_errors", + "range" => %{end: %{character: 19, line: 39}, start: %{character: 7, line: 39}} + }, + %{ + "newText" => "handle_errors", + "range" => %{end: %{character: 19, line: 28}, start: %{character: 7, line: 28}} + } + ] + } == List.first(changes) + end + + test "rename function with multiple heads" do + file_path = FixtureHelpers.get_path("rename_example.exs") + text = File.read!(file_path) + uri = SourceFile.path_to_uri(file_path) + + # |> _handle_error + {line, char} = {29, 8} + + assert {:ok, %{"documentChanges" => [changes]}} = + Rename.rename( + %SourceFile{text: text, version: 0}, + uri, + line, + char, + "handle_errors" + ) + + # missed second function head on line 40: handle_error({:error, changeset}) + assert %{ + "textDocument" => %{ + "uri" => uri, + "version" => 1 + }, + "edits" => [ + %{ + "newText" => "handle_errors", + "range" => %{end: %{character: 19, line: 37}, start: %{character: 7, line: 37}} + }, + %{ + "newText" => "handle_errors", + "range" => %{end: %{character: 19, line: 28}, start: %{character: 7, line: 28}} + } + ] + } == changes + end + end +end diff --git a/apps/language_server/test/support/fixtures/rename_example.exs b/apps/language_server/test/support/fixtures/rename_example.exs new file mode 100644 index 000000000..35c3a57be --- /dev/null +++ b/apps/language_server/test/support/fixtures/rename_example.exs @@ -0,0 +1,67 @@ +defmodule Example.Pipeline do + use Broadway + + alias Broadway.Message + + def start_link(_) do + {module, opts} = Application.get_env(:broadway_sqs, :producer_module) + + Broadway.start_link(__MODULE__, + name: __MODULE__, + producer: [ + module: {module, opts} + ], + processors: [ + default: [] + ], + batchers: [ + default: [ + batch_size: 10, + batch_timeout: 2000 + ] + ] + ) + end + + def handle_message(_, %Message{data: data} = message, _) do + info + |> create_message + |> handle_error + + message + end + + def handle_batch(_, messages, _, _) do + messages + end + + defp handle_error({:ok, message}), do: message + + defp handle_error({:error, changeset}) do + raise Example.SqsPipelineException, message: changeset.errors + end + + defp create_message(data) do + data + |> Jason.decode(save_message) + |> prepare_params + |> save_message + end + + defp save_message({:ok, params}) do + %Collector.Message{} + |> Collector.Message.changeset(params) + |> Collector.Repo.insert() + end + + defp prepare_params({:ok, %{"action" => _, "payload" => _}} = params), + do: params + + defp prepare_params(_) do + raise Example.PipelineException, message: "Message does not contain all necessary keys." + end +end + +defmodule Example.PipelineException do + defexception [:message] +end \ No newline at end of file From 2e769851d3fc62638225c0399c17f250981deb5d Mon Sep 17 00:00:00 2001 From: Tim Gent Date: Thu, 20 Oct 2022 16:50:13 +0100 Subject: [PATCH 2/4] Fix renaming functions with multiple headers --- .../lib/language_server/providers/rename.ex | 64 ++++++++++++++----- .../test/providers/rename_test.exs | 36 ++++++----- 2 files changed, 67 insertions(+), 33 deletions(-) diff --git a/apps/language_server/lib/language_server/providers/rename.ex b/apps/language_server/lib/language_server/providers/rename.ex index 2c31af2bb..5f524fe12 100644 --- a/apps/language_server/lib/language_server/providers/rename.ex +++ b/apps/language_server/lib/language_server/providers/rename.ex @@ -6,7 +6,6 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do """ alias ElixirLS.LanguageServer.SourceFile - #import ElixirLS.LanguageServer.Protocol def rename(%SourceFile{} = source_file, start_uri, line, character, new_name) do edits = @@ -15,23 +14,24 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do %ElixirSense.Location{} = definition <- ElixirSense.definition(source_file.text, line, character), references <- ElixirSense.references(source_file.text, line, character) do - - length_old = length(char_ident) - [ - %{ - uri: start_uri, - range: - adjust_range( - definition.line, - definition.column, - definition.line, - definition.column + length_old + definition_references = + case definition do + %{type: :function} -> + parse_definition_source_code(definition, source_file.text) + |> get_all_fn_header_positions(char_ident) + |> positions_to_references(start_uri, length_old) + + _ -> + positions_to_references( + [{definition.line, definition.column}], + start_uri, + length_old ) - } - | repack_references(references, start_uri) - ] + end + + definition_references ++ repack_references(references, start_uri) else _ -> [] @@ -62,7 +62,9 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do begin: {start_line, start_col}, end: {end_line, end_col}, context: {context, char_ident} - } when context in [:local_or_var, :local_call] <- Code.Fragment.surround_context(source_file.text, {line, character}) do + } + when context in [:local_or_var, :local_call] <- + Code.Fragment.surround_context(source_file.text, {line, character}) do %{ range: adjust_range(start_line, start_col, end_line, end_col), placeholder: to_string(char_ident) @@ -91,6 +93,36 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do end end + defp parse_definition_source_code(definition, source_text) + + defp parse_definition_source_code(%{file: nil}, source_text) do + ElixirSense.Core.Parser.parse_string(source_text, true, true, 0) + end + + defp parse_definition_source_code(%{file: file}, _) do + ElixirSense.Core.Parser.parse_file(file, true, true, 0) + end + + defp get_all_fn_header_positions(parsed_source, char_ident) do + parsed_source.mods_funs_to_positions + |> Map.filter(fn + {{_, fn_name, _}, _} -> Atom.to_charlist(fn_name) == char_ident + end) + |> Enum.flat_map(fn {_, %{positions: positions}} -> positions end) + |> Enum.uniq() + end + + defp positions_to_references(header_positions, start_uri, length_old) + when is_list(header_positions) do + header_positions + |> Enum.map(fn {line, column} -> + %{ + uri: start_uri, + range: adjust_range(line, column, line, column + length_old) + } + end) + end + defp adjust_range(start_line, start_character, end_line, end_character) do %{ start: %{line: start_line - 1, character: start_character - 1}, diff --git a/apps/language_server/test/providers/rename_test.exs b/apps/language_server/test/providers/rename_test.exs index ad3793c5b..ffe51b1c5 100644 --- a/apps/language_server/test/providers/rename_test.exs +++ b/apps/language_server/test/providers/rename_test.exs @@ -132,18 +132,15 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do ] } == changes end - end - describe "not yet (fully) supported/working renaming cases" do - test "rename started with cursor at function definition" do + test "rename function with multiple heads: handle_error -> handle_errors" do file_path = FixtureHelpers.get_path("rename_example.exs") text = File.read!(file_path) uri = SourceFile.path_to_uri(file_path) - # defp _handle_error({:ok, message}) - {line, char} = {4, 8} + {line, char} = {29, 8} - assert {:ok, %{"documentChanges" => changes}} = + assert {:ok, %{"documentChanges" => [changes]}} = Rename.rename( %SourceFile{text: text, version: 0}, uri, @@ -152,7 +149,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do "handle_errors" ) - refute %{ + assert %{ "textDocument" => %{ "uri" => uri, "version" => 1 @@ -160,29 +157,31 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do "edits" => [ %{ "newText" => "handle_errors", - "range" => %{end: %{character: 19, line: 37}, start: %{character: 7, line: 37}} + "range" => %{end: %{character: 19, line: 39}, start: %{character: 7, line: 39}} }, %{ "newText" => "handle_errors", - "range" => %{end: %{character: 19, line: 39}, start: %{character: 7, line: 39}} + "range" => %{end: %{character: 19, line: 37}, start: %{character: 7, line: 37}} }, %{ "newText" => "handle_errors", "range" => %{end: %{character: 19, line: 28}, start: %{character: 7, line: 28}} } ] - } == List.first(changes) + } == changes end + end - test "rename function with multiple heads" do + describe "not yet (fully) supported/working renaming cases" do + test "rename started with cursor at function definition" do file_path = FixtureHelpers.get_path("rename_example.exs") text = File.read!(file_path) uri = SourceFile.path_to_uri(file_path) - # |> _handle_error - {line, char} = {29, 8} + # defp _handle_error({:ok, message}) + {line, char} = {4, 8} - assert {:ok, %{"documentChanges" => [changes]}} = + assert {:ok, %{"documentChanges" => changes}} = Rename.rename( %SourceFile{text: text, version: 0}, uri, @@ -191,8 +190,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do "handle_errors" ) - # missed second function head on line 40: handle_error({:error, changeset}) - assert %{ + refute %{ "textDocument" => %{ "uri" => uri, "version" => 1 @@ -202,12 +200,16 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do "newText" => "handle_errors", "range" => %{end: %{character: 19, line: 37}, start: %{character: 7, line: 37}} }, + %{ + "newText" => "handle_errors", + "range" => %{end: %{character: 19, line: 39}, start: %{character: 7, line: 39}} + }, %{ "newText" => "handle_errors", "range" => %{end: %{character: 19, line: 28}, start: %{character: 7, line: 28}} } ] - } == changes + } == List.first(changes) end end end From 1daf1c8ef52ff1486ce9384aa75e3c0fc7bcbfce Mon Sep 17 00:00:00 2001 From: Tim Gent Date: Fri, 21 Oct 2022 10:47:26 +0100 Subject: [PATCH 3/4] Simplify tests with rename_example - Make rename_example a `.ex` file so we can later test multi-file refactors - Simplify rename_example to make it easier to follow - Extract some common patterns out in the tests --- .../test/providers/rename_test.exs | 220 +++++++++--------- .../test/support/fixtures/rename_example.ex | 15 ++ .../test/support/fixtures/rename_example.exs | 67 ------ 3 files changed, 128 insertions(+), 174 deletions(-) create mode 100644 apps/language_server/test/support/fixtures/rename_example.ex delete mode 100644 apps/language_server/test/support/fixtures/rename_example.exs diff --git a/apps/language_server/test/providers/rename_test.exs b/apps/language_server/test/providers/rename_test.exs index ffe51b1c5..c20a9b563 100644 --- a/apps/language_server/test/providers/rename_test.exs +++ b/apps/language_server/test/providers/rename_test.exs @@ -36,25 +36,18 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do # _a + b {line, char} = {3, 5} - assert {:ok, %{"documentChanges" => changes}} = - Rename.rename(%SourceFile{text: text, version: 0}, @fake_uri, line, char, "test") - - assert %{ - "textDocument" => %{ - "uri" => @fake_uri, - "version" => 1 - }, - "edits" => [ - %{ - "range" => %{end: %{character: 11, line: 1}, start: %{character: 10, line: 1}}, - "newText" => "test" - }, - %{ - "range" => %{end: %{character: 5, line: 2}, start: %{character: 4, line: 2}}, - "newText" => "test" - } - ] - } == List.first(changes) + edits = + Rename.rename(%SourceFile{text: text, version: 0}, @fake_uri, line, char, "test") + |> assert_return_structure_and_get_edits(@fake_uri, 1) + + expected_edits = + [ + %{line: 1, start_char: 10, end_char: 11}, + %{line: 2, start_char: 4, end_char: 5} + ] + |> get_expected_edits("test") + + assert sort_edit_by_start_line(edits) == expected_edits end test "nema -> name" do @@ -69,112 +62,91 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do # "Hello " <> ne_ma {line, char} = {3, 19} - assert {:ok, %{"documentChanges" => [changes]}} = - Rename.rename( - %SourceFile{text: text, version: 0}, - @fake_uri, - line, - char, - "name" - ) - - assert %{ - "textDocument" => %{ - "uri" => @fake_uri, - "version" => 1 - }, - "edits" => [ - %{ - "range" => %{end: %{character: 16, line: 1}, start: %{character: 12, line: 1}}, - "newText" => "name" - }, - %{ - "range" => %{end: %{character: 20, line: 2}, start: %{character: 16, line: 2}}, - "newText" => "name" - } - ] - } == changes + edits = + Rename.rename( + %SourceFile{text: text, version: 0}, + @fake_uri, + line, + char, + "name" + ) + |> assert_return_structure_and_get_edits(@fake_uri, 1) + + expected_edits = + [ + %{line: 1, start_char: 12, end_char: 16}, + %{line: 2, start_char: 16, end_char: 20} + ] + |> get_expected_edits("name") + + assert sort_edit_by_start_line(edits) == expected_edits end end describe "renaming local function" do - test "create_message -> store_message" do - file_path = FixtureHelpers.get_path("rename_example.exs") + test "subtract -> new_subtract" do + file_path = FixtureHelpers.get_path("rename_example.ex") text = File.read!(file_path) uri = SourceFile.path_to_uri(file_path) - # |> _create_message - {line, char} = {28, 8} - - assert {:ok, %{"documentChanges" => [changes]}} = - Rename.rename( - %SourceFile{text: text, version: 0}, - uri, - line, - char, - "store_message" - ) - - assert %{ - "textDocument" => %{ - "uri" => uri, - "version" => 1 - }, - "edits" => [ - %{ - "newText" => "store_message", - "range" => %{end: %{character: 21, line: 43}, start: %{character: 7, line: 43}} - }, - %{ - "newText" => "store_message", - "range" => %{end: %{character: 21, line: 27}, start: %{character: 7, line: 27}} - } - ] - } == changes + # d = subtract(a, b) + {line, char} = {6, 10} + + edits = + Rename.rename( + %SourceFile{text: text, version: 0}, + uri, + line, + char, + "new_subtract" + ) + |> assert_return_structure_and_get_edits(uri, 1) + + expected_edits = + [ + %{line: 5, start_char: 8, end_char: 16}, + %{line: 13, start_char: 7, end_char: 15} + ] + |> get_expected_edits("new_subtract") + + assert sort_edit_by_start_line(edits) == expected_edits end test "rename function with multiple heads: handle_error -> handle_errors" do - file_path = FixtureHelpers.get_path("rename_example.exs") + file_path = FixtureHelpers.get_path("rename_example.ex") text = File.read!(file_path) uri = SourceFile.path_to_uri(file_path) - {line, char} = {29, 8} - - assert {:ok, %{"documentChanges" => [changes]}} = - Rename.rename( - %SourceFile{text: text, version: 0}, - uri, - line, - char, - "handle_errors" - ) - - assert %{ - "textDocument" => %{ - "uri" => uri, - "version" => 1 - }, - "edits" => [ - %{ - "newText" => "handle_errors", - "range" => %{end: %{character: 19, line: 39}, start: %{character: 7, line: 39}} - }, - %{ - "newText" => "handle_errors", - "range" => %{end: %{character: 19, line: 37}, start: %{character: 7, line: 37}} - }, - %{ - "newText" => "handle_errors", - "range" => %{end: %{character: 19, line: 28}, start: %{character: 7, line: 28}} - } - ] - } == changes + # c = add(a, b) + {line, char} = {5, 9} + + edits = + Rename.rename( + %SourceFile{text: text, version: 0}, + uri, + line, + char, + "new_add" + ) + |> assert_return_structure_and_get_edits(uri, 1) + + expected_edits = + [ + %{line: 4, start_char: 8, end_char: 11}, + %{line: 6, start_char: 4, end_char: 7}, + %{line: 9, start_char: 7, end_char: 10}, + %{line: 10, start_char: 7, end_char: 10}, + %{line: 11, start_char: 7, end_char: 10} + ] + |> get_expected_edits("new_add") + + assert sort_edit_by_start_line(edits) == expected_edits end end describe "not yet (fully) supported/working renaming cases" do test "rename started with cursor at function definition" do - file_path = FixtureHelpers.get_path("rename_example.exs") + file_path = FixtureHelpers.get_path("rename_example.ex") text = File.read!(file_path) uri = SourceFile.path_to_uri(file_path) @@ -212,4 +184,38 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do } == List.first(changes) end end + + defp get_expected_edits(edits, new_text) when is_list(edits), + do: Enum.map(edits, &get_expected_edits(&1, new_text)) + + defp get_expected_edits(%{line: line, start_char: start_char, end_char: end_char}, new_text) do + %{ + "newText" => new_text, + "range" => %{ + start: %{line: line, character: start_char}, + end: %{line: line, character: end_char} + } + } + end + + defp assert_return_structure_and_get_edits(rename_result, uri, version) do + assert {:ok, + %{ + "documentChanges" => [ + %{ + "textDocument" => %{ + "uri" => ^uri, + "version" => ^version + }, + "edits" => edits + } + ] + }} = rename_result + + edits + end + + defp sort_edit_by_start_line(edits) do + Enum.sort(edits, &(&1["range"].start.line < &2["range"].start.line)) + end end diff --git a/apps/language_server/test/support/fixtures/rename_example.ex b/apps/language_server/test/support/fixtures/rename_example.ex new file mode 100644 index 000000000..df86abea5 --- /dev/null +++ b/apps/language_server/test/support/fixtures/rename_example.ex @@ -0,0 +1,15 @@ +defmodule ElixirLS.Test.RenameExample do + def main do + a = 5 + b = 10 + c = add(a, b) + d = subtract(a, b) + add(c, d) + end + + defp add(a, b) + defp add(a, b) when is_integer(a) and is_integer(b), do: a + b + defp add(a, b) when is_binary(a) and is_binary(b), do: a <> b + + defp subtract(a, b), do: a - b +end diff --git a/apps/language_server/test/support/fixtures/rename_example.exs b/apps/language_server/test/support/fixtures/rename_example.exs deleted file mode 100644 index 35c3a57be..000000000 --- a/apps/language_server/test/support/fixtures/rename_example.exs +++ /dev/null @@ -1,67 +0,0 @@ -defmodule Example.Pipeline do - use Broadway - - alias Broadway.Message - - def start_link(_) do - {module, opts} = Application.get_env(:broadway_sqs, :producer_module) - - Broadway.start_link(__MODULE__, - name: __MODULE__, - producer: [ - module: {module, opts} - ], - processors: [ - default: [] - ], - batchers: [ - default: [ - batch_size: 10, - batch_timeout: 2000 - ] - ] - ) - end - - def handle_message(_, %Message{data: data} = message, _) do - info - |> create_message - |> handle_error - - message - end - - def handle_batch(_, messages, _, _) do - messages - end - - defp handle_error({:ok, message}), do: message - - defp handle_error({:error, changeset}) do - raise Example.SqsPipelineException, message: changeset.errors - end - - defp create_message(data) do - data - |> Jason.decode(save_message) - |> prepare_params - |> save_message - end - - defp save_message({:ok, params}) do - %Collector.Message{} - |> Collector.Message.changeset(params) - |> Collector.Repo.insert() - end - - defp prepare_params({:ok, %{"action" => _, "payload" => _}} = params), - do: params - - defp prepare_params(_) do - raise Example.PipelineException, message: "Message does not contain all necessary keys." - end -end - -defmodule Example.PipelineException do - defexception [:message] -end \ No newline at end of file From 987456e9db68280103840d31588ad04897fd2f4c Mon Sep 17 00:00:00 2001 From: Tim Gent Date: Fri, 21 Oct 2022 12:22:22 +0100 Subject: [PATCH 4/4] Implement rename across multiple files --- .../lib/language_server/providers/rename.ex | 31 +++++++---- .../test/providers/rename_test.exs | 52 ++++++++++++++++++- .../test/support/fixtures/rename_example.ex | 2 +- .../test/support/fixtures/rename_example_b.ex | 3 ++ 4 files changed, 76 insertions(+), 12 deletions(-) create mode 100644 apps/language_server/test/support/fixtures/rename_example_b.ex diff --git a/apps/language_server/lib/language_server/providers/rename.ex b/apps/language_server/lib/language_server/providers/rename.ex index 5f524fe12..2897e9d9b 100644 --- a/apps/language_server/lib/language_server/providers/rename.ex +++ b/apps/language_server/lib/language_server/providers/rename.ex @@ -9,8 +9,8 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do def rename(%SourceFile{} = source_file, start_uri, line, character, new_name) do edits = - with %{context: {context, char_ident}} when context in [:local_or_var, :local_call] <- - Code.Fragment.surround_context(source_file.text, {line, character}), + with char_ident when not is_nil(char_ident) <- + get_char_ident(source_file.text, line, character), %ElixirSense.Location{} = definition <- ElixirSense.definition(source_file.text, line, character), references <- ElixirSense.references(source_file.text, line, character) do @@ -18,11 +18,16 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do definition_references = case definition do - %{type: :function} -> - parse_definition_source_code(definition, source_file.text) + %{file: nil, type: :function} -> + parse_definition_source_code(source_file.text) |> get_all_fn_header_positions(char_ident) |> positions_to_references(start_uri, length_old) + %{file: separate_file_path, type: :function} -> + parse_definition_source_code(definition) + |> get_all_fn_header_positions(char_ident) + |> positions_to_references(SourceFile.path_to_uri(separate_file_path), length_old) + _ -> positions_to_references( [{definition.line, definition.column}], @@ -93,14 +98,12 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do end end - defp parse_definition_source_code(definition, source_text) - - defp parse_definition_source_code(%{file: nil}, source_text) do - ElixirSense.Core.Parser.parse_string(source_text, true, true, 0) + defp parse_definition_source_code(%{file: file}) do + ElixirSense.Core.Parser.parse_file(file, true, true, 0) end - defp parse_definition_source_code(%{file: file}, _) do - ElixirSense.Core.Parser.parse_file(file, true, true, 0) + defp parse_definition_source_code(source_text) when is_binary(source_text) do + ElixirSense.Core.Parser.parse_string(source_text, true, true, 0) end defp get_all_fn_header_positions(parsed_source, char_ident) do @@ -129,4 +132,12 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do end: %{line: end_line - 1, character: end_character - 1} } end + + defp get_char_ident(text, line, character) do + case Code.Fragment.surround_context(text, {line, character}) do + %{context: {context, char_ident}} when context in [:local_or_var, :local_call] -> char_ident + %{context: {:dot, _, char_ident}} -> char_ident + _ -> nil + end + end end diff --git a/apps/language_server/test/providers/rename_test.exs b/apps/language_server/test/providers/rename_test.exs index c20a9b563..934ac38a3 100644 --- a/apps/language_server/test/providers/rename_test.exs +++ b/apps/language_server/test/providers/rename_test.exs @@ -112,7 +112,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do assert sort_edit_by_start_line(edits) == expected_edits end - test "rename function with multiple heads: handle_error -> handle_errors" do + test "rename function with multiple heads: add -> new_add" do file_path = FixtureHelpers.get_path("rename_example.ex") text = File.read!(file_path) uri = SourceFile.path_to_uri(file_path) @@ -142,6 +142,56 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do assert sort_edit_by_start_line(edits) == expected_edits end + + test "rename function defined in a different file ten -> new_ten" do + file_path = FixtureHelpers.get_path("rename_example.ex") + text = File.read!(file_path) + uri = SourceFile.path_to_uri(file_path) + + fn_definition_file_uri = + FixtureHelpers.get_path("rename_example_b.ex") |> SourceFile.path_to_uri() + + # b = ElixirLS.Test.RenameExampleB.ten() + {line, char} = {4, 38} + + assert {:ok, + %{ + "documentChanges" => [ + %{ + "textDocument" => %{ + "uri" => ^uri, + "version" => 1 + }, + "edits" => file_edits + }, + %{ + "textDocument" => %{ + "uri" => ^fn_definition_file_uri, + "version" => 1 + }, + "edits" => fn_definition_file_edits + } + ] + }} = + Rename.rename( + %SourceFile{text: text, version: 0}, + uri, + line, + char, + "new_ten" + ) + + expected_edits_fn_definition_file = + [%{line: 1, start_char: 6, end_char: 9}] |> get_expected_edits("new_ten") + + assert sort_edit_by_start_line(fn_definition_file_edits) == + expected_edits_fn_definition_file + + expected_edits = [%{line: 3, start_char: 37, end_char: 40}] |> get_expected_edits("new_ten") + + assert sort_edit_by_start_line(file_edits) == + expected_edits + end end describe "not yet (fully) supported/working renaming cases" do diff --git a/apps/language_server/test/support/fixtures/rename_example.ex b/apps/language_server/test/support/fixtures/rename_example.ex index df86abea5..2110e22a3 100644 --- a/apps/language_server/test/support/fixtures/rename_example.ex +++ b/apps/language_server/test/support/fixtures/rename_example.ex @@ -1,7 +1,7 @@ defmodule ElixirLS.Test.RenameExample do def main do a = 5 - b = 10 + b = ElixirLS.Test.RenameExampleB.ten() c = add(a, b) d = subtract(a, b) add(c, d) diff --git a/apps/language_server/test/support/fixtures/rename_example_b.ex b/apps/language_server/test/support/fixtures/rename_example_b.ex new file mode 100644 index 000000000..06839db1d --- /dev/null +++ b/apps/language_server/test/support/fixtures/rename_example_b.ex @@ -0,0 +1,3 @@ +defmodule ElixirLS.Test.RenameExampleB do + def ten, do: 10 +end