Fix renaming functions with multiple headers

Author: timgent

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},

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,37 +149,39 @@ defmodule ElixirLS.LanguageServer.Providers.RenameTest do
                  "handle_errors"
                )
 
-      refute %{
+      assert %{
                "textDocument" => %{
                  "uri" => uri,
                  "version" => 1
                },
                "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