Skip to content

Commit

Permalink
Merge pull request #1072 from viniciusmuller/add-duplicated-alias-check
Browse files Browse the repository at this point in the history
Add duplicated alias check
  • Loading branch information
rrrene authored Feb 8, 2024
2 parents a2d15a5 + 600a730 commit 311ac71
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 0 deletions.
1 change: 1 addition & 0 deletions .credo.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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]},
Expand Down
57 changes: 57 additions & 0 deletions lib/credo/check/readability/duplicated_aliases.ex
Original file line number Diff line number Diff line change
@@ -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
80 changes: 80 additions & 0 deletions test/credo/check/readability/duplicated_aliases_test.exs
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 311ac71

Please sign in to comment.