Repository: adobe/elixir-styler
Pull request #101 (2023-12-08 19:11:48)

add test ensuring all nodes have line meta


Patch:
diff --git a/lib/style.ex b/lib/style.ex
index 82ffacc..2eba371 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 4b0d0b1..9e6de65 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 42b9b44..28c5bb7 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()


Test patch:
diff --git a/test/support/style_case.ex b/test/support/style_case.ex
index 2d560bc..ef52deb 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)