diff --git a/.credo.exs b/.credo.exs index baa1ea244..c78136331 100644 --- a/.credo.exs +++ b/.credo.exs @@ -95,6 +95,7 @@ ## Readability Checks # {Credo.Check.Readability.AliasOrder, []}, + {Credo.Check.Readability.DuplicatedAliases, []}, {Credo.Check.Readability.FunctionNames, []}, {Credo.Check.Readability.LargeNumbers, []}, {Credo.Check.Readability.MaxLineLength, [priority: :low, max_length: 120]}, diff --git a/lib/credo/check/readability/duplicated_aliases.ex b/lib/credo/check/readability/duplicated_aliases.ex new file mode 100644 index 000000000..8a2e256b3 --- /dev/null +++ b/lib/credo/check/readability/duplicated_aliases.ex @@ -0,0 +1,57 @@ +defmodule Credo.Check.Readability.DuplicatedAliases do + use Credo.Check, + base_priority: :low, + category: :readability, + explanations: [ + check: """ + Sometimes during code reviews in large projects with modules that use many + aliases, there can be issues when solving conflicts and some duplicated + may end up not being noticed by reviewers and get merged into the main + branch. + + These duplicated alias can accumulate over many different files over time + and make the aliases section of a file larger and more confusing. + """ + ] + + alias Credo.SourceFile + + def run(source_file, params \\ []) do + issue_meta = IssueMeta.for(source_file, params) + source_ast = SourceFile.ast(source_file) + + {_, {_, _, issues}} = Macro.prewalk(source_ast, {%{}, issue_meta, []}, &traverse(&1, &2)) + issues + end + + defp traverse( + {:alias, _, [{:__aliases__, meta, alias_} | _]} = ast, + {cache, issue_meta, issues} + ) do + if Map.has_key?(cache, alias_) do + existing_alias_meta = Map.fetch!(cache, alias_) + issue = build_issue(alias_, meta[:line], existing_alias_meta[:line], issue_meta) + {ast, {cache, issue_meta, [issue | issues]}} + else + {ast, {Map.put(cache, alias_, meta), issue_meta, issues}} + end + end + + defp traverse(ast, acc), do: {ast, acc} + + defp build_issue(trigger, line_no, existing_alias_line_no, issue_meta) do + format_issue( + issue_meta, + message: + "Duplicated alias: #{format_alias(trigger)}, already defined in line #{existing_alias_line_no}", + trigger: format_alias(trigger), + line_no: line_no + ) + end + + defp format_alias(a) do + a + |> List.wrap() + |> Enum.join(".") + end +end diff --git a/test/credo/check/readability/duplicated_aliases_test.exs b/test/credo/check/readability/duplicated_aliases_test.exs new file mode 100644 index 000000000..4854cab8f --- /dev/null +++ b/test/credo/check/readability/duplicated_aliases_test.exs @@ -0,0 +1,80 @@ +defmodule Credo.Check.Readability.DuplicatedAliasesTest do + use Credo.Test.Case + + @described_check Credo.Check.Readability.DuplicatedAliases + + test "should raise an issue for duplicated aliases" do + file = """ + defmodule M1 do + alias URI + alias URI + end + """ + + file + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "should raise an issue for duplicated nested aliases" do + file = """ + defmodule M1 do + alias IO.ANSI + alias IO.ANSI + end + """ + + file + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "should raise an issue if duplicated alias in function" do + file = """ + defmodule M1 do + alias IO.ANSI + + def test do + alias IO.ANSI + + :ok + end + end + """ + + file + |> to_source_file + |> run_check(@described_check) + |> assert_issue() + end + + test "should NOT raise an issue for single line alias + duplicated multi-alias" do + file = """ + defmodule M1 do + alias IO.ANSI + alias {IO.ANSI, URI} + end + """ + + file + |> to_source_file + |> run_check(@described_check) + |> refute_issues() + end + + test "should NOT raise an issue for duplicated alias between multi-aliases" do + file = """ + defmodule M1 do + alias {IO.ANSI, URI} + alias {File, IO.ANSI} + end + """ + + file + |> to_source_file + |> run_check(@described_check) + |> refute_issues() + end +end