configs: improve comment handling on initial runs

lib/style.ex

@@ -229,4 +229,15 @@ defmodule Styler.Style do
       do: do_fix_lines(nodes, max, [shift_line(node, max - line - 2) | acc]),
       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(ast) do
+    {_, 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

lib/style/blocks.ex

@@ -121,7 +121,7 @@ defmodule Styler.Style.Blocks do
         # disable keyword `, do:` since there will be multiple statements in the body
         with_meta =
           if Enum.any?(postroll),
-            do: Keyword.merge(with_meta, do: [line: with_meta[:line]], end: [line: max_line(children) + 1]),
+            do: Keyword.merge(with_meta, do: [line: with_meta[:line]], end: [line: Style.max_line(children) + 1]),
             else: with_meta
 
         with_children = Enum.reverse(reversed_clauses, [[{do_block, do_body} | elses]])
@@ -186,7 +186,7 @@ defmodule Styler.Style.Blocks do
         {:cont, Zipper.replace(zipper, {:if, m, [head, [do_block]]}), ctx}
 
       [head, [do_, else_]] ->
-        if max_line(do_) > max_line(else_) do
+        if Style.max_line(do_) > Style.max_line(else_) do
           # we inverted the if/else blocks of this `if` statement in a previous pass (due to negators or unless)
           # shift comments etc to make it happy now
           if_ast(zipper, head, do_, else_, ctx)
@@ -268,8 +268,8 @@ defmodule Styler.Style.Blocks do
     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)
+    max_do_line = Style.max_line(do_body)
+    max_else_line = Style.max_line(else_body)
     end_line = max(max_do_line, max_else_line)
 
     # Change ast meta and comment lines to fit the `if` ast
@@ -303,16 +303,6 @@ defmodule Styler.Style.Blocks do
     |> run(%{ctx | comments: comments})
   end
 
-  defp 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)
-
-    max_line
-  end
-
   defp invert({:!=, m, [a, b]}), do: {:==, m, [a, b]}
   defp invert({:!==, m, [a, b]}), do: {:===, m, [a, b]}
   defp invert({_, _, [expr]}), do: expr

lib/style/configs.ex

@@ -47,13 +47,21 @@ defmodule Styler.Style.Configs do
     {:skip, zipper, Map.put(ctx, :mix_config?, true)}
   end
 
-  def run({{:config, _, [_, _ | _]} = config, zm}, %{mix_config?: true} = ctx) do
+  def run({{:config, cfm, [_, _ | _]} = config, zm}, %{mix_config?: true, comments: comments} = ctx) do
     # all of these list are reversed due to the reduce
     {configs, assignments, rest} = accumulate(zm.r, [], [])
-
-    left_siblings = assignments |> Enum.reverse() |> Style.reset_newlines() |> Enum.reverse(zm.l)
-
-    [config | left_siblings] =
+    # @TODO
+    # okay so comments between nodes that we moved.......
+    # lets just push them out of the way (???). so
+    # 1. figure out first/last possible lines we're talking about here
+    # 2. only pass comments in that range off
+    # 3. split those comments into "moved, didn't move"
+    # 4. for any "didn't move" comments... move them to the top?
+    #
+    # also, should i just do a scan of the configs ++ assignments, and see if any of them have lines out of order,
+    # and decide from there whether or not i want to do set_lines
+
+    configs =
       [config | configs]
       |> Enum.group_by(fn
         {:config, _, [{:__block__, _, [app]} | _]} -> app
@@ -65,10 +73,38 @@ defmodule Styler.Style.Configs do
         |> Enum.sort_by(&Style.without_meta/1)
         |> Style.reset_newlines()
       end)
-      |> Style.fix_line_numbers(List.first(rest))
-      |> Enum.reverse(left_siblings)
 
