diff --git a/lib/sourceror/comments.ex b/lib/sourceror/comments.ex index 4df9d7b..b1ea9b5 100644 --- a/lib/sourceror/comments.ex +++ b/lib/sourceror/comments.ex @@ -14,6 +14,10 @@ defmodule Sourceror.Comments do `:trailing_comments` field. """ @spec merge_comments(Macro.t(), list(map)) :: Macro.t() + def merge_comments({:__block__, _meta, []} = quoted, comments) do + trailing_block({quoted, comments}) + end + def merge_comments(quoted, comments) do {quoted, leftovers} = Macro.traverse(quoted, comments, &do_merge_comments/2, &merge_leftovers/2) @@ -134,27 +138,47 @@ defmodule Sourceror.Comments do quoted end - Macro.postwalk(quoted, [], fn - {_, _, _} = quoted, acc -> - do_extract_comments(quoted, acc, collapse_comments) + output = + Macro.prewalk(quoted, [], fn + {_, meta, _} = quoted, acc -> + traling_block? = meta[:__sourceror__][:trailing_block] || false + do_extract_comments(quoted, acc, collapse_comments, traling_block?) - other, acc -> - {other, acc} - end) + other, acc -> + {other, acc} + end) + + output end - defp do_extract_comments({_, meta, _} = quoted, acc, collapse_comments) do - leading_comments = Keyword.get(meta, :leading_comments, []) + defp do_extract_comments({_, meta, [quoted]}, [], collapse_comments, true) do + {quoted, comments} = do_extract_comments(quoted, [], collapse_comments, false) + quoted = update_empty_quoted(quoted) + + {start, span} = span(quoted) + + {leading_comments, end_leading_comments} = + trailing_block_comments(meta[:leading_comments], collapse_comments, start) - leading_comments_count = length(leading_comments) + end_line = + if span > 0 do + end_leading_comments + start + span + else + max(end_leading_comments, 1) + end + + {trailing_comments, _} = + trailing_block_comments(meta[:trailing_comments], collapse_comments, end_line) + + {quoted, Enum.concat([leading_comments, comments, trailing_comments])} + end + + defp do_extract_comments({_, meta, _} = quoted, acc, collapse_comments, false) do + leading_comments = Keyword.get(meta, :leading_comments, []) leading_comments = if collapse_comments do - for {comment, i} <- Enum.with_index(leading_comments, 0) do - next_eol_correction = max(0, comment.next_eol_count - 1) - line = max(1, meta[:line] - (leading_comments_count - i + next_eol_correction)) - %{comment | line: line} - end + collapse_comments(meta[:line], leading_comments) else leading_comments end @@ -163,7 +187,7 @@ defmodule Sourceror.Comments do trailing_comments = if collapse_comments do - collapse_trailing_comments(quoted, trailing_comments) + quoted |> Sourceror.get_end_line() |> collapse_comments(trailing_comments) else trailing_comments end @@ -190,34 +214,59 @@ defmodule Sourceror.Comments do {quoted, acc} end - defp collapse_trailing_comments(quoted, trailing_comments) do - meta = Sourceror.get_meta(quoted) - trailing_block? = meta[:__sourceror__][:trailing_block] + defp span({:__block__, meta, []}), do: {meta[:line], 0} + + defp span(quoted) do + %{start: range_start, end: range_end} = Sourceror.get_range(quoted, include_comments: true) + {range_start[:line], range_end[:line] - range_start[:line] + 1} + end + + defp update_empty_quoted({:__block__, meta, []}) do + {:__block__, Keyword.put(meta, :line, 1), []} + end + + defp update_empty_quoted(quoted), do: quoted + + defp trailing_block_comments([], _collapse_comments, line), do: {[], line - 1} - comments = - Enum.map(trailing_comments, fn comment -> - line = meta[:end_of_expression][:line] || meta[:line] + defp trailing_block_comments(comments, false, line), do: {comments, line} - %{comment | line: line - 1} + defp trailing_block_comments([comment | _] = comments, true, line) do + prev = min(comment.previous_eol_count, 2) - 1 + line = line + prev + + {comments, line} = + Enum.reduce(comments, {[], line}, fn comment, {acc, line} -> + comment = %{comment | line: line} + line = line + min(comment.next_eol_count, 2) + {[comment | acc], line} end) - comments = - case comments do - [first | rest] -> - prev_eol_count = if trailing_block?, do: first.previous_eol_count, else: 0 + {Enum.reverse(comments), line} + end - [%{first | previous_eol_count: prev_eol_count} | rest] + defp collapse_comments(_line, []), do: [] - _ -> - comments - end + defp collapse_comments(line, comments) do + comments + |> Enum.reverse() + |> Enum.reduce({[], line}, fn comment, {acc, line} -> + line = line - comment.next_eol_count + comment = %{comment | line: line} + {[comment | acc], line} + end) + |> elem(0) + end - case List.pop_at(comments, -1) do - {last, rest} when is_map(last) -> - rest ++ [%{last | next_eol_count: 0}] + defp trailing_block({quoted, []}), do: quoted - _ -> - comments - end + defp trailing_block({{_form, meta, _args} = quoted, leftovers}) do + {:__block__, + [ + __sourceror__: %{trailing_block: true}, + trailing_comments: leftovers, + leading_comments: [], + line: meta[:line] + ], [quoted]} end end diff --git a/lib/sourceror/lines_corrector.ex b/lib/sourceror/lines_corrector.ex index ab21461..57e9549 100644 --- a/lib/sourceror/lines_corrector.ex +++ b/lib/sourceror/lines_corrector.ex @@ -14,76 +14,112 @@ defmodule Sourceror.LinesCorrector do * If a node has a line number lower than the one before, its line number is recursively incremented by the line number difference, if it's not a pipeline operator. - * If a node has leading comments, it's line number is incremented by the - length of the comments list - * If a node has trailing comments, it's end_of_expression and end line - metadata are set to the line of their last child plus the trailing comments - list length + * If a node has leading comments, its line number is incremented by the + newlines for those comments. + * If a node has trailing comments, its end_of_expression and end line + metadata are set to the line of their last child plus the newlines of the + trailing comments list. """ def correct(quoted) do - {ast, _} = - Macro.traverse(quoted, %{last_line: 1, last_form: nil}, &pre_correct/2, &post_correct/2) + case trailing_block(quoted) do + {:ok, block, line} -> + block + |> do_correct(line) + |> into_trailing_block(quoted) + + :error -> + do_correct(quoted, 1) + end + end - ast + defp into_trailing_block(quoted, {:__block__, meta, [_]}) do + {:__block__, meta, [quoted]} end - defp pre_correct({form, meta, args} = quoted, state) do - {quoted, state} = - cond do - is_nil(meta[:line]) -> - meta = Keyword.put(meta, :line, state.last_line) - {{form, meta, args}, %{state | last_form: form}} + defp trailing_block({:__block__, meta, [quoted]}) do + if meta[:__sourceror__][:trailing_block] || false do + {:ok, quoted, comments_eol_count(meta[:leading_comments])} + else + :error + end + end - get_line(quoted) <= state.last_line and not is_binary_op(form) -> - correction = state.last_line - get_line(quoted) - quoted = recursive_correct_lines(quoted, correction) - {quoted, %{state | last_line: get_line(quoted), last_form: form}} + defp trailing_block(_quoted), do: :error - true -> - {quoted, %{state | last_form: form}} - end + defp do_correct(quoted, line) do + quoted + |> Macro.traverse(%{last_line: line}, &pre_correct/2, &post_correct/2) + |> elem(0) + end - if has_leading_comments?(quoted) do - leading_comments = length(meta[:leading_comments]) - quoted = recursive_correct_lines(quoted, leading_comments + 1) - {quoted, %{state | last_line: meta[:line]}} - else - {quoted, state} - end + defp pre_correct({_form, _meta, _args} = quoted, state) do + do_pre_correct(quoted, state) end defp pre_correct(quoted, state) do {quoted, state} end - defp post_correct({_, meta, _} = quoted, state) do - quoted = - with {form, meta, [{_, _, _} = left, right]} when is_binary_op(form) <- quoted do - # We must ensure that, for binary operators, the operator line number is - # not greater than the left operand. Otherwise the comment eol counts - # will be ignored by the formatter - left_line = get_line(left) - - if left_line > get_line(quoted) do - {form, Keyword.put(meta, :line, left_line), [left, right]} - else - quoted - end - end + defp do_pre_correct({form, meta, args} = quoted, state) do + case correction(form, meta[:line], meta[:leading_comments], state) do + nil -> + meta = Keyword.put(meta, :line, state.last_line) + {{form, meta, args}, state} + + 0 -> + {quoted, state} + + correction -> + quoted = recursive_correct_lines(quoted, correction) + state = %{state | last_line: get_line(quoted)} + {quoted, state} + end + end + + defp correction(_form, nil, _comments, _state), do: nil + + defp correction(form, _line, _comments, _state) when is_binary_op(form), do: 0 + + defp correction(_form, line, nil, state) do + max(state.last_line - line, 0) + end + + defp correction(_form, line, [], state) do + max(state.last_line - line, 0) + end + + defp correction(_form, line, comments, state) do + comments_last_line = comments_last_line(comments) + extra_lines = if state.last_line == line, do: 1, else: 0 + + extra_lines = + if line <= comments_last_line, + do: extra_lines + (comments_last_line - line) + 2, + else: extra_lines - last_line = Sourceror.get_end_line(quoted, state.last_line) + max(state.last_line + extra_lines + comments_eol_count(comments) - line, 0) + end + + defp post_correct({_form, _meta, _args} = quoted, state) do + do_post_correct(quoted, state) + end + + defp post_correct(quoted, state) do + {quoted, state} + end + + defp do_post_correct({_, meta, _} = quoted, state) do + quoted = maybe_correct_binary_op(quoted) last_line = if has_trailing_comments?(quoted) do - if Sourceror.Identifier.do_block?(quoted) do - last_line + length(meta[:trailing_comments] || []) + 2 - else - last_line + length(meta[:trailing_comments] || []) + 1 - end + state.last_line + comments_eol_count(meta[:trailing_comments]) + 1 else - last_line + state.last_line end + last_line = Sourceror.get_end_line(quoted, last_line) + quoted = quoted |> maybe_correct_end_of_expression(last_line) @@ -93,10 +129,22 @@ defmodule Sourceror.LinesCorrector do {quoted, %{state | last_line: last_line}} end - defp post_correct(quoted, state) do - {quoted, state} + defp maybe_correct_binary_op({form, meta, [{_, _, _} = left, right]} = quoted) + when is_binary_op(form) do + # We must ensure that, for binary operators, the operator line number is + # not greater than the left operand. Otherwise the comment eol counts + # will be ignored by the formatter + left_line = get_line(left) + + if left_line > get_line(quoted) do + {form, Keyword.put(meta, :line, left_line), [left, right]} + else + quoted + end end + defp maybe_correct_binary_op(quoted), do: quoted + defp maybe_correct_end_of_expression({form, meta, args} = quoted, last_line) do meta = if meta[:end_of_expression] || has_trailing_comments?(quoted) do @@ -159,4 +207,24 @@ defmodule Sourceror.LinesCorrector do ast end) end + + defp comments_last_line(comments) do + last = List.last(comments) + last.line + end + + defp comments_eol_count(comments, count \\ nil) + + defp comments_eol_count(nil, _count), do: 0 + + defp comments_eol_count([], count), do: count || 0 + + defp comments_eol_count([comment | _] = comments, nil) do + line = comment.previous_eol_count - 1 + comments_eol_count(comments, line) + end + + defp comments_eol_count([comment | comments], lines) do + comments_eol_count(comments, lines + comment.next_eol_count) + end end diff --git a/test/comments_test.exs b/test/comments_test.exs index 507bd28..687ef3b 100644 --- a/test/comments_test.exs +++ b/test/comments_test.exs @@ -2,6 +2,23 @@ defmodule SourcerorTest.CommentsTest do use ExUnit.Case, async: true doctest Sourceror.Comments + defmacrop assert_to_string(quoted, expected) do + quote bind_quoted: [quoted: quoted, expected: expected] do + expected_formatted = expected |> Code.format_string!([]) |> IO.iodata_to_binary() + + if Version.match?(System.version(), "~> 1.16") do + assert expected == expected_formatted, """ + The given expected code is not formatted. + Formatted code: + #{expected_formatted} + """ + end + + assert Sourceror.to_string(quoted, collapse_comments: true, correct_lines: true) == + expected_formatted + end + end + describe "merge_comments/2" do test "merges leading comments" do quoted = @@ -125,10 +142,18 @@ defmodule SourcerorTest.CommentsTest do {_quoted, comments} = Sourceror.Comments.extract_comments(quoted, collapse_comments: true) assert [ - %{line: 1, text: "# A"}, + %{line: 0, text: "# A"}, %{line: 1, text: "# B"} ] = comments + {_quoted, comments} = + Sourceror.Comments.extract_comments(quoted, collapse_comments: true, correct_lines: true) + + assert [ + %{line: 3, text: "# A"}, + %{line: 4, text: "# B"} + ] = comments + quoted = Sourceror.parse_string!(""" def a do @@ -140,9 +165,18 @@ defmodule SourcerorTest.CommentsTest do {_quoted, comments} = Sourceror.Comments.extract_comments(quoted, collapse_comments: true, correct_lines: true) + assert_to_string(quoted, """ + def a do + :ok + # A + end + + # B\ + """) + assert [ - %{line: 5, text: "# A"}, - %{line: 7, text: "# B"} + %{line: 3, text: "# A"}, + %{line: 4, text: "# B"} ] = comments quoted = @@ -156,22 +190,123 @@ defmodule SourcerorTest.CommentsTest do {_quoted, comments} = Sourceror.Comments.extract_comments(quoted, collapse_comments: true, correct_lines: true) + assert_to_string(quoted, """ + Foo.{ + A + # A + } + + # B\ + """) + assert [ - %{line: 5, text: "# A"}, - %{line: 7, text: "# B"} + %{line: 3, text: "# A"}, + %{line: 4, text: "# B"} ] = comments - assert Sourceror.to_string(quoted, collapse_comments: true, correct_lines: true) == - """ - Foo.{ - A + quoted = + Sourceror.parse_string!(""" + Foo.{ + A + # A + # B + # C + } # X + # Y + # Z + """) + + {_quoted, comments} = + Sourceror.Comments.extract_comments(quoted, collapse_comments: true, correct_lines: true) + + assert_to_string(quoted, """ + Foo.{ + A + # A + # B + # C + } + + # X + # Y + # Z\ + """) + + assert [ + %{line: 3, text: "# A"}, + %{line: 4, text: "# B"}, + %{line: 5, text: "# C"}, + %{line: 6, text: "# X"}, + %{line: 7, text: "# Y"}, + %{line: 8, text: "# Z"} + ] = comments + + quoted = + Sourceror.parse_string!(""" + if a do + :b # yes + else + :a # no + end + """) + + {_quoted, comments} = + Sourceror.Comments.extract_comments(quoted, collapse_comments: true, correct_lines: true) + + assert [ + %{line: 2, text: "# yes"}, + %{line: 4, text: "# no"} + ] = comments + + assert_to_string(quoted, """ + if a do + # yes + :b + else + # no + :a + end\ + """) + end + + test "remove this line" do + quoted = + Sourceror.parse_string!(""" + cond do + # cond 1 + a -> # if a + :a + # cond 2 + b -> # if b + :b + end + """) + + {_quoted, comments} = + Sourceror.Comments.extract_comments(quoted, collapse_comments: true, correct_lines: true) + + assert [ + %{line: 3, text: "# cond 1"}, + %{line: 4, text: "# if a"}, + %{line: 9, text: "# cond 2"}, + %{line: 10, text: "# if b"} + ] = comments + + assert_to_string(quoted, """ + cond do + # cond 1 + + # if a + a -> + :a - # A - } + # cond 2 - # B - """ - |> String.trim() + # if b + b -> + :b + end\ + """) end end end diff --git a/test/lines_corrector_test.exs b/test/lines_corrector_test.exs index b1270fe..7ecb71b 100644 --- a/test/lines_corrector_test.exs +++ b/test/lines_corrector_test.exs @@ -57,8 +57,8 @@ defmodule SourcerorTest.LinesCorrectorTest do assert {:foo, foo_meta, [[{_, {:bar, bar_meta, _}}]]} = corrected assert foo_meta[:line] == 1 - assert bar_meta[:line] == 3 - assert foo_meta[:end][:line] == 3 + assert bar_meta[:line] == 5 + assert foo_meta[:end][:line] == 5 assert Sourceror.to_string(corrected) == ~S""" diff --git a/test/sourceror_test.exs b/test/sourceror_test.exs index 9c68151..78dce15 100644 --- a/test/sourceror_test.exs +++ b/test/sourceror_test.exs @@ -2,14 +2,27 @@ defmodule SourcerorTest do use ExUnit.Case, async: true doctest Sourceror - defmacro assert_same(string, opts \\ []) do + defmacrop assert_same(string, opts \\ []) do quote bind_quoted: [string: string, opts: opts] do - string = String.trim(string) - assert string == Sourceror.parse_string!(string) |> Sourceror.to_string(opts) + formatted = string |> Code.format_string!([]) |> IO.iodata_to_binary() + + if Version.match?(System.version(), "~> 1.16") do + assert formatted == String.trim(string), """ + The given expected code is not formatted. + Formatted code: + #{formatted} + """ + end + + assert formatted == Sourceror.parse_string!(string) |> Sourceror.to_string(opts) end end describe "parse_string!/2 and to_string/2 comment position preservation" do + test "empty string" do + assert_same("") + end + test "just some comments" do assert_same(~S""" # comment @@ -180,9 +193,94 @@ defmodule SourcerorTest do def request(%S3{http_method: :head} = op), do: head_object(op) """) end + + test "after expression" do + code = ~S""" + bar() + # test-comment + foo() + """ + + assert_same(code) + end + + test "last line in def" do + code = ~S""" + def a_fun do + :return_value + # test-comment + end + """ + + assert_same(code) + end + + test "in directly executed anonymous function" do + code = ~S""" + (fn x -> + # test-comment + x * x + end).() + """ + + assert_same(code) + end + + test "multiple comment blocks" do + code = ~S""" + defmodule Z do + alias B + alias A + + def a do + # test1 + # test2 + # line1 + # line2 + :ok + end + + # test + defp z do + 1 + end + end + """ + + assert_same(code) + end + + test "multiline comments in list" do + code = ~S""" + [ + :field_1, + + ####################################################### + ### Another comment + ####################################################### + + :field_2 + ] + """ + + assert_same(code) + end + + test "with a :do in the args" do + assert_same(""" + defp foo(state, {:do, :code}) do + # Lorem ispum + state + end + """) + end end describe "parse_string!/2" do + test "returns an empty block for an emtpy string" do + assert Sourceror.parse_string!("") == {:__block__, [], []} + end + test "raises on invalid string" do assert_raise SyntaxError, fn -> Sourceror.parse_string!(":ok end") @@ -375,10 +473,16 @@ defmodule SourcerorTest do code = """ [ 1, - ] + ]\ """ - assert_same(code, quoted_to_algebra: quoted_to_algebra, trailing_comma: true) + assert code == + code + |> Sourceror.parse_string!() + |> Sourceror.to_string( + quoted_to_algebra: quoted_to_algebra, + trailing_comma: true + ) end end @@ -620,7 +724,6 @@ defmodule SourcerorTest do ~S""" foo do :ok - # B # A end @@ -643,7 +746,6 @@ defmodule SourcerorTest do ~S""" foo do :ok - # B end """ @@ -662,7 +764,6 @@ defmodule SourcerorTest do ~S""" Foo.{ Bar - # B } """ @@ -753,7 +854,6 @@ defmodule SourcerorTest do ~S""" foo do :ok - # B end """ @@ -772,7 +872,6 @@ defmodule SourcerorTest do ~S""" Foo.{ Bar - # B } """