From bafa887647e8db347233eccd3e5802fc360060cf Mon Sep 17 00:00:00 2001 From: Christian Koch Date: Wed, 15 Jun 2022 13:09:53 -0500 Subject: [PATCH] Comments and typos --- lib/ex_factor.ex | 2 +- lib/ex_factor/changer.ex | 12 +++- lib/mix/tasks/ex_factor.ex | 97 +++++++++++++++++++++++++++++++-- test/ex_factor/changer_test.exs | 7 ++- test/support/support.ex | 2 +- 5 files changed, 109 insertions(+), 11 deletions(-) diff --git a/lib/ex_factor.ex b/lib/ex_factor.ex index 7320691..ae87b1a 100644 --- a/lib/ex_factor.ex +++ b/lib/ex_factor.ex @@ -5,7 +5,7 @@ defmodule ExFactor do By identifying a Module, function name, and arity, it will identify all non-test usages and extract them to a new Module. - If the Module exists, it add the function to the end of the file and change all calls to the + If the Module exists, it adds the function to the end of the file and change all calls to the new module's name. """ diff --git a/lib/ex_factor/changer.ex b/lib/ex_factor/changer.ex index 9e1821d..961eeea 100644 --- a/lib/ex_factor/changer.ex +++ b/lib/ex_factor/changer.ex @@ -157,7 +157,8 @@ defmodule ExFactor.Changer do end defp change_alias({state, contents_list}, target_string) do - elem = Enum.find(contents_list, &(str_match?(&1, target_string, "alias"))) + elem = Enum.find(contents_list, &str_match?(&1, target_string, "alias")) + if elem do {[:alias_exists | state], contents_list} else @@ -169,15 +170,21 @@ defmodule ExFactor.Changer 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 = + 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 @@ -197,6 +204,7 @@ defmodule ExFactor.Changer do 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}")} else diff --git a/lib/mix/tasks/ex_factor.ex b/lib/mix/tasks/ex_factor.ex index 6df5211..1889e83 100644 --- a/lib/mix/tasks/ex_factor.ex +++ b/lib/mix/tasks/ex_factor.ex @@ -69,18 +69,49 @@ defmodule Mix.Tasks.ExFactor do defp cli_output(map, opts) do verbose = Keyword.get(opts, :verbose, false) + # dry_run = Keyword.get(opts, :dryrun, false) format_entry(Map.get(map, :additions), "Additions", :light_cyan_background, verbose) format_entry(Map.get(map, :changes), "Changes", :light_green_background, verbose) format_entry(Map.get(map, :removals), "Removals", :light_red_background, verbose) + message(false) end defp format_entry(entry, title, color, verbose) do IO.puts(IO.ANSI.format([color, IO.ANSI.format([:black, title])])) - IO.puts(IO.ANSI.format([color, IO.ANSI.format([:black, "================================================================================"])])) - IO.puts(IO.ANSI.format([color, IO.ANSI.format([:black, "vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv"])])) + + IO.puts( + IO.ANSI.format([ + color, + IO.ANSI.format([ + :black, + "================================================================================" + ]) + ]) + ) + + IO.puts( + IO.ANSI.format([ + color, + IO.ANSI.format([ + :black, + "vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv" + ]) + ]) + ) + handle_entry(entry, color, verbose) - IO.puts(IO.ANSI.format([color, IO.ANSI.format([:black, "================================================================================"])])) + + IO.puts( + IO.ANSI.format([ + color, + IO.ANSI.format([ + :black, + "================================================================================" + ]) + ]) + ) + IO.puts("") end @@ -89,14 +120,70 @@ defmodule Mix.Tasks.ExFactor do end defp handle_entry(entry, color, true) do - handle_entry(entry, color , false) + handle_entry(entry, color, false) IO.puts(" File contents: \n#{entry.file_contents}") IO.puts(" State: #{inspect(entry.state)}") end - defp handle_entry(entry, color , false) do + + defp handle_entry(entry, color, false) do IO.puts(IO.ANSI.format([color, IO.ANSI.format([:black, " ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"])])) IO.puts(IO.ANSI.format([color, IO.ANSI.format([:black, " Module: #{entry.module}"])])) IO.puts(IO.ANSI.format([color, IO.ANSI.format([:black, " Path: #{entry.path}"])])) IO.puts(IO.ANSI.format([color, IO.ANSI.format([:black, " Message: #{entry.message}"])])) end + + @message """ + `ExFactor` (by design), does not change test files. + + There are two important and practical reason for this. + + Reason1: + The test failures provide a safety net to help ensure that the ExFactor-ed functions + continue to behave as expected. + + Reason2: + Because .exs files are evaluated at runtime the introspection provided by the compiler + is not available. + + In future revisions, we hope to address this issue, but for now, refactoring test files + remains your responsibility. + """ + defp message(true), do: "" + + defp message(false) do + IO.puts( + IO.ANSI.format([ + :magenta_background, + IO.ANSI.format([ + :bright, + "⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠⚠" + ]) + ]) + ) + + IO.puts(IO.ANSI.format([:magenta_background, IO.ANSI.format([:bright, "IMPORTANT NOTE:"])])) + + IO.puts( + IO.ANSI.format([ + :magenta_background, + IO.ANSI.format([ + :bright, + "✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖✖" + ]) + ]) + ) + + IO.puts( + IO.ANSI.format([ + :magenta_background, + IO.ANSI.format([ + :bright, + "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" + ]) + ]) + ) + + IO.puts(IO.ANSI.format([:magenta_background, IO.ANSI.format([:bright, ""])])) + IO.puts(IO.ANSI.format([:magenta_background, IO.ANSI.format([:bright, @message])])) + end end diff --git a/test/ex_factor/changer_test.exs b/test/ex_factor/changer_test.exs index 839172d..2874324 100644 --- a/test/ex_factor/changer_test.exs +++ b/test/ex_factor/changer_test.exs @@ -191,7 +191,6 @@ defmodule ExFactor.ChangerTest do end) end - test "handle alias exists with :as" do content = """ defmodule ExFactor.Tmp.SourceMod do @@ -379,7 +378,11 @@ defmodule ExFactor.ChangerTest do refute caller =~ "alias ExFactor.Tmp.TargetModule" refute caller =~ "TargetModule.refactor1(arg_a)" - assert Enum.find(change_map.state, fn el -> String.match?(Atom.to_string(el), ~r/alias_/) end) + + 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 diff --git a/test/support/support.ex b/test/support/support.ex index be14393..79155b4 100644 --- a/test/support/support.ex +++ b/test/support/support.ex @@ -1,6 +1,6 @@ defmodule ExFactor.Support do @moduledoc """ - Support moduel for `ExFactor` testing. + Support module for `ExFactor` testing. """ # use alias as: to verify the caller is found.