Repository: rrrene/credo
Pull request #707 (2019-10-18 00:18:43)

Add check to ensure aliases and requires are together in a file.
Adds check to ensure aliases and requires are together in a file.

For example:
```
    defmodule Foo do
      require Logger
      alias Foo.Bar

      alias Foo.Baz
      require Integer
      ...
    end
```
should be changed to:
```
    defmodule Foo do
      require Integer
      require Logger

      alias Foo.Bar
      alias Foo.Baz
```
Will fail check if aliases appear on non-consecutive lines within a file, or if requires do likewise.
Ignores code in quotes to allow separate aliases/requires in things like `__using__`.
Accepts multiline multi-alias including surrounding bracket line and newline, and an alias-as that wraps including surrounding newlines.


Patch:
diff --git a/.credo.exs b/.credo.exs
index 231035d2..f05da7ba 100644
--- a/.credo.exs
+++ b/.credo.exs
@@ -93,6 +93,7 @@
         {Credo.Check.Readability.PreferImplicitTry, []},
         {Credo.Check.Readability.RedundantBlankLines, []},
         {Credo.Check.Readability.Semicolons, []},
+        {Credo.Check.Readability.SeparateAliasRequire, []},
         {Credo.Check.Readability.SpaceAfterCommas, []},
         {Credo.Check.Readability.StringSigils, []},
         {Credo.Check.Readability.TrailingBlankLine, []},
diff --git a/lib/credo/check/readability/separate_alias_import_require_use.ex b/lib/credo/check/readability/separate_alias_import_require_use.ex
new file mode 100644
index 00000000..1b785c26
--- /dev/null
+++ b/lib/credo/check/readability/separate_alias_import_require_use.ex
@@ -0,0 +1,139 @@
+defmodule Credo.Check.Readability.SeparateAliasRequire do
+  @checkdoc """
+    All instances of alias should be consecutive within a file.
+    Likewise, all instances of require should be consecutive within a file.
+
+    For example:
+
+    defmodule Foo do
+      require Logger
+      alias Foo.Bar
+
+      alias Foo.Baz
+      require Integer
+
+      ...
+    end
+
+    should be changed to:
+
+    defmodule Foo do
+      require Integer
+      require Logger
+
+      alias Foo.Bar
+      alias Foo.Baz
+
+      ...
+    end
+  """
+
+  @explanation [
+    check: @checkdoc,
+    params: []
+  ]
+  @default_params []
+
+  # you can configure the basics of your check via the `use Credo.Check` call
+  use Credo.Check, base_priority: :low
+
+  alias Credo.Code
+
+  @doc false
+  def run(source_file, params \\ []) do
+    issue_meta = IssueMeta.for(source_file, params)
+
+    line_map =
+      source_file
+      |> to_types()
+      |> Enum.into(%{})
+
+    [:alias, :require]
+    |> Enum.map(&find_issue(&1, line_map, issue_meta))
+    |> Enum.reject(&is_nil/1)
+  end
+
+  defp to_types(source_file) do
+    Code.prewalk(source_file, &traverse(&1, &2))
+  end
+
+  defp line_number({_, opts, _}), do: Keyword.get(opts, :line)
+
+  defp traverse({:quote, _, _} = _ast, acc) do
+    {nil, acc}
+  end
+
+  defp traverse({:alias, _, [{:__aliases__, _, _}, [as: as_ast]]} = ast, acc) do
+    alias_line_number = line_number(ast)
+    as_line_number = line_number(as_ast)
+
+    if alias_line_number == as_line_number do
+      {ast, [{alias_line_number, :alias} | acc]}
+    else
+      lines = [alias_line_number - 1, alias_line_number, as_line_number, as_line_number + 1]
+      {ast, acc ++ Enum.map(lines, &{&1, :alias})}
+    end
+  end
+
+  defp traverse(
+         {:alias, _, [{{_, _, [{:__aliases__, _, _base_alias}, :{}]}, _, multi_aliases}]} = ast,
+         acc
+       ) do
+    base_line_number = line_number(ast)
+
+    acc =
+      case multi_aliases do
+        [] ->
+          acc
+
+        [hd | _] ->
+          if line_number(hd) == base_line_number do
+            # single line multi-alias
+            [{base_line_number, :alias} | acc]
+          else
+            # multiple line multi-alias
+            lines = Enum.map(multi_aliases, &line_number/1)
+
+            max = Enum.max(lines)
+            lines = [max + 1, max + 2, base_line_number, base_line_number - 1 | lines]
+            acc ++ Enum.map(lines, &{&1, :alias})
+          end
+      end
+
+    {ast, acc}
+  end
+
+  defp traverse({type, _, [{:__aliases__, _, _} | _]} = ast, acc)
+       when type in [:alias, :require],
+       do: {ast, [{line_number(ast), type} | acc]}
+
+  defp traverse(ast, acc) do
+    {ast, acc}
+  end
+
+  defp find_issue(type, line_map, issue_meta) do
+    line_map
+    |> Enum.filter(&(elem(&1, 1) == type))
+    |> Enum.map(&elem(&1, 0))
+    |> Enum.sort()
+    |> is_issue(type, issue_meta)
+  end
+
+  defp is_issue([], _, _), do: nil
+  defp is_issue([_], _, _), do: nil
+
+  defp is_issue([x, y | tl], type, issue_meta) when x == y - 1,
+    do: is_issue([y | tl], type, issue_meta)
+
+  defp is_issue([_x, y | _], type, issue_meta), do: issue_for(issue_meta, y, type)
+
+  defp issue_for(issue_meta, line_no, type) do
+    format_issue(issue_meta,
+      message: "#{plural(type)} should be consecutive within a file",
+      line_no: line_no
+    )
+  end
+
+  defp plural(:alias), do: "aliases"
+  defp plural(:require), do: "requires"
+end


