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 01/10] 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 02/10] 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 03/10] 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 04/10] 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 From 4171632aca5ebcee341aff8051749c82acb2a2ed Mon Sep 17 00:00:00 2001 From: Tim Gent Date: Thu, 10 Nov 2022 15:42:02 +0000 Subject: [PATCH 05/10] Fix rename file version & file paths --- .../lib/language_server/providers/rename.ex | 10 ++++++---- apps/language_server/test/providers/rename_test.exs | 12 ++++++------ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/apps/language_server/lib/language_server/providers/rename.ex b/apps/language_server/lib/language_server/providers/rename.ex index 2897e9d9b..9333229f3 100644 --- a/apps/language_server/lib/language_server/providers/rename.ex +++ b/apps/language_server/lib/language_server/providers/rename.ex @@ -49,7 +49,7 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do %{ "textDocument" => %{ "uri" => uri, - "version" => source_file.version + 1 + "version" => nil }, "edits" => Enum.map(edits, fn edit -> @@ -83,8 +83,10 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do {:ok, result} end - defp repack_references(references, uri) do + defp repack_references(references, start_uri) do for reference <- references do + uri = if reference.uri, do: SourceFile.path_to_uri(reference.uri), else: start_uri + %{ uri: uri, range: %{ @@ -99,11 +101,11 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do end defp parse_definition_source_code(%{file: file}) do - ElixirSense.Core.Parser.parse_file(file, true, true, 0) + ElixirSense.Core.Parser.parse_file(file, true, true, nil) end defp parse_definition_source_code(source_text) when is_binary(source_text) do - ElixirSense.Core.Parser.parse_string(source_text, true, true, 0) + ElixirSense.Core.Parser.parse_string(source_text, true, true, nil) end defp get_all_fn_header_positions(parsed_source, char_ident) do diff --git a/apps/language_server/test/providers/rename_test.exs b/apps/language_server/test/providers/rename_test.exs index 934ac38a3..7afce1c8d 100644 --- a/apps/language_server/test/providers/rename_test.exs +++ b/apps/language_server/test/providers/rename_test.exs @@ -38,7 +38,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do edits = Rename.rename(%SourceFile{text: text, version: 0}, @fake_uri, line, char, "test") - |> assert_return_structure_and_get_edits(@fake_uri, 1) + |> assert_return_structure_and_get_edits(@fake_uri, nil) expected_edits = [ @@ -70,7 +70,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do char, "name" ) - |> assert_return_structure_and_get_edits(@fake_uri, 1) + |> assert_return_structure_and_get_edits(@fake_uri, nil) expected_edits = [ @@ -100,7 +100,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do char, "new_subtract" ) - |> assert_return_structure_and_get_edits(uri, 1) + |> assert_return_structure_and_get_edits(uri, nil) expected_edits = [ @@ -128,7 +128,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do char, "new_add" ) - |> assert_return_structure_and_get_edits(uri, 1) + |> assert_return_structure_and_get_edits(uri, nil) expected_edits = [ @@ -160,14 +160,14 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do %{ "textDocument" => %{ "uri" => ^uri, - "version" => 1 + "version" => nil }, "edits" => file_edits }, %{ "textDocument" => %{ "uri" => ^fn_definition_file_uri, - "version" => 1 + "version" => nil }, "edits" => fn_definition_file_edits } From 0df099669d3a95843af1fc5c6650124e206af317 Mon Sep 17 00:00:00 2001 From: Tim Gent Date: Thu, 10 Nov 2022 16:04:58 +0000 Subject: [PATCH 06/10] Fix rename issue with duplicate entries when renaming var definition --- .../lib/language_server/providers/rename.ex | 2 +- .../test/providers/rename_test.exs | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/apps/language_server/lib/language_server/providers/rename.ex b/apps/language_server/lib/language_server/providers/rename.ex index 9333229f3..4b87e2a42 100644 --- a/apps/language_server/lib/language_server/providers/rename.ex +++ b/apps/language_server/lib/language_server/providers/rename.ex @@ -36,7 +36,7 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do ) end - definition_references ++ repack_references(references, start_uri) + Enum.uniq(definition_references ++ repack_references(references, start_uri)) else _ -> [] diff --git a/apps/language_server/test/providers/rename_test.exs b/apps/language_server/test/providers/rename_test.exs index 7afce1c8d..74ff62b16 100644 --- a/apps/language_server/test/providers/rename_test.exs +++ b/apps/language_server/test/providers/rename_test.exs @@ -81,6 +81,39 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do assert sort_edit_by_start_line(edits) == expected_edits end + + test "renaming a variable definition works original -> new_original" do + text = """ + defmodule MyModule do + def hello do + original = "original" + new = original <> " new stuff" + end + end + """ + + # new = "#{original} + new stuff!" + {line, char} = {3, 6} + + edits = + Rename.rename( + %SourceFile{text: text, version: 0}, + @fake_uri, + line, + char, + "new_original" + ) + |> assert_return_structure_and_get_edits(@fake_uri, nil) + + expected_edits = + [ + %{line: 2, start_char: 4, end_char: 12}, + %{line: 3, start_char: 10, end_char: 18} + ] + |> get_expected_edits("new_original") + + assert sort_edit_by_start_line(edits) == expected_edits + end end describe "renaming local function" do From 42c4d30273379532b260b0f475691d735e3d35cb Mon Sep 17 00:00:00 2001 From: Tim Gent Date: Sun, 13 Nov 2022 21:18:42 +0000 Subject: [PATCH 07/10] Tests for rename prepare and fix rename prepare across multiple files --- .../lib/language_server/providers/rename.ex | 22 +++++- .../test/providers/rename_test.exs | 78 +++++++++++++++---- 2 files changed, 80 insertions(+), 20 deletions(-) diff --git a/apps/language_server/lib/language_server/providers/rename.ex b/apps/language_server/lib/language_server/providers/rename.ex index 4b87e2a42..225efd060 100644 --- a/apps/language_server/lib/language_server/providers/rename.ex +++ b/apps/language_server/lib/language_server/providers/rename.ex @@ -66,10 +66,10 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do 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 + char_ident: char_ident + } = res + when not is_nil(res) <- + get_begin_end_and_char_ident(source_file.text, line, character) do %{ range: adjust_range(start_line, start_col, end_line, end_col), placeholder: to_string(char_ident) @@ -142,4 +142,18 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do _ -> nil end end + + defp get_begin_end_and_char_ident(text, line, character) do + case Code.Fragment.surround_context(text, {line, character}) do + %{begin: begin, end: the_end, context: {context, char_ident}} + when context in [:local_or_var, :local_call] -> + %{begin: begin, end: the_end, char_ident: char_ident} + + %{begin: begin, end: the_end, context: {:dot, _, char_ident}} -> + %{begin: begin, end: the_end, 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 74ff62b16..14f4983f5 100644 --- a/apps/language_server/test/providers/rename_test.exs +++ b/apps/language_server/test/providers/rename_test.exs @@ -36,14 +36,20 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do # _a + b {line, char} = {3, 5} + source = %SourceFile{text: text, version: 0} + target_range = %{line: 2, start_char: 4, end_char: 5} + + Rename.prepare(source, @fake_uri, line, char) + |> assert_prepare_range_and_placeholder_is(target_range, "a") + edits = - Rename.rename(%SourceFile{text: text, version: 0}, @fake_uri, line, char, "test") + Rename.rename(source, @fake_uri, line, char, "test") |> assert_return_structure_and_get_edits(@fake_uri, nil) expected_edits = [ %{line: 1, start_char: 10, end_char: 11}, - %{line: 2, start_char: 4, end_char: 5} + target_range ] |> get_expected_edits("test") @@ -62,9 +68,15 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do # "Hello " <> ne_ma {line, char} = {3, 19} + source = %SourceFile{text: text, version: 0} + target_range = %{line: 2, start_char: 16, end_char: 20} + + Rename.prepare(source, @fake_uri, line, char) + |> assert_prepare_range_and_placeholder_is(target_range, "nema") + edits = Rename.rename( - %SourceFile{text: text, version: 0}, + source, @fake_uri, line, char, @@ -75,7 +87,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do expected_edits = [ %{line: 1, start_char: 12, end_char: 16}, - %{line: 2, start_char: 16, end_char: 20} + target_range ] |> get_expected_edits("name") @@ -94,10 +106,15 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do # new = "#{original} + new stuff!" {line, char} = {3, 6} + source = %SourceFile{text: text, version: 0} + target_range = %{line: 2, start_char: 4, end_char: 12} + + Rename.prepare(source, @fake_uri, line, char) + |> assert_prepare_range_and_placeholder_is(target_range, "original") edits = Rename.rename( - %SourceFile{text: text, version: 0}, + source, @fake_uri, line, char, @@ -107,7 +124,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do expected_edits = [ - %{line: 2, start_char: 4, end_char: 12}, + target_range, %{line: 3, start_char: 10, end_char: 18} ] |> get_expected_edits("new_original") @@ -120,14 +137,19 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do 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) + uri = SourceFile.Path.to_uri(file_path) # d = subtract(a, b) {line, char} = {6, 10} + source = %SourceFile{text: text, version: 0} + target_range = %{line: 5, start_char: 8, end_char: 16} + + Rename.prepare(source, @fake_uri, line, char) + |> assert_prepare_range_and_placeholder_is(target_range, "subtract") edits = Rename.rename( - %SourceFile{text: text, version: 0}, + source, uri, line, char, @@ -137,7 +159,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do expected_edits = [ - %{line: 5, start_char: 8, end_char: 16}, + target_range, %{line: 13, start_char: 7, end_char: 15} ] |> get_expected_edits("new_subtract") @@ -148,14 +170,19 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest 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) + uri = SourceFile.Path.to_uri(file_path) # c = add(a, b) {line, char} = {5, 9} + source = %SourceFile{text: text, version: 0} + target_range = %{line: 4, start_char: 8, end_char: 11} + + Rename.prepare(source, @fake_uri, line, char) + |> assert_prepare_range_and_placeholder_is(target_range, "add") edits = Rename.rename( - %SourceFile{text: text, version: 0}, + source, uri, line, char, @@ -165,7 +192,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do expected_edits = [ - %{line: 4, start_char: 8, end_char: 11}, + target_range, %{line: 6, start_char: 4, end_char: 7}, %{line: 9, start_char: 7, end_char: 10}, %{line: 10, start_char: 7, end_char: 10}, @@ -179,13 +206,17 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do 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) + uri = SourceFile.Path.to_uri(file_path) fn_definition_file_uri = - FixtureHelpers.get_path("rename_example_b.ex") |> SourceFile.path_to_uri() + FixtureHelpers.get_path("rename_example_b.ex") |> SourceFile.Path.to_uri() # b = ElixirLS.Test.RenameExampleB.ten() {line, char} = {4, 38} + source = %SourceFile{text: text, version: 0} + + Rename.prepare(source, uri, line, char) + |> assert_prepare_range_and_placeholder_is(%{line: 3, start_char: 8, end_char: 40}, "ten") assert {:ok, %{ @@ -207,7 +238,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do ] }} = Rename.rename( - %SourceFile{text: text, version: 0}, + source, uri, line, char, @@ -231,7 +262,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do test "rename started with cursor at function definition" do file_path = FixtureHelpers.get_path("rename_example.ex") text = File.read!(file_path) - uri = SourceFile.path_to_uri(file_path) + uri = SourceFile.Path.to_uri(file_path) # defp _handle_error({:ok, message}) {line, char} = {4, 8} @@ -301,4 +332,19 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do defp sort_edit_by_start_line(edits) do Enum.sort(edits, &(&1["range"].start.line < &2["range"].start.line)) end + + defp assert_prepare_range_and_placeholder_is( + prepare_result, + %{line: line, start_char: start_char, end_char: end_char} = _expected_range, + expected_placeholder + ) do + assert {:ok, + %{ + placeholder: expected_placeholder, + range: %{ + start: %{line: line, character: start_char}, + end: %{line: line, character: end_char} + } + }} == prepare_result + end end From faf8d25115f9c4a952e5857c962df3b244bb80c0 Mon Sep 17 00:00:00 2001 From: Tim Gent Date: Sun, 13 Nov 2022 21:53:27 +0000 Subject: [PATCH 08/10] Fix rename issues following merge with master --- .../lib/language_server/providers/rename.ex | 8 +++--- .../test/providers/rename_test.exs | 26 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/apps/language_server/lib/language_server/providers/rename.ex b/apps/language_server/lib/language_server/providers/rename.ex index 225efd060..98d8bc732 100644 --- a/apps/language_server/lib/language_server/providers/rename.ex +++ b/apps/language_server/lib/language_server/providers/rename.ex @@ -8,12 +8,14 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do alias ElixirLS.LanguageServer.SourceFile def rename(%SourceFile{} = source_file, start_uri, line, character, new_name) do + trace = ElixirLS.LanguageServer.Tracer.get_trace() + edits = 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 + references <- ElixirSense.references(source_file.text, line, character, trace) do length_old = length(char_ident) definition_references = @@ -26,7 +28,7 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do %{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(SourceFile.Path.to_uri(separate_file_path), length_old) _ -> positions_to_references( @@ -85,7 +87,7 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do defp repack_references(references, start_uri) do for reference <- references do - uri = if reference.uri, do: SourceFile.path_to_uri(reference.uri), else: start_uri + uri = if reference.uri, do: SourceFile.Path.to_uri(reference.uri), else: start_uri %{ uri: uri, diff --git a/apps/language_server/test/providers/rename_test.exs b/apps/language_server/test/providers/rename_test.exs index 14f4983f5..8f63d26f7 100644 --- a/apps/language_server/test/providers/rename_test.exs +++ b/apps/language_server/test/providers/rename_test.exs @@ -1,13 +1,39 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do use ExUnit.Case, async: true + alias ElixirLS.LanguageServer.Build alias ElixirLS.LanguageServer.Providers.Rename alias ElixirLS.LanguageServer.SourceFile alias ElixirLS.LanguageServer.Test.FixtureHelpers + alias ElixirLS.LanguageServer.Tracer # mix cmd --app language_server mix test test/providers/rename_test.exs @fake_uri "file:///World/Netherlands/Amsterdam/supercomputer/amazing.ex" + setup_all context do + File.rm_rf!(FixtureHelpers.get_path(".elixir_ls/calls.dets")) + {:ok, pid} = Tracer.start_link([]) + Tracer.set_project_dir(FixtureHelpers.get_path("")) + + compiler_options = Code.compiler_options() + Build.set_compiler_options(ignore_module_conflict: true) + + on_exit(fn -> + Process.monitor(pid) + Process.unlink(pid) + GenServer.stop(pid) + + receive do + {:DOWN, _, _, _, _} -> :ok + end + end) + + Code.compile_file(FixtureHelpers.get_path("rename_example.ex")) + Code.compile_file(FixtureHelpers.get_path("rename_example_b.ex")) + + {:ok, context} + end + test "rename blank space" do text = """ defmodule MyModule do From fab20320cb44a70417ce688a444ce0b2fb64e285 Mon Sep 17 00:00:00 2001 From: Tim Gent Date: Tue, 15 Nov 2022 21:05:13 +0000 Subject: [PATCH 09/10] Rename to only rename fn headers of correct arity --- .../lib/language_server/providers/rename.ex | 20 ++++++++++++------- .../test/providers/rename_test.exs | 2 +- .../test/support/fixtures/rename_example.ex | 2 ++ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/apps/language_server/lib/language_server/providers/rename.ex b/apps/language_server/lib/language_server/providers/rename.ex index 98d8bc732..d4588009b 100644 --- a/apps/language_server/lib/language_server/providers/rename.ex +++ b/apps/language_server/lib/language_server/providers/rename.ex @@ -22,12 +22,12 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do case definition do %{file: nil, type: :function} -> parse_definition_source_code(source_file.text) - |> get_all_fn_header_positions(char_ident) + |> get_all_fn_header_positions(char_ident, definition) |> 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) + |> get_all_fn_header_positions(char_ident, definition) |> positions_to_references(SourceFile.Path.to_uri(separate_file_path), length_old) _ -> @@ -78,7 +78,7 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do } else _ -> - # Not a variable or local call, skipping for now + # Not a variable or function call, skipping nil end @@ -86,7 +86,7 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do end defp repack_references(references, start_uri) do - for reference <- references do + Enum.map(references, fn reference -> uri = if reference.uri, do: SourceFile.Path.to_uri(reference.uri), else: start_uri %{ @@ -99,7 +99,7 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do } } } - end + end) end defp parse_definition_source_code(%{file: file}) do @@ -110,10 +110,16 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do ElixirSense.Core.Parser.parse_string(source_text, true, true, nil) end - defp get_all_fn_header_positions(parsed_source, char_ident) do + defp get_all_fn_header_positions( + parsed_source, + definition_name, + %{column: column, line: line} = _definition + ) do parsed_source.mods_funs_to_positions |> Map.filter(fn - {{_, fn_name, _}, _} -> Atom.to_charlist(fn_name) == char_ident + {{_, fn_name, fn_arity}, %{positions: fn_positions}} -> + Atom.to_charlist(fn_name) === definition_name and not is_nil(fn_arity) and + Enum.member?(fn_positions, {line, column}) end) |> Enum.flat_map(fn {_, %{positions: positions}} -> positions end) |> Enum.uniq() diff --git a/apps/language_server/test/providers/rename_test.exs b/apps/language_server/test/providers/rename_test.exs index 8f63d26f7..62f97f235 100644 --- a/apps/language_server/test/providers/rename_test.exs +++ b/apps/language_server/test/providers/rename_test.exs @@ -186,7 +186,7 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do expected_edits = [ target_range, - %{line: 13, start_char: 7, end_char: 15} + %{line: 15, start_char: 7, end_char: 15} ] |> get_expected_edits("new_subtract") diff --git a/apps/language_server/test/support/fixtures/rename_example.ex b/apps/language_server/test/support/fixtures/rename_example.ex index 2110e22a3..2ee17656f 100644 --- a/apps/language_server/test/support/fixtures/rename_example.ex +++ b/apps/language_server/test/support/fixtures/rename_example.ex @@ -11,5 +11,7 @@ defmodule ElixirLS.Test.RenameExample do 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 + def add(a, b, c), do: a + b + c + defp subtract(a, b), do: a - b end From 532bf88bcc4f83366c43baa40125ea3cffb25cea Mon Sep 17 00:00:00 2001 From: Tim Gent Date: Tue, 15 Nov 2022 21:07:36 +0000 Subject: [PATCH 10/10] Simplify get_char_ident fn in rename provider --- .../lib/language_server/providers/rename.ex | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/apps/language_server/lib/language_server/providers/rename.ex b/apps/language_server/lib/language_server/providers/rename.ex index d4588009b..24f815b3a 100644 --- a/apps/language_server/lib/language_server/providers/rename.ex +++ b/apps/language_server/lib/language_server/providers/rename.ex @@ -144,10 +144,9 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do 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 + case get_begin_end_and_char_ident(text, line, character) do + nil -> nil + %{char_ident: char_ident} -> char_ident end end