maintain line meta when rewriting to if statements

lib/style/blocks.ex

@@ -28,35 +28,27 @@ defmodule Styler.Style.Blocks do
   @behaviour Styler.Style
 
   alias Styler.Style
+  alias Styler.Zipper
 
   # @TODO handle comments https://github.com/adobe/elixir-styler/issues/79
   def run({node, meta}, ctx), do: {:cont, {style(node), meta}, ctx}
 
-  defmacrop trivial_case(head, a, a_body, b, b_body) do
-    quote do
-      {:case, _,
-       [
-         unquote(head),
-         [
-           {_,
-            [
-              {:->, _, [[unquote(a)], unquote(a_body)]},
-              {:->, _, [[unquote(b)], unquote(b_body)]}
-            ]}
-         ]
-       ]}
+  # case statement with exactly 2 `->` cases
+  # rewrite to `if` if it's any of 3 trivial cases
+  defp style({:case, _, [head, [{_, [{:->, _, [[lhs_a], a]}, {:->, _, [[lhs_b], b]}]}]]} = trivial_case_ast) do
+    case {lhs_a, lhs_b} do
+      {{_, _, [true]}, {_, _, [false]}} -> if_ast(head, a, b)
+      {{_, _, [true]}, {:_, _, _}} -> if_ast(head, a, b)
+      {{_, _, [false]}, {_, _, [true]}} -> if_ast(head, b, a)
+      _ -> trivial_case_ast
     end
   end
 
-  defp style(trivial_case(head, {_, _, [true]}, do_, {_, _, [false]}, else_)), do: styled_if(head, do_, else_)
-  defp style(trivial_case(head, {_, _, [false]}, else_, {_, _, [true]}, do_)), do: styled_if(head, do_, else_)
-  defp style(trivial_case(head, {_, _, [true]}, do_, {:_, _, _}, else_)), do: styled_if(head, do_, else_)
-
   # `Credo.Check.Refactor.CondStatements`
   # This also detects strings and lists...
   defp style({:cond, _, [[{_, [{:->, _, [[expr], do_body]}, {:->, _, [[{:__block__, _, [truthy]}], else_body]}]}]]})
        when is_atom(truthy) and truthy not in [nil, false] do
-    styled_if(expr, do_body, else_body)
+    if_ast(expr, do_body, else_body)
   end
 
   # Credo.Check.Readability.WithSingleClause
@@ -126,7 +118,8 @@ defmodule Styler.Style.Blocks do
   defp style({:if, m, [{negator, _, [expr]}, [{do_, do_body}, {else_, else_body}]]}) when negator in [:!, :not],
     do: style({:if, m, [expr, [{do_, else_body}, {else_, do_body}]]})
 
-  defp style({:if, m, [head, [do_block, {_, {:__block__, _, [nil]}}]]}), do: {:if, m, [head, [do_block]]}
+  defp style({:if, m, [head, [do_block, {_, {:__block__, _, [nil]}}]]}),
+    do: {:if, m, [head, [do_block]]}
 
   defp style(node), do: node
 
@@ -138,11 +131,47 @@ defmodule Styler.Style.Blocks do
     Style.update_all_meta(a, fn _ -> nil end) == Style.update_all_meta(b, fn _ -> nil end)
   end
 
-  defp styled_if(head, do_body, else_body) do
-    {_, meta, _} = head
+  defp if_ast({_, meta, _} = head, do_body, else_body) do
     line = meta[:line]
