From 140488bdb19a35d650ecafdd0cd7613c4177bba4 Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Wed, 27 Jul 2022 10:27:45 -0700 Subject: [PATCH 01/12] Add autocorrect to default suggest pipeline At this point the pipeline for the default "suggest" task has been modified to include an Autocorrect step. That's all hooked up, and a callback for `autocorrect/1` has been added to the `Credo.Check` behaviour and the default implementation. This can be implemented for each check individually, and it's a noop for now. --- lib/credo/check.ex | 9 +++++++ .../cli/command/suggest/suggest_command.ex | 1 + lib/credo/cli/task/run_autocorrect.ex | 26 +++++++++++++++++++ lib/credo/execution.ex | 1 + 4 files changed, 37 insertions(+) create mode 100644 lib/credo/cli/task/run_autocorrect.ex diff --git a/lib/credo/check.ex b/lib/credo/check.ex index d85ac16d4..3d5747fc6 100644 --- a/lib/credo/check.ex +++ b/lib/credo/check.ex @@ -153,6 +153,9 @@ defmodule Credo.Check do @callback format_issue(issue_meta :: Credo.IssueMeta.t(), opts :: Keyword.t()) :: Credo.Issue.t() + @doc false + @callback autocorrect(source_file :: String.t()) :: String.t() + @base_category_exit_status_map %{ consistency: 1, design: 2, @@ -395,6 +398,12 @@ defmodule Credo.Check do throw("Implement me") end + @doc false + @impl true + def autocorrect(source_file) do + source_file + end + defoverridable Credo.Check defp append_issues_and_timings([] = _issues, exec) do diff --git a/lib/credo/cli/command/suggest/suggest_command.ex b/lib/credo/cli/command/suggest/suggest_command.ex index c8a284999..87bec2136 100644 --- a/lib/credo/cli/command/suggest/suggest_command.ex +++ b/lib/credo/cli/command/suggest/suggest_command.ex @@ -41,6 +41,7 @@ defmodule Credo.CLI.Command.Suggest.SuggestCommand do print_before_analysis: [__MODULE__.PrintBeforeInfo], run_analysis: [Task.RunChecks], filter_issues: [Task.SetRelevantIssues], + run_autocorrect: [Task.RunAutocorrect], print_after_analysis: [__MODULE__.PrintResultsAndSummary] ) end diff --git a/lib/credo/cli/task/run_autocorrect.ex b/lib/credo/cli/task/run_autocorrect.ex new file mode 100644 index 000000000..3c2a51390 --- /dev/null +++ b/lib/credo/cli/task/run_autocorrect.ex @@ -0,0 +1,26 @@ +defmodule Credo.CLI.Task.RunAutocorrect do + @moduledoc false + + use Credo.Execution.Task + + def call(exec, opts) do + issues = Keyword.get_lazy(opts, :issues, fn -> Execution.get_issues(exec) end) + + issues + |> group_by_file + |> Enum.each(fn {file_name, issues} -> + file = File.read!(file_name) + Enum.reduce(issues, file, fn issue, corrected_file -> + issue.check.autocorrect(corrected_file) + end) + end) + + exec + end + + defp group_by_file(issues) do + Enum.reduce(issues, %{}, fn issue, acc -> + Map.update(acc, issue.filename, [issue], &[issue | &1]) + end) + end +end diff --git a/lib/credo/execution.ex b/lib/credo/execution.ex index 414eb5987..dda146527 100644 --- a/lib/credo/execution.ex +++ b/lib/credo/execution.ex @@ -29,6 +29,7 @@ defmodule Credo.Execution do plugins: [], parse_timeout: 5000, strict: false, + autocorrect: false, # options, set by the command line format: nil, From 0ef3eb793b749b8bf24c860069360d4144c57caf Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Wed, 27 Jul 2022 10:48:19 -0700 Subject: [PATCH 02/12] Add run autocorrect task & tests At this point the task in the execution pipeline for running the autocorrect is implemented & tested. It's naive, but it should work in basic cases! --- lib/credo/cli/task/run_autocorrect.ex | 24 +++++--- test/credo/cli/task/run_autocorrect_test.exs | 64 ++++++++++++++++++++ 2 files changed, 79 insertions(+), 9 deletions(-) create mode 100644 test/credo/cli/task/run_autocorrect_test.exs diff --git a/lib/credo/cli/task/run_autocorrect.ex b/lib/credo/cli/task/run_autocorrect.ex index 3c2a51390..2799be9a7 100644 --- a/lib/credo/cli/task/run_autocorrect.ex +++ b/lib/credo/cli/task/run_autocorrect.ex @@ -3,17 +3,23 @@ defmodule Credo.CLI.Task.RunAutocorrect do use Credo.Execution.Task - def call(exec, opts) do - issues = Keyword.get_lazy(opts, :issues, fn -> Execution.get_issues(exec) end) + def call(exec, opts, read_fun \\ &File.read!/1, write_fun \\ &File.write!/2) do + if exec.autocorrect do + issues = Keyword.get_lazy(opts, :issues, fn -> Execution.get_issues(exec) end) - issues - |> group_by_file - |> Enum.each(fn {file_name, issues} -> - file = File.read!(file_name) - Enum.reduce(issues, file, fn issue, corrected_file -> - issue.check.autocorrect(corrected_file) + issues + |> group_by_file + |> Enum.each(fn {file_path, issues} -> + file = read_fun.(file_path) + + corrected = + Enum.reduce(issues, file, fn issue, corrected_file -> + issue.check.autocorrect(corrected_file) + end) + + write_fun.(file_path, corrected) end) - end) + end exec end diff --git a/test/credo/cli/task/run_autocorrect_test.exs b/test/credo/cli/task/run_autocorrect_test.exs new file mode 100644 index 000000000..efbc1f3d9 --- /dev/null +++ b/test/credo/cli/task/run_autocorrect_test.exs @@ -0,0 +1,64 @@ +defmodule Credo.CLI.Task.RunAutocorrectTest do + use ExUnit.Case, async: true + + alias Credo.CLI.Task.RunAutocorrect + + defmodule CheckOne do + def autocorrect(file) do + send(self(), {:check_one, file}) + "check_one" + end + end + + defmodule CheckTwo do + def autocorrect(file) do + send(self(), {:check_two, file}) + "check_two" + end + end + + defmodule CheckThree do + def autocorrect(file) do + send(self(), {:check_three, file}) + "check_three" + end + end + + describe "run/2" do + test "calls `autocorrect/1` for each issue with the correct arguments if autocorrect is true" do + issues = [ + %Credo.Issue{filename: "a", check: CheckTwo}, + %Credo.Issue{filename: "a", check: CheckOne}, + %Credo.Issue{filename: "b", check: CheckThree} + ] + + exec = %Credo.Execution{autocorrect: true} + read_fun = fn _ -> "start" end + write_fun = fn file_path, contents -> send(self(), {:write, file_path, contents}) end + RunAutocorrect.call(exec, [issues: issues], read_fun, write_fun) + assert_receive({:check_one, "start"}) + assert_receive({:check_two, "check_one"}) + assert_receive({:check_three, "start"}) + assert_receive({:write, "a", "check_two"}) + assert_receive({:write, "b", "check_three"}) + end + + test "does nothing if autocorrect is false" do + issues = [ + %Credo.Issue{filename: "a", check: CheckTwo}, + %Credo.Issue{filename: "a", check: CheckOne}, + %Credo.Issue{filename: "b", check: CheckThree} + ] + + exec = %Credo.Execution{autocorrect: false} + read_fun = fn _ -> "start" end + write_fun = fn file_path, contents -> send(self(), {:write, file_path, contents}) end + RunAutocorrect.call(exec, [issues: issues], read_fun, write_fun) + refute_receive({:check_one, "start"}) + refute_receive({:check_two, "check_one"}) + refute_receive({:check_three, "start"}) + refute_receive({:write, "a", "check_two"}) + refute_receive({:write, "b", "check_three"}) + end + end +end From 7a0dada05625591dd7c3eb7ca827768fe8100999 Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Wed, 27 Jul 2022 10:55:38 -0700 Subject: [PATCH 03/12] Autocorrect implemented & working for a single check At this point we have autocorrect functionality implemented & working for the TrailingBlankLine check. I've verified this with manual testing. The UX still has a ton of work to do, but for now the bulk of the work is actually working. --- lib/credo/check/readability/trailing_blank_line.ex | 4 ++++ lib/credo/execution.ex | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/credo/check/readability/trailing_blank_line.ex b/lib/credo/check/readability/trailing_blank_line.ex index 7215e90e8..a72477dfd 100644 --- a/lib/credo/check/readability/trailing_blank_line.ex +++ b/lib/credo/check/readability/trailing_blank_line.ex @@ -39,4 +39,8 @@ defmodule Credo.Check.Readability.TrailingBlankLine do line_no: line_no ) end + + def autocorrect(file) do + "#{file}\n" + end end diff --git a/lib/credo/execution.ex b/lib/credo/execution.ex index dda146527..edecb14f7 100644 --- a/lib/credo/execution.ex +++ b/lib/credo/execution.ex @@ -29,7 +29,7 @@ defmodule Credo.Execution do plugins: [], parse_timeout: 5000, strict: false, - autocorrect: false, + autocorrect: true, # options, set by the command line format: nil, From 93cba5d9d00b8c7f6e3f03fe5a09648bae432cfa Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Wed, 27 Jul 2022 11:02:31 -0700 Subject: [PATCH 04/12] Now able to pass `--autocorrect` flag to `mix credo` We can now pass an `--autocorrect` flag to the default `suggest` command when running `mix credo` and it works! --- lib/credo/cli/command/suggest/suggest_command.ex | 1 + lib/credo/config_builder.ex | 9 +++++++++ lib/credo/execution.ex | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/credo/cli/command/suggest/suggest_command.ex b/lib/credo/cli/command/suggest/suggest_command.ex index 87bec2136..99b222454 100644 --- a/lib/credo/cli/command/suggest/suggest_command.ex +++ b/lib/credo/cli/command/suggest/suggest_command.ex @@ -30,6 +30,7 @@ defmodule Credo.CLI.Command.Suggest.SuggestCommand do Switch.boolean("read_from_stdin"), Switch.boolean("strict"), Switch.boolean("verbose"), + Switch.boolean("autocorrect"), Switch.boolean("watch") ] diff --git a/lib/credo/config_builder.ex b/lib/credo/config_builder.ex index 689002a42..55e75e2da 100644 --- a/lib/credo/config_builder.ex +++ b/lib/credo/config_builder.ex @@ -81,6 +81,7 @@ defmodule Credo.ConfigBuilder do |> add_switch_ignore(switches) |> add_switch_mute_exit_status(switches) |> add_switch_only(switches) + |> add_switch_autocorrect(switches) |> add_switch_read_from_stdin(switches) |> add_switch_strict(switches) |> add_switch_min_priority(switches) @@ -262,6 +263,14 @@ defmodule Credo.ConfigBuilder do defp add_switch_only(exec, _), do: exec + # add_switch_autocorrect + + defp add_switch_autocorrect(exec, %{autocorrect: true}) do + %Execution{exec | autocorrect: true} + end + + defp add_switch_autocorrect(exec, _), do: exec + # add_switch_ignore # exclude/ignore certain checks diff --git a/lib/credo/execution.ex b/lib/credo/execution.ex index edecb14f7..dda146527 100644 --- a/lib/credo/execution.ex +++ b/lib/credo/execution.ex @@ -29,7 +29,7 @@ defmodule Credo.Execution do plugins: [], parse_timeout: 5000, strict: false, - autocorrect: true, + autocorrect: false, # options, set by the command line format: nil, From a0042a535cd6c7a01b2ccf5b592f1806e1c4169e Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Wed, 27 Jul 2022 11:27:35 -0700 Subject: [PATCH 05/12] Remove autocorrected issues before displaying results Now when we autocorrect an issue, we remove that issue from the stored list of issues so it isn't shown to the user (since technically it's no longer an issue since it was autocorrected). One other thing we might do is to change the formatter to display when an issue was there and to indicate that it was autocorrected, also making sure the exit code is 0 for any autocorrected issues, but for now this seemed like a reasonable path forward and much simpler than this option. --- lib/credo/cli/task/run_autocorrect.ex | 16 ++++++++++++---- lib/credo/execution.ex | 8 ++++++++ lib/credo/execution/execution_issues.ex | 12 ++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/lib/credo/cli/task/run_autocorrect.ex b/lib/credo/cli/task/run_autocorrect.ex index 2799be9a7..e71781cc3 100644 --- a/lib/credo/cli/task/run_autocorrect.ex +++ b/lib/credo/cli/task/run_autocorrect.ex @@ -12,10 +12,7 @@ defmodule Credo.CLI.Task.RunAutocorrect do |> Enum.each(fn {file_path, issues} -> file = read_fun.(file_path) - corrected = - Enum.reduce(issues, file, fn issue, corrected_file -> - issue.check.autocorrect(corrected_file) - end) + corrected = Enum.reduce(issues, file, &run_autocorrect(&1, &2, exec)) write_fun.(file_path, corrected) end) @@ -29,4 +26,15 @@ defmodule Credo.CLI.Task.RunAutocorrect do Map.update(acc, issue.filename, [issue], &[issue | &1]) end) end + + defp run_autocorrect(issue, file, exec) do + case issue.check.autocorrect(file) do + ^file -> + file + + corrected_file -> + Execution.remove_issue(exec, issue) + corrected_file + end + end end diff --git a/lib/credo/execution.ex b/lib/credo/execution.ex index dda146527..a129ec933 100644 --- a/lib/credo/execution.ex +++ b/lib/credo/execution.ex @@ -494,6 +494,14 @@ defmodule Credo.Execution do |> Map.get(filename, []) end + @doc """ + This removes an issue for the given `exec` struct if the issue has been automatically resolved + before the run has completed. + """ + def remove_issue(exec, issue) do + ExecutionIssues.remove_issue(exec, issue) + end + @doc """ Sets the issues for the given `exec` struct, overwriting any existing issues. """ diff --git a/lib/credo/execution/execution_issues.ex b/lib/credo/execution/execution_issues.ex index 0b9ab32fe..e85b578c2 100644 --- a/lib/credo/execution/execution_issues.ex +++ b/lib/credo/execution/execution_issues.ex @@ -46,6 +46,10 @@ defmodule Credo.Execution.ExecutionIssues do GenServer.call(pid, :to_map) end + def remove_issue(%Execution{issues_pid: pid}, issue) do + GenServer.call(pid, {:remove_issue, issue}) + end + # callbacks def init(_) do @@ -73,4 +77,12 @@ defmodule Credo.Execution.ExecutionIssues do def handle_call(:to_map, _from, current_state) do {:reply, current_state, current_state} end + + def handle_call({:remove_issue, issue}, _from, current_state) do + existing_issues = List.wrap(current_state[issue.filename]) + new_issue_list = List.delete(existing_issues, issue) + new_current_state = Map.put(current_state, issue.filename, new_issue_list) + + {:reply, new_current_state, new_current_state} + end end From b98842fb52cf8ccd185081cd0399cb32cdc19cd5 Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Wed, 27 Jul 2022 13:47:16 -0700 Subject: [PATCH 06/12] Implement most basic version of alias ordering autocorrect At this point it will autocorrect the most basic version of alias ordering, but it doesn't yet handle multi-aliases or handle groupings of aliases. That will come in a later commit. --- lib/credo/check/readability/alias_order.ex | 26 +++++++++++++++++ .../check/readability/alias_order_test.exs | 28 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/lib/credo/check/readability/alias_order.ex b/lib/credo/check/readability/alias_order.ex index eeee15cd4..ec7cefd8b 100644 --- a/lib/credo/check/readability/alias_order.ex +++ b/lib/credo/check/readability/alias_order.ex @@ -223,4 +223,30 @@ defmodule Credo.Check.Readability.AliasOrder do line_no: line_no ) end + + def autocorrect(file) do + {:ok, quoted} = :"Elixir.Code".string_to_quoted(file) + + modified = + quoted + |> Macro.prewalk(&do_autocorrect/1) + |> Macro.to_string() + |> :"Elixir.Code".format_string!() + |> to_string() + + "#{modified}\n" + end + + defp do_autocorrect({:__block__ = op, meta, [{:alias, _, _} | _] = aliases}) do + mapper = fn {:alias, _, [{:__aliases__, _, _} = node]} -> compare_name(node) end + + modified = + Enum.sort_by(aliases, mapper, fn left, right -> + Enum.sort([left, right]) == [left, right] + end) + + {op, meta, modified} + end + + defp do_autocorrect(ast), do: ast end diff --git a/test/credo/check/readability/alias_order_test.exs b/test/credo/check/readability/alias_order_test.exs index 62793ae41..2eea86675 100644 --- a/test/credo/check/readability/alias_order_test.exs +++ b/test/credo/check/readability/alias_order_test.exs @@ -228,4 +228,32 @@ defmodule Credo.Check.Readability.AliasOrderTest do assert issue.trigger == "Sorter" end) end + + describe "autocorrect/1" do + test "puts all the aliases in the right order" do + starting = """ + defmodule CredoSampleModule do + alias Credo.CLI.Sorter + alias Credo.CLI.Command + alias Credo.CLI.Filename + end + """ + + expected = """ + defmodule CredoSampleModule do + alias Credo.CLI.Command + alias Credo.CLI.Filename + alias Credo.CLI.Sorter + end + """ + + assert @described_check.autocorrect(starting) == expected + end + + test "works with multi-aliases" do + end + + test "works with as: option" do + end + end end From f5f1343e484c1ecf8b813374032af58224598053 Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Wed, 27 Jul 2022 14:19:41 -0700 Subject: [PATCH 07/12] More alias ordering autocorrect behavior implemented More basics of the autocorrect behavior for the alias ordering check have been implemented, and now we just need to handle blocks of aliases with whitespace between them. --- lib/credo/check/readability/alias_order.ex | 32 ++++++++++++++---- .../check/readability/alias_order_test.exs | 33 +++++++++++++++++++ test/credo/cli/task/run_autocorrect_test.exs | 4 +-- 3 files changed, 61 insertions(+), 8 deletions(-) diff --git a/lib/credo/check/readability/alias_order.ex b/lib/credo/check/readability/alias_order.ex index ec7cefd8b..b0cefa919 100644 --- a/lib/credo/check/readability/alias_order.ex +++ b/lib/credo/check/readability/alias_order.ex @@ -238,15 +238,35 @@ defmodule Credo.Check.Readability.AliasOrder do end defp do_autocorrect({:__block__ = op, meta, [{:alias, _, _} | _] = aliases}) do - mapper = fn {:alias, _, [{:__aliases__, _, _} = node]} -> compare_name(node) end - modified = - Enum.sort_by(aliases, mapper, fn left, right -> - Enum.sort([left, right]) == [left, right] - end) + aliases + |> Macro.prewalk(&remove_line_numbers/1) + |> Enum.map(&sort_multi_aliases/1) + |> sort_aliases() - {op, meta, modified} + {op, Keyword.delete(meta, :line), modified} end defp do_autocorrect(ast), do: ast + + defp sort_multi_aliases({op, meta, [{op2, meta2, [{_, _, _} | _] = aliases}]}) do + {op, meta, [{op2, meta2, sort_aliases(aliases)}]} + end + + defp sort_multi_aliases(node), do: node + + defp sort_aliases(aliases) do + Enum.sort_by(aliases, &name_for_sorting/1, fn left, right -> + Enum.sort([left, right]) == [left, right] + end) + end + + defp remove_line_numbers({op, meta, args}) when is_list(meta), + do: {op, Keyword.delete(meta, :line), args} + + defp remove_line_numbers(ast), do: ast + + defp name_for_sorting({_, _, [{_, _, node}, [as: _]]}), do: compare_name(node) + defp name_for_sorting({_, _, [{_, _, _} = node]}), do: compare_name(node) + defp name_for_sorting({_, _, [node]}), do: compare_name(node) end diff --git a/test/credo/check/readability/alias_order_test.exs b/test/credo/check/readability/alias_order_test.exs index 2eea86675..e70f07317 100644 --- a/test/credo/check/readability/alias_order_test.exs +++ b/test/credo/check/readability/alias_order_test.exs @@ -251,9 +251,42 @@ defmodule Credo.Check.Readability.AliasOrderTest do end test "works with multi-aliases" do + starting = """ + defmodule CredoSampleModule do + alias Credo.CLI.{Sorter, Command} + alias Credo.CLI.Filename + end + """ + + expected = """ + defmodule CredoSampleModule do + alias Credo.CLI.{Command, Sorter} + alias Credo.CLI.Filename + end + """ + + assert @described_check.autocorrect(starting) == expected end test "works with as: option" do + starting = """ + defmodule CredoSampleModule do + alias App.Module2 + alias App.Module1, as: Module3 + end + """ + + expected = """ + defmodule CredoSampleModule do + alias App.Module1, as: Module3 + alias App.Module2 + end + """ + + assert @described_check.autocorrect(starting) == expected + end + + test "works with multiple blocks of aliases including multi-aliases" do end end end diff --git a/test/credo/cli/task/run_autocorrect_test.exs b/test/credo/cli/task/run_autocorrect_test.exs index efbc1f3d9..2eb6c0382 100644 --- a/test/credo/cli/task/run_autocorrect_test.exs +++ b/test/credo/cli/task/run_autocorrect_test.exs @@ -1,5 +1,5 @@ defmodule Credo.CLI.Task.RunAutocorrectTest do - use ExUnit.Case, async: true + use Credo.Test.Case alias Credo.CLI.Task.RunAutocorrect @@ -32,7 +32,7 @@ defmodule Credo.CLI.Task.RunAutocorrectTest do %Credo.Issue{filename: "b", check: CheckThree} ] - exec = %Credo.Execution{autocorrect: true} + exec = %Credo.Execution{autocorrect: true} |> Credo.Execution.ExecutionIssues.start_server() read_fun = fn _ -> "start" end write_fun = fn file_path, contents -> send(self(), {:write, file_path, contents}) end RunAutocorrect.call(exec, [issues: issues], read_fun, write_fun) From 0d844906ee925008bf624f257f94c6b67ef6dc41 Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Wed, 27 Jul 2022 15:10:23 -0700 Subject: [PATCH 08/12] "Finish" implementing autocorrect for ordering aliases We've now handled pretty much all the use cases for ordering aliases automatically. There is some formatter whackiness that will need to be figured out, but beyond that this is a good example of how an autocorrect callback might be implemented for a more involved check than the first one. --- lib/credo/check/readability/alias_order.ex | 35 +++++++++++++++++-- .../check/readability/alias_order_test.exs | 24 +++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/lib/credo/check/readability/alias_order.ex b/lib/credo/check/readability/alias_order.ex index b0cefa919..e099a924e 100644 --- a/lib/credo/check/readability/alias_order.ex +++ b/lib/credo/check/readability/alias_order.ex @@ -240,15 +240,44 @@ defmodule Credo.Check.Readability.AliasOrder do defp do_autocorrect({:__block__ = op, meta, [{:alias, _, _} | _] = aliases}) do modified = aliases - |> Macro.prewalk(&remove_line_numbers/1) - |> Enum.map(&sort_multi_aliases/1) - |> sort_aliases() + |> group_aliases() + |> Enum.map(fn {line, group} -> + group + |> Macro.prewalk(&remove_line_numbers/1) + |> Enum.map(&sort_multi_aliases/1) + |> sort_aliases() + |> put_line_number(line) + end) + |> List.flatten() {op, Keyword.delete(meta, :line), modified} end defp do_autocorrect(ast), do: ast + defp put_line_number([{op, meta, args} | tail], line) do + modified = {op, Keyword.put(meta, :line, line), args} + [modified | tail] + end + + defp group_aliases(aliases) do + chunk_fun = fn + {_, meta, _} = node, {_, []} -> + {:cont, {meta[:line], [node]}} + + {_, meta, _} = node, {line, [{_, meta2, _} | _] = chunk} -> + if meta[:line] - 1 > meta2[:line] do + {:cont, {line, chunk}, {meta[:line], [node]}} + else + {:cont, {line, [node | chunk]}} + end + end + + after_fun = fn acc -> {:cont, acc, []} end + + Enum.chunk_while(aliases, {nil, []}, chunk_fun, after_fun) + end + defp sort_multi_aliases({op, meta, [{op2, meta2, [{_, _, _} | _] = aliases}]}) do {op, meta, [{op2, meta2, sort_aliases(aliases)}]} end diff --git a/test/credo/check/readability/alias_order_test.exs b/test/credo/check/readability/alias_order_test.exs index e70f07317..fb494bcb2 100644 --- a/test/credo/check/readability/alias_order_test.exs +++ b/test/credo/check/readability/alias_order_test.exs @@ -287,6 +287,30 @@ defmodule Credo.Check.Readability.AliasOrderTest do end test "works with multiple blocks of aliases including multi-aliases" do + starting = """ + defmodule CredoSampleModule do + alias Credo.CLI.Filename + alias Credo.CLI.{Sorter, Command} + + alias App.Module2 + alias App.Module1 + end + """ + + # NOTE: For some reason I couldn't get the formatter to keep the newline between these + # groups. I'm likely missing some of the metadata that influences how the formatter is told + # to keep user-inserted newlines, but I'll need to dig deeper into the formatter to make + # that stuff work. For now, this has almost all the behavior that we want! + expected = """ + defmodule CredoSampleModule do + alias Credo.CLI.{Command, Sorter} + alias Credo.CLI.Filename + alias App.Module1 + alias App.Module2 + end + """ + + assert @described_check.autocorrect(starting) == expected end end end From fb4be0e4c557bae4abfb5f1697b0354aa15e5be7 Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Thu, 28 Jul 2022 10:06:33 -0700 Subject: [PATCH 09/12] Add autocorrect callback for line endings consistency check This is the first implementation of an autocorrect callback for a consistency check. I don't _love_ how we're doing it right now since we're not actually storing the state of what the "correct" behavior is that should be changed to, so instead we're just using the issue message to give us information about what the autocorrect behavior should be. It's not great, but it works! --- lib/credo/check.ex | 4 +-- lib/credo/check/consistency/line_endings.ex | 11 ++++++ lib/credo/check/readability/alias_order.ex | 2 +- .../check/readability/trailing_blank_line.ex | 2 +- lib/credo/cli/task/run_autocorrect.ex | 2 +- .../check/consistency/line_endings_test.exs | 25 ++++++++++++++ .../check/readability/alias_order_test.exs | 8 ++--- test/credo/cli/task/run_autocorrect_test.exs | 34 ++++++++----------- 8 files changed, 60 insertions(+), 28 deletions(-) diff --git a/lib/credo/check.ex b/lib/credo/check.ex index 3d5747fc6..762ad5c64 100644 --- a/lib/credo/check.ex +++ b/lib/credo/check.ex @@ -154,7 +154,7 @@ defmodule Credo.Check do Credo.Issue.t() @doc false - @callback autocorrect(source_file :: String.t()) :: String.t() + @callback autocorrect(source_file :: String.t(), issue :: Issue.t()) :: String.t() @base_category_exit_status_map %{ consistency: 1, @@ -400,7 +400,7 @@ defmodule Credo.Check do @doc false @impl true - def autocorrect(source_file) do + def autocorrect(source_file, _issue) do source_file end diff --git a/lib/credo/check/consistency/line_endings.ex b/lib/credo/check/consistency/line_endings.ex index becb10e39..bcaffabfc 100644 --- a/lib/credo/check/consistency/line_endings.ex +++ b/lib/credo/check/consistency/line_endings.ex @@ -44,4 +44,15 @@ defmodule Credo.Check.Consistency.LineEndings do defp message_for(:windows = _expected) do "File is using unix line endings while most of the files use windows line endings." end + + def autocorrect(file, issue) do + if issue.message == message_for(:windows) do + do_autocorrect(file, :unix_to_windows) + else + do_autocorrect(file, :windows_to_unix) + end + end + + defp do_autocorrect(file, :windows_to_unix), do: String.replace(file, "\r\n", "\n") + defp do_autocorrect(file, :unix_to_windows), do: String.replace(file, "\n", "\r\n") end diff --git a/lib/credo/check/readability/alias_order.ex b/lib/credo/check/readability/alias_order.ex index e099a924e..dcd486bd9 100644 --- a/lib/credo/check/readability/alias_order.ex +++ b/lib/credo/check/readability/alias_order.ex @@ -224,7 +224,7 @@ defmodule Credo.Check.Readability.AliasOrder do ) end - def autocorrect(file) do + def autocorrect(file, _issue) do {:ok, quoted} = :"Elixir.Code".string_to_quoted(file) modified = diff --git a/lib/credo/check/readability/trailing_blank_line.ex b/lib/credo/check/readability/trailing_blank_line.ex index a72477dfd..69cad00ad 100644 --- a/lib/credo/check/readability/trailing_blank_line.ex +++ b/lib/credo/check/readability/trailing_blank_line.ex @@ -40,7 +40,7 @@ defmodule Credo.Check.Readability.TrailingBlankLine do ) end - def autocorrect(file) do + def autocorrect(file, _issue) do "#{file}\n" end end diff --git a/lib/credo/cli/task/run_autocorrect.ex b/lib/credo/cli/task/run_autocorrect.ex index e71781cc3..c384a6791 100644 --- a/lib/credo/cli/task/run_autocorrect.ex +++ b/lib/credo/cli/task/run_autocorrect.ex @@ -28,7 +28,7 @@ defmodule Credo.CLI.Task.RunAutocorrect do end defp run_autocorrect(issue, file, exec) do - case issue.check.autocorrect(file) do + case issue.check.autocorrect(file, issue) do ^file -> file diff --git a/test/credo/check/consistency/line_endings_test.exs b/test/credo/check/consistency/line_endings_test.exs index d4aed95fa..1bcb3b1c9 100644 --- a/test/credo/check/consistency/line_endings_test.exs +++ b/test/credo/check/consistency/line_endings_test.exs @@ -55,4 +55,29 @@ defmodule Credo.Check.Readability.LineEndingsTest do |> run_check(@described_check) |> assert_issue end + + describe "autocorrect/1" do + test "switches unix to windows line endings" do + starting = "defmodule Credo.Sample do\n@test_attribute :foo\nend\n" + expected = "defmodule Credo.Sample do\r\n@test_attribute :foo\r\nend\r\n" + + issue = %Credo.Issue{message: "File is using unix line endings while most of the files use windows line endings."} + + assert @described_check.autocorrect(starting, issue) == expected + end + + test "switches windows to unix line endings" do + starting = """ + defmodule Credo.Sample do\r\n@test_attribute :foo\r\nend + """ + + expected = """ + defmodule Credo.Sample do\n@test_attribute :foo\nend + """ + + issue = %Credo.Issue{message: "File is using windows line endings while most of the files use unix line endings."} + + assert @described_check.autocorrect(starting, issue) == expected + end + end end diff --git a/test/credo/check/readability/alias_order_test.exs b/test/credo/check/readability/alias_order_test.exs index fb494bcb2..f25303eb9 100644 --- a/test/credo/check/readability/alias_order_test.exs +++ b/test/credo/check/readability/alias_order_test.exs @@ -247,7 +247,7 @@ defmodule Credo.Check.Readability.AliasOrderTest do end """ - assert @described_check.autocorrect(starting) == expected + assert @described_check.autocorrect(starting, nil) == expected end test "works with multi-aliases" do @@ -265,7 +265,7 @@ defmodule Credo.Check.Readability.AliasOrderTest do end """ - assert @described_check.autocorrect(starting) == expected + assert @described_check.autocorrect(starting, nil) == expected end test "works with as: option" do @@ -283,7 +283,7 @@ defmodule Credo.Check.Readability.AliasOrderTest do end """ - assert @described_check.autocorrect(starting) == expected + assert @described_check.autocorrect(starting, nil) == expected end test "works with multiple blocks of aliases including multi-aliases" do @@ -310,7 +310,7 @@ defmodule Credo.Check.Readability.AliasOrderTest do end """ - assert @described_check.autocorrect(starting) == expected + assert @described_check.autocorrect(starting, nil) == expected end end end diff --git a/test/credo/cli/task/run_autocorrect_test.exs b/test/credo/cli/task/run_autocorrect_test.exs index 2eb6c0382..0e3ae6cb3 100644 --- a/test/credo/cli/task/run_autocorrect_test.exs +++ b/test/credo/cli/task/run_autocorrect_test.exs @@ -4,41 +4,41 @@ defmodule Credo.CLI.Task.RunAutocorrectTest do alias Credo.CLI.Task.RunAutocorrect defmodule CheckOne do - def autocorrect(file) do - send(self(), {:check_one, file}) + def autocorrect(file, issue) do + send(self(), {:check_one, file, issue}) "check_one" end end defmodule CheckTwo do - def autocorrect(file) do - send(self(), {:check_two, file}) + def autocorrect(file, issue) do + send(self(), {:check_two, file, issue}) "check_two" end end defmodule CheckThree do - def autocorrect(file) do - send(self(), {:check_three, file}) + def autocorrect(file, issue) do + send(self(), {:check_three, file, issue}) "check_three" end end describe "run/2" do test "calls `autocorrect/1` for each issue with the correct arguments if autocorrect is true" do - issues = [ - %Credo.Issue{filename: "a", check: CheckTwo}, - %Credo.Issue{filename: "a", check: CheckOne}, - %Credo.Issue{filename: "b", check: CheckThree} - ] + issue1 = %Credo.Issue{filename: "a", check: CheckOne} + issue2 = %Credo.Issue{filename: "a", check: CheckTwo} + issue3 = %Credo.Issue{filename: "b", check: CheckThree} + + issues = [issue2, issue1, issue3] exec = %Credo.Execution{autocorrect: true} |> Credo.Execution.ExecutionIssues.start_server() read_fun = fn _ -> "start" end write_fun = fn file_path, contents -> send(self(), {:write, file_path, contents}) end RunAutocorrect.call(exec, [issues: issues], read_fun, write_fun) - assert_receive({:check_one, "start"}) - assert_receive({:check_two, "check_one"}) - assert_receive({:check_three, "start"}) + assert_receive({:check_one, "start", ^issue1}) + assert_receive({:check_two, "check_one", ^issue2}) + assert_receive({:check_three, "start", ^issue3}) assert_receive({:write, "a", "check_two"}) assert_receive({:write, "b", "check_three"}) end @@ -54,11 +54,7 @@ defmodule Credo.CLI.Task.RunAutocorrectTest do read_fun = fn _ -> "start" end write_fun = fn file_path, contents -> send(self(), {:write, file_path, contents}) end RunAutocorrect.call(exec, [issues: issues], read_fun, write_fun) - refute_receive({:check_one, "start"}) - refute_receive({:check_two, "check_one"}) - refute_receive({:check_three, "start"}) - refute_receive({:write, "a", "check_two"}) - refute_receive({:write, "b", "check_three"}) + refute_receive(_) end end end From ab77dcd44b1f553de9fade74c653d985c24f8fd1 Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Thu, 28 Jul 2022 10:16:34 -0700 Subject: [PATCH 10/12] Add autocorrect for unless with else check This autocorrects an unless with else check so that we're changing it to an `if` statement. --- lib/credo/check/refactor/unless_with_else.ex | 19 +++++++++++ .../check/refactor/unless_with_else_test.exs | 34 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/lib/credo/check/refactor/unless_with_else.ex b/lib/credo/check/refactor/unless_with_else.ex index fbd282387..a842f869c 100644 --- a/lib/credo/check/refactor/unless_with_else.ex +++ b/lib/credo/check/refactor/unless_with_else.ex @@ -71,4 +71,23 @@ defmodule Credo.Check.Refactor.UnlessWithElse do line_no: line_no ) end + + def autocorrect(file, _issue) do + {:ok, quoted} = :"Elixir.Code".string_to_quoted(file) + + modified = + quoted + |> Macro.prewalk(&do_autocorrect/1) + |> Macro.to_string() + |> :"Elixir.Code".format_string!() + |> to_string() + + "#{modified}\n" + end + + defp do_autocorrect({:unless, meta, [clause, [do: falsy, else: truthy]]}) do + {:if, meta, [clause, [do: truthy, else: falsy]]} + end + + defp do_autocorrect(ast), do: ast end diff --git a/test/credo/check/refactor/unless_with_else_test.exs b/test/credo/check/refactor/unless_with_else_test.exs index fac35cb36..00858093d 100644 --- a/test/credo/check/refactor/unless_with_else_test.exs +++ b/test/credo/check/refactor/unless_with_else_test.exs @@ -47,4 +47,38 @@ defmodule Credo.Check.Refactor.UnlessWithElseTest do |> run_check(@described_check) |> assert_issue() end + + describe "autocorrect/2" do + test "changes the unless with else to an if clause" do + starting = """ + defmodule CredoSampleModule do + def some_function(parameter1, parameter2) do + unless allowed? do + unless other_condition? do + something + end + else + something_else + end + end + end + """ + + expected = """ + defmodule CredoSampleModule do + def some_function(parameter1, parameter2) do + if allowed? do + something_else + else + unless other_condition? do + something + end + end + end + end + """ + + assert @described_check.autocorrect(starting, nil) == expected + end + end end From acd96bb528ce30eff1f47c26521f2d27460f140a Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Thu, 28 Jul 2022 11:09:32 -0700 Subject: [PATCH 11/12] Add basics for unused string operation autocorrect This adds the first bits of an implementation for how we might want to autocorrect an unused String operation. Unfortunately because there's nothing in the AST that represents **nothing**, anything we replace that unused String operation with in the AST will be compiled into something - even if just the literal atom `nil`. That would probably be better than unused String operations since those would just be noops in the runtime, but they would be potentially confusing in the code. If we want to actually do this there's probably a _lot_ more that would need to be done for this check, but what we have offers a glimpse into how this kind of check could be autocorrected as a proof of concept. --- .../check/warning/unused_string_operation.ex | 44 ++++++++++++++++ .../warning/unused_string_operation_test.exs | 51 +++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/lib/credo/check/warning/unused_string_operation.ex b/lib/credo/check/warning/unused_string_operation.ex index 93142953f..e3d6238d3 100644 --- a/lib/credo/check/warning/unused_string_operation.ex +++ b/lib/credo/check/warning/unused_string_operation.ex @@ -28,6 +28,7 @@ defmodule Credo.Check.Warning.UnusedStringOperation do """ ] + alias Credo.Check.Warning.UnusedFunctionReturnHelper alias Credo.Check.Warning.UnusedOperation @checked_module :String @@ -44,4 +45,47 @@ defmodule Credo.Check.Warning.UnusedStringOperation do &format_issue/2 ) end + + def autocorrect(file, _issue) do + {_, quoted} = Credo.Code.ast(file) + source_file = SourceFile.parse(file, "nofile") + + unused_calls = + UnusedFunctionReturnHelper.find_unused_calls( + source_file, + [], + [@checked_module], + @funs_with_return_value + ) + + modified = + quoted + |> Macro.prewalk(&do_autocorrect(&1, unused_calls)) + |> Macro.to_string() + |> :"Elixir.Code".format_string!() + |> to_string() + + "#{modified}\n" + end + + defp do_autocorrect({:__block__, meta, [{:|>, _pipe_meta, pipe_args} | tail] = args}, unused_calls) do + args = + if List.last(pipe_args) in unused_calls do + [hd(pipe_args) | tail] + else + args + end + + {:__block__, meta, args} + end + + defp do_autocorrect({:__block__, meta, args}, unused_calls) do + {:__block__, meta, Enum.reject(args, & &1 in unused_calls)} + end + + defp do_autocorrect({:do, node}, unused_calls) do + {:do, do_autocorrect(node, unused_calls)} + end + + defp do_autocorrect(ast, _), do: ast end diff --git a/test/credo/check/warning/unused_string_operation_test.exs b/test/credo/check/warning/unused_string_operation_test.exs index 3d44d80c3..1b9baaea8 100644 --- a/test/credo/check/warning/unused_string_operation_test.exs +++ b/test/credo/check/warning/unused_string_operation_test.exs @@ -720,4 +720,55 @@ defmodule Credo.Check.Warning.UnusedStringOperationTest do assert "String.to_float" == issue.trigger end) end + + describe "autocorrect/2" do + test "removes the unused String function" do + starting = """ + defmodule CredoSampleModule do + def some_function(parameter1, parameter2) do + x = parameter1 + parameter2 + + String.split(parameter1) + + parameter1 + end + end + """ + + expected = """ + defmodule CredoSampleModule do + def some_function(parameter1, parameter2) do + x = parameter1 + parameter2 + parameter1 + end + end + """ + + assert @described_check.autocorrect(starting, nil) == expected + end + + test "it should report a violation when end of pipe" do + starting = """ + defmodule CredoSampleModule do + def some_function(parameter1, parameter2) do + parameter1 + parameter2 + |> String.split(parameter1) + + parameter1 + end + end + """ + + expected = """ + defmodule CredoSampleModule do + def some_function(parameter1, parameter2) do + parameter1 + parameter2 + parameter1 + end + end + """ + + assert @described_check.autocorrect(starting, nil) == expected + end + end end From 4bb4095f3528b54360a44f7f55b3094bcdb28b8d Mon Sep 17 00:00:00 2001 From: Devon Estes Date: Fri, 29 Jul 2022 09:51:55 -0700 Subject: [PATCH 12/12] change autocorrect to autofix --- lib/credo/check.ex | 4 ++-- lib/credo/check/consistency/line_endings.ex | 10 ++++----- lib/credo/check/readability/alias_order.ex | 8 +++---- .../check/readability/trailing_blank_line.ex | 2 +- lib/credo/check/refactor/unless_with_else.ex | 8 +++---- .../check/warning/unused_string_operation.ex | 14 ++++++------ .../cli/command/suggest/suggest_command.ex | 4 ++-- lib/credo/cli/task/run_autocorrect.ex | 10 ++++----- lib/credo/config_builder.ex | 10 ++++----- lib/credo/execution.ex | 2 +- .../check/consistency/line_endings_test.exs | 6 ++--- .../check/readability/alias_order_test.exs | 10 ++++----- .../check/refactor/unless_with_else_test.exs | 4 ++-- .../warning/unused_string_operation_test.exs | 6 ++--- test/credo/cli/task/run_autocorrect_test.exs | 22 +++++++++---------- 15 files changed, 60 insertions(+), 60 deletions(-) diff --git a/lib/credo/check.ex b/lib/credo/check.ex index 762ad5c64..a6ad226bd 100644 --- a/lib/credo/check.ex +++ b/lib/credo/check.ex @@ -154,7 +154,7 @@ defmodule Credo.Check do Credo.Issue.t() @doc false - @callback autocorrect(source_file :: String.t(), issue :: Issue.t()) :: String.t() + @callback autofix(source_file :: String.t(), issue :: Issue.t()) :: String.t() @base_category_exit_status_map %{ consistency: 1, @@ -400,7 +400,7 @@ defmodule Credo.Check do @doc false @impl true - def autocorrect(source_file, _issue) do + def autofix(source_file, _issue) do source_file end diff --git a/lib/credo/check/consistency/line_endings.ex b/lib/credo/check/consistency/line_endings.ex index bcaffabfc..8fc327ab2 100644 --- a/lib/credo/check/consistency/line_endings.ex +++ b/lib/credo/check/consistency/line_endings.ex @@ -45,14 +45,14 @@ defmodule Credo.Check.Consistency.LineEndings do "File is using unix line endings while most of the files use windows line endings." end - def autocorrect(file, issue) do + def autofix(file, issue) do if issue.message == message_for(:windows) do - do_autocorrect(file, :unix_to_windows) + do_autofix(file, :unix_to_windows) else - do_autocorrect(file, :windows_to_unix) + do_autofix(file, :windows_to_unix) end end - defp do_autocorrect(file, :windows_to_unix), do: String.replace(file, "\r\n", "\n") - defp do_autocorrect(file, :unix_to_windows), do: String.replace(file, "\n", "\r\n") + defp do_autofix(file, :windows_to_unix), do: String.replace(file, "\r\n", "\n") + defp do_autofix(file, :unix_to_windows), do: String.replace(file, "\n", "\r\n") end diff --git a/lib/credo/check/readability/alias_order.ex b/lib/credo/check/readability/alias_order.ex index dcd486bd9..2a53e0e49 100644 --- a/lib/credo/check/readability/alias_order.ex +++ b/lib/credo/check/readability/alias_order.ex @@ -224,12 +224,12 @@ defmodule Credo.Check.Readability.AliasOrder do ) end - def autocorrect(file, _issue) do + def autofix(file, _issue) do {:ok, quoted} = :"Elixir.Code".string_to_quoted(file) modified = quoted - |> Macro.prewalk(&do_autocorrect/1) + |> Macro.prewalk(&do_autofix/1) |> Macro.to_string() |> :"Elixir.Code".format_string!() |> to_string() @@ -237,7 +237,7 @@ defmodule Credo.Check.Readability.AliasOrder do "#{modified}\n" end - defp do_autocorrect({:__block__ = op, meta, [{:alias, _, _} | _] = aliases}) do + defp do_autofix({:__block__ = op, meta, [{:alias, _, _} | _] = aliases}) do modified = aliases |> group_aliases() @@ -253,7 +253,7 @@ defmodule Credo.Check.Readability.AliasOrder do {op, Keyword.delete(meta, :line), modified} end - defp do_autocorrect(ast), do: ast + defp do_autofix(ast), do: ast defp put_line_number([{op, meta, args} | tail], line) do modified = {op, Keyword.put(meta, :line, line), args} diff --git a/lib/credo/check/readability/trailing_blank_line.ex b/lib/credo/check/readability/trailing_blank_line.ex index 69cad00ad..0a7226b74 100644 --- a/lib/credo/check/readability/trailing_blank_line.ex +++ b/lib/credo/check/readability/trailing_blank_line.ex @@ -40,7 +40,7 @@ defmodule Credo.Check.Readability.TrailingBlankLine do ) end - def autocorrect(file, _issue) do + def autofix(file, _issue) do "#{file}\n" end end diff --git a/lib/credo/check/refactor/unless_with_else.ex b/lib/credo/check/refactor/unless_with_else.ex index a842f869c..7f8f7ea39 100644 --- a/lib/credo/check/refactor/unless_with_else.ex +++ b/lib/credo/check/refactor/unless_with_else.ex @@ -72,12 +72,12 @@ defmodule Credo.Check.Refactor.UnlessWithElse do ) end - def autocorrect(file, _issue) do + def autofix(file, _issue) do {:ok, quoted} = :"Elixir.Code".string_to_quoted(file) modified = quoted - |> Macro.prewalk(&do_autocorrect/1) + |> Macro.prewalk(&do_autofix/1) |> Macro.to_string() |> :"Elixir.Code".format_string!() |> to_string() @@ -85,9 +85,9 @@ defmodule Credo.Check.Refactor.UnlessWithElse do "#{modified}\n" end - defp do_autocorrect({:unless, meta, [clause, [do: falsy, else: truthy]]}) do + defp do_autofix({:unless, meta, [clause, [do: falsy, else: truthy]]}) do {:if, meta, [clause, [do: truthy, else: falsy]]} end - defp do_autocorrect(ast), do: ast + defp do_autofix(ast), do: ast end diff --git a/lib/credo/check/warning/unused_string_operation.ex b/lib/credo/check/warning/unused_string_operation.ex index e3d6238d3..ad66c525c 100644 --- a/lib/credo/check/warning/unused_string_operation.ex +++ b/lib/credo/check/warning/unused_string_operation.ex @@ -46,7 +46,7 @@ defmodule Credo.Check.Warning.UnusedStringOperation do ) end - def autocorrect(file, _issue) do + def autofix(file, _issue) do {_, quoted} = Credo.Code.ast(file) source_file = SourceFile.parse(file, "nofile") @@ -60,7 +60,7 @@ defmodule Credo.Check.Warning.UnusedStringOperation do modified = quoted - |> Macro.prewalk(&do_autocorrect(&1, unused_calls)) + |> Macro.prewalk(&do_autofix(&1, unused_calls)) |> Macro.to_string() |> :"Elixir.Code".format_string!() |> to_string() @@ -68,7 +68,7 @@ defmodule Credo.Check.Warning.UnusedStringOperation do "#{modified}\n" end - defp do_autocorrect({:__block__, meta, [{:|>, _pipe_meta, pipe_args} | tail] = args}, unused_calls) do + defp do_autofix({:__block__, meta, [{:|>, _pipe_meta, pipe_args} | tail] = args}, unused_calls) do args = if List.last(pipe_args) in unused_calls do [hd(pipe_args) | tail] @@ -79,13 +79,13 @@ defmodule Credo.Check.Warning.UnusedStringOperation do {:__block__, meta, args} end - defp do_autocorrect({:__block__, meta, args}, unused_calls) do + defp do_autofix({:__block__, meta, args}, unused_calls) do {:__block__, meta, Enum.reject(args, & &1 in unused_calls)} end - defp do_autocorrect({:do, node}, unused_calls) do - {:do, do_autocorrect(node, unused_calls)} + defp do_autofix({:do, node}, unused_calls) do + {:do, do_autofix(node, unused_calls)} end - defp do_autocorrect(ast, _), do: ast + defp do_autofix(ast, _), do: ast end diff --git a/lib/credo/cli/command/suggest/suggest_command.ex b/lib/credo/cli/command/suggest/suggest_command.ex index 99b222454..b0d65ce9d 100644 --- a/lib/credo/cli/command/suggest/suggest_command.ex +++ b/lib/credo/cli/command/suggest/suggest_command.ex @@ -30,7 +30,7 @@ defmodule Credo.CLI.Command.Suggest.SuggestCommand do Switch.boolean("read_from_stdin"), Switch.boolean("strict"), Switch.boolean("verbose"), - Switch.boolean("autocorrect"), + Switch.boolean("autofix"), Switch.boolean("watch") ] @@ -42,7 +42,7 @@ defmodule Credo.CLI.Command.Suggest.SuggestCommand do print_before_analysis: [__MODULE__.PrintBeforeInfo], run_analysis: [Task.RunChecks], filter_issues: [Task.SetRelevantIssues], - run_autocorrect: [Task.RunAutocorrect], + run_autofix: [Task.RunAutofix], print_after_analysis: [__MODULE__.PrintResultsAndSummary] ) end diff --git a/lib/credo/cli/task/run_autocorrect.ex b/lib/credo/cli/task/run_autocorrect.ex index c384a6791..ace9839d7 100644 --- a/lib/credo/cli/task/run_autocorrect.ex +++ b/lib/credo/cli/task/run_autocorrect.ex @@ -1,10 +1,10 @@ -defmodule Credo.CLI.Task.RunAutocorrect do +defmodule Credo.CLI.Task.RunAutofix do @moduledoc false use Credo.Execution.Task def call(exec, opts, read_fun \\ &File.read!/1, write_fun \\ &File.write!/2) do - if exec.autocorrect do + if exec.autofix do issues = Keyword.get_lazy(opts, :issues, fn -> Execution.get_issues(exec) end) issues @@ -12,7 +12,7 @@ defmodule Credo.CLI.Task.RunAutocorrect do |> Enum.each(fn {file_path, issues} -> file = read_fun.(file_path) - corrected = Enum.reduce(issues, file, &run_autocorrect(&1, &2, exec)) + corrected = Enum.reduce(issues, file, &run_autofix(&1, &2, exec)) write_fun.(file_path, corrected) end) @@ -27,8 +27,8 @@ defmodule Credo.CLI.Task.RunAutocorrect do end) end - defp run_autocorrect(issue, file, exec) do - case issue.check.autocorrect(file, issue) do + defp run_autofix(issue, file, exec) do + case issue.check.autofix(file, issue) do ^file -> file diff --git a/lib/credo/config_builder.ex b/lib/credo/config_builder.ex index 55e75e2da..e8832b3a5 100644 --- a/lib/credo/config_builder.ex +++ b/lib/credo/config_builder.ex @@ -81,7 +81,7 @@ defmodule Credo.ConfigBuilder do |> add_switch_ignore(switches) |> add_switch_mute_exit_status(switches) |> add_switch_only(switches) - |> add_switch_autocorrect(switches) + |> add_switch_autofix(switches) |> add_switch_read_from_stdin(switches) |> add_switch_strict(switches) |> add_switch_min_priority(switches) @@ -263,13 +263,13 @@ defmodule Credo.ConfigBuilder do defp add_switch_only(exec, _), do: exec - # add_switch_autocorrect + # add_switch_autofix - defp add_switch_autocorrect(exec, %{autocorrect: true}) do - %Execution{exec | autocorrect: true} + defp add_switch_autofix(exec, %{autofix: true}) do + %Execution{exec | autofix: true} end - defp add_switch_autocorrect(exec, _), do: exec + defp add_switch_autofix(exec, _), do: exec # add_switch_ignore diff --git a/lib/credo/execution.ex b/lib/credo/execution.ex index a129ec933..6339d4276 100644 --- a/lib/credo/execution.ex +++ b/lib/credo/execution.ex @@ -29,7 +29,7 @@ defmodule Credo.Execution do plugins: [], parse_timeout: 5000, strict: false, - autocorrect: false, + autofix: false, # options, set by the command line format: nil, diff --git a/test/credo/check/consistency/line_endings_test.exs b/test/credo/check/consistency/line_endings_test.exs index 1bcb3b1c9..18dd8ccc7 100644 --- a/test/credo/check/consistency/line_endings_test.exs +++ b/test/credo/check/consistency/line_endings_test.exs @@ -56,14 +56,14 @@ defmodule Credo.Check.Readability.LineEndingsTest do |> assert_issue end - describe "autocorrect/1" do + describe "autofix/1" do test "switches unix to windows line endings" do starting = "defmodule Credo.Sample do\n@test_attribute :foo\nend\n" expected = "defmodule Credo.Sample do\r\n@test_attribute :foo\r\nend\r\n" issue = %Credo.Issue{message: "File is using unix line endings while most of the files use windows line endings."} - assert @described_check.autocorrect(starting, issue) == expected + assert @described_check.autofix(starting, issue) == expected end test "switches windows to unix line endings" do @@ -77,7 +77,7 @@ defmodule Credo.Check.Readability.LineEndingsTest do issue = %Credo.Issue{message: "File is using windows line endings while most of the files use unix line endings."} - assert @described_check.autocorrect(starting, issue) == expected + assert @described_check.autofix(starting, issue) == expected end end end diff --git a/test/credo/check/readability/alias_order_test.exs b/test/credo/check/readability/alias_order_test.exs index f25303eb9..b15aee85a 100644 --- a/test/credo/check/readability/alias_order_test.exs +++ b/test/credo/check/readability/alias_order_test.exs @@ -229,7 +229,7 @@ defmodule Credo.Check.Readability.AliasOrderTest do end) end - describe "autocorrect/1" do + describe "autofix/1" do test "puts all the aliases in the right order" do starting = """ defmodule CredoSampleModule do @@ -247,7 +247,7 @@ defmodule Credo.Check.Readability.AliasOrderTest do end """ - assert @described_check.autocorrect(starting, nil) == expected + assert @described_check.autofix(starting, nil) == expected end test "works with multi-aliases" do @@ -265,7 +265,7 @@ defmodule Credo.Check.Readability.AliasOrderTest do end """ - assert @described_check.autocorrect(starting, nil) == expected + assert @described_check.autofix(starting, nil) == expected end test "works with as: option" do @@ -283,7 +283,7 @@ defmodule Credo.Check.Readability.AliasOrderTest do end """ - assert @described_check.autocorrect(starting, nil) == expected + assert @described_check.autofix(starting, nil) == expected end test "works with multiple blocks of aliases including multi-aliases" do @@ -310,7 +310,7 @@ defmodule Credo.Check.Readability.AliasOrderTest do end """ - assert @described_check.autocorrect(starting, nil) == expected + assert @described_check.autofix(starting, nil) == expected end end end diff --git a/test/credo/check/refactor/unless_with_else_test.exs b/test/credo/check/refactor/unless_with_else_test.exs index 00858093d..340f947e0 100644 --- a/test/credo/check/refactor/unless_with_else_test.exs +++ b/test/credo/check/refactor/unless_with_else_test.exs @@ -48,7 +48,7 @@ defmodule Credo.Check.Refactor.UnlessWithElseTest do |> assert_issue() end - describe "autocorrect/2" do + describe "autofix/2" do test "changes the unless with else to an if clause" do starting = """ defmodule CredoSampleModule do @@ -78,7 +78,7 @@ defmodule Credo.Check.Refactor.UnlessWithElseTest do end """ - assert @described_check.autocorrect(starting, nil) == expected + assert @described_check.autofix(starting, nil) == expected end end end diff --git a/test/credo/check/warning/unused_string_operation_test.exs b/test/credo/check/warning/unused_string_operation_test.exs index 1b9baaea8..533cda557 100644 --- a/test/credo/check/warning/unused_string_operation_test.exs +++ b/test/credo/check/warning/unused_string_operation_test.exs @@ -721,7 +721,7 @@ defmodule Credo.Check.Warning.UnusedStringOperationTest do end) end - describe "autocorrect/2" do + describe "autofix/2" do test "removes the unused String function" do starting = """ defmodule CredoSampleModule do @@ -744,7 +744,7 @@ defmodule Credo.Check.Warning.UnusedStringOperationTest do end """ - assert @described_check.autocorrect(starting, nil) == expected + assert @described_check.autofix(starting, nil) == expected end test "it should report a violation when end of pipe" do @@ -768,7 +768,7 @@ defmodule Credo.Check.Warning.UnusedStringOperationTest do end """ - assert @described_check.autocorrect(starting, nil) == expected + assert @described_check.autofix(starting, nil) == expected end end end diff --git a/test/credo/cli/task/run_autocorrect_test.exs b/test/credo/cli/task/run_autocorrect_test.exs index 0e3ae6cb3..1400b1ef0 100644 --- a/test/credo/cli/task/run_autocorrect_test.exs +++ b/test/credo/cli/task/run_autocorrect_test.exs @@ -1,41 +1,41 @@ -defmodule Credo.CLI.Task.RunAutocorrectTest do +defmodule Credo.CLI.Task.RunAutofixTest do use Credo.Test.Case - alias Credo.CLI.Task.RunAutocorrect + alias Credo.CLI.Task.RunAutofix defmodule CheckOne do - def autocorrect(file, issue) do + def autofix(file, issue) do send(self(), {:check_one, file, issue}) "check_one" end end defmodule CheckTwo do - def autocorrect(file, issue) do + def autofix(file, issue) do send(self(), {:check_two, file, issue}) "check_two" end end defmodule CheckThree do - def autocorrect(file, issue) do + def autofix(file, issue) do send(self(), {:check_three, file, issue}) "check_three" end end describe "run/2" do - test "calls `autocorrect/1` for each issue with the correct arguments if autocorrect is true" do + test "calls `autofix/1` for each issue with the correct arguments if autofix is true" do issue1 = %Credo.Issue{filename: "a", check: CheckOne} issue2 = %Credo.Issue{filename: "a", check: CheckTwo} issue3 = %Credo.Issue{filename: "b", check: CheckThree} issues = [issue2, issue1, issue3] - exec = %Credo.Execution{autocorrect: true} |> Credo.Execution.ExecutionIssues.start_server() + exec = %Credo.Execution{autofix: true} |> Credo.Execution.ExecutionIssues.start_server() read_fun = fn _ -> "start" end write_fun = fn file_path, contents -> send(self(), {:write, file_path, contents}) end - RunAutocorrect.call(exec, [issues: issues], read_fun, write_fun) + RunAutofix.call(exec, [issues: issues], read_fun, write_fun) assert_receive({:check_one, "start", ^issue1}) assert_receive({:check_two, "check_one", ^issue2}) assert_receive({:check_three, "start", ^issue3}) @@ -43,17 +43,17 @@ defmodule Credo.CLI.Task.RunAutocorrectTest do assert_receive({:write, "b", "check_three"}) end - test "does nothing if autocorrect is false" do + test "does nothing if autofix is false" do issues = [ %Credo.Issue{filename: "a", check: CheckTwo}, %Credo.Issue{filename: "a", check: CheckOne}, %Credo.Issue{filename: "b", check: CheckThree} ] - exec = %Credo.Execution{autocorrect: false} + exec = %Credo.Execution{autofix: false} read_fun = fn _ -> "start" end write_fun = fn file_path, contents -> send(self(), {:write, file_path, contents}) end - RunAutocorrect.call(exec, [issues: issues], read_fun, write_fun) + RunAutofix.call(exec, [issues: issues], read_fun, write_fun) refute_receive(_) end end