refactor(changer): Use Traces

Refactor Changer code and update tests using new compiled artifacts.
ckochx · Aug 20, 2022 · 35817d3 · 35817d3
1 parent 4eb61df
commit 35817d3
Show file tree

Hide file tree

Showing 9 changed files with 578 additions and 980 deletions.
diff --git a/README.md b/README.md
@@ -26,20 +26,21 @@ Use at your peril, _for now._
   - [X] Add and configure CHANGELOG tracking.
   - [X] Support opt-out of format-ing
   - [X] Option to only change the module name throughout the project
+  - [X] update code to rely on compilation tracers, instead of XREF
+  - [X] With module-only option, ensure we remove changed aliases
 
 ## Roadmap TODO
 
-  - [] With module-only option, ensure we remove changed aliases
+  - [] Add test for one file containing more than one `defmodule`
+  - [] Add test for nested defmodules.
+  - [] update test file refs by CLI option
   - [] find dead functions
   - [] find module attrs and also move them?
   - [] find types referenced in the moved specs
   - [] find private functions references in refactored fn bodies.
   - [] git stage all changes?
-  - [] Add test for one file containing more than one `defmodule`
-  - [] Add test for nested defmodules.
   - [] How does this work with macro code? Does that even make sense as a case to handle?
   - [] Update .exs files too?
-  - [] update test file refs by CLI option
   - [] Write tests to ensure we can find modules across umbrella apps.
   - [] Add configuration hooks?
   - [] ElixirLS integration for VSCode?

diff --git a/lib/ex_factor/changer.ex b/lib/ex_factor/changer.ex
@@ -7,18 +7,6 @@ defmodule ExFactor.Changer do
   alias ExFactor.Callers
   alias ExFactor.Parser
 
-  # @doc """
-  # Given all the Callers of a module, find the instances of usage of the module and refactor the
-  # module reference to the new module. Respect any existing aliases.
-  # """
-  # def rename_module(opts) do
-  #   source_module = Keyword.fetch!(opts, :source_module)
-
-  #   source_module
-  #   |> Callers.callers()
-  #   |> update_caller_groups(opts)
-  # end
-
   @doc """
   Given all the Callers to a module, find the instances of the target function and refactor the
   function module reference to the new module. Respect any existing aliases.
@@ -45,29 +33,10 @@ defmodule ExFactor.Changer do
     ]
   end
 
-  # defp update_caller_groups(callers, opts) do
-  #   dry_run = Keyword.get(opts, :dry_run, false)
-
-  #   Enum.map(callers, fn {file, [first | _] = grouped_callers} ->
-  #     file_list =
-  #       File.read!(file)
-  #       |> String.split("\n")
-
-  #     grouped_callers
-  #     |> Enum.reduce({[:unchanged], file_list}, fn %{line: line}, acc ->
-  #       find_and_replace(acc, opts, line)
-  #     end)
-  #     |> maybe_add_import(opts)
-  #     |> maybe_add_alias(opts)
-  #     |> write_file(first.caller_module, file, dry_run)
-  #   end)
-  # end
-
   defp update_caller_groups(callers, opts) do
     dry_run = Keyword.get(opts, :dry_run, false)
     source_module = Keyword.fetch!(opts, :source_module)
     source_function = Keyword.get(opts, :source_function)
-    |> IO.inspect(label: "source_function")
     mod = Callers.cast(source_module)
 
     case source_function do
@@ -79,36 +48,26 @@ defmodule ExFactor.Changer do
           end)
         end)
       fun ->
-        # fun |> IO.inspect(label: "function")
-        # callers |> IO.inspect(label: "callers")
         fun_atom = Callers.cast(fun)
         Enum.filter(callers, fn {{file_path, module}, fn_calls} ->
           Enum.find(fn_calls, fn
           {_, _, _, ^mod, ^fun_atom, _} -> true
           _ -> false
           end)
         end)
-        # arity = Keyword.get(opts, :arity)
-        # |> IO.inspect(label: "arity")
-        # matching_functions = Enum.filter(fn_calls, fn
-          # {_, _, _, ^mod, ^fun_atom, _} -> true
-          # _ -> false
     end
-    |> IO.inspect(label: "before mapping something?")
     |> Enum.map(fn {{file_path, module}, fn_calls} ->
       file_list =
         file_path
         |> File.read!()
         |> String.split("\n")
-      # |> IO.inspect(label: "matching functions")
       Enum.reduce(fn_calls, {[:unchanged], file_list}, fn
-        {_, line, _, ^mod, _, _}, acc ->
+        {_, line, _, ^mod, _, _} = fn_call, acc ->
           find_and_replace(acc, opts, line)
         _, acc -> acc
       end)
-      # |> IO.inspect(label: "FUNS")
-      |> maybe_add_import(opts)
       |> maybe_add_alias(opts)
+      |> maybe_add_import(opts)
       |> write_file(module, file_path, dry_run)
     end)
   end
@@ -119,78 +78,8 @@ defmodule ExFactor.Changer do
       nil -> update_module(tuple, opts, line)
       _source_function -> update_module_and_function(tuple, opts, line)
     end
-    # source_module = Keyword.fetch!(opts, :source_module)
-    # target_module = Keyword.fetch!(opts, :target_module)
-
-    # # modified values
-    # source_string = to_string(source_module)
-    # source_modules = String.split(source_module, ".")
-    # source_alias = Enum.at(source_modules, -1)
-    # target_alias = preferred_alias(file_list, target_module)
-    # source_alias_alt = find_alias_as(file_list, source_module)
-    # fn_line = Enum.at(file_list, line - 1)
-
-    # {new_state, new_line} =
-    #   cond do
-    #     # match full module name
-    #     String.match?(fn_line, ~r/#{source_string}\.#{source_function}/) ->
-    #       fn_line = String.replace(fn_line, source_module, target_alias)
-    #       {set_state(state, :changed), fn_line}
-
-    #     # match aliased module name
-    #     String.match?(fn_line, ~r/#{source_alias}\.#{source_function}/) ->
-    #       fn_line = String.replace(fn_line, source_alias, target_alias)
-    #       {set_state(state, :changed), fn_line}
-
-    #     # match module name aliased :as
-    #     String.match?(fn_line, ~r/#{source_alias_alt}\.#{source_function}/) ->
-    #       fn_line = String.replace(fn_line, source_alias_alt, target_alias)
-    #       {set_state(state, :changed), fn_line}
-
-    #     true ->
-    #       {state, fn_line}
-    #   end
-
-    # {new_state, List.replace_at(file_list, line - 1, new_line)}
   end
 
-  # defp find_and_replace_module({state, file_list}, opts, line) do
-  #   # opts values
-  #   source_module = Keyword.fetch!(opts, :source_module)
-  #   target_module = Keyword.fetch!(opts, :target_module)
-
-  #   # modified values
-  #   source_string = to_string(source_module)
-  #   source_modules = String.split(source_module, ".")
-  #   source_alias = Enum.at(source_modules, -1)
-  #   target_alias = preferred_alias(file_list, target_module)
-  #   source_alias_alt = find_alias_as(file_list, source_module)
-  #   fn_line = Enum.at(file_list, line - 1)
-
-  #   {new_state, new_line} =
-  #     cond do
-  #       # match full module name
-  #       String.match?(fn_line, ~r/#{source_string}/) ->
-  #         fn_line = String.replace(fn_line, source_module, target_alias)
-  #         {set_state(state, :changed), fn_line}
-
-  #       # match aliased module name
-  #       String.match?(fn_line, ~r/#{source_alias}/) ->
-  #         fn_line = String.replace(fn_line, source_alias, target_alias)
-  #         {set_state(state, :changed), fn_line}
-
-  #       # match module name aliased :as
-  #       String.match?(fn_line, ~r/#{source_alias_alt}/) ->
-  #         fn_line = String.replace(fn_line, source_alias_alt, target_alias)
-  #         {set_state(state, :changed), fn_line}
-
-  #       true ->
-  #         {state, fn_line}
-  #     end
-
-  #   {new_state, List.replace_at(file_list, line - 1, new_line)}
-  # end
-
   defp update_module_and_function({state, file_list}, opts, line) do
     # opts values
     source_module = Keyword.fetch!(opts, :source_module)
@@ -244,6 +133,10 @@ defmodule ExFactor.Changer do
 
     {new_state, new_line} =
       cond do
+        # already changed
+        String.match?(fn_line, ~r/#{target_alias}/) ->
+          {state, fn_line}
+
         # match full module name
         String.match?(fn_line, ~r/#{source_string}/) ->
           fn_line = String.replace(fn_line, source_module, target_alias)
@@ -254,11 +147,6 @@ defmodule ExFactor.Changer do
           fn_line = String.replace(fn_line, source_alias, target_alias)
           {set_state(state, :changed), fn_line}
 
-        # match module name aliased :as
-        String.match?(fn_line, ~r/#{source_alias_alt}/) ->
-          fn_line = String.replace(fn_line, source_alias_alt, target_alias)
-          {set_state(state, :changed), fn_line}
-
         true ->
           {state, fn_line}
       end
@@ -267,10 +155,10 @@ defmodule ExFactor.Changer do
   end
 
   defp find_alias_as(list, module) do
-    aalias = Enum.find(list, "", fn el -> str_match?(el, module) end)
+    alias_as = Enum.find(list, "", fn el -> str_match?(el, module) end)
 
-    if String.match?(aalias, ~r/, as: /) do
-      aalias
+    if String.match?(alias_as, ~r/, as: /) do
+      alias_as
       |> String.split("as:", trim: true)
       |> Enum.at(-1)
     else
@@ -316,8 +204,8 @@ defmodule ExFactor.Changer do
   defp list_to_string(contents_list) do
     Enum.join(contents_list, "\n")
   end
-
   defp maybe_add_alias({[:unchanged], _} = resp, _), do: resp
+
   defp maybe_add_alias({state, contents_list}, opts) do
     target_module = Keyword.fetch!(opts, :target_module)
     target_string = to_string(target_module)
@@ -358,10 +246,10 @@ defmodule ExFactor.Changer do
         {state, contents_list}
 
       :changed in state ->
-        index =
+        alias_index =
           Enum.find_index(contents_list, fn el -> str_match?(el, target_string, "alias") end)
 
-        index = index || 2
+        index = alias_index || 2
         contents_list = List.insert_at(contents_list, index - 1, "alias #{target_string}")
         {set_state(state, :alias_added), contents_list}
 
@@ -374,7 +262,7 @@ defmodule ExFactor.Changer do
   defp maybe_add_import({state, contents_list}, opts) do
     source_module = Keyword.fetch!(opts, :source_module)
     target_module = Keyword.fetch!(opts, :target_module)
-    target_string = to_string(target_module)
+    target_alias = preferred_alias([], target_module)
     source_modules = String.split(source_module, ".")
     source_alias = Enum.at(source_modules, -1)
     source_alias_alt = find_alias_as(contents_list, source_module)
@@ -384,10 +272,8 @@ defmodule ExFactor.Changer do
       Enum.find_index(contents_list, fn el -> str_match?(el, source_alias, "import") end) ||
         Enum.find_index(contents_list, fn el -> str_match?(el, source_alias_alt, "import") end)
 
-    new_state = set_state(state, :import_added)
-
     if index do
-      {new_state, List.insert_at(contents_list, index + 1, "import #{target_string}")}
+      {set_state(state, :import_added), List.insert_at(contents_list, index + 1, "import #{target_alias}")}
     else
       {state, contents_list}
     end

diff --git a/lib/ex_factor/neighbors.ex b/lib/ex_factor/neighbors.ex
@@ -2,6 +2,7 @@ defmodule ExFactor.Neighbors do
   @moduledoc """
   Documentation for `ExFactor.Neighbors`.
   """
+  @excluded_neighbors ~w(alias use import require)a
 
   @doc """
   Walk the AST and find all the elements before the target function and the previous function.
@@ -41,7 +42,7 @@ defmodule ExFactor.Neighbors do
     {[], acc}
   end
 
-  defp eval_elem({type, _, _}, {pending, acc}, _name, _artiy) when type in [:alias] do
+  defp eval_elem({type, _, _}, {pending, acc}, _name, _artiy) when type in @excluded_neighbors do
     {pending, acc}
   end
 

diff --git a/lib/ex_factor/traces.ex b/lib/ex_factor/traces.ex
@@ -1,8 +1,8 @@
 defmodule ExFactor.Traces do
   def setup do
     Code.compiler_options(parser_options: [columns: true])
-    ExFactor.Tracer = Code.ensure_loaded!(ExFactor.Tracer)
     ExFactor.Server = Code.ensure_loaded!(ExFactor.Server)
+    ExFactor.Tracer = Code.ensure_loaded!(ExFactor.Tracer)
 
     _ = ExFactor.Server.start_link(__MODULE__)
   end