Skip to content

Commit

Permalink
fix: MultiAliasImportRequireUse collection
Browse files Browse the repository at this point in the history
- prevents false positives when multiple modules exist in one file
  • Loading branch information
msutkowski committed Nov 19, 2023
1 parent a3c0608 commit 2b0b797
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,34 @@ defmodule Credo.Check.Consistency.MultiAliasImportRequireUse.Collector do

def collect_matches(source_file, _params) do
source_file
|> Credo.Code.prewalk(&traverse/2, [])
|> group_usages
|> count_occurrences
|> Credo.Code.prewalk(&traverse/2, {%{}, nil})
|> elem(0)
|> Enum.map(fn {_mod_name, aliases} ->
aliases
|> group_usages
|> count_occurrences
end)
|> merge_module_stats()
end

def find_locations_not_matching(expected, source_file) do
source_file
|> Credo.Code.prewalk(&traverse/2, [])
|> group_usages
|> drop_locations(expected)
|> Credo.Code.prewalk(&traverse/2, {%{}, nil})
|> elem(0)
|> Enum.map(fn {_mod_name, aliases} ->
aliases
|> group_usages
|> drop_locations(expected)
end)
|> List.flatten()
end

defp traverse({directive, meta, arguments} = ast, acc)
when directive in @directives do
defp traverse({:defmodule, _, [{:__aliases__, _, mod_name} | _]} = ast, {acc, _}) do
{ast, {Map.put(acc, mod_name, []), mod_name}}
end

defp traverse({directive, meta, arguments} = ast, {acc, current_module})
when directive in @directives and not is_nil(current_module) do
aliases =
case arguments do
[{:__aliases__, _, nested_modules}] when length(nested_modules) > 1 ->
Expand All @@ -35,9 +49,12 @@ defmodule Credo.Check.Consistency.MultiAliasImportRequireUse.Collector do
end

if aliases do
{ast, [{directive, aliases, meta[:line]} | acc]}
updated_acc = Map.update(acc, current_module, [], fn entries ->
[{directive, aliases, meta[:line]} | entries]
end)
{ast, {updated_acc, current_module}}
else
{ast, acc}
{ast, {acc, current_module}}
end
end

Expand Down Expand Up @@ -94,4 +111,12 @@ defmodule Credo.Check.Consistency.MultiAliasImportRequireUse.Collector do

{:lists.reverse(acc1), :lists.reverse(acc2)}
end

defp merge_module_stats(stats_list) do
Enum.reduce(stats_list, %{}, fn stats, acc ->
Enum.reduce(stats, acc, fn {key, val}, acc ->
Map.update(acc, key, val, &(&1 + val))
end)
end)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,32 @@ defmodule Credo.Check.Consistency.MultiAliasImportRequireUseTest do
require Foo.Quux
end
"""
@multi_module_same_file """
defmodule CredoMultiAliasExample.SetMultiAliasToSingles do
@moduledoc "This modules does many aliases to set the consistency to multi-alias"
alias Credo.CLI.{Command, Output}
alias Config.{Reader, Provider}
end
defmodule CredoMultiAliasExample.Foo do
@moduledoc "This module has a function"
def test, do: :ok
end
defmodule CredoMultiAliasExample.Bar do
@moduledoc "This module aliases another module"
alias CredoMultiAliasExample.Foo
end
defmodule CredoMultiAliasExample.Baz do
@moduledoc "This module aliases a different module under the same parent"
alias CredoMultiAliasExample.Bar
end
"""

#
# cases NOT raising issues
Expand All @@ -36,6 +62,13 @@ defmodule Credo.Check.Consistency.MultiAliasImportRequireUseTest do
|> refute_issues()
end

test "it should NOT report errors when there are multiple modules in the same file when the multi syntax is used consistently" do
[@multi_module_same_file]
|> to_source_files
|> run_check(@described_check)
|> refute_issues()
end

test "it should NOT report errors when the single syntax is used consistently" do
[@single]
|> to_source_files
Expand All @@ -50,6 +83,13 @@ defmodule Credo.Check.Consistency.MultiAliasImportRequireUseTest do
|> refute_issues()
end

test "it should not report errors when the single syntax is used consistently" do
[@single]
|> to_source_files
|> run_check(@described_check)
|> refute_issues()
end

#
# cases raising issues
#
Expand All @@ -61,10 +101,10 @@ defmodule Credo.Check.Consistency.MultiAliasImportRequireUseTest do
|> assert_issue()
end

test "it should not report errors when the single syntax is used consistently" do
[@single]
test "it should report errors when the multi and single syntaxes are mixed (two files, one multi-module)" do
[@single, @multi_module_same_file]
|> to_source_files
|> run_check(@described_check)
|> refute_issues()
|> assert_issue()
end
end

0 comments on commit 2b0b797

Please sign in to comment.