Don't lift require aliases due to our ordering rules

Author: JesseHerrick

lib/style/module_directives.ex

@@ -433,11 +433,12 @@ defmodule Styler.Style.ModuleDirectives do
 
   defp apply_aliases(acc, inverted_env) when map_size(inverted_env) == 0, do: acc
 
-  defp apply_aliases(%{require: requires, nondirectives: nondirectives, alias_env: alias_env} = acc, inverted_env) do
-    # applying aliases to requires can change their ordering again
-    requires = requires |> apply_aliases(inverted_env, alias_env) |> sort()
+  defp apply_aliases(%{nondirectives: nondirectives, alias_env: alias_env} = acc, inverted_env) do
+    # Requires are intentionally left alone: they sort above aliases in strict layout,
+    # so shortening `require Foo.Bar` to `require Bar` would reference an alias declared
+    # below it (broken Elixir).
     nondirectives = apply_aliases(nondirectives, inverted_env, alias_env)
-    %{acc | require: requires, nondirectives: nondirectives}
+    %{acc | nondirectives: nondirectives}
   end
 
   # applies the aliases withi `to_as` across the given ast

test/style/module_directives/alias_lifting_test.exs

@@ -176,7 +176,7 @@ defmodule Styler.Style.ModuleDirectives.AliasLiftingTest do
 
         import A.B.C
 
-        require C
+        require A.B.C
 
         alias A.B.C
 
@@ -216,24 +216,59 @@ defmodule Styler.Style.ModuleDirectives.AliasLiftingTest do
     )
   end
 
-  test "re-sorts requires after lifting" do
+  test "lifting does not rewrite require directives" do
+    # Requires sort above aliases, so a `require A.B.C` cannot be shortened to
+    # `require C` — at that point in the file, `C` isn't aliased yet.
     assert_style(
       """
       defmodule A do
         require A.B.C
         require B
 
         A.B.C.foo()
+        A.B.C.foo()
       end
       """,
       """
       defmodule A do
+        require A.B.C
         require B
-        require C
 
         alias A.B.C
 
         C.foo()
+        C.foo()
+      end
+      """
+    )
+  end
+
+  test "lifting via a require sighting does not rewrite the require itself" do
+    # Regression test: one require + one nondirective use of the same FQDN used to
+    # produce a `require Bar` ordered above its `alias Foo.Bar` (broken Elixir).
+    assert_style(
+      """
+      defmodule PayrollBillableItem do
+        @moduledoc false
+
+        require Tiger.Payrolling.OutputPayElements.PayrollBillableItemSchema
+
+        def define do
+          Tiger.Payrolling.OutputPayElements.PayrollBillableItemSchema.define()
+        end
+      end
+      """,
+      """
+      defmodule PayrollBillableItem do
+        @moduledoc false
+
+        require Tiger.Payrolling.OutputPayElements.PayrollBillableItemSchema
+
+        alias Tiger.Payrolling.OutputPayElements.PayrollBillableItemSchema
+
+        def define do
+          PayrollBillableItemSchema.define()
+        end
       end
       """
     )

test/style/module_directives_test.exs

@@ -294,7 +294,7 @@ defmodule Styler.Style.ModuleDirectivesTest do
         alias A.A
         """,
         """
-        require A
+        require A.A
         require A.C
 
         alias A.A