Use a brand new diff strategy and apply some special handling for ren…

…aming non-alias reference modules.

This can fix some errors that occur when renaming without changing the local name.

apps/remote_control/lib/lexical/remote_control/code_mod/rename/entry.ex

@@ -0,0 +1,38 @@
+defmodule Lexical.RemoteControl.CodeMod.Rename.Entry do
+  @moduledoc """
+  """
+  alias Lexical.RemoteControl.Search.Indexer, as: Indexer
+
+  # When renaming, we rely on the `Indexer.Entry`,
+  # and we also need some other fields used exclusively for renaming, such as `edit_range`.
+  @type t :: %__MODULE__{
+          id: Indexer.Entry.entry_id(),
+          path: Lexical.path(),
+          subject: Indexer.Entry.subject(),
+          block_range: Lexical.Document.Range.t() | nil,
+          range: Lexical.Document.Range.t(),
+          edit_range: Lexical.Document.Range.t(),
+          subtype: Indexer.Entry.entry_subtype()
+        }
+  defstruct [
+    :id,
+    :path,
+    :subject,
+    :block_range,
+    :range,
+    :edit_range,
+    :subtype
+  ]
+
+  def new(%Indexer.Entry{} = indexer_entry) do
+    %__MODULE__{
+      id: indexer_entry.id,
+      path: indexer_entry.path,
+      subject: indexer_entry.subject,
+      subtype: indexer_entry.subtype,
+      block_range: indexer_entry.block_range,
+      range: indexer_entry.range,
+      edit_range: indexer_entry.range
+    }
+  end
+end

apps/remote_control/lib/lexical/remote_control/code_mod/rename/file.ex

@@ -5,8 +5,9 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.File do
   alias Lexical.Project
   alias Lexical.RemoteControl
   alias Lexical.RemoteControl.CodeIntelligence.Entity
+  alias Lexical.RemoteControl.CodeMod.Rename.Entry
   alias Lexical.RemoteControl.Search.Indexer
-  alias Lexical.RemoteControl.Search.Indexer.Entry
+  alias Lexical.RemoteControl.Search.Indexer.Entry, as: IndexerEntry
 
   @spec maybe_rename(Document.t(), Entry.t(), String.t()) :: Document.Changes.rename_file()
   def maybe_rename(%Document{} = document, %Entry{} = entry, new_suffix) do
@@ -15,7 +16,7 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.File do
     end
   end
 
-  defp root_module?(entry, document) do
+  defp root_module?(%Entry{} = entry, document) do
     entries =
       ProcessCache.trans("#{document.uri}-entries", 50, fn ->
         with {:ok, entries} <-
@@ -25,15 +26,15 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.File do
       end)
 
     case Enum.filter(entries, &(&1.block_id == :root)) do
-      [root_module] ->
+      [%IndexerEntry{} = root_module] ->
         root_module.subject == entry.subject and root_module.block_range == entry.block_range
 
       _ ->
         false
     end
   end
 
-  defp rename_file(document, entry, new_suffix) do
+  defp rename_file(document, %Entry{} = entry, new_suffix) do
     root_path = root_path()
     relative_path = Path.relative_to(entry.path, root_path)
 
@@ -61,13 +62,13 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.File do
     Project.root_path(RemoteControl.get_project())
   end
 
-  defp fetch_new_name(document, entry, new_suffix) do
-    text_edits = [Document.Edit.new(new_suffix, entry.range)]
+  defp fetch_new_name(document, %Entry{} = entry, new_suffix) do
+    text_edits = [Document.Edit.new(new_suffix, entry.edit_range)]
 
     with {:ok, edited_document} <-
            Document.apply_content_changes(document, document.version + 1, text_edits),
          {:ok, %{context: {:alias, alias}}} <-
-           Ast.surround_context(edited_document, entry.range.start) do
+           Ast.surround_context(edited_document, entry.edit_range.start) do
       {:ok, to_string(alias)}
     else
       _ -> :error

apps/remote_control/lib/lexical/remote_control/code_mod/rename/module.ex

@@ -9,6 +9,8 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.Module do
   alias Lexical.Formats
   alias Lexical.RemoteControl.CodeIntelligence.Entity
   alias Lexical.RemoteControl.CodeMod.Rename
+  alias Lexical.RemoteControl.CodeMod.Rename.Entry
+  alias Lexical.RemoteControl.CodeMod.Rename.Module
   alias Lexical.RemoteControl.Search.Store
   require Logger
 
@@ -41,11 +43,11 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.Module do
 
   @spec rename(Range.t(), String.t(), atom()) :: [Document.Changes.t()]
   def rename(%Range{} = old_range, new_name, module) do
-    {old_suffix, new_suffix} = old_range |> range_text() |> diff(new_name)
-    results = exacts(module, old_suffix) ++ descendants(module, old_suffix)
+    {to_be_renamed, replacement} = old_range |> range_text() |> Module.Diff.diff(new_name)
+    results = exacts(module, to_be_renamed) ++ descendants(module, to_be_renamed)
 
     for {uri, entries} <- Enum.group_by(results, &Document.Path.ensure_uri(&1.path)),
