diff --git a/lib/ex_factor/changer.ex b/lib/ex_factor/changer.ex index ace05ac..c8319e7 100644 --- a/lib/ex_factor/changer.ex +++ b/lib/ex_factor/changer.ex @@ -41,11 +41,11 @@ defmodule ExFactor.Changer do |> String.split("\n") callers - |> Enum.reduce({:unchanged, file_list}, fn %{line: line}, acc -> + |> 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) + |> maybe_add_alias(opts) |> write_file(source_function, file, dry_run) end) end @@ -69,17 +69,17 @@ defmodule ExFactor.Changer do # match full module name String.match?(fn_line, ~r/#{source_string}\.#{source_function}/) -> fn_line = String.replace(fn_line, source_module, target_alias) - {:changed, fn_line} + {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) - {:changed, fn_line} + {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) - {:changed, fn_line} + {set_state(state, :changed), fn_line} true -> {state, fn_line} @@ -115,7 +115,7 @@ defmodule ExFactor.Changer do defp write_file({state, contents_list}, _, target_path, true) do %ExFactor{ path: target_path, - state: [:dry_run | [state]], + state: [:dry_run | state], message: "--dry_run changes to make", file_contents: list_to_string(contents_list) } @@ -138,40 +138,47 @@ defmodule ExFactor.Changer do end defp maybe_add_alias({state, contents_list}, opts) do - source_module = Keyword.fetch!(opts, :source_module) - source_string = Util.module_to_string(source_module) target_module = Keyword.fetch!(opts, :target_module) 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 + {state, contents_list} + |> change_alias(target_string) + |> add_alias(target_string) + |> then(fn {state, list} -> {state, maybe_reverse(list)} end) + end - if Enum.find(contents_list, fn el -> str_match?(el, target_string) end) do - {state, contents_list} + defp maybe_reverse([head | _tail] = list) do + if String.match?(head, ~r/defmodule/) do + list else - contents_list - |> Enum.reduce({:none, []}, fn elem, {prev, acc} -> - cond 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 - end) - |> then(fn {state, list} -> {state, Enum.reverse(list)} end) + Enum.reverse(list) + end + end + + defp change_alias({state, contents_list}, target_string) do + elem = Enum.find(contents_list, &(str_match?(&1, target_string, "alias"))) + if elem do + {[:alias_exists | state], contents_list} + else + {state, contents_list} + end + end + + defp add_alias({state, contents_list}, target_string) do + cond do + :alias_changed in state -> + {state, contents_list} + :alias_added in state -> + {state, contents_list} + :alias_exists in state -> + {state, contents_list} + :changed in state -> + index = Enum.find_index(contents_list, fn el -> str_match?(el, target_string, "alias") end) + index = index || 2 + contents_list = List.insert_at(contents_list, index - 1, "alias #{target_string}") + {set_state(state, :alias_added), contents_list} + true -> + {state, contents_list} end end @@ -188,13 +195,7 @@ 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 = - if state == :unchanged do - [:import_added] - else - [:import_added | [state]] - end - + new_state = set_state(state, :import_added) if index do {new_state, List.insert_at(contents_list, index + 1, "import #{target_string}")} else @@ -211,4 +212,7 @@ defmodule ExFactor.Changer do defp str_match?(string, module_string, token) do String.match?(string, ~r/(^|\s)#{token} #{module_string}(\s|$|\,)/) end + + defp set_state([:unchanged], new_state), do: [new_state] + defp set_state(state, new_state), do: [new_state | state] end diff --git a/test/ex_factor/changer_test.exs b/test/ex_factor/changer_test.exs index 1ef561a..839172d 100644 --- a/test/ex_factor/changer_test.exs +++ b/test/ex_factor/changer_test.exs @@ -130,6 +130,51 @@ defmodule ExFactor.ChangerTest do arity: 1 ] + Changer.change(opts) + caller = File.read!("lib/ex_factor/tmp/caller_module.ex") + caller_list = String.split(caller, "\n") + assert caller =~ "alias ExFactor.Tmp.TargetModule" + assert caller =~ "TargetModule.refactor1(arg_a)" + + assert 1 == + Enum.count(caller_list, fn el -> + el =~ "alias ExFactor.Tmp.TargetModule" + end) + end + + test "add alias entry when the target alias is missing" do + content = """ + defmodule ExFactor.Tmp.SourceMod do + def refactor1(arg1) do + {:ok, arg1} + end + end + """ + + File.write("lib/ex_factor/tmp/source_module.ex", content) + + content = """ + defmodule ExFactor.Tmp.CallerModule do + alias ExFactor.Tmp.OtherModule + def pub1(arg_a) do + ExFactor.Tmp.SourceMod.refactor1(arg_a) + end + + def pub2 do + OtherModule + end + end + """ + + File.write("lib/ex_factor/tmp/caller_module.ex", content) + + opts = [ + target_module: "ExFactor.Tmp.TargetModule", + source_module: "ExFactor.Tmp.SourceMod", + source_function: "refactor1", + arity: 1 + ] + Mix.Tasks.Compile.Elixir.run([]) Changer.change(opts) @@ -146,6 +191,7 @@ defmodule ExFactor.ChangerTest do end) end + test "handle alias exists with :as" do content = """ defmodule ExFactor.Tmp.SourceMod do @@ -183,11 +229,8 @@ defmodule ExFactor.ChangerTest do ] Mix.Tasks.Compile.Elixir.run([]) - Changer.change(opts) - caller = File.read!("lib/ex_factor/tmp/caller_module.ex") - caller_list = String.split(caller, "\n") assert caller =~ "alias ExFactor.Tmp.TargetModule, as: TM" assert caller =~ "TM.refactor1(arg_a)" @@ -271,7 +314,7 @@ defmodule ExFactor.ChangerTest do def pub1(arg_a) do ExFactor.Tmp.SourceMod.refactor1(arg_a) end - def alias2, do: TM + def alias2, do: TheOtherModule end """ @@ -330,15 +373,14 @@ defmodule ExFactor.ChangerTest do arity: 1 ] - Mix.Tasks.Compile.Elixir.run([]) - [change_map] = Changer.change(opts) caller = File.read!("lib/ex_factor/tmp/caller_module.ex") refute caller =~ "alias ExFactor.Tmp.TargetModule" refute caller =~ "TargetModule.refactor1(arg_a)" - assert change_map.state == [:dry_run, :changed] + assert Enum.find(change_map.state, fn el -> String.match?(Atom.to_string(el), ~r/alias_/) end) + # change_map.state == [:dry_run, :alias_changed, :changed] assert change_map.message == "--dry_run changes to make" end @@ -372,8 +414,6 @@ defmodule ExFactor.ChangerTest do File.write("lib/ex_factor/tmp/caller_module.ex", content) - # ExFactor.Parser.read_file("lib/ex_factor/tmp/caller_module.ex") - # |> IO.inspect(label: "") opts = [ target_module: "ExFactor.Tmp.TargetModule", source_module: "ExFactor.Tmp.SourceMod", @@ -381,13 +421,9 @@ defmodule ExFactor.ChangerTest do arity: 1 ] - Mix.Tasks.Compile.Elixir.run([]) - [change_map] = Changer.change(opts) - caller = File.read!("lib/ex_factor/tmp/caller_module.ex") - assert caller =~ "alias ExFactor.Tmp.TargetModule" assert caller =~ "import ExFactor.Tmp.TargetModule" assert caller =~ " refactor1(arg_a)" assert change_map.state == [:import_added]