-    {:skip, {config, %{zm | l: left_siblings, r: rest}}, ctx}
+    nodes =
+      assignments
+      |> Enum.reverse()
+      |> Style.reset_newlines()
+      |> Enum.concat(configs)
+
+    # `set_lines` performs better than `fix_line_numbers` for a large number of nodes moving, as it moves their comments with them
+    # however, it will also move any comments not associated with a node, causing wildly unpredictable sad times!
+    # so i'm trying to guess which change will be less damaging.
+    # moving >=3 nodes hints that this is an initial run, where `set_lines` definitely outperforms.
+    {nodes, comments} =
+      case change_count(nodes) do
+        0 ->
+          {nodes, comments}
+
+        n when n < 3 ->
+          {Style.fix_line_numbers(nodes, List.last(rest)), comments}
+
+        _ ->
+          # after running, this block should take up the same # of lines that it did before
+          # the first node of `rest` is greater than the highest line in configs, assignments
+          # config line is the first line to be used as part of this block
+          # that will change when we consider preceding comments
+          {node_comments, _} = comments_for_node(config, comments)
+          first_line = min(List.last(node_comments)[:line] || cfm[:line], cfm[:line])
+          set_lines(nodes, comments, first_line)
+      end
+
+    [config | left_siblings] = Enum.reverse(nodes, zm.l)
+
+    {:skip, {config, %{zm | l: left_siblings, r: rest}}, %{ctx | comments: comments}}
   end
 
   def run(zipper, %{config?: true} = ctx) do
@@ -77,12 +113,89 @@ defmodule Styler.Style.Configs do
 
   def run(zipper, %{file: file} = ctx) do
     if file =~ ~r|config/.*\.exs| or file =~ ~r|rel/overlays/.*\.exs| do
+      # @TODO have this run forward to `import Config`, then run forward from there until we find `config` itself. no need for multi function head
       {:cont, zipper, Map.put(ctx, :config?, true)}
     else
       {:halt, zipper, ctx}
     end
   end
 
+  defp change_count(nodes, n \\ 0)
+
+  defp change_count([{_, am, _}, {_, bm, _} = b | tail], n) do
+    n = if am[:line] > bm[:line], do: n + 1, else: n
+    change_count([b | tail], n)
+  end
+
+  defp change_count(_, n), do: n
+
+  defp set_lines(nodes, comments, first_line) do
+    {nodes, comments, node_comments} = set_lines(nodes, comments, first_line, [], [])
+    # @TODO if there are dangling comments between the nodes min/max, push them somewhere?
+    # likewise deal with conflicting line comments?
+    {nodes, Enum.sort_by(comments ++ node_comments, & &1.line)}
+  end
+
+  def set_lines([], comments, _, node_acc, c_acc), do: {Enum.reverse(node_acc), comments, c_acc}
+
+  def set_lines([{_, meta, _} = node | nodes], comments, start_line, n_acc, c_acc) do
+    line = meta[:line]
+    last_line = meta[:end_of_expression][:line] || Style.max_line(node)
+
+    {node, node_comments, comments} =
+      if start_line == line do
+        {node, [], comments}
+      else
+        {mine, comments} = comments_for_lines(comments, line, last_line)
+        line_with_comments = (List.first(mine)[:line] || line) - (List.first(mine)[:previous_eol_count] || 1) + 1
+
+        if line_with_comments == start_line do
+          {node, mine, comments}
+        else
+          shift = start_line - line_with_comments
+          node = Style.shift_line(node, shift)
+
+          mine = Enum.map(mine, &%{&1 | line: &1.line + shift})
+          {node, mine, comments}
+        end
+      end
+
+    {_, meta, _} = node
+    # @TODO what about comments that were free floating between blocks? i'm just ignoring them and maybe always will...
+    # kind of just want to shove them to the end though, so that they don't interrupt existing stanzas.
+    # i think that's accomplishable by doing a final call above that finds all comments in the comments list that weren't moved
+    # and which are in the range of start..finish and sets their lines to finish!
+    last_line = meta[:end_of_expression][:line] || Style.max_line(node)
+    last_line = (meta[:end_of_expression][:newlines] || 1) + last_line
+    set_lines(nodes, comments, last_line, [node | n_acc], node_comments ++ c_acc)
+  end
+
+  defp comments_for_node({_, m, _} = node, comments) do
+    last_line = m[:end_of_expression][:line] || Style.max_line(node)
+    comments_for_lines(comments, m[:line], last_line)
+  end
+
+  defp comments_for_lines(comments, start_line, last_line) do
+    comments
+    |> Enum.reverse()
+    |> comments_for_lines(start_line, last_line, [], [])
+  end
+
+  defp comments_for_lines(reversed_comments, start, last, match, acc)
+
+  defp comments_for_lines([], _, _, match, acc), do: {Enum.reverse(match), acc}
+
+  defp comments_for_lines([%{line: line} = comment | rev_comments], start, last, match, acc) do
+    cond do
+      line > last -> comments_for_lines(rev_comments, start, last, match, [comment | acc])
+      line >= start -> comments_for_lines(rev_comments, start, last, [comment | match], acc)
+      # @TODO bug: match line looks like `x = :foo # comment for x`
+      # could account for that by pre-running the formatter on config files :/
+      line == start - 1 -> comments_for_lines(rev_comments, start - 1, last, [comment | match], acc)
+      true -> {match, Enum.reverse(rev_comments, [comment | acc])}
+    end
+  end
+
   defp accumulate([{:config, _, [_, _ | _]} = c | siblings], cs, as), do: accumulate(siblings, [c | cs], as)
   defp accumulate([{:=, _, [_lhs, _rhs]} = a | siblings], cs, as), do: accumulate(siblings, cs, [a | as])
   defp accumulate(rest, configs, assignments), do: {configs, assignments, rest}

