WIP ensure consistent returns, add tests for edge cases.

TODO: handle aliases better

lib/ex_factor/changer.ex

@@ -19,7 +19,10 @@ defmodule ExFactor.Changer do
     |> update_callers(opts)
   end
 
-  defp update_callers([], _), do: %ExFactor{}
+  defp update_callers([], opts) do
+    source_module = Keyword.fetch!(opts, :source_module)
+    [%ExFactor{state: [:unchanged], message: "module: #{source_module} not found"}]
+  end
 
   defp update_callers(callers, opts) do
     Enum.map(callers, fn caller ->
@@ -93,20 +96,20 @@ defmodule ExFactor.Changer do
     end
   end
 
-  defp write_file({:unchanged, contents_list}, source_function, target_path, true) do
+  defp write_file({state, contents_list}, _, target_path, true) do
     %ExFactor{
       path: target_path,
-      state: [:unchanged],
-      message: "#{source_function} not found, no changes to make",
+      state: [:dry_run, state],
+      message: "--dry_run changes to make",
       file_contents: list_to_string(contents_list)
     }
   end
 
-  defp write_file({state, contents_list}, _, target_path, true) do
+  defp write_file({:unchanged, contents_list}, source_function, target_path, _dry_run) do
     %ExFactor{
       path: target_path,
-      state: [:dry_run, state],
-      message: "--dry_run changes to make",
+      state: [:unchanged],
+      message: "function: #{source_function} not found, no changes to make",
       file_contents: list_to_string(contents_list)
     }
   end
@@ -137,18 +140,24 @@ defmodule ExFactor.Changer do
     target_module = Keyword.fetch!(opts, :target_module)
     target_string = Util.module_to_string(target_module)
 
+    # when module has not aliases
+    contents_list = if Enum.find(contents_list, fn el -> match_alias?(el, "") end) do
+      contents_list
+    else
+      List.insert_at(contents_list, 1, "alias #{target_string}")
+    end
+
     if Enum.find(contents_list, fn el -> match_alias?(el, target_string) end) do
-      # IO.puts "#{target_string} FOUND THE ALIAS"
       {state, contents_list}
     else
       contents_list
       |> Enum.reduce({:none, []}, fn elem, {prev, acc} ->
         cond do
           match_alias?(elem, source_string) ->
             new_alias = String.replace(elem, source_string, target_string)
-            {:alias, [new_alias | [elem | acc]]}
+            {:alias, [elem | [new_alias | acc]]}
           prev == :alias and not match_alias?(elem, "") ->
-            {:none, [ "alias #{target_string}" | [elem | acc]]}
+            {:none, [elem | ["alias #{target_string}" | acc]]}
           match_alias?(elem, "") ->
             {:alias, [elem | acc]}
           true ->

test/ex_factor/changer_test.exs

@@ -11,10 +11,12 @@ defmodule ExFactor.ChangerTest do
   end
 
   describe "change/1" do
-    test "it finds all the callers of a module, function, and arity, and updates the calls to the new module " do
+    test "it finds all the callers of a module, function, and arity, and updates the calls to the new module" do
       content = """
       defmodule ExFactor.Tmp.SourceMod do
-        @moduledoc "This is moduedoc"
+        @moduledoc "
+        This is a multiline moduedoc
+        "
         @doc "this is some documentation for refactor1/1"
         def refactor1([]) do
           :empty
@@ -29,12 +31,20 @@ defmodule ExFactor.ChangerTest do
 
       content = """
       defmodule ExFactor.Tmp.CallerModule do
+        @moduledoc "
+        This is a multiline moduedoc.
+        Its in the caller module
+        "
         alias ExFactor.Tmp.SourceMod
         alias ExFactor.Tmp.SourceMod.Other
         def pub1(arg_a) do
           SourceMod.refactor1(arg_a)
         end
         def pub2(), do: Other
+
+        def pub3(arg_a) do
+          SourceMod.refactor1(arg_a)
+        end
       end
       """
 
@@ -66,9 +76,12 @@ defmodule ExFactor.ChangerTest do
       # ensure we don't match dumbly
       assert caller =~ "alias ExFactor.Tmp.SourceMod.Other"
       refute caller =~ "alias ExFactor.Tmp.TargetModule.Other"
+      # assert the alias doesn't get spliced into the moduledoc
+      refute caller =~ "Its in the caller module\nalias ExFactor.Tmp.TargetModule\n  \""
       assert caller =~ "TargetModule.refactor1(arg_a)"
       # asser the function uses the alias
       refute caller =~ "ExFactor.Tmp.TargetModule.refactor1(arg_a)"
+      assert caller =~ "def pub3(arg_a) do\n    TargetModule.refactor1(arg_a)"
 
       caller_two = File.read!("lib/ex_factor/tmp/caller_two_module.ex")
       assert caller_two =~ "alias ExFactor.Tmp.TargetModule"
@@ -172,7 +185,7 @@ defmodule ExFactor.ChangerTest do
       caller = File.read!("lib/ex_factor/tmp/caller_module.ex")
 
       caller_list = String.split(caller, "\n")
-      assert caller =~ "alias ExFactor.Tmp.TargetModule"
+      assert caller =~ "alias ExFactor.Tmp.TargetModule, as: TM"
       assert caller =~ "TM.refactor1(arg_a)"
 
       assert 1 ==
@@ -181,6 +194,7 @@ defmodule ExFactor.ChangerTest do
                end)
     end
 
+
     test "it finds all the callers of a module by an alias, function, and arity, and updates the calls to the new module " do
       content = """
       defmodule ExFactor.Tmp.SourceMod do
@@ -217,20 +231,100 @@ defmodule ExFactor.ChangerTest do
       assert caller =~ "TargetModule.refactor1(arg_a)"
     end
 
-    test "matches the arity" do
-    end
+    test "handles no functions found to change, messages correctly" do
+      content = """
+      defmodule ExFactor.Tmp.SourceMod do
+        def refactor1(_arg1, _opt2 \\\\ []) do
+          :ok
+        end
+      end
+      """
 
-    test "changes multiple functions" do
-    end
+      File.write("lib/ex_factor/tmp/source_module.ex", content)
 
-    test "handles no functions found to change, messages correctly" do
+      content = """
+      defmodule ExFactor.Tmp.CallerModule do
+        alias ExFactor.Tmp.SourceMod, as: SM
+        def pub1(_arg_a) do
+          SM
+        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
+      ]
+
+      [change] = Changer.change(opts)
+      assert change.message == "function: refactor1 not found, no changes to make"
+      assert change.state == [:unchanged]
     end
 
     test "handles no modules found to change, messages correctly" do
+      opts = [
+        target_module: "ExFactor.Tmp.TargetMissing",
+        source_module: "ExFactor.Tmp.SourceModMissing",
+        source_function: :refactor1,
+        arity: 1
+      ]
+
+      [change] = Changer.change(opts)
+      assert change.message == "module: ExFactor.Tmp.SourceModMissing not found"
+      assert change.state == [:unchanged]
     end
 
-    # update the annoying alias style: alias Foo.{Bar, Baz, Biz}
-    # find and update when the module is used but not aliased
+    test "updates a mod-fn-arity when the function is not aliased" do
+      content = """
+      defmodule ExFactor.Tmp.SourceMod do
+        @moduledoc "This is moduedoc"
+        @doc "this is some documentation for refactor1/1"
+        def refactor1([]) do
+          :empty
+        end
+        def refactor1(arg1) do
+          {:ok, arg1}
+        end
+      end
+      """
+
+      File.write("lib/ex_factor/tmp/source_module.ex", content)
+
+      content = """
+      defmodule ExFactor.Tmp.CallerModule do
+        def pub1(arg_a) do
+          ExFactor.Tmp.SourceMod.refactor1(arg_a)
+        end
+        def alias2, do: TM
+      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
+      ]
+
+      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 "takes a dry_run argument and doesn't update the files" do
       content = """
@@ -271,5 +365,12 @@ defmodule ExFactor.ChangerTest do
       assert change_map.state == [:dry_run, :changed]
       assert change_map.message == "--dry_run changes to make"
     end
+
+    # functions to fill in
+    test "update the alternate alias style: alias Foo.{Bar, Baz, Biz}" do
+    end
+
+    test "matches the arity" do
+    end
   end
 end