From 2b0b7976bdbd146de25bf9ce5904932af14d06fd Mon Sep 17 00:00:00 2001
From: Matt Sutkowski <msutkowski@gmail.com>
Date: Sat, 18 Nov 2023 17:27:22 -0800
Subject: [PATCH] fix: MultiAliasImportRequireUse collection

- prevents false positives when multiple modules exist in one file
---
 .../collector.ex                              | 45 ++++++++++++++----
 .../multi_alias_import_require_use_test.exs   | 46 +++++++++++++++++--
 2 files changed, 78 insertions(+), 13 deletions(-)

diff --git a/lib/credo/check/consistency/multi_alias_import_require_use/collector.ex b/lib/credo/check/consistency/multi_alias_import_require_use/collector.ex
index fc6f6af27..0c499780d 100644
--- a/lib/credo/check/consistency/multi_alias_import_require_use/collector.ex
+++ b/lib/credo/check/consistency/multi_alias_import_require_use/collector.ex
@@ -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 ->
@@ -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
 
@@ -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
diff --git a/test/credo/check/consistency/multi_alias_import_require_use_test.exs b/test/credo/check/consistency/multi_alias_import_require_use_test.exs
index 010d4043f..27f55f06c 100644
--- a/test/credo/check/consistency/multi_alias_import_require_use_test.exs
+++ b/test/credo/check/consistency/multi_alias_import_require_use_test.exs
@@ -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
@@ -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
@@ -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
   #
@@ -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