From d444a9c52ba3c0c5ee9973409c776d94be0c0e14 Mon Sep 17 00:00:00 2001 From: Matt Enlow Date: Thu, 7 Nov 2024 11:16:37 -0700 Subject: [PATCH] Oneline unpiped assignments. Closes #181 --- CHANGELOG.md | 4 ++ lib/style.ex | 30 +++++++++---- lib/style/pipes.ex | 91 ++++++++++++++++++++++++++++++-------- test/style/pipes_test.exs | 93 +++++++++++++++++++++++++++++++++++---- 4 files changed, 183 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15db7758..11460ae8 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/style.ex b/lib/style.ex index 0f20ced8..6368293c 100644 --- a/lib/style.ex +++ b/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 diff --git a/lib/style/pipes.ex b/lib/style/pipes.ex index 9aecaed3..5b88e3ab 100644 --- a/lib/style/pipes.ex +++ b/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 accommodate + 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]} diff --git a/test/style/pipes_test.exs b/test/style/pipes_test.exs index 5c363551..93ba461f 100644 --- a/test/style/pipes_test.exs +++ b/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