refactor changer to work better and use new callers return

lib/ex_factor/callers.ex

@@ -19,7 +19,6 @@ defmodule ExFactor.Callers do
   end
 
   def callers(mod, func, arity) do
-
     # Code.compiler_options(parser_options: [columns: true])
     # Mix.Task.rerun("compile.elixir", ["--force", "--tracer", "ExFactor.CallerTracer"])
 
@@ -34,9 +33,12 @@ defmodule ExFactor.Callers do
   end
 
   def cast(value) when is_atom(value), do: value
+
   def cast(value) do
     cond do
-      String.downcase(value) == value -> String.to_atom(value)
+      String.downcase(value) == value ->
+        String.to_atom(value)
+
       true ->
         cast_module(value)
     end

lib/ex_factor/changer.ex

@@ -19,91 +19,53 @@ defmodule ExFactor.Changer do
 
     source_module
     |> Callers.callers(source_function, arity)
-    |> update_callers_from3(opts)
+    |> Enum.group_by(& &1.file)
+    |> update_caller_groups(opts)
   end
 
-  # defp update_callers([], opts) do
-  #   source_module = Keyword.fetch!(opts, :source_module)
-  #   [%ExFactor{state: [:unchanged], message: "module: #{source_module} not found"}]
-  # end
+  defp update_caller_groups(empty_map, opts) when empty_map == %{},
+    do: update_caller_groups([], opts)
 
-  # defp update_callers(callers, opts) do
-  #   Enum.map(callers, fn caller ->
-  #     File.read!(caller.filepath)
-  #     |> String.split("\n")
-  #     |> find_and_replace_target(opts, caller.filepath)
-  #   end)
-  # end
-
-  defp update_callers_from3([], opts) do
+  defp update_caller_groups([], opts) do
     source_module = Keyword.fetch!(opts, :source_module)
     [%ExFactor{state: [:unchanged], message: "module: #{source_module} not found"}]
   end
 
-  defp update_callers_from3(callers, opts) do
-    Enum.map(callers, fn caller ->
-      File.read!(caller.file)
-      |> String.split("\n")
-      |> find_and_replace_target(opts, caller.file, caller.line)
+  defp update_caller_groups(callers, opts) do
+    dry_run = Keyword.get(opts, :dry_run, false)
+    source_function = Keyword.fetch!(opts, :source_function)
+
+    Enum.map(callers, fn {file, callers} ->
+      file_list =
+        File.read!(file)
+        |> String.split("\n")
+
+      callers
+      |> Enum.reduce({:unchanged, file_list}, fn %{line: line}, acc ->
+        find_and_replace_function(acc, opts, line)
+      end)
+      |> maybe_add_alias(opts)
+      |> maybe_add_import(opts)
+      |> write_file(source_function, file, dry_run)
     end)
   end
 
-  # defp find_and_replace_target(list, opts, filepath) do
-  #   # opts values
-  #   source_module = Keyword.fetch!(opts, :source_module)
-  #   source_function = Keyword.fetch!(opts, :source_function)
-  #   target_module = Keyword.fetch!(opts, :target_module)
-  #   _arity = Keyword.fetch!(opts, :arity)
-  #   dry_run = Keyword.get(opts, :dry_run, false)
-
-  #   # modified values
-  #   source_string = Util.module_to_string(source_module)
-  #   source_modules = String.split(source_module, ".")
-  #   source_alias = Enum.at(source_modules, -1)
-  #   target_alias = preferred_alias(list, target_module)
-  #   source_alias_alt = find_alias_as(list, source_module)
-
-  #   Enum.reduce(list, {:unchanged, []}, fn elem, {state, acc} ->
-  #     cond do
-  #       # match full module name
-  #       String.match?(elem, ~r/#{source_string}\.#{source_function}/) ->
-  #         elem = String.replace(elem, source_module, target_alias)
-  #         {:changed, [elem | acc]}
-
-  #       # match aliased module name
-  #       String.match?(elem, ~r/#{source_alias}\.#{source_function}/) ->
-  #         elem = String.replace(elem, source_alias, target_alias)
-  #         {:changed, [elem | acc]}
-
-  #       # match module name aliased :as
-  #       String.match?(elem, ~r/#{source_alias_alt}\.#{source_function}/) ->
-  #         elem = String.replace(elem, source_alias_alt, target_alias)
-  #         {:changed, [elem | acc]}
-
-  #       true ->
-  #         {state, [elem | acc]}
-  #     end
-  #   end)
-  #   |> maybe_add_alias(opts)
-  #   |> write_file(source_function, filepath, dry_run)
-  # end
-
-  defp find_and_replace_target(list, opts, filepath, line) do
+  defp find_and_replace_function({state, file_list}, opts, line) do
     # opts values
     source_module = Keyword.fetch!(opts, :source_module)
     source_function = Keyword.fetch!(opts, :source_function)
     target_module = Keyword.fetch!(opts, :target_module)
-    dry_run = Keyword.get(opts, :dry_run, false)
 
     # modified values
     source_string = Util.module_to_string(source_module)
     source_modules = String.split(source_module, ".")
     source_alias = Enum.at(source_modules, -1)
-    target_alias = preferred_alias(list, target_module)
-    source_alias_alt = find_alias_as(list, source_module)
+    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)
 