-        result = to_document_changes(uri, entries, new_suffix),
+        result = to_document_changes(uri, entries, replacement),
         match?({:ok, _}, result) do
       {:ok, document_changes} = result
       document_changes
@@ -90,40 +92,25 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.Module do
     {module, range}
   end
 
-  defp diff(old_range_text, new_name) do
-    diff = String.myers_difference(old_range_text, new_name)
-
-    eq =
-      if match?([{:eq, _eq} | _], diff) do
-        diff |> hd() |> elem(1)
-      else
-        ""
-      end
-
-    old_suffix = String.replace(old_range_text, ~r"^#{eq}", "")
-    new_suffix = String.replace(new_name, ~r"^#{eq}", "")
-    {old_suffix, new_suffix}
-  end
-
-  defp exacts(module, old_suffix) do
+  defp exacts(module, to_be_renamed) do
     module
     |> query_for_exacts()
-    |> Enum.filter(&entry_matching?(&1, old_suffix))
-    |> adjust_range_for_exacts(old_suffix)
+    |> Enum.filter(&entry_matching?(&1, to_be_renamed))
+    |> adjust_range_for_exacts(to_be_renamed)
   end
 
-  defp descendants(module, old_suffix) do
+  defp descendants(module, to_be_renamed) do
     module
     |> query_for_descendants()
-    |> Enum.filter(&(entry_matching?(&1, old_suffix) and has_dots_in_range?(&1)))
-    |> adjust_range_for_descendants(module, old_suffix)
+    |> Enum.filter(&(entry_matching?(&1, to_be_renamed) and has_dots_in_range?(&1)))
+    |> adjust_range_for_descendants(module, to_be_renamed)
   end
 
   defp query_for_exacts(module) do
     module_string = Formats.module(module)
 
     case Store.exact(module_string, type: :module) do
-      {:ok, entries} -> entries
+      {:ok, entries} -> Enum.map(entries, &Entry.new/1)
       {:error, _} -> []
     end
   end
@@ -133,43 +120,43 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.Module do
     prefix = "#{module_string}."
 
     case Store.prefix(prefix, type: :module) do
-      {:ok, entries} -> entries
+      {:ok, entries} -> Enum.map(entries, &Entry.new/1)
       {:error, _} -> []
     end
   end
 
-  defp maybe_rename_file(document, entries, new_suffix) do
+  defp maybe_rename_file(document, entries, replacement) do
     entries
-    |> Enum.map(&Rename.File.maybe_rename(document, &1, new_suffix))
+    |> Enum.map(&Rename.File.maybe_rename(document, &1, replacement))
     # every group should have only one `rename_file`
     |> Enum.find(&(not is_nil(&1)))
   end
 
-  defp entry_matching?(entry, old_suffix) do
-    entry.range |> range_text() |> String.contains?(old_suffix)
+  defp entry_matching?(entry, to_be_renamed) do
+    entry.range |> range_text() |> String.contains?(to_be_renamed)
   end
 
   defp has_dots_in_range?(entry) do
-    entry.range |> range_text() |> String.contains?(".")
+    entry.edit_range |> range_text() |> String.contains?(".")
   end
 
-  defp adjust_range_for_exacts(entries, old_suffix) do
-    old_suffix_length = String.length(old_suffix)
+  defp adjust_range_for_exacts(entries, to_be_renamed) do
+    old_suffix_length = String.length(to_be_renamed)
 
-    for entry <- entries do
-      start_character = entry.range.end.character - old_suffix_length
-      put_in(entry.range.start.character, start_character)
+    for %Entry{} = entry <- entries do
+      start_character = entry.edit_range.end.character - old_suffix_length
+      put_in(entry.edit_range.start.character, start_character)
     end
   end
 
-  defp adjust_range_for_descendants(entries, module, old_suffix) do
-    for entry <- entries,
-        range_text = range_text(entry.range),
-        matches = matches(range_text, old_suffix),
+  defp adjust_range_for_descendants(entries, module, to_be_renamed) do
+    for %Entry{} = entry <- entries,
+        range_text = range_text(entry.edit_range),
+        matches = matches(range_text, to_be_renamed),
         result = resolve_module_range(entry, module, matches),
         match?({:ok, _}, result) do
       {_, range} = result
-      %{entry | range: range}
+      %{entry | edit_range: range}
     end
   end
 
@@ -183,7 +170,7 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.Module do
   end
 
   defp resolve_module_range(entry, module, [[{start, length}]]) do
-    range = adjust_range_characters(entry.range, {start, length})
+    range = adjust_range_characters(entry.edit_range, {start, length})
 
     with {:ok, {:module, ^module}, _} <- resolve(entry.path, range.start) do
       {:ok, range}
