Add new AliasOrder check

.credo.exs

@@ -82,6 +82,7 @@
         #
         ## Readability Checks
         #
+        {Credo.Check.Readability.AliasOrder},
         {Credo.Check.Readability.FunctionNames},
         {Credo.Check.Readability.LargeNumbers},
         {Credo.Check.Readability.MaxLineLength, priority: :low, max_length: 80},

lib/credo/check/readability/alias_order.ex

@@ -0,0 +1,174 @@
+defmodule Credo.Check.Readability.AliasOrder do
+  @moduledoc """
+  Alphabetically ordered lists are more easily scannable by the read.
+
+      # preferred
+
+      alias Module1
+      alias Module2
+      alias Module3
+
+      # NOT preferred
+
+      alias Module1
+      alias Module3
+      alias Module2
+
+  Alias should be alphabetically ordered among their group:
+
+      # preferred
+
+      alias Module3
+      alias Module4
+
+      alias Module1
+      alias Module2
+
+      # NOT preferred
+
+      alias Module3
+      alias Module4
+
+      alias Module2
+      alias Module1
+
+  Like all `Readability` issues, this one is not a technical concern.
+  But you can improve the odds of others reading and liking your code by making
+  it easier to follow.
+  """
+
+  @explanation [check: @moduledoc]
+
+  alias Credo.Code
+  alias Credo.Code.Name
+
+  use Credo.Check, base_priority: :high
+
+  @doc false
+  def run(source_file, params \\ []) do
+    issue_meta = IssueMeta.for(source_file, params)
+
+    Code.prewalk(source_file, &traverse(&1, &2, issue_meta))
+  end
+
+  defp traverse({:defmodule, _, _} = ast, issues, issue_meta) do
+    new_issues =
+      ast
+      |> extract_alias_groups()
+      |> Enum.reduce([], &traverse_groups(&1, &2, issue_meta))
+
+    {ast, issues ++ new_issues}
+  end
+
+  defp traverse(ast, issues, _), do: {ast, issues}
+
+  defp traverse_groups(group, acc, issue_meta) do
+    chunk_method =
+      if function_exported?(Enum, :chunk_every, 2) do
+        :chunk_every
+      else
+        :chunk
+      end
+
+    group
+    |> (&apply(Enum, chunk_method, [&1, 2, 1])).()
+    |> Enum.reduce_while(nil, &process_group/2)
+    |> case do
+      nil ->
+        acc
+
+      {line_no, trigger} ->
+        acc ++ [issue_for(issue_meta, line_no, trigger)]
+    end
+  end
+
+  defp process_group([{_, first_line_content}, {_, second_line_content} = second_line], _)
+       when first_line_content > second_line_content,
+       do: {:halt, second_line}
+
+  defp process_group(_, _), do: {:cont, nil}
+
+  defp extract_alias_groups({:defmodule, _, _} = ast) do
+    ast
+    |> Code.postwalk(&find_alias_groups/2)
+    |> Enum.reverse()
+    |> Enum.reduce([[]], fn definition, acc ->
+      case definition do
+        nil ->
+          [[]] ++ acc
+
+        definition ->
+          [group | groups] = acc
+          [group ++ [definition]] ++ groups
+      end
+    end)
+    |> Enum.reverse()
+  end
+
+  defp find_alias_groups(
+         {:alias, _, [{:__aliases__, [line: line, column: _], mod_list}]} = ast,
+         aliases
+       ),
+       do: process_single_alias(ast, line, mod_list, aliases)
+
+  defp find_alias_groups({:alias, _, [{:__aliases__, [line: line], mod_list}]} = ast, aliases),
+    do: process_single_alias(ast, line, mod_list, aliases)
+
+  defp find_alias_groups(
+         {:alias, _,
+          [
+            {{:., _, [{:__aliases__, [line: line, column: _], mod_list}, :{}]}, _, multi_mod_list}
+          ]} = ast,
+         aliases
+       ),
+       do: process_multi_alias(ast, line, mod_list, multi_mod_list, aliases)
+
+  defp find_alias_groups(
+         {:alias, _,
+          [
+            {{:., _, [{:__aliases__, [line: line], mod_list}, :{}]}, _, multi_mod_list}
+          ]} = ast,
+         aliases
+       ),
+       do: process_multi_alias(ast, line, mod_list, multi_mod_list, aliases)
+
+  defp find_alias_groups(ast, aliases), do: {ast, aliases}
+
+  defp process_multi_alias(ast, line, mod_list, multi_mod_list, aliases) do
+    modules =
+      multi_mod_list
+      |> Enum.map(&{line, Name.full([Name.full(mod_list), Name.full(&1)])})
+      |> Enum.reverse()
+
+    accumulate_alias_into_group(ast, modules, line, aliases)
+  end
+
+  defp process_single_alias(ast, line, mod_list, aliases) do
+    modules =
+      mod_list
+      |> Name.full()
+      |> List.wrap()
+      |> Enum.join()
+      |> (&[{line, &1}]).()
+
+    accumulate_alias_into_group(ast, modules, line, aliases)
+  end
+
+  defp accumulate_alias_into_group(ast, modules, line, [{line_no, _} | _] = aliases)
+       when line_no != 0 and line_no != line - 1 do
+    {ast, modules ++ [nil] ++ aliases}
+  end
+
+  defp accumulate_alias_into_group(ast, modules, _, aliases) do
+    {ast, modules ++ aliases}
+  end
+
+  defp issue_for(issue_meta, line_no, trigger) do
+    format_issue(
+      issue_meta,
+      message: "The alias `#{trigger}` is not alphabetically ordered among its group.",
+      trigger: trigger,
+      line_no: line_no
+    )
+  end
+end

test/credo/check/readability/alias_order_test.exs

@@ -3,8 +3,6 @@ defmodule Credo.Check.Readability.AliasOrderTest do
 
   @described_check Credo.Check.Readability.AliasOrder
 
-  @moduletag :to_be_implemented
-
   #
   # cases NOT raising issues
   #
@@ -38,6 +36,24 @@ defmodule Credo.Check.Readability.AliasOrderTest do
     |> refute_issues(@described_check)
   end
 
+  test "it should NOT report violation for multi-aliases when they are alpha-ordered" do
+    """
+    defmodule Test do
+      alias App.CLI.{Filename,Sorter}
+      alias App.Foo.{Bar,Baz}
+
+      alias Credo.CLI.Command
+      alias Credo.CLI.Filename
+      alias Credo.CLI.Sorter
+
+      alias App.Module1
+      alias App.Module2
+    end
+    """
+    |> to_source_file
+    |> refute_issues(@described_check)
+  end
+
   test "it should work with __MODULE__" do
     """
     defmodule Test do
@@ -68,7 +84,7 @@ defmodule Credo.Check.Readability.AliasOrderTest do
     |> assert_issue(@described_check)
   end
 
-  test "it should report a violation /2" do
+  test "it should report a violation with alias groups" do
     """
     defmodule CredoSampleModule do
       alias Credo.CLI.Command
@@ -82,4 +98,22 @@ defmodule Credo.Check.Readability.AliasOrderTest do
     |> to_source_file
     |> assert_issue(@described_check)
   end
+
+  test "it should report a violation with multi-alias" do
+    """
+    defmodule CredoSampleModule do
+      alias App.CLI.{Bar,Baz}
+      alias App.Foo.{Sorter,Command,Filename}
+
+      alias Credo.CLI.Command
+      alias Credo.CLI.Filename
+      alias Credo.CLI.Sorter
+
+      alias App.Module1
+      alias App.Module2
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
 end