Add dry run option and tests

lib/ex_factor/extractor.ex

@@ -4,7 +4,6 @@ defmodule ExFactor.Extractor do
   """
   alias ExFactor.Neighbors
   alias ExFactor.Parser
-  alias ExFactor.Remover
 
   @doc """
   Given a keyword list of opts, find the function in the specified source.
@@ -29,51 +28,52 @@ defmodule ExFactor.Extractor do
     arity = Keyword.fetch!(opts, :arity)
     target_path = Keyword.get(opts, :target_path, path(target_module))
     source_path = Keyword.get(opts, :source_path, path(source_module))
+    dry_run = Keyword.get(opts, :dry_run, false)
     {_ast, block_contents} = Parser.block_contents(source_path)
-    to_extract = Neighbors.walk(block_contents, source_function, arity)
+    to_extract = block_contents
+    |> Neighbors.walk(source_function, arity)
+    |> Enum.map(&(Macro.to_string(&1)))
+    |> IO.inspect(label: "to string")
+
+    string_fns = Enum.join(to_extract, "\n")
 
     case File.exists?(target_path) do
       true ->
         {ast, list} = Parser.read_file(target_path)
         {:defmodule, [do: [line: _begin_line], end: [line: end_line], line: _], _} = ast
-        string_fns = Macro.to_string(to_extract)
+        # string_fns = Macro.to_string(to_extract)
+        # string_fns = Enum.join(to_extract, "\n")
 
         list
         |> List.insert_at(end_line - 1, refactor_message())
         |> List.insert_at(end_line, string_fns)
         |> Enum.join("\n")
-        |> then(fn contents -> File.write(target_path, contents, [:write]) end)
+        |> then(fn contents -> write_file(target_path, contents, dry_run) end)
 
       _ ->
-        content =
+        contents =
           quote generated: true do
             defmodule unquote(target_module) do
               @moduledoc false
-              unquote(to_extract)
+              unquote(Macro.unescape_string(string_fns))
             end
           end
           |> Macro.to_string()
 
-        File.write(target_path, content)
+        write_file(target_path, contents, dry_run)
     end
   end
 
-  @doc """
-  Remove the indicated function and its spec from it's original file.
-  """
-  def remove(opts) do
-    source_module = Keyword.fetch!(opts, :source_module)
-    source_function = Keyword.fetch!(opts, :source_function)
-    arity = Keyword.fetch!(opts, :arity)
-    source_path = Keyword.get(opts, :source_path, path(source_module))
-    |> IO.inspect(label: "REMOVE source_path")
-
-    Remover.remove(source_path, source_function, arity)
-  end
-
   defp path(module) do
     Path.join(["lib", Macro.underscore(module) <> ".ex"])
   end
 
   defp refactor_message, do: "#refactored function moved with ExFactor"
+
+  defp write_file(_target_path, contents, true) do
+    contents
+  end
+  defp write_file(target_path, contents, _) do
+    File.write(target_path, contents, [:write])
+  end
 end

lib/ex_factor/remover.ex

@@ -5,9 +5,19 @@ defmodule ExFactor.Remover do
 
   alias ExFactor.Parser
 
-  def remove(source_path, fn_name, arity) do
+  @doc """
+  Remove the indicated function and its spec from it's original file.
+  """
+  def remove(opts) do
+    source_function = Keyword.fetch!(opts, :source_function)
+    arity = Keyword.fetch!(opts, :arity)
+    source_path = Keyword.fetch!(opts, :source_path)
+    dry_run = Keyword.get(opts, :dry_run, false)
+    # |> IO.inspect(label: "REMOVE source_path")
+
+
     {_ast, block_contents} = Parser.all_functions(source_path)
-    fns_to_remove = Enum.filter(block_contents, &(&1.name == fn_name))
+    fns_to_remove = Enum.filter(block_contents, &(&1.name == source_function))
     {_ast, line_list} = Parser.read_file(source_path)
 
     Enum.reduce(fns_to_remove, line_list, fn function, acc ->
@@ -20,10 +30,10 @@ defmodule ExFactor.Remover do
       |> Enum.reduce(acc, fn idx, acc ->
         List.delete_at(acc, idx - 1)
       end)
-      |> List.insert_at(function.start_line, comment(fn_name, arity, function.defn))
+      |> List.insert_at(function.start_line, comment(source_function, arity, function.defn))
     end)
     |> Enum.join("\n")
-    |> then(fn str -> File.write(source_path, str, [:write]) end)
+    |> then(fn str -> write_file(source_path, str, dry_run) end)
   end
 
   defp comment(name, arity, "@spec") do
