ModuleDirectives: fix bugs (#12)

- handle single child do blocks
- handle parentless directives
- handle defmodule with no directives and no moduledoc added

CHANGELOG.md

@@ -5,6 +5,14 @@
 ### Improvements
 
 * Enabled `Defs` style and overhauled it to properly handles comments
+* Optimized and tweaked `ModuleDirectives` style
+    * Now culls newlines between "groups" of the same directive
+    * sorts `@behaviour` directives
+    * orders directives within non defmodule contexts (eg, a `def do`) if there's at least one `alias|require|use|import`
+
+### Fixes
+
+* `Pipes` will try to keep single-pipe rewrites on one line
 
 ## v0.2.0
 

lib/style/module_directives.ex

@@ -76,36 +76,53 @@ defmodule Styler.Style.ModuleDirectives do
 
   def run({{:defmodule, _, children}, _} = zipper, ctx) do
     [{:__aliases__, _, aliases}, [{{:__block__, do_meta, [:do]}, _module_body}]] = children
-    # Move the zipper's focus to the module's body
-    name = aliases |> List.last() |> to_string()
-    add_moduledoc? = do_meta[:format] != :keyword and not String.ends_with?(name, @dont_moduledoc)
-    body_zipper = zipper |> Zipper.down() |> Zipper.right() |> Zipper.down() |> Zipper.down() |> Zipper.right()
-
-    case Zipper.node(body_zipper) do
-      {:__block__, _, _} ->
-        {:skip, organize_directives(body_zipper, add_moduledoc?), ctx}
-
-      {:@, _, [{:moduledoc, _, _}]} ->
-        # a module whose only child is a moduledoc. nothing to do here!
-        # seems weird at first blush but lots of projects/libraries do this with their root namespace module
-        {:skip, zipper, ctx}
-
-      only_child ->
-        # There's only one child, and it's not a moduledoc. Conditionally add a moduledoc, then style the only_child
-        if add_moduledoc? do
-          body_zipper
-          |> Zipper.replace({:__block__, [], [@moduledoc_false, only_child]})
-          |> Zipper.down()
-          |> Zipper.right()
-          |> run(ctx)
-        else
-          run(body_zipper, ctx)
-        end
+
+    if do_meta[:format] == :keyword do
+      {:skip, zipper, ctx}
+    else
+      name = aliases |> List.last() |> to_string()
+      add_moduledoc? = not String.ends_with?(name, @dont_moduledoc)
+      # Move the zipper's focus to the module's body
+      body_zipper = zipper |> Zipper.down() |> Zipper.right() |> Zipper.down() |> Zipper.down() |> Zipper.right()
+
+      case Zipper.node(body_zipper) do
+        {:__block__, _, _} ->
+          {:skip, organize_directives(body_zipper, add_moduledoc?), ctx}
+
+        {:@, _, [{:moduledoc, _, _}]} ->
+          # a module whose only child is a moduledoc. nothing to do here!
+          # seems weird at first blush but lots of projects/libraries do this with their root namespace module
+          {:skip, zipper, ctx}
+
+        only_child ->
+          # There's only one child, and it's not a moduledoc. Conditionally add a moduledoc, then style the only_child
+          if add_moduledoc? do
+            body_zipper
+            |> Zipper.replace({:__block__, [], [@moduledoc_false, only_child]})
+            |> Zipper.down()
+            |> Zipper.right()
+            |> run(ctx)
+          else
+            run(body_zipper, ctx)
+          end
+      end
     end
   end
 
-  def run({{d, _, _}, _} = zipper, ctx) when d in @directives do
-    {:skip, zipper |> Zipper.up() |> organize_directives(), ctx}
+  def run({{d, _, _} = directive, _} = zipper, ctx) when d in @directives do
+    parent =
+      case Zipper.up(zipper) do
+        nil ->
+          Zipper.replace(zipper, {:__block__, [], [directive]})
+
+        {{{:__block__, _, [:do]}, _only_child}, _} ->
+          Zipper.replace(zipper, {:__block__, [], [directive]})
+
+        parent ->
+          parent
+      end
+
+    {:skip, organize_directives(parent), ctx}
   end
 
   def run(zipper, ctx), do: {:cont, zipper, ctx}
@@ -148,13 +165,21 @@ defmodule Styler.Style.ModuleDirectives do
         requires
       ])
 
-    parent = Zipper.update(parent, &Zipper.replace_children(&1, directives))
+    cond do
+      Enum.empty?(directives) ->
+        parent
 
-    if Enum.empty?(nondirectives) do
-      parent
-    else
-      {last_directive, meta} = parent |> Zipper.down() |> Zipper.rightmost()
-      {last_directive, %{meta | r: nondirectives}}
+      Enum.empty?(nondirectives) ->
+        Zipper.update(parent, &Zipper.replace_children(&1, directives))
+
+      true ->
+        {last_directive, meta} =
+          parent
+          |> Zipper.update(&Zipper.replace_children(&1, directives))
+          |> Zipper.down()
+          |> Zipper.rightmost()
+
+        {last_directive, %{meta | r: nondirectives}}
     end
   end
 

test/style/module_directives_test.exs

@@ -12,7 +12,31 @@ defmodule Styler.Style.ModuleDirectivesTest do
   @moduledoc false
   use Styler.StyleCase, style: Styler.Style.ModuleDirectives, async: true
 
-  describe "module directive sorting" do
+  describe "defmodule features" do
+    test "handles module with no directives" do
+      assert_style("""
+      defmodule Test do
+        def foo, do: :ok
+      end
+      """)
+    end
+
+    test "module with single child" do
+      assert_style(
+        """
+        defmodule ATest do
+          alias Foo.{A, B}
+        end
+        """,
+        """
+        defmodule ATest do
+          alias Foo.A
+          alias Foo.B
+        end
+        """
+      )
+    end
+
     test "adds moduledoc" do
       assert_style(
         """
@@ -38,8 +62,6 @@ defmodule Styler.Style.ModuleDirectivesTest do
           use Bar
         end
 
-        def Foo, do: :ok
-
         defmodule Foo do
           alias Foo.{Bar, Baz}
         end
@@ -72,8 +94,6 @@ defmodule Styler.Style.ModuleDirectivesTest do
           use Bar
         end
 
-        def Foo, do: :ok
-
         defmodule Foo do
           @moduledoc false
           alias Foo.Bar
@@ -83,6 +103,10 @@ defmodule Styler.Style.ModuleDirectivesTest do
       )
     end
 
+    test "skips keyword defmodules" do
+      assert_style("defmodule Foo, do: use(Bar)")
+    end
+
     test "doesn't add moduledoc to modules of specific names" do
       for verboten <- ~w(Test Mixfile Controller Endpoint Repo Router Socket View HTML JSON) do
         assert_style("""
@@ -177,7 +201,38 @@ defmodule Styler.Style.ModuleDirectivesTest do
     end
   end
 
+  describe "quote blocks" do
+    test "quote do with one child" do
+      assert_style(
+        """
+        quote do
+          alias A.{C, B}
+        end
+        """,
+        """
+        quote do
+          alias A.B
+          alias A.C
+        end
+        """
+      )
+    end
+
+    test "quote do with multiple children" do
+      assert_style("""
+      quote do
+        import A
+        import B
+      end
+      """)
+    end
+  end
+
   describe "directive sort/dedupe/expansion" do
+    test "handles a lonely lonely directive" do
+      assert_style("import Foo")
+    end
+
     test "sorts, dedupes & expands alias/require/import while respecting groups" do
       for d <- ~w(alias require import) do
         assert_style(