Merge branch 'main' into me/no-more-unless

CHANGELOG.md

@@ -3,6 +3,17 @@
 **Note** Styler's only public API is its usage as a formatter plugin. While you're welcome to play with its internals,
 they can and will change without that change being reflected in Styler's semantic version.
 
+## main
+
+### Improvements
+
+* `pipes`: optimize `|> Stream.{each|map}(fun) |> Stream.run()` to `|> Enum.each(fun)`
+
+### Fixes
+
+* `pipes`: optimizations reducing 2 pipes to 1 no longer squeeze all pipes onto one line (#180)
+* `if`: fix infinite loop rewriting negated if with empty do body `if x != y, do: (), else: :ok` (#196, h/t @itamm15)
+
 ## 1.0.0
 
 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.

docs/pipes.md

@@ -98,6 +98,13 @@ a |> Map.new(mapper) |> ...
 a |> Enum.map(mapper) |> Enum.into(%{}) |> ...
 # Styled:
 a |> Map.new(mapper) |> ...
+
+# Given:
+a |> b() |> Stream.each(fun) |> Stream.run()
+a |> b() |> Stream.map(fun) |> Stream.run()
+# Styled:
+a |> b() |> Enum.each(fun)
+a |> b() |> Enum.each(fun)
 ```
 
 ### Unpiping Single Pipes

lib/style/blocks.ex

@@ -195,6 +195,10 @@ defmodule Styler.Style.Blocks do
       [negator, [{do_, do_body}, {else_, else_body}]] when is_negator(negator) ->
         zipper |> Zipper.replace({:if, m, [invert(negator), [{do_, else_body}, {else_, do_body}]]}) |> run(ctx)
 
+      # drop `else end`
+      [head, [do_block, {_, {:__block__, _, []}}]] ->
+        {:cont, Zipper.replace(zipper, {:if, m, [head, [do_block]]}), ctx}
+
       # drop `else: nil`
       [head, [do_block, {_, {:__block__, _, [nil]}}]] ->
         {:cont, Zipper.replace(zipper, {:if, m, [head, [do_block]]}), ctx}

lib/style/pipes.ex

@@ -161,8 +161,8 @@ defmodule Styler.Style.Pipes do
 
   # `pipe_chain(a, b, c)` generates the ast for `a |> b |> c`
   # the intention is to make it a little easier to see what the fix_pipe functions are matching on =)
-  defmacrop pipe_chain(a, b, c) do
-    quote do: {:|>, _, [{:|>, _, [unquote(a), unquote(b)]}, unquote(c)]}
+  defmacrop pipe_chain(pm, a, b, c) do
+    quote do: {:|>, unquote(pm), [{:|>, _, [unquote(a), unquote(b)]}, unquote(c)]}
   end
 
   # a |> fun => a |> fun()
@@ -203,55 +203,74 @@ defmodule Styler.Style.Pipes do
   # `lhs |> Enum.reverse() |> Enum.concat(enum)` => `lhs |> Enum.reverse(enum)`
   defp fix_pipe(
          pipe_chain(
+           pm,
            lhs,
            {{:., _, [{_, _, [:Enum]}, :reverse]} = reverse, meta, []},
            {{:., _, [{_, _, [:Enum]}, :concat]}, _, [enum]}
          )
        ) do
-    {:|>, [line: meta[:line]], [lhs, {reverse, [line: meta[:line]], [enum]}]}
+    {:|>, pm, [lhs, {reverse, [line: meta[:line]], [enum]}]}
   end
 
   # `lhs |> Enum.reverse() |> Kernel.++(enum)` => `lhs |> Enum.reverse(enum)`
   defp fix_pipe(
          pipe_chain(
+           pm,
            lhs,
            {{:., _, [{_, _, [:Enum]}, :reverse]} = reverse, meta, []},
            {{:., _, [{_, _, [:Kernel]}, :++]}, _, [enum]}
          )
        ) do
-    {:|>, [line: meta[:line]], [lhs, {reverse, [line: meta[:line]], [enum]}]}
+    {:|>, pm, [lhs, {reverse, [line: meta[:line]], [enum]}]}
   end
 
   # `lhs |> Enum.filter(filterer) |> Enum.count()` => `lhs |> Enum.count(count)`
   defp fix_pipe(
          pipe_chain(
+           pm,
            lhs,
            {{:., _, [{_, _, [mod]}, :filter]}, meta, [filterer]},
            {{:., _, [{_, _, [:Enum]}, :count]} = count, _, []}
          )
        )
        when mod in @enum do
-    {:|>, [line: meta[:line]], [lhs, {count, [line: meta[:line]], [filterer]}]}
+    {:|>, pm, [lhs, {count, [line: meta[:line]], [filterer]}]}
+  end
+
+  # `lhs |> Stream.map(fun) |> Stream.run()` => `lhs |> Enum.each(fun)`
+  # `lhs |> Stream.each(fun) |> Stream.run()` => `lhs |> Enum.each(fun)`
+  defp fix_pipe(
+         pipe_chain(
+           pm,
+           lhs,
+           {{:., dm, [{a, am, [:Stream]}, map_or_each]}, fm, fa},
+           {{:., _, [{_, _, [:Stream]}, :run]}, _, []}
+         )
+       )
+       when map_or_each in [:map, :each] do
+    {:|>, pm, [lhs, {{:., dm, [{a, am, [:Enum]}, :each]}, fm, fa}]}
   end
 
   # `lhs |> Enum.map(mapper) |> Enum.join(joiner)` => `lhs |> Enum.map_join(joiner, mapper)`
   defp fix_pipe(
          pipe_chain(
+           pm,
            lhs,
            {{:., dm, [{_, _, [mod]}, :map]}, em, map_args},
            {{:., _, [{_, _, [:Enum]} = enum, :join]}, _, join_args}
          )
        )
        when mod in @enum do
     rhs = Style.set_line({{:., dm, [enum, :map_join]}, em, join_args ++ map_args}, dm[:line])
-    {:|>, [line: dm[:line]], [lhs, rhs]}
+    {:|>, pm, [lhs, rhs]}
   end
 
   # `lhs |> Enum.map(mapper) |> Enum.into(empty_map)` => `lhs |> Map.new(mapper)`
   # or
   # `lhs |> Enum.map(mapper) |> Enum.into(collectable)` => `lhs |> Enum.into(collectable, mapper)
   defp fix_pipe(
          pipe_chain(
+           pm,
            lhs,
            {{:., dm, [{_, _, [mod]}, :map]}, _, [mapper]},
            {{:., _, [{_, _, [:Enum]}, :into]} = into, _, [collectable]}
@@ -270,15 +289,20 @@ defmodule Styler.Style.Pipes do
           {into, dm, [collectable, mapper]}
       end
 
-    Style.set_line({:|>, [], [lhs, rhs]}, dm[:line])
+    Style.set_line({:|>, pm, [lhs, rhs]}, dm[:line])
   end
 
   # `lhs |> Enum.map(mapper) |> Map.new()` => `lhs |> Map.new(mapper)`
   defp fix_pipe(
-         pipe_chain(lhs, {{:., _, [{_, _, [enum]}, :map]}, _, [mapper]}, {{:., _, [{_, _, [mod]}, :new]} = new, nm, []})
+         pipe_chain(
+           pm,
+           lhs,
+           {{:., _, [{_, _, [enum]}, :map]}, _, [mapper]},
+           {{:., _, [{_, _, [mod]}, :new]} = new, nm, []}
+         )
        )
        when mod in @collectable and enum in @enum do
-    Style.set_line({:|>, [], [lhs, {new, nm, [mapper]}]}, nm[:line])
+    Style.set_line({:|>, pm, [lhs, {new, nm, [mapper]}]}, nm[:line])
   end
 
   defp fix_pipe(node), do: node

test/style/blocks_test.exs

@@ -879,6 +879,24 @@ defmodule Styler.Style.BlocksTest do
       )
     end
 
+    test "regression: negation with empty do body" do
+      assert_style(
+        """
+        if a != b do
+          # comment
+        else
+          :ok
+        end
+        """,
+        """
+        if a == b do
+          # comment
+          :ok
+        end
+        """
+      )
+    end
+
     test "Credo.Check.Refactor.UnlessWithElse" do
       for negator <- ["!", "not "] do
         assert_style(

test/style/pipes_test.exs

@@ -589,11 +589,14 @@ defmodule Styler.Style.PipesTest do
         assert_style(
           """
           a
+          |> b()
           |> #{enum}.filter(fun)
           |> Enum.count()
           """,
           """
-          Enum.count(a, fun)
+          a
+          |> b()
+          |> Enum.count(fun)
           """
         )
 
@@ -621,9 +624,43 @@ defmodule Styler.Style.PipesTest do
       end
     end
 
+    test "Stream.{each/map}/Stream.run" do
+      assert_style("a |> Stream.each(fun) |> Stream.run()", "Enum.each(a, fun)")
+      assert_style("a |> Stream.map(fun) |> Stream.run()", "Enum.each(a, fun)")
+
+      assert_style(
+        """
+        a
+        |> foo
+        |> Stream.map(fun)
+        |> Stream.run()
+        """,
+        """
+        a
+        |> foo()
+        |> Enum.each(fun)
+        """
+      )
+    end
+
     test "map/join" do
       for enum <- ~w(Enum Stream) do
         assert_style("a |> #{enum}.map(mapper) |> Enum.join()", "Enum.map_join(a, mapper)")
+
+        assert_style(
+          """
+          a
+          |> b()
+          |> #{enum}.map(mapper)
+          |> Enum.join()
+          """,
+          """
+          a
+          |> b()
+          |> Enum.map_join(mapper)
+          """
+        )
+
         assert_style("a |> #{enum}.map(mapper) |> Enum.join(joiner)", "Enum.map_join(a, joiner, mapper)")
       end
     end