@@ -42,4 +52,12 @@ defmodule ExFactor.Remover do
     #
     """
   end
+
+  defp write_file(_path, contents, true) do
+    contents
+  end
+
+  defp write_file(path, contents, _) do
+    File.write(path, contents, [:write])
+  end
 end

test/ex_factor/extractor_test.exs

@@ -2,17 +2,23 @@ defmodule ExFactor.ExtractorTest do
   use ExUnit.Case
   alias ExFactor.Extractor
 
+  setup_all do
+    File.mkdir_p("test/tmp")
+
+    on_exit(fn ->
+      File.rm_rf("test/tmp")
+    end)
+  end
+
   describe "emplace/1" do
     test "requires some options" do
       opts = [
-        # target_module: ExFactor.NewMod,
         source_module: ExFactorSampleModule,
         source_function: :pub1,
         arity: 1
       ]
 
       assert_raise KeyError, "key :target_module not found in: #{inspect(opts)}", fn -> Extractor.emplace(opts) end
-      # assert message == ""
     end
 
     test "write a new file with the function" do
@@ -28,16 +34,16 @@ defmodule ExFactor.ExtractorTest do
       end
       """
 
-      File.write("test/support/source_module.ex", content)
+      File.write("test/tmp/source_module.ex", content)
 
-      target_path = "test/support/target_module.ex"
+      target_path = "test/tmp/target_module.ex"
       File.rm(target_path)
 
       opts = [
         target_path: target_path,
         target_module: ExFactor.NewMod,
         source_module: ExFactorSampleModule,
-        source_path: "test/support/source_module.ex",
+        source_path: "test/tmp/source_module.ex",
         source_function: :pub1,
         arity: 1
       ]
@@ -50,10 +56,45 @@ defmodule ExFactor.ExtractorTest do
       # includes additional attrs
       assert file =~ "@spec(pub1(term()) :: term())"
       assert file =~ "@somedoc(\"This is somedoc\")"
+      # assert the added elements get flattened correctly
+      refute file =~ "[@somedoc(\"This is somedoc\"), "
       # comments don't get moved
       refute file =~ "# a comment and no aliases"
-      File.rm("test/support/source_module.ex")
-      File.rm("test/support/target_module.ex")
+      File.rm("test/tmp/source_module.ex")
+      File.rm("test/tmp/target_module.ex")
+    end
+
+    test " with dry_run option, don't write the file." do
+      content = """
+      defmodule ExFactorSampleModule do
+        @somedoc "This is somedoc"
+        # a comment and no aliases
+        _docp = "here's an arbitrary module underscore"
+        @spec pub1(term()) :: term()
+        def pub1(arg1) do
+          :ok
+        end
+      end
+      """
+
+      File.write("test/tmp/source_module.ex", content)
+
+      target_path = "test/tmp/target_module.ex"
+      File.rm(target_path)
+
+      opts = [
+        target_path: target_path,
+        target_module: ExFactor.NewMod,
+        source_module: ExFactorSampleModule,
+        source_path: "test/tmp/source_module.ex",
+        source_function: :pub1,
+        arity: 1,
+        dry_run: true
+      ]
+
+      output = Extractor.emplace(opts)
+      assert {:error, :enoent} = File.read(target_path)
+      assert output =~ "defmodule(ExFactor.NewMod) do"
     end
 
     test "write a new file with the function, infer some defaults" do
@@ -101,7 +142,7 @@ defmodule ExFactor.ExtractorTest do
       end
       """
 
-      File.write("lib/ex_factor/source_module.ex", content)
+      File.write("test/tmp/source_module.ex", content)
 
       content = """
       defmodule ExFactor.TargetModule do
