Skip to content

Commit

Permalink
ensure all nodes have line meta
Browse files Browse the repository at this point in the history
  • Loading branch information
novaugust committed Dec 9, 2023
1 parent c01f2e1 commit 9d5ed6b
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 16 deletions.
2 changes: 1 addition & 1 deletion lib/style.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion lib/style/blocks.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 12 additions & 10 deletions lib/style/module_directives.ex
Original file line number Diff line number Diff line change
Expand Up @@ -72,21 +72,21 @@ defmodule Styler.Style.ModuleDirectives do
@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

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!
Expand All @@ -95,9 +95,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)
Expand All @@ -122,16 +122,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()
Expand All @@ -148,7 +150,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()
Expand Down
42 changes: 38 additions & 4 deletions test/support/style_case.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -45,12 +47,44 @@ defmodule Styler.StyleCase do
IO.puts("========================\n")
end

# Ensure that every node has `line` meta so that we get better comments behaviour
styled_ast
|> Styler.Zipper.zip()
|> Styler.Zipper.traverse(fn {
{_, meta, _} = node, _} = zipper ->
assert meta[:line], "missing `:line` meta in \nnode:\n#{inspect node} \ntree:\n#{inspect 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
dbg(up && Zipper.node(up))
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)
Expand Down

0 comments on commit 9d5ed6b

Please sign in to comment.