-    fn_line = Enum.at(list, line - 1)
-      {state, new_line} = cond do
+    {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)
@@ -120,13 +82,10 @@ defmodule ExFactor.Changer do
           {:changed, fn_line}
 
         true ->
-          {:unchanged, fn_line}
+          {state, fn_line}
       end
 
-    {state, List.replace_at(list, line - 1, new_line)}
-    |> maybe_add_alias(opts)
-    |> maybe_add_import(opts)
-    |> write_file(source_function, filepath, dry_run)
+    {new_state, List.replace_at(file_list, line - 1, new_line)}
   end
 
   defp find_alias_as(list, module) do
@@ -175,9 +134,7 @@ defmodule ExFactor.Changer do
   end
 
   defp list_to_string(contents_list) do
-    contents_list
-    |> Enum.reverse()
-    |> Enum.join("\n")
+    Enum.join(contents_list, "\n")
   end
 
   defp maybe_add_alias({state, contents_list}, opts) do
@@ -187,11 +144,12 @@ defmodule ExFactor.Changer do
     target_string = Util.module_to_string(target_module)
 
     # when module has no aliases
-    contents_list = if Enum.find(contents_list, fn el -> str_match?(el, "") end) do
-      contents_list
-    else
-      List.insert_at(contents_list, 1, "alias #{target_string}")
-    end
+    contents_list =
+      if Enum.find(contents_list, fn el -> str_match?(el, "") end) do
+        contents_list
+      else
+        List.insert_at(contents_list, 1, "alias #{target_string}")
+      end
 
     if Enum.find(contents_list, fn el -> str_match?(el, target_string) end) do
       {state, contents_list}
@@ -202,10 +160,13 @@ defmodule ExFactor.Changer do
           str_match?(elem, source_string) ->
             new_alias = String.replace(elem, source_string, target_string)
             {:alias, [elem | [new_alias | acc]]}
+
           prev == :alias and not str_match?(elem, "") ->
             {:alias_added, [elem | ["alias #{target_string}" | acc]]}
+
           str_match?(elem, "") ->
             {state, [elem | acc]}
+
           true ->
             {state, [elem | acc]}
         end
@@ -223,14 +184,17 @@ defmodule ExFactor.Changer do
     source_alias_alt = find_alias_as(contents_list, source_module)
 
     # when module has no imports
-    index = 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)
+    index =
+      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 =
+      if state == :unchanged do
+        [:import_added]
+      else
+        [:import_added | [state]]
+      end
 