@@ -203,6 +244,7 @@ defmodule ExFactor.ExtractorTest do
 
       content = """
       defmodule ExFactor.TargetModule do
+        @moduledoc false
         @somedoc "This is somedoc TargetModule"
         @doc "some docs"
         def pub_exists(arg_exists) do
@@ -226,6 +268,7 @@ defmodule ExFactor.ExtractorTest do
       Extractor.emplace(opts)
 
       file = File.read!("lib/ex_factor/target_module.ex")
+      |> IO.inspect(label: "")
       assert file =~ "def(refactor1(arg1)) do"
       assert file =~ "def(refactor1([])) do"
       assert file =~ " @doc \"some docs\""
@@ -234,37 +277,4 @@ defmodule ExFactor.ExtractorTest do
       File.rm("lib/ex_factor/target_module.ex")
     end
   end
-
-  describe "remove/1" do
-    test "remove the given function from the source module" do
-      content = """
-      defmodule ExFactorSampleModule do
-        @somedoc "This is somedoc"
-        # a comment and no aliases
-        _docp = "here's an arbitrary module underscore"
-        def pub1(arg1) do
-          :ok
-        end
-      end
-      """
-
-      File.write("test/support/source_module.ex", content)
-      source_path = "test/support/source_module.ex"
-
-      opts = [
-        source_module: ExFactorSampleModule,
-        source_path: "test/support/source_module.ex",
-        source_function: :pub1,
-        arity: 1
-      ]
-
-      Extractor.remove(opts)
-
-      file = File.read!(source_path)
-      assert file =~ "defmodule ExFactorSampleModule do"
-      assert file =~ "_docp = \"here's an arbitrary module underscore"
-      refute file =~ "def pub1(arg1) do"
-      File.rm("test/support/source_module.ex")
-    end
-  end
 end

test/ex_factor/parser_test.exs

@@ -439,6 +439,7 @@ defmodule ExFactor.ParserTest do
     test "it reads the file into an AST and list of each line" do
       content = """
       defmodule ExFactorSampleModule do
+        alias ExFactorOtherModule
         @somedoc "This is somedoc"
         # comments get dropped
         _docp = "arbitrary module-level elem"

test/ex_factor/remover_test.exs

@@ -11,6 +11,37 @@ defmodule ExFactor.RemoverTest do
   end
 
   describe "remove/1" do
+    test "remove the given function from the source module" do
+      content = """
+      defmodule ExFactorSampleModule do
+        @somedoc "This is somedoc"
+        # a comment and no aliases
+        _docp = "here's an arbitrary module underscore"
+        def pub1(arg1) do
+          :ok
+        end
+      end
+      """
+
+      File.write("test/tmp/source_module.ex", content)
+      source_path = "test/tmp/source_module.ex"
+
+      opts = [
+        source_module: ExFactorSampleModule,
+        source_path: "test/tmp/source_module.ex",
+        source_function: :pub1,
+        arity: 1
+      ]
+
+      Remover.remove(opts)
+
+      file = File.read!(source_path)
+      assert file =~ "defmodule ExFactorSampleModule do"
+      assert file =~ "_docp = \"here's an arbitrary module underscore"
+      refute file =~ "def pub1(arg1) do"
+      File.rm("test/support/source_module.ex")
+    end
+
     test "it rewrites the source file and removes code blocks" do
       module = """
       defmodule ExFactorSampleModule do
@@ -38,7 +69,13 @@ defmodule ExFactor.RemoverTest do
       """
 
       File.write("test/tmp/source_module.ex", module)
-      Remover.remove("test/tmp/source_module.ex", :pub1, 1)
+      opts = [
+        source_module: ExFactorSampleModule,
+        source_path: "test/tmp/source_module.ex",
+        source_function: :pub1,
+        arity: 1
+      ]
+      Remover.remove(opts)
 
       updated_file = File.read!("test/tmp/source_module.ex")
       refute updated_file =~ "def pub1(arg1) do"
@@ -47,5 +84,52 @@ defmodule ExFactor.RemoverTest do
       # it removes specs too
       refute updated_file =~ "@spec pub1(term()) :: term()"
     end
+
+    test "takes a dry_run option to only report intended changes" do
+      module = """
+      defmodule ExFactorSampleModule do
+        @somedoc "This is somedoc"
+        # comments get dropped
+        @doc "
+        multiline
+        documentation for pub1
+        "
+        @spec pub1(term()) :: term()
+        def pub1(arg1) do
+          :ok
+        end
+        _docp = "arbitrary module-level elem"
+        defp priv1(arg1) do
+          :ok
+        end
+
+        def pub2(arg1)
+          do
+            :ok
+          end
+
+      end
+      """
+
+      File.write("test/tmp/source_module.ex", module)
+      opts = [
+        dry_run: true,
+        source_module: ExFactorSampleModule,
+        source_path: "test/tmp/source_module.ex",
+        source_function: :pub1,
+        arity: 1
+      ]
+      changes = Remover.remove(opts)
+
+      unchanged_file = File.read!("test/tmp/source_module.ex")
+      assert unchanged_file =~ "def pub1(arg1) do"
+      refute unchanged_file =~ "Function: pub1/1 removed by ExFactor"
+      assert unchanged_file =~ "@spec pub1(term()) :: term()"
+
+      assert changes =~ "Function: pub1/1 removed by ExFactor"
+      refute changes =~ "@spec pub1(term()) :: term()"
+      refute changes =~ "def pub1(arg1) do"
+    end
+
   end
 end