diff --git a/lib/style.ex b/lib/style.ex index 82ffacc7..2eba3710 100644 --- a/lib/style.ex +++ b/lib/style.ex @@ -94,7 +94,7 @@ defmodule Styler.Style do # give it a block parent, then step back to the child - we can insert next to it now that it's in a block defp wrap_in_block(zipper) do zipper - |> Zipper.update(fn {_, meta, _} = node -> {:__block__, [line: meta[:line] || 99_999], [node]} end) + |> Zipper.update(fn {_, meta, _} = node -> {:__block__, Keyword.take(meta, [:line]), [node]} end) |> Zipper.down() end diff --git a/lib/style/blocks.ex b/lib/style/blocks.ex index 4b0d0b16..9e6de656 100644 --- a/lib/style/blocks.ex +++ b/lib/style/blocks.ex @@ -92,7 +92,7 @@ defmodule Styler.Style.Blocks do Enum.any?(postroll) -> {node, do_body_meta, do_children} = do_body do_children = if node == :__block__, do: do_children, else: [do_body] - do_body = {:__block__, [line: do_body_meta[:line]], Enum.reverse(postroll, do_children)} + do_body = {:__block__, Keyword.take(do_body_meta, [:line]), Enum.reverse(postroll, do_children)} {reversed_clauses, do_body} # Credo.Check.Refactor.RedundantWithClauseResult diff --git a/lib/style/module_directives.ex b/lib/style/module_directives.ex index 42b9b447..28c5bb70 100644 --- a/lib/style/module_directives.ex +++ b/lib/style/module_directives.ex @@ -71,8 +71,7 @@ defmodule Styler.Style.ModuleDirectives do @directives ~w(alias import require use)a @attr_directives ~w(moduledoc shortdoc behaviour)a - # @TODO put line meta in moduledoc false usages - @moduledoc_false {:@, [], [{:moduledoc, [], [{:__block__, [], [false]}]}]} + @moduledoc_false {:@, [line: nil], [{:moduledoc, [line: nil], [{:__block__, [line: nil], [false]}]}]} def run({{:defmodule, _, children}, _} = zipper, ctx) do [name, [{{:__block__, do_meta, [:do]}, _body}]] = children @@ -80,13 +79,13 @@ defmodule Styler.Style.ModuleDirectives do if do_meta[:format] == :keyword do {:skip, zipper, ctx} else - add_moduledoc? = add_moduledoc?(name) + moduledoc = moduledoc(name) # Move the zipper's focus to the module's body body_zipper = zipper |> Zipper.down() |> Zipper.right() |> Zipper.down() |> Zipper.down() |> Zipper.right() case Zipper.node(body_zipper) do {:__block__, _, _} -> - {:skip, organize_directives(body_zipper, add_moduledoc?), ctx} + {:skip, organize_directives(body_zipper, moduledoc), ctx} {:@, _, [{:moduledoc, _, _}]} -> # a module whose only child is a moduledoc. nothing to do here! @@ -95,9 +94,9 @@ defmodule Styler.Style.ModuleDirectives do only_child -> # There's only one child, and it's not a moduledoc. Conditionally add a moduledoc, then style the only_child - if add_moduledoc? do + if moduledoc do body_zipper - |> Zipper.replace({:__block__, [], [@moduledoc_false, only_child]}) + |> Zipper.replace({:__block__, [], [moduledoc, only_child]}) |> Zipper.down() |> Zipper.right() |> run(ctx) @@ -122,16 +121,18 @@ defmodule Styler.Style.ModuleDirectives do def run(zipper, ctx), do: {:cont, zipper, ctx} - defp add_moduledoc?({:__aliases__, _, aliases}) do + defp moduledoc({:__aliases__, m, aliases}) do name = aliases |> List.last() |> to_string() # module names ending with these suffixes will not have a default moduledoc appended - not String.ends_with?(name, ~w(Test Mixfile MixProject Controller Endpoint Repo Router Socket View HTML JSON)) + unless String.ends_with?(name, ~w(Test Mixfile MixProject Controller Endpoint Repo Router Socket View HTML JSON)) do + Style.set_line(@moduledoc_false, m[:line] + 1) + end end # a dynamic module name, like `defmodule my_variable do ... end` - defp add_moduledoc?(_), do: false + defp moduledoc(_), do: nil - defp organize_directives(parent, add_moduledoc? \\ false) do + defp organize_directives(parent, moduledoc \\ nil) do {directives, nondirectives} = parent |> Zipper.children() @@ -148,7 +149,7 @@ defmodule Styler.Style.ModuleDirectives do end) shortdocs = directives[:"@shortdoc"] || [] - moduledocs = directives[:"@moduledoc"] || if add_moduledoc?, do: [@moduledoc_false], else: [] + moduledocs = directives[:"@moduledoc"] || List.wrap(moduledoc) behaviours = expand_and_sort(directives[:"@behaviour"] || []) uses = (directives[:use] || []) |> Enum.flat_map(&expand_directive/1) |> reset_newlines() diff --git a/test/support/style_case.ex b/test/support/style_case.ex index 2d560bc1..ef52deb3 100644 --- a/test/support/style_case.ex +++ b/test/support/style_case.ex @@ -24,6 +24,8 @@ defmodule Styler.StyleCase do expected = expected || before quote bind_quoted: [before: before, expected: expected] do + alias Styler.Zipper + expected = String.trim(expected) {styled_ast, styled, styled_comments} = style(before) @@ -45,6 +47,47 @@ defmodule Styler.StyleCase do IO.puts("========================\n") end + # Ensure that every node has `line` meta so that we get better comments behaviour + styled_ast + |> Zipper.zip() + |> Zipper.traverse(fn + {{node, meta, _} = ast, _} = zipper -> + up = Zipper.up(zipper) + # body blocks - for example, the block node for an anonymous function - don't have line meta + # yes, i just did `&& case`. sometimes it's funny to write ugly things in my project that's all about style. + # i believe they calls that one "irony" + is_body_block? = + node == :__block__ && + case up && Zipper.node(up) do + # top of a snippet + nil -> true + # do/else/etc + {{:__block__, _, [_]}, {:__block__, [], _}} -> true + # anon fun + {:->, _, _} -> true + _ -> false + end + + unless meta[:line] || is_body_block? do + IO.puts("missing `:line` meta in node:") + dbg(ast) + + IO.puts("tree:") + dbg(styled_ast) + + IO.puts("expected:") + dbg(elem(Styler.string_to_quoted_with_comments(expected), 0)) + + IO.puts("code:\n#{styled}") + flunk("") + end + + zipper + + zipper -> + zipper + end) + assert expected == styled {_, restyled, _} = style(styled)