Merge branch 'main' into me/docs

CHANGELOG.md

@@ -2,14 +2,21 @@
 
 ## main
 
-## 1.0.0-alpha.0
+## 1.0.0-rc.0
 
-An alpha! You probably want to hold off for the beta, but feel free to take this for a spin and give feedback!
+At this point, 1.0.0 feels feature complete. Two things remains for a full release:
 
-Documentation to come.
+1. feedback!
+2. documentation overhaul! [monitor progress here](https://github.com/adobe/elixir-styler/pull/166)
 
 ### Improvements
 
+Styler's two biggest outstanding bugs have been fixed, both related to compilation breaking during module directive organization. One was references to aliases being moved above where the aliases were declared, and the other was similarly module directives being moved after their uses in module directives.
+
+In both cases, Styler is now smart enough to auto-apply the fixes we recommended in the old Readme.
+
+Other than that, a slew of powerful new features have been added, the neatest one (in the author's opinion anyways) being Alias Lifting.
+
 #### Alias Lifting
 
 Along the lines of `Credo.Check.Design.AliasUsage`, Styler now "lifts" deeply nested aliases (depth >= 3, ala `A.B.C....`) that are used more than once.
@@ -41,13 +48,44 @@ end
 
 To exclude modules ending in `.Foo` from being lifted, add `styler: [alias_lifting_exclude: [Foo]]` to your `.formatter.exs`
 
+#### Module Attribute Lifting
+
+A long outstanding breakage of a first pass with Styler was breaking directives that relied on module attributes which Styler moved _after_ their uses. Styler now detects these potential breakages and automatically applies our suggested fix, which is creating a variable before the module. This usually happened when folks were using a library that autogenerated their moduledocs for them.
+
+In code, this module:
+
+```elixir
+defmodule MyGreatLibrary do
+  @library_options [...]
+  @moduledoc make_pretty_docs(@library_options)
+  use OptionsMagic, my_opts: @library_options
+
+  ...
+end
+```
+
+Will now be styled like so:
+
+```elixir
+library_options = [...]
+
+defmodule MyGreatLibrary do
+  @moduledoc make_pretty_docs(library_options)
+  use OptionsMagic, my_opts: unquote(library_options)
+
+  @library_options library_options
+
+  ...
+end
+```
+
 #### Mix Config File Organization
 
 Styler now organizes `Mix.Config.config/2,3` stanzas according to erlang term sorting. This helps manage large configuration files, removing the "where should I put this" burden from developers AND helping find duplicated configuration stanzas.
 
 See the moduledoc for `Styler.Style.Configs` for more.
 
-#### Other Improvements
+#### Everything Else
 
 * `if`/`unless`: invert if and unless with `!=` or `!==`, like we do for `!` and `not` #132
 * `@derive`: move `@derive` before `defstruct|schema|embedded_schema` declarations (fixes compiler warning!) #134
@@ -69,6 +107,7 @@ See the moduledoc for `Styler.Style.Configs` for more.
 ### Breaking Changes
 
 * drop support for elixir `1.14`
+* ModuleDirectives: group callback attributes (`before_compile after_compile after_verify`) with nondirectives (previously, were grouped with `use`, their relative order maintained). to keep the desired behaviour, you can make new `use` macros that wrap these callbacks. Apologies if this makes using Styler untenable for your codebase, but it's probably not a good tool for macro-heavy libraries.
 * sorting configs for the first time can change your configuration; see `Styler.Style.Configs` moduledoc for more
 
 ## v0.11.9

lib/style/module_directives.ex

@@ -93,7 +93,6 @@ defmodule Styler.Style.ModuleDirectives do
   alias Styler.Zipper
 
   @directives ~w(alias import require use)a
-  @callback_attrs ~w(before_compile after_compile after_verify)a
   @attr_directives ~w(moduledoc shortdoc behaviour)a
   @defstruct ~w(schema embedded_schema defstruct)a
 
@@ -191,65 +190,119 @@ defmodule Styler.Style.ModuleDirectives do
   defp moduledoc(_), do: nil
 
   @acc %{
-    "@shortdoc": [],
-    "@moduledoc": [],
-    "@behaviour": [],
+    shortdoc: [],
+    moduledoc: [],
+    behaviour: [],
     use: [],
     import: [],
     alias: [],
     require: [],
     nondirectives: [],
-    dealiases: %{}
+    dealiases: %{},
+    attrs: MapSet.new(),
+    attr_lifts: []
   }
 
+  defp lift_module_attrs({node, _, _} = ast, %{attrs: attrs} = acc) do
+    if Enum.empty?(attrs) do
+      {ast, acc}
+    else
+      use? = node == :use
+
+      Macro.prewalk(ast, acc, fn
+        {:@, m, [{attr, _, _} = var]} = ast, acc ->
+          if attr in attrs do
+            replacement =
+              if use?,
+                do: {:unquote, [closing: [line: m[:line]], line: m[:line]], [var]},
+                else: var
+
+            {replacement, %{acc | attr_lifts: [attr | acc.attr_lifts]}}
+          else
+            {ast, acc}
+          end
+
+        ast, acc ->
+          {ast, acc}
+      end)
+    end
+  end
+
   defp organize_directives(parent, moduledoc \\ nil) do
     acc =
       parent
       |> Zipper.children()
       |> Enum.reduce(@acc, fn
+        {:@, _, [{attr_directive, _, _}]} = ast, acc when attr_directive in @attr_directives ->
+          # attr_directives are moved above aliases, so we need to dealias them
+          {ast, acc} = acc.dealiases |> Dealias.apply(ast) |> lift_module_attrs(acc)
+          %{acc | attr_directive => [ast | acc[attr_directive]]}
+
         {:@, _, [{attr, _, _}]} = ast, acc ->
-          key =
-            cond do
-              # TODO drop for a 1.0 release?
-              # the order of callbacks relative to use can matter if the use is also doing callbacks
-              # looking back, this is probably a hack to support one person's weird hackery 🤣
-              attr in @callback_attrs -> :use
-              attr in @attr_directives -> :"@#{attr}"
-              true -> :nondirectives
-            end
-
-          # both callback and attr_directives are moved above aliases, so we need to dealias them
-          ast = if key == :nondirectives, do: ast, else: Dealias.apply(acc.dealiases, ast)
-          %{acc | key => [ast | acc[key]]}
+          %{acc | nondirectives: [ast | acc.nondirectives], attrs: MapSet.put(acc.attrs, attr)}
 
         {directive, _, _} = ast, acc when directive in @directives ->
+          {ast, acc} = lift_module_attrs(ast, acc)
           ast = expand(ast)
           # import and used get hoisted above aliases, so need to dealias
           ast = if directive in ~w(import use)a, do: Dealias.apply(acc.dealiases, ast), else: ast
           dealiases = if directive == :alias, do: Dealias.put(acc.dealiases, ast), else: acc.dealiases
+
           # the reverse accounts for `expand` putting things in reading order, whereas we're accumulating in reverse
-          %{acc | directive => Enum.reverse(ast, acc[directive]), :dealiases => dealiases}
+          %{acc | directive => Enum.reverse(ast, acc[directive]), dealiases: dealiases}
 
         ast, acc ->
           %{acc | nondirectives: [ast | acc.nondirectives]}
       end)
       # Reversing once we're done accumulating since `reduce`ing into list accs means you're reversed!
       |> Map.new(fn
-        {:"@moduledoc", []} -> {:"@moduledoc", List.wrap(moduledoc)}
+        {:moduledoc, []} -> {:moduledoc, List.wrap(moduledoc)}
         {:use, uses} -> {:use, uses |> Enum.reverse() |> Style.reset_newlines()}
-        {directive, to_sort} when directive in ~w(@behaviour import alias require)a -> {directive, sort(to_sort)}
+        {directive, to_sort} when directive in ~w(behaviour import alias require)a -> {directive, sort(to_sort)}
         {:dealiases, d} -> {:dealiases, d}
         {k, v} -> {k, Enum.reverse(v)}
       end)
       |> lift_aliases()
 
+    # Not happy with it, but this does the work to move module attribute assignments above the module or quote or whatever
+    # Given that it'll only be run once and not again, i'm okay with it being inefficient
+    {acc, parent} =
+      if Enum.any?(acc.attr_lifts) do
+        lifts = acc.attr_lifts
+
+        nondirectives =
+          Enum.map(acc.nondirectives, fn
+            {:@, m, [{attr, am, _}]} = ast -> if attr in lifts, do: {:@, m, [{attr, am, [{attr, am, nil}]}]}, else: ast
+            ast -> ast
+          end)
+
+        assignments =
+          Enum.flat_map(acc.nondirectives, fn
+            {:@, m, [{attr, am, [val]}]} -> if attr in lifts, do: [{:=, m, [{attr, am, nil}, val]}], else: []
+            _ -> []
+          end)
+
+        {past, _} = parent
+
+        parent =
+          parent
+          |> Zipper.up()
+          |> Style.find_nearest_block()
+          |> Zipper.prepend_siblings(assignments)
+          |> Zipper.find(&(&1 == past))
+
+        {%{acc | nondirectives: nondirectives}, parent}
+      else
+        {acc, parent}
+      end
+
     nondirectives = acc.nondirectives
 
     directives =
       [
-        acc."@shortdoc",
-        acc."@moduledoc",
-        acc."@behaviour",
+        acc.shortdoc,
+        acc.moduledoc,
+        acc.behaviour,
         acc.use,
         acc.import,
         acc.alias,

mix.exs

@@ -12,7 +12,7 @@ defmodule Styler.MixProject do
   use Mix.Project
 
   # Don't forget to bump the README when doing non-patch version changes
-  @version "1.0.0-alpha.0"
+  @version "1.0.0-rc.0"
   @url "https://github.com/adobe/elixir-styler"
 
   def project do

test/style/module_directives_test.exs

@@ -388,17 +388,6 @@ defmodule Styler.Style.ModuleDirectivesTest do
       )
     end
 
-    test "groups module callbacks with uses, keeping ordering" do
-      assert_style """
-      defmacro __using__(_) do
-        quote do
-          @after_compile Foo
-          use Bar
-        end
-      end
-      """
-    end
-
     test "interwoven directives w/o the context of a module" do
       assert_style(
         """
@@ -584,4 +573,50 @@ defmodule Styler.Style.ModuleDirectivesTest do
       """
     )
   end
+
+  describe "module attribute lifting" do
+    test "replaces uses in other attributes and `use` correctly" do
+      assert_style(
+        """
+        defmodule MyGreatLibrary do
+          @library_options [...]
+          @moduledoc make_pretty_docs(@library_options)
+          use OptionsMagic, my_opts: @library_options
+        end
+        """,
+        """
+        library_options = [...]
+
+        defmodule MyGreatLibrary do
+          @moduledoc make_pretty_docs(library_options)
+          use OptionsMagic, my_opts: unquote(library_options)
+
+          @library_options library_options
+        end
+        """
+      )
+    end
+
+    test "works with `quote`" do
+      assert_style(
+        """
+        quote do
+          @library_options [...]
+          @moduledoc make_pretty_docs(@library_options)
+          use OptionsMagic, my_opts: @library_options
+        end
+        """,
+        """
+        library_options = [...]
+
+        quote do
+          @moduledoc make_pretty_docs(library_options)
+          use OptionsMagic, my_opts: unquote(library_options)
+
+          @library_options library_options
+        end
+        """
+      )
+    end
+  end
 end