Oneline unpiped assignments. Closes #181

CHANGELOG.md

@@ -4,6 +4,10 @@
 they can and will change without that change being reflected in Styler's semantic version.
 ## main
 
+### Improvements
+
+* `pipes`: unpiping assignments will make the assignment one-line when possible (Closes #181)
+
 ### Fixes
 
 * `pipes`: optimizations are less likely to move comments (Closes #176)

lib/style.ex

@@ -144,7 +144,7 @@ defmodule Styler.Style do
     comments
     |> Enum.map(fn comment ->
       if delta = Enum.find_value(shifts, fn {range, delta} -> comment.line in range && delta end) do
-        %{comment | line: comment.line + delta}
+        %{comment | line: max(comment.line + delta, 1)}
       else
         comment
       end
@@ -230,14 +230,28 @@ defmodule Styler.Style do
       else: do_fix_lines(nodes, line, [node | acc])
   end
 
-  # @TODO can i shortcut and just return end_of_expression[:line] when it's available?
+  def max_line([_ | _] = list), do: list |> List.last() |> max_line()
+
   def max_line(ast) do
-    {_, max_line} =
-      Macro.prewalk(ast, 0, fn
-        {_, meta, _} = ast, max -> {ast, max(meta[:line] || max, max)}
-        ast, max -> {ast, max}
-      end)
+    meta =
+      case ast do
+        {_, meta, _} ->
+          meta
+
+        _ ->
+          []
+      end
 
-    max_line
+    if max_line = meta[:closing][:line] do
+      max_line
+    else
+      {_, max_line} =
+        Macro.prewalk(ast, 0, fn
+          {_, meta, _} = ast, max -> {ast, max(meta[:line] || max, max)}
+          ast, max -> {ast, max}
+        end)
+
+      max_line
+    end
   end
 end

lib/style/pipes.ex

@@ -50,25 +50,79 @@ defmodule Styler.Style.Pipes do
           {{:|>, _, [_, {:unquote, _, [_]}]}, _} = single_pipe_unquote_zipper ->
             {:cont, single_pipe_unquote_zipper, ctx}
 
+          # unpipe a single pipe zipper
           {{:|>, _, [lhs, rhs]}, _} = single_pipe_zipper ->
-            {_, meta, _} = lhs
-            # try to get everything on one line if we can
-            line = meta[:line]
-            {fun, meta, args} = rhs
+            {fun, rhs_meta, args} = rhs
+            {_, lhs_meta, _} = lhs
+            lhs_line = lhs_meta[:line]
             args = args || []
-
-            # no way multi-headed fn fits on one line; everything else (?) is just a matter of line length
-            args =
-              if Enum.any?(args, &match?({:fn, _, [{:->, _, _}, {:->, _, _} | _]}, &1)) do
-                Style.shift_line(args, -1)
-              else
-                Style.set_line(args, line)
+            # Every branch ends with the zipper being replaced with a function call
+            # `lhs |> rhs(...args)` => `rhs(lhs, ...args)`
+            # The differences are just figuring out what line number updates to make
+            # in order to get the following properties:
+            #
+            # 1. write the function call on one line if reasonable
+            # 2. keep comments well behaved (by doing meta line-number gymnastics)
+
+            # if we see multiple `->`, there's no way we can online this
+            # future heuristics would include finding multiple lines
+            definitively_multiline? =
+              Enum.any?(args, fn
+                {:fn, _, [{:->, _, _}, {:->, _, _} | _]} -> true
+                {:fn, _, [{:->, _, [_, _]}]} -> true
+                _ -> false
+              end)
+
+            if definitively_multiline? do
+              # shift rhs up to hang out with lhs
+              # 1   lhs
+              # 2   |> fun(
+              # 3     ...args...
+              # n   )
+              # =>
+              # 1   fun(lhs
+              # 2     ... args...
+              # n-1 )
+
+              # because there could be comments between lhs and rhs, or the dev may have a bunch of empty lines,
+              # we need to calculate the distance between the two ("shift")
+              rhs_line = rhs_meta[:line]
+              shift = lhs_line - rhs_line
+              {fun, meta, args} = Style.shift_line(rhs, shift)
+
+              # Not going to lie, no idea why the `shift + 1` is correct but it makes tests pass ¯\_(ツ)_/¯
+              rhs_max_line = Style.max_line(rhs)
+
+              comments =
+                ctx.comments
+                |> Style.displace_comments(lhs_line..(rhs_line - 1))
+                |> Style.shift_comments(rhs_line..rhs_max_line, shift + 1)
+
+              {:cont, Zipper.replace(single_pipe_zipper, {fun, meta, [lhs | args]}), %{ctx | comments: comments}}
+            else
+              # try to get everything on one line.
+              # formatter will kick it back to multiple if line-length doesn't acocommodate
+              case Zipper.up(single_pipe_zipper) do
+                # if the parent is an assignment, put it on the same line as the `=`
+                {{:=, am, [{_, vm, _} = var, _single_pipe]}, _} = assignment_parent ->
+                  # 1 var =
+                  # 2   lhs
+                  # 3   |> rhs(...args)
+                  # =>
+                  # 1 var = rhs(lhs, ...args)
+                  oneline_assignment = Style.set_line({:=, am, [var, {fun, rhs_meta, [lhs | args]}]}, vm[:line])
+                  # skip so we don't re-traverse
+                  {:cont, Zipper.replace(assignment_parent, oneline_assignment), ctx}
+
+                _ ->
+                  # lhs
+                  # |> rhs(...args)
+                  # =>
+                  # rhs(lhs, ...)
+                  oneline_function_call = Style.set_line({fun, rhs_meta, [lhs | args]}, lhs_line)
+                  {:cont, Zipper.replace(single_pipe_zipper, oneline_function_call), ctx}
               end
-
-            lhs = Style.set_line(lhs, line)
-            {_, meta, _} = Style.set_line({:ignore, meta, []}, line)
-            function_call_zipper = Zipper.replace(single_pipe_zipper, {fun, meta, [lhs | args]})
-            {:cont, function_call_zipper, ctx}
+            end
         end
 
       non_pipe ->
@@ -162,7 +216,7 @@ 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(pm, a, b, c) do
-    quote do: {:|>, unquote(pm), [{:|>, _, [unquote(a), unquote(b)]}, unquote(c)]}
+    quote do: {:|>, _, [{:|>, unquote(pm), [unquote(a), unquote(b)]}, unquote(c)]}
   end
 
   # a |> fun => a |> fun()
@@ -286,7 +340,8 @@ defmodule Styler.Style.Pipes do
           {{:., dm, [{:__aliases__, dm, [:Map]}, :new]}, em, [mapper]}
 
         _ ->
-          {into, em, [Style.set_line(collectable, dm[:line]), mapper]}
+          {into, m, collectable} = Style.set_line({into, em, [collectable]}, dm[:line])
+          {into, m, [collectable, mapper]}
       end
 
     {:|>, pm, [lhs, rhs]}

test/style/pipes_test.exs

@@ -432,6 +432,18 @@ defmodule Styler.Style.PipesTest do
         """
       )
     end
+
+    test "onelines assignments" do
+      assert_style(
+        """
+        x =
+          y
+          |> Enum.map(&f/1)
+          |> Enum.join()
+        """,
+        "x = Enum.map_join(y, &f/1)"
+      )
+    end
   end
 
   describe "valid pipe starts & unpiping" do
@@ -677,16 +689,24 @@ defmodule Styler.Style.PipesTest do
 
         assert_style(
           """
+          # a
+          # b
           a_multiline_mapper
           |> #{enum}.map(fn %{gets: shrunk, down: to_a_more_reasonable} ->
+            # c
             IO.puts "woo!"
+            # d
             {shrunk, to_a_more_reasonable}
           end)
           |> Enum.into(size)
           """,
           """
+          # a
+          # b
           Enum.into(a_multiline_mapper, size, fn %{gets: shrunk, down: to_a_more_reasonable} ->
+            # c
             IO.puts("woo!")
+            # d
             {shrunk, to_a_more_reasonable}
           end)
           """
@@ -751,16 +771,16 @@ defmodule Styler.Style.PipesTest do
     test "unpiping doesn't move comment in anonymous function" do
       assert_style(
         """
-          aliased =
-            aliases
-            |> MapSet.new(fn
-              {:alias, _, [{:__aliases__, _, aliases}]} -> List.last(aliases)
-              {:alias, _, [{:__aliases__, _, _}, [{_as, {:__aliases__, _, [as]}}]]} -> as
-              # alias __MODULE__ or other oddities
-              {:alias, _, _} -> nil
-            end)
+        aliased =
+          aliases
+          |> MapSet.new(fn
+            {:alias, _, [{:__aliases__, _, aliases}]} -> List.last(aliases)
+            {:alias, _, [{:__aliases__, _, _}, [{_as, {:__aliases__, _, [as]}}]]} -> as
+            # alias __MODULE__ or other oddities
+            {:alias, _, _} -> nil
+          end)
 
-          excluded_first = MapSet.union(aliased, @excluded_namespaces)
+        excluded_first = MapSet.union(aliased, @excluded_namespaces)
         """,
         """
         aliased =
@@ -774,6 +794,61 @@ defmodule Styler.Style.PipesTest do
         excluded_first = MapSet.union(aliased, @excluded_namespaces)
         """
       )
+
+      assert_style(
+        """
+        foo =
+          # bar
+          bar
+          # baz
+          |> baz(fn ->
+            # a
+            a
+            # b
+            b
+          end)
+        """,
+        """
+        # bar
+        # baz
+        foo =
+          baz(bar, fn ->
+            # a
+            a
+            # b
+            b
+          end)
+        """
+      )
+
+      assert_style(
+        """
+        foo =
+          # bar
+          bar
+          # baz
+
+
+
+          |> baz(fn ->
+            # a
+            a
+            # b
+            b
+          end)
+        """,
+        """
+        # bar
+        # baz
+        foo =
+          baz(bar, fn ->
+            # a
+            a
+            # b
+            b
+          end)
+        """
+      )
     end
   end