Initial spike on Autocorrect

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 autofix(source_file :: String.t(), issue :: Issue.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 autofix(source_file, _issue) do
+        source_file
+      end
+
       defoverridable Credo.Check
 
       defp append_issues_and_timings([] = _issues, exec) do

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 autofix(file, issue) do
+    if issue.message == message_for(:windows) do
+      do_autofix(file, :unix_to_windows)
+    else
+      do_autofix(file, :windows_to_unix)
+    end
+  end
+
+  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

lib/credo/check/readability/alias_order.ex

@@ -223,4 +223,79 @@ defmodule Credo.Check.Readability.AliasOrder do
       line_no: line_no
     )
   end
+
+  def autofix(file, _issue) do
+    {:ok, quoted} = :"Elixir.Code".string_to_quoted(file)
+
+    modified =
+      quoted
+      |> Macro.prewalk(&do_autofix/1)
+      |> Macro.to_string()
+      |> :"Elixir.Code".format_string!()
+      |> to_string()
+
+    "#{modified}\n"
+  end
+
+  defp do_autofix({:__block__ = op, meta, [{:alias, _, _} | _] = aliases}) do
+    modified =
+      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_autofix(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
+
+  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

lib/credo/check/readability/trailing_blank_line.ex

@@ -39,4 +39,8 @@ defmodule Credo.Check.Readability.TrailingBlankLine do
       line_no: line_no
     )
   end
+
+  def autofix(file, _issue) do
+    "#{file}\n"
+  end
 end

lib/credo/check/refactor/unless_with_else.ex

@@ -71,4 +71,23 @@ defmodule Credo.Check.Refactor.UnlessWithElse do
       line_no: line_no
     )
   end
+
+  def autofix(file, _issue) do
+    {:ok, quoted} = :"Elixir.Code".string_to_quoted(file)
+
+    modified =
+      quoted
+      |> Macro.prewalk(&do_autofix/1)
+      |> Macro.to_string()
+      |> :"Elixir.Code".format_string!()
+      |> to_string()
+
+    "#{modified}\n"
+  end
+
+  defp do_autofix({:unless, meta, [clause, [do: falsy, else: truthy]]}) do
+    {:if, meta, [clause, [do: truthy, else: falsy]]}
+  end
+
+  defp do_autofix(ast), do: ast
 end

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 autofix(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_autofix(&1, unused_calls))
+      |> Macro.to_string()
+      |> :"Elixir.Code".format_string!()
+      |> to_string()
+
+    "#{modified}\n"
+  end
+
+  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]
+      else
+        args
+      end
+
+    {:__block__, meta, args}
+  end
+
+  defp do_autofix({:__block__, meta, args}, unused_calls) do
+    {:__block__, meta, Enum.reject(args, & &1 in unused_calls)}
+  end
+
+  defp do_autofix({:do, node}, unused_calls) do
+    {:do, do_autofix(node, unused_calls)}
+  end
+
+  defp do_autofix(ast, _), do: ast
 end

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("autofix"),
       Switch.boolean("watch")
     ]
 
@@ -41,6 +42,7 @@ defmodule Credo.CLI.Command.Suggest.SuggestCommand do
       print_before_analysis: [__MODULE__.PrintBeforeInfo],
       run_analysis: [Task.RunChecks],
       filter_issues: [Task.SetRelevantIssues],
+      run_autofix: [Task.RunAutofix],
       print_after_analysis: [__MODULE__.PrintResultsAndSummary]
     )
   end

lib/credo/cli/task/run_autocorrect.ex

@@ -0,0 +1,40 @@
+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.autofix do
+      issues = Keyword.get_lazy(opts, :issues, fn -> Execution.get_issues(exec) end)
+
+      issues
+      |> group_by_file
+      |> Enum.each(fn {file_path, issues} ->
+        file = read_fun.(file_path)
+
+        corrected = Enum.reduce(issues, file, &run_autofix(&1, &2, exec))
+
+        write_fun.(file_path, corrected)
+      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
+
+  defp run_autofix(issue, file, exec) do
+    case issue.check.autofix(file, issue) do
+      ^file ->
+        file
+
+      corrected_file ->
+        Execution.remove_issue(exec, issue)
+        corrected_file
+    end
+  end
+end

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_autofix(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_autofix
+
+  defp add_switch_autofix(exec, %{autofix: true}) do
+    %Execution{exec | autofix: true}
+  end
+
+  defp add_switch_autofix(exec, _), do: exec
+
   # add_switch_ignore
 
   # exclude/ignore certain checks

lib/credo/execution.ex

@@ -29,6 +29,7 @@ defmodule Credo.Execution do
             plugins: [],
             parse_timeout: 5000,
             strict: false,
+            autofix: false,
 
             # options, set by the command line
             format: nil,
@@ -493,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.
   """

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

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 "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.autofix(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.autofix(starting, issue) == expected
+    end
+  end
 end