lib/zipper.ex

@@ -261,8 +261,8 @@ defmodule Styler.Zipper do
   end
 
   @doc """
-  Returns the previous zipper in depth-first pre-order. If it's already at
-  the end, it returns nil.
+  Returns the previous zipper in depth-first pre-order.
+  Returns nil if the tree is already at the top.
   """
   @spec prev(zipper) :: zipper | nil
   def prev(zipper) do

test/style/configs_test.exs

@@ -218,15 +218,11 @@ defmodule Styler.Style.ConfigsTest do
         import Config
 
         dog_sound = :woof
-        # z is best when configged w/ dog sounds
-        # dog sounds ftw
 
         # this is my big c
         # comment i'd like to leave c
         # about c
         c = :c
-        # Multiline comment
-        # comment in a block
 
         # this is my big my_app
         # comment i'd like to leave my_app
@@ -244,8 +240,12 @@ defmodule Styler.Style.ConfigsTest do
 
         config :a,
           a_longer_name: :a_longer_value,
+          # Multiline comment
+          # comment in a block
           multiple_things: :that_could_all_fit_on_one_line_though
 
+        # z is best when configged w/ dog sounds
+        # dog sounds ftw
         config :z, :x, dog_sound
 
         config my_app, :nooooooooo
@@ -263,5 +263,92 @@ defmodule Styler.Style.ConfigsTest do
         """
       )
     end
+
+    test "comments, more nuanced" do
+      assert_style(
+        """
+        # start config
+        # import
+        import Config
+
+        # random noise
+
+        config :c,
+          # ca
+          ca: :ca,
+          # cb 1
+          # cb 2
+          cb: :cb,
+          cc: :cc,
+          # cd
+          cd: :cd
+
+        # yeehaw
+        config :b, :yeehaw, :meow
+        config :b, :apples, :oranges
+        config :b,
+          a: :b,
+          # bcd
+          c: :d,
+          e: :f
+
+        # some junk after b, idk
+
+        config :a,
+          # aa
+          aa: :aa,
+          # ab 1
+          # ab 2
+          ab: :ab,
+          ac: :ac,
+          # ad
+          ad: :cd
+
+        # end of config
+        """,
+        """
+        # start config
+        # import
+        import Config
+
+        # random noise
+
+        config :a,
+          # aa
+          aa: :aa,
+          # ab 1
+          # ab 2
+          ab: :ab,
+          ac: :ac,
+          # ad
+          ad: :cd
+
+        config :b, :apples, :oranges
+
+        # yeehaw
+        config :b, :yeehaw, :meow
+
+        config :b,
+          a: :b,
+          # bcd
+          c: :d,
+          e: :f
+
+        config :c,
+          # some junk after b, idk
+
+          # ca
+          ca: :ca,
+          # cb 1
+          # cb 2
+          cb: :cb,
+          cc: :cc,
+          # cd
+          cd: :cd
+
+        # end of config
+        """
+      )
+    end
   end
 end