diff --git a/apps/language_server/lib/language_server/providers/rename.ex b/apps/language_server/lib/language_server/providers/rename.ex index 2c31af2bb..2897e9d9b 100644 --- a/apps/language_server/lib/language_server/providers/rename.ex +++ b/apps/language_server/lib/language_server/providers/rename.ex @@ -6,32 +6,37 @@ 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 = - 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 - - 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 + %{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}], + start_uri, + length_old ) - } - | repack_references(references, start_uri) - ] + end + + definition_references ++ repack_references(references, start_uri) else _ -> [] @@ -62,7 +67,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,10 +98,46 @@ defmodule ElixirLS.LanguageServer.Providers.Rename do end end + defp parse_definition_source_code(%{file: file}) do + ElixirSense.Core.Parser.parse_file(file, true, true, 0) + end + + 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 + 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}, 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 ad3793c5b..934ac38a3 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") + edits = + Rename.rename(%SourceFile{text: text, version: 0}, @fake_uri, line, char, "test") + |> assert_return_structure_and_get_edits(@fake_uri, 1) - 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) + 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,74 +62,141 @@ 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" - ) + edits = + Rename.rename( + %SourceFile{text: text, version: 0}, + @fake_uri, + line, + char, + "name" + ) + |> assert_return_structure_and_get_edits(@fake_uri, 1) - 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 + 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) + + # 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: add -> new_add" do + file_path = FixtureHelpers.get_path("rename_example.ex") + text = File.read!(file_path) + uri = SourceFile.path_to_uri(file_path) + + # 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 + + 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) - # |> _create_message - {line, char} = {28, 8} + fn_definition_file_uri = + FixtureHelpers.get_path("rename_example_b.ex") |> SourceFile.path_to_uri() - assert {:ok, %{"documentChanges" => [changes]}} = + # 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, - "store_message" + "new_ten" ) - 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 + 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 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) @@ -173,41 +233,39 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do ] } == List.first(changes) end + 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) + defp get_expected_edits(edits, new_text) when is_list(edits), + do: Enum.map(edits, &get_expected_edits(&1, new_text)) - # |> _handle_error - {line, char} = {29, 8} + 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 - assert {:ok, %{"documentChanges" => [changes]}} = - Rename.rename( - %SourceFile{text: text, version: 0}, - uri, - line, - char, - "handle_errors" - ) + defp assert_return_structure_and_get_edits(rename_result, uri, version) do + assert {:ok, + %{ + "documentChanges" => [ + %{ + "textDocument" => %{ + "uri" => ^uri, + "version" => ^version + }, + "edits" => edits + } + ] + }} = rename_result - # 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 + 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..2110e22a3 --- /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 = ElixirLS.Test.RenameExampleB.ten() + 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 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