-    # @TODO figure out appropriate line meta for `else` and `if->end->line`
-    children = [head, [{{:__block__, [line: line], [:do]}, do_body}, {{:__block__, [], [:else]}, else_body}]]
-    style({:if, [line: line, do: [line: line], end: []], children})
+    # else body gets moved down by a line to give the new `else` keyword its own line
+    # else_body = Style.shift_line(else_body, 1)
+    do_body = Macro.update_meta(do_body, &Keyword.delete(&1, :end_of_expression))
+    else_body = Macro.update_meta(else_body, &Keyword.delete(&1, :end_of_expression))
+
+    max_do_line = max_line(do_body)
+    max_else_line = max_line(else_body)
+
+    {do_block, else_block, end_line} =
+      if max_do_line >= max_else_line do
+        shift = max_else_line - line
+        dbg(shift)
+        do_block = {{:__block__, [line: line], [:do]}, Style.shift_line(do_body, -shift)}
+        # +1 to fit the else keyword
+        else_body = Style.shift_line(else_body, shift + 1)
+        else_block = {{:__block__, [line: max_else_line + 1], [:else]}, else_body}
+        {do_block, else_block, max_do_line + 2}
+      else
+
+        do_block = {{:__block__, [line: line], [:do]}, do_body}
+        # move everything down by 1 to put the `else` keyword on its own line
+        #@TODO shift comments w/ this
+        else_block = Style.shift_line({{:__block__, [line: max_do_line], [:else]}, else_body}, 1)
+        {do_block, else_block, max_else_line + 2}
+      end
+    dbg()
+
+    # end line is max_line + 2 to accomodate `else` and `end` both having their own line
+    style({:if, [do: [line: line], end: [line: end_line], line: line], [head, [do_block, else_block]]})
+  end
+
+  defp max_line(ast, start \\ 0) do
+    {_, max_line} =  ast
+      |> Zipper.zip()
+      |> Zipper.traverse(start, fn
+        {{_, meta, _}, _} = z, max -> {z, max(meta[:line] || max, max)}
+        z, max -> {z, max}
+      end)
+
+    max_line
   end
 end

test/style/blocks_test.exs

@@ -9,7 +9,7 @@
 # governing permissions and limitations under the License.
 
 defmodule Styler.Style.BlocksTest do
-  use Styler.StyleCase, async: true
+  use Styler.StyleCase, strict: true, async: true
 
   describe "case to if" do
     test "rewrites case true false to if else" do

test/style/pipes_test.exs

@@ -470,7 +470,7 @@ defmodule Styler.Style.PipesTest do
   describe "simple rewrites" do
     test "{Keyword/Map}.merge/2 of a single key => *.put/3" do
       for module <- ~w(Map Keyword) do
-        assert_style "foo |> #{module}.merge(%{one_key: :bar}) |> bop()", "foo |> #{module}.put(:one_key, :bar) |> bop()"
+        assert_style("foo |> #{module}.merge(%{one_key: :bar}) |> bop()", "foo |> #{module}.put(:one_key, :bar) |> bop()")
       end
     end
 

test/support/style_case.ex

@@ -14,9 +14,11 @@ defmodule Styler.StyleCase do
   """
   use ExUnit.CaseTemplate
 
-  using do
+  using opts do
     quote do
       import unquote(__MODULE__), only: [assert_style: 1, assert_style: 2, style: 1]
+
+      @strict unquote(opts[:strict]) || false
     end
   end
 
@@ -26,6 +28,8 @@ defmodule Styler.StyleCase do
     quote bind_quoted: [before: before, expected: expected] do
       expected = String.trim(expected)
       {styled_ast, styled, styled_comments} = style(before)
+      {expected_ast, expected_comments} = Styler.string_to_quoted_with_comments(expected)
+      {_, restyled, _} = style(styled)
 
       if styled != expected and ExUnit.configuration()[:trace] do
         IO.puts("\n======Given=============\n")
@@ -35,7 +39,6 @@ defmodule Styler.StyleCase do
         dbg(before_comments)
         IO.puts("======Expected==========\n")
         IO.puts(expected)
-        {expected_ast, expected_comments} = Styler.string_to_quoted_with_comments(expected)
         dbg(expected_ast)
         dbg(expected_comments)
         IO.puts("======Got===============\n")
@@ -46,7 +49,10 @@ defmodule Styler.StyleCase do
       end
 
       assert expected == styled
-      {_, restyled, _} = style(styled)
+
+      if @strict do
+        assert styled_ast == expected_ast
+      end
 
       assert restyled == styled, """
       expected styling to be idempotent, but a second pass resulted in more changes.