fix: correctly order aliases (#322)

Lines were being mangled due to deletes being incorrectly applied after
inserts, and newlines being messed up.

Author: doorgan

apps/engine/lib/engine/code_mod/directives.ex

@@ -54,7 +54,7 @@ defmodule Engine.CodeMod.Directives do
       |> String.trim_trailing()
 
     zeroed = put_in(insert_position.character, 1)
-    new_range = Range.new(zeroed, zeroed)
+    insert_range = Range.new(zeroed, zeroed)
 
     block_text =
       if is_binary(trailer) do
@@ -63,41 +63,39 @@ defmodule Engine.CodeMod.Directives do
         block_text
       end
 
-    edits = remove_old_directives(all_ranges)
+    delete_edits = remove_old_directives(all_ranges)
 
-    edits ++ [Edit.new(block_text, new_range)]
+    case delete_edits do
+      [] ->
+        insert_edit = Edit.new(block_text, insert_range)
+        [insert_edit]
+
+      _ ->
+        # When deleting existing directives, the delete edits remove entire lines
+        # including their newlines. So we need to add a trailing newline to the
+        # inserted block to maintain proper line structure.
+        insert_edit = Edit.new(block_text <> "\n", insert_range)
+
+        [insert_edit | delete_edits]
+    end
   end
 
   defp indent(text, spaces) do
     String.duplicate(" ", spaces) <> text
   end
 
   defp remove_old_directives(ranges) do
-    ranges =
-      ranges
-      |> Enum.sort_by(& &1.start.line, :desc)
-      |> Enum.uniq_by(& &1)
-      |> Enum.map(fn %Range{} = range ->
-        orig_range = range
-
-        orig_range
-        |> put_in([:start, :character], 1)
-        |> update_in([:end], fn %Position{} = pos ->
-          %Position{pos | character: 1, line: pos.line + 1}
-        end)
-      end)
-
-    first_index = length(ranges) - 1
-
     ranges
-    |> Enum.with_index()
-    |> Enum.map(fn
-      {range, ^first_index} ->
-        Edit.new("\n", range)
-
-      {range, _} ->
-        Edit.new("", range)
+    |> Enum.sort_by(& &1.start.line, :desc)
+    |> Enum.uniq_by(& &1)
+    |> Enum.map(fn %Range{} = range ->
+      range
+      |> put_in([:start, :character], 1)
+      |> update_in([:end], fn %Position{} = pos ->
+        %Position{pos | character: 1, line: pos.line + 1}
+      end)
     end)
+    |> Enum.map(&Edit.new("", &1))
     |> merge_adjacent_edits()
   end
 
@@ -106,12 +104,12 @@ defmodule Engine.CodeMod.Directives do
 
   defp merge_adjacent_edits([edit | rest]) do
     rest
-    |> Enum.reduce([edit], fn %Edit{} = current, [%Edit{} = last | rest] = edits ->
+    |> Enum.reduce([edit], fn %Edit{} = current, [%Edit{} = last | acc_rest] = edits ->
       with {same_text, same_text} <- {last.text, current.text},
            {same, same} <- {to_tuple(current.range.end), to_tuple(last.range.start)} do
         collapsed = put_in(current.range.end, last.range.end)
 
-        [collapsed | rest]
+        [collapsed | acc_rest]
       else
         _ ->
           [current | edits]

apps/engine/test/engine/code_action/handlers/organize_aliases_test.exs

@@ -132,6 +132,29 @@ defmodule Engine.CodeAction.Handlers.OrganizeAliasesTest do
       assert expected == organized
     end
 
+    test "aliases are sorted alphabetically" do
+      {:ok, organized} =
+        ~q[
+          defmodule SortAliases do
+            alias A|
+            alias C
+            alias D
+            alias B
+          end
+        ]
+        |> organize_aliases()
+
+      expected = ~q[
+      defmodule SortAliases do
+        alias A
+        alias B
+        alias C
+        alias D
+      end
+    ]t
+      assert expected == organized
+    end
+
     test "aliases are removed duplicate aliases" do
       {:ok, organized} =
         ~q[

apps/engine/test/engine/code_mod/directives_test.exs

@@ -92,11 +92,11 @@ defmodule Engine.CodeMod.DirectivesTest do
           range: &range_of/1
         )
 
-      # expect old ranges removed (replaced/emptied) and new block inserted
-      assert length(edits) == 3
-      assert Enum.any?(edits, &(&1.text == "\n"))
-      assert Enum.any?(edits, &String.contains?(&1.text, "decl a"))
-      assert Enum.any?(edits, &String.contains?(&1.text, "decl b"))
+      assert length(edits) >= 2
+      [insert_edit | delete_edits] = edits
+      assert String.contains?(insert_edit.text, "decl a")
+      assert String.contains?(insert_edit.text, "decl b")
+      assert Enum.all?(delete_edits, &(&1.text == ""))
     end
   end
 end

apps/forge/lib/test/code_mod_case.ex

@@ -43,7 +43,10 @@ defmodule Forge.Test.CodeMod.Case do
   def apply_edits(original, text_edits, opts) do
     document = Document.new("file:///file.ex", original, 0)
     utf8_edits = Enum.map(text_edits, &convert_edit_utf16_to_utf8(document, &1))
-    {:ok, edited_document} = Document.apply_content_changes(document, 1, utf8_edits)
+
+    sorted_edits = sort_edits_for_application(utf8_edits)
+
+    {:ok, edited_document} = Document.apply_content_changes(document, 1, sorted_edits)
     edited_document = Document.to_string(edited_document)
 
     if Keyword.get(opts, :trim, true) do
@@ -53,6 +56,21 @@ defmodule Forge.Test.CodeMod.Case do
     end
   end
 
+  defp sort_edits_for_application(edits) do
+    edits
+    |> Enum.with_index()
+    |> Enum.sort_by(fn {edit, original_index} ->
+      case edit.range do
+        nil ->
+          {0, 0, 0, original_index}
+
+        range ->
+          {-range.end.line, -range.end.character, -range.start.line, original_index}
+      end
+    end)
+    |> Enum.map(fn {edit, _index} -> edit end)
+  end
+
   defp convert_edit_utf16_to_utf8(document, %Document.Edit{} = edit) do
     case edit.range do
       nil ->