@@ -195,8 +182,8 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.Module do
     # For example, if we have a module named `Foo.Bar.Foo.Bar` and we want to rename it to `Foo.Bar.Baz`
     # The `Foo.Bar` will be duplicated in the range text, so we need to resolve the correct range
     # and only rename the second occurrence of `Foo.Bar`
-    start_character = entry.range.start.character + start
-    position = %{entry.range.start | character: start_character}
+    start_character = entry.edit_range.start.character + start
+    position = %{entry.edit_range.start | character: start_character}
 
     with {:ok, {:module, result}, range} <- resolve(entry.path, position) do
       if result == module do
@@ -209,15 +196,15 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.Module do
   end
 
   defp matches(range_text, "") do
-    # When expanding a module, the old_suffix is an empty string,
+    # When expanding a module, the to_be_renamed is an empty string,
     # so we need to scan the module before the period
     for [{start, length}] <- Regex.scan(~r/\w+(?=\.)/, range_text, return: :index) do
       [{start + length, 0}]
     end
   end
 
-  defp matches(range_text, old_suffix) do
-    Regex.scan(~r/#{old_suffix}/, range_text, return: :index)
+  defp matches(range_text, to_be_renamed) do
+    Regex.scan(~r/#{to_be_renamed}/, range_text, return: :index)
   end
 
   defp adjust_range_characters(%Range{} = range, {start, length} = _matched_old_suffix) do
@@ -229,12 +216,29 @@ defmodule Lexical.RemoteControl.CodeMod.Rename.Module do
     |> put_in([:end, :character], end_character)
   end
 
-  defp to_document_changes(uri, entries, new_suffix) do
-    edits = Enum.map(entries, &Edit.new(new_suffix, &1.range))
-
+  defp to_document_changes(uri, entries, replacement) do
     with {:ok, document} <- Document.Store.open_temporary(uri) do
-      rename_file = maybe_rename_file(document, entries, new_suffix)
+      rename_file = maybe_rename_file(document, entries, replacement)
+
+      edits =
+        Enum.map(entries, fn entry ->
+          reference? = entry.subtype == :reference
+
+          if reference? and not ancestor_is_alias?(document, entry.edit_range.start) do
+            replacement = replacement |> String.split(".") |> Enum.at(-1)
+            Edit.new(replacement, entry.edit_range)
+          else
+            Edit.new(replacement, entry.edit_range)
+          end
+        end)
+
       {:ok, Document.Changes.new(document, edits, rename_file)}
     end
   end
+
+  defp ancestor_is_alias?(%Document{} = document, %Position{} = position) do
+    document
+    |> Ast.cursor_path(position)
+    |> Enum.any?(&match?({:alias, _, _}, &1))
+  end
 end

apps/remote_control/lib/lexical/remote_control/code_mod/rename/module/diff.ex

@@ -0,0 +1,27 @@
+defmodule Lexical.RemoteControl.CodeMod.Rename.Module.Diff do
+  def diff(old_name, new_name) do
+    with [{:eq, eq} | _] <- String.myers_difference(old_name, new_name),
+         equal_segment <- trim_last_dot_part(eq),
+         true <- not is_nil(equal_segment) do
+      to_be_renamed = replace_leading_eq(old_name, equal_segment)
+      replacement = replace_leading_eq(new_name, equal_segment)
+      {to_be_renamed, replacement}
+    else
+      _ ->
+        {old_name, new_name}
+    end
+  end
+
+  defp trim_last_dot_part(module) do
+    split = module |> String.reverse() |> String.split(".", parts: 2)
+
+    if length(split) == 2 do
+      [_, rest] = split
+      rest |> String.reverse()
+    end
+  end
+
+  defp replace_leading_eq(module, eq) do
+    module |> String.replace(~r"^#{eq}", "") |> String.trim_leading(".")
+  end
+end

apps/remote_control/test/lexical/remote_control/code_mod/rename/module/diff_test.exs

@@ -0,0 +1,27 @@
+defmodule Lexical.RemoteControl.CodeMod.Rename.Module.DiffTest do
+  alias Lexical.RemoteControl.CodeMod.Rename.Module.Diff
+
+  use ExUnit.Case, async: true
+
+  describe "diff/2" do
+    test "returns the local module pair if only the local name is changed" do
+      assert Diff.diff("A.B.C", "A.B.D") == {"C", "D"}
+    end
+
+    test "returns the local module pair even if the part of local name is changed" do
+      assert Diff.diff("A.B.CD", "A.B.CC") == {"CD", "CC"}
+    end
+
+    test "returns the suffix when extending the middle part" do
+      assert Diff.diff("Foo.Bar", "Foo.Baz.Bar") == {"Bar", "Baz.Bar"}
+    end
+
+    test "returns the suffix if the middle part is removed" do
+      assert Diff.diff("Foo.Baz.Bar", "Foo.Bar") == {"Baz.Bar", "Bar"}
+    end
+
+    test "returns the entire module pair if the change starts from the first module" do
+      assert Diff.diff("Foo.Bar", "Foa.Bar") == {"Foo.Bar", "Foa.Bar"}
+    end
+  end
+end