Test patch:
diff --git a/test/credo/check/readability/separate_alias_import_require_use_test.exs b/test/credo/check/readability/separate_alias_import_require_use_test.exs
new file mode 100644
index 00000000..296096a3
--- /dev/null
+++ b/test/credo/check/readability/separate_alias_import_require_use_test.exs
@@ -0,0 +1,160 @@
+defmodule Credo.Check.Readability.SeparateAliasImportRequireUseTest do
+  use Credo.TestHelper
+
+  @described_check Credo.Check.Readability.SeparateAliasRequire
+
+  test "it should NOT report violation on consecutive aliases" do
+    """
+    defmodule Test do
+      alias App.Module1
+      alias App.Module2
+    end
+    """
+    |> to_source_file
+    |> refute_issues(@described_check)
+  end
+
+  test "it should report violation on separate aliases" do
+    """
+    defmodule Test do
+      alias App.Module1
+
+      alias App.Module2
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should NOT report violation on consecutive single-line multi-aliases" do
+    """
+    defmodule Test do
+      alias App.{Module1, Module2}
+      alias App.Module2
+    end
+    """
+    |> to_source_file
+    |> refute_issues(@described_check)
+  end
+
+  test "it should report violation on separate single-line multi-aliases" do
+    """
+    defmodule Test do
+      alias App.{Module1, Module2}
+
+      alias App.Module2
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should NOT report violation on consecutive multi-line multi-aliases" do
+    """
+    defmodule Test do
+      alias App
+
+      alias App.{
+        Module1,
+        Module2
+      }
+
+      alias App.Module3
+    end
+    """
+    |> to_source_file
+    |> refute_issues(@described_check)
+  end
+
+  test "it should report violation on separate multi-line multi-aliases" do
+    """
+    defmodule Test do
+      alias App
+
+      alias App.{
+        Module1,
+        Module2
+      }
+
+
+      alias App.Module3
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should NOT report violation on consecutive requires" do
+    """
+    defmodule Test do
+      require App.Module1
+      require App.Module2
+    end
+    """
+    |> to_source_file
+    |> refute_issues(@described_check)
+  end
+
+  test "it should report violation on separate requires" do
+    """
+    defmodule Test do
+      require App.Module1
+
+      require App.Module2
+    end
+    """
+    |> to_source_file
+    |> assert_issue(@described_check)
+  end
+
+  test "it should not report violation on functions named require or alias" do
+    """
+    defmodule Test do
+      alias Foo
+      require Foo
+
+      defp require do
+        :foo
+      end
+
+      defp alias do
+        :foo
+      end
+    end
+    """
+    |> to_source_file
+    |> refute_issues(@described_check)
+  end
+
+  test "it should not report violation on multiline alias as" do
+    """
+    defmodule Test do
+      alias App.Module1
+
+      alias App.Module2,
+        as: Module3
+
+      alias App.Module4
+    end
+    """
+    |> to_source_file
+    |> refute_issues(@described_check)
+  end
+
+  test "it should not report violation on macro quotes" do
+    """
+    defmodule Test do
+      alias App.Module1
+
+      defmacro __using__ do
+        quote do
+          alias App.Module2
+        end
+      end
+
+    end
+    """
+    |> to_source_file
+    |> refute_issues(@described_check)
+  end
+end