-    new_state = if state == :unchanged do
-      [:import_added]
-    else
-      [:import_added | [state]]
-    end
     if index do
       {new_state, List.insert_at(contents_list, index + 1, "import #{target_string}")}
     else
@@ -239,15 +203,12 @@ defmodule ExFactor.Changer do
   end
 
   defp str_match?(string, match, token \\ "alias")
+
   defp str_match?(string, "", token) do
     String.match?(string, ~r/(^|\s)#{token}\s/)
   end
 
   defp str_match?(string, module_string, token) do
     String.match?(string, ~r/(^|\s)#{token} #{module_string}(\s|$|\,)/)
   end
-
-  # defp try_replace(string, "", _), do: string
-  # defp try_replace(string, nil, _), do: string
-  # defp try_replace(string, source, target), do: String.replace(string, source, target)
 end

lib/ex_factor/extractor.ex

@@ -51,7 +51,8 @@ defmodule ExFactor.Extractor do
         |> then(fn contents -> write_file(target_path, contents, target_module, dry_run) end)
 
       _ ->
-        target_mod = Module.concat [target_module]
+        target_mod = Module.concat([target_module])
+
         quote generated: true do
           defmodule unquote(target_mod) do
             @moduledoc false
@@ -79,6 +80,7 @@ defmodule ExFactor.Extractor do
 
   defp write_file(target_path, contents, module, _dry_run) do
     _ = File.write(target_path, contents, [:write])
+
     %{
       module: module,
       path: target_path,

lib/ex_factor/neighbors.ex

@@ -9,6 +9,7 @@ defmodule ExFactor.Neighbors do
   AST elements. Ignore certain elements, such as :alias.
   """
   def walk(block, fn_name, arity \\ :unmatched)
+
   def walk(block, fn_name, arity) when is_binary(fn_name) do
     # fn_name_atom = String.to_atom(fn_name)
     walk(block, String.to_atom(fn_name), arity)

lib/ex_factor/remover.ex

@@ -8,9 +8,11 @@ defmodule ExFactor.Remover do
   Remove the indicated function and its spec from it's original file.
   """
   def remove(opts) do
-    source_function = opts
-    |> Keyword.fetch!(:source_function)
-    |> function_name()
+    source_function =
+      opts
+      |> Keyword.fetch!(:source_function)
+      |> function_name()
+
     source_module = Keyword.get(opts, :source_module)
 
     arity = Keyword.fetch!(opts, :arity)
@@ -66,6 +68,7 @@ defmodule ExFactor.Remover do
 
   defp write_file(path, contents, source_module, _) do
     File.write(path, contents, [:write])
+
     %ExFactor{
       module: source_module,
       path: path,
@@ -80,5 +83,6 @@ defmodule ExFactor.Remover do
   defp function_name(name) when is_binary(name) do
     String.to_atom(name)
   end
+
   defp function_name(name), do: name
 end

test/ex_factor/callers_test.exs

@@ -18,17 +18,21 @@ defmodule ExFactor.CallersTest do
 
   describe "callers/3" do
     test "it should report callers of a module" do
-      [one, _two, _three] = Callers.callers(ExFactor.Parser, :all_functions, 1)
+      [one, _two, _three] =
+        ExFactor.Parser
+        |> Callers.callers(:all_functions, 1)
+        |> Enum.sort_by(& &1.file)
 
       assert one.caller_module == ExFactor.Callers
       assert one.file == "lib/ex_factor/callers.ex"
       assert one.line == 9
     end
 
     test "it converts strings to atoms" do
-      [one, _two, _three] = "ExFactor.Parser"
-      |> Callers.callers("all_functions", 1)
-      |> Enum.sort_by(&(&1.file))
+      [one, _two, _three] =
+        "ExFactor.Parser"
+        |> Callers.callers("all_functions", 1)
+        |> Enum.sort_by(& &1.file)
 
       assert one.caller_module == ExFactor.Callers
       assert one.file == "lib/ex_factor/callers.ex"