From 3cb10f2410fda70277750e8395722a1cd5829822 Mon Sep 17 00:00:00 2001
From: Matt Enlow <enlow@adobe.com>
Date: Mon, 8 Apr 2024 11:14:30 -0600
Subject: [PATCH] fix it up

---
 .formatter.exs                                |  1 +
 lib/style/module_directives.ex                | 61 +++++++++++++++----
 .../module_directives/alias_lifting_test.exs  | 23 ++++---
 3 files changed, 64 insertions(+), 21 deletions(-)

diff --git a/.formatter.exs b/.formatter.exs
index b41a4519..77c8bc58 100644
--- a/.formatter.exs
+++ b/.formatter.exs
@@ -8,5 +8,6 @@
     assert_style: 2
   ],
   plugins: [Styler],
+  styler: [alias_lifting_exclude: []],
   line_length: 122
 ]
diff --git a/lib/style/module_directives.ex b/lib/style/module_directives.ex
index bbe95dac..ad9177b4 100644
--- a/lib/style/module_directives.ex
+++ b/lib/style/module_directives.ex
@@ -27,10 +27,12 @@ defmodule Styler.Style.ModuleDirectives do
     * `Credo.Check.Readability.UnnecessaryAliasExpansion`
     * `Credo.Check.Design.AliasUsage`
 
-  ## Strict Layout
+  ## Breakages
 
   **This can break your code.**
 
+  ### Strict Layout: alias referents
+
   Modules directives are sorted into the following order:
 
     * `@shortdoc`
@@ -63,6 +65,26 @@ defmodule Styler.Style.ModuleDirectives do
     ```
 
   For now, it's up to you to come up with a fix for this issue. Sorry!
+
+  ### Strict Layout: interwoven conflicting aliases
+
+  Ideally no one writes code like this as it's hard for our human brains to notice the context switching!
+  Still, it's a possible source of breakages in Styler.
+
+
+    alias Foo.Bar
+    Bar.Baz.bop()
+
+    alias Baz.Bar
+    Bar.Baz.bop()
+
+    # becomes
+
+    alias Baz.Bar
+    alias Baz.Bar.Baz
+    alias Foo.Bar
+    Baz.bop() # was Foo.Bar.Baz, is now Baz.Bar.Baz
+    Baz.bop()
   """
   @behaviour Styler.Style
 
@@ -223,14 +245,16 @@ defmodule Styler.Style.ModuleDirectives do
   end
 
   defp lift_aliases(aliases, requires, nondirectives) do
-    excluded =
-      Enum.reduce(aliases, Styler.Config.get(:lifting_excludes), fn
-        {:alias, _, [{:__aliases__, _, aliases}]}, excluded -> MapSet.put(excluded, List.last(aliases))
-        {:alias, _, [{:__aliases__, _, _}, [{_as, {:__aliases__, _, [as]}}]]}, excluded -> MapSet.put(excluded, as)
+    aliasing =
+      Map.new(aliases, fn
+        {:alias, _, [{:__aliases__, _, aliases}]} -> {List.last(aliases), aliases}
+        {:alias, _, [{:__aliases__, _, aliases}, [{_as, {:__aliases__, _, [as]}}]]} -> {as, aliases}
         # `alias __MODULE__` or other oddities
-        {:alias, _, _}, excluded -> excluded
+        {:alias, _, _} -> {nil, nil}
       end)
 
+    excluded = aliasing |> Map.keys() |> MapSet.new() |> MapSet.union(Styler.Config.get(:lifting_excludes))
+
     liftable = find_liftable_aliases(requires ++ nondirectives, excluded)
 
     if Enum.any?(liftable) do
@@ -238,7 +262,19 @@ defmodule Styler.Style.ModuleDirectives do
       # the `cap_line` algo was designed to handle high-line stuff moving up into low line territory, so we set our
       # new node to have an abritrarily high line annnnd comments behave! i think.
       line = 99_999
-      new_aliases = Enum.map(liftable, &{:alias, [line: line], [{:__aliases__, [last: [line: line], line: line], &1}]})
+
+      new_aliases =
+        Enum.map(liftable, fn [first | rest] = modules ->
+          # if there's an existing alias for this alias, make sure the new one we make is the compound alias
+          modules =
+            case aliasing[first] do
+              nil -> modules
+              parent_alias -> parent_alias ++ rest
+            end
+
+          {:alias, [line: line], [{:__aliases__, [last: [line: line], line: line], modules}]}
+        end)
+
       aliases = expand_and_sort(aliases ++ new_aliases)
       requires = do_lift_aliases(requires, liftable)
       nondirectives = do_lift_aliases(nondirectives, liftable)
@@ -272,10 +308,13 @@ defmodule Styler.Style.ModuleDirectives do
       {{:quote, _, _}, _} = zipper, acc ->
         {:skip, zipper, acc}
 
-      {{:__aliases__, _, [_, _, _ | _] = aliases}, _} = zipper, {lifts, excluded} = acc ->
-        if List.last(aliases) in excluded or List.first(aliases) in excluded or not Enum.all?(aliases, &is_atom/1),
-          do: {:skip, zipper, acc},
-          else: {:skip, zipper, {Map.update(lifts, aliases, false, fn _ -> true end), excluded}}
+      {{:__aliases__, _, [_, _, _ | _] = modlist}, _} = zipper, {lifts, excluded} = acc ->
+        acc =
+          if Enum.any?(modlist, &(not is_atom(&1))) or List.last(modlist) in excluded,
+            do: acc,
+            else: {Map.update(lifts, modlist, false, fn _ -> true end), excluded}
+
+        {:skip, zipper, acc}
 
       zipper, acc ->
         {:cont, zipper, acc}
diff --git a/test/style/module_directives/alias_lifting_test.exs b/test/style/module_directives/alias_lifting_test.exs
index 5187db84..1c360b03 100644
--- a/test/style/module_directives/alias_lifting_test.exs
+++ b/test/style/module_directives/alias_lifting_test.exs
@@ -136,19 +136,22 @@ defmodule Styler.Style.ModuleDirectives.AliasLiftingTest do
   end
 
   test "deep nesting of an alias" do
-    assert_style("""
-    alias Foo.Bar.Baz
+    assert_style(
+      """
+      alias Foo.Bar.Baz
 
-    Baz.Bop.Boom.wee()
-    Baz.Bop.Boom.wee()
+      Baz.Bop.Boom.wee()
+      Baz.Bop.Boom.wee()
 
-    """, """
-    alias Foo.Bar.Baz
-    alias Foo.Bar.Baz.Bop.Boom
+      """,
+      """
+      alias Foo.Bar.Baz
+      alias Foo.Bar.Baz.Bop.Boom
 
-    Boom.wee()
-    Boom.wee()
-    """)
+      Boom.wee()
+      Boom.wee()
+      """
+    )
   end
 
   describe "comments stay put" do