Ensure single trailing newline in generated files

And no unintentional consecutive blank lines in the generated content.

Those two issues are rather aesthetic, but downstream users notice and
they are a common source of code churn and maintenance overheard that we
can prevent from now on.

The single trailing newline is a common convention in Unix and POSIX
systems, and it is also what the Elixir formatter does
(`Code.format_file!` always appends a trailing newline [1]).

The consecutive blank lines in Elixir code are also automatically
removed by the Elixir formatter. It is not a strict rule for other
files, but generally they are not expected and most of the time added
unintentionally, e.g. when using conditional EEx templates or
concatenating strings (AGENTS.md / usage rules).

Except for a handful of generated files (favicon.ico, phoenix.png and
*.pem certificates), all generated files are candidates for those two
rules. Instead of updating lots of tests (which would cause a lot of
churn and be a future maintenance burden, easy to miss in new tests), we
update `MixHelper.assert_file/1` to check for those two rules in all
current and future generated files. If we need more exceptions in the
future, it is easy to change `assert_file/1` to add them.

[1]: https://github.com/elixir-lang/elixir/blob/545dddf138e4cb1ee874e6f2c26882c9b438f551/lib/elixir/lib/code.ex#L1137-L1141

Author: rhcarvalho

installer/lib/phx_new/generator.ex

@@ -128,7 +128,7 @@ defmodule Phx.New.Generator do
           project.binding[:javascript] && project.binding[:css] &&
             @new_project_rules_files["assets.md"],
           # generic usage rules
-          "\n<!-- usage-rules-start -->",
+          "<!-- usage-rules-start -->",
           [
             "<!-- phoenix:elixir-start -->\n",
             @rules_files["elixir.md"],
@@ -157,7 +157,7 @@ defmodule Phx.New.Generator do
               @rules_files["liveview.md"],
               "\n<!-- phoenix:liveview-end -->"
             ],
-          "<!-- usage-rules-end -->"
+          "<!-- usage-rules-end -->\n"
         ]
         |> Enum.reject(fn part -> part == nil or part == false end)
         |> Enum.intersperse("\n\n")

installer/templates/phx_assets/app.js.eex

@@ -15,8 +15,8 @@
 //     import "some-package"
 //
 // If you have dependencies that try to import CSS, esbuild will generate a separate `app.css` file.
-// To load it, simply add a second `<link>` to your `root.html.heex` file.
-<%= if @html do %>
+// To load it, simply add a second `<link>` to your `root.html.heex` file.<%= if @html do %>
+
 // Include phoenix_html to handle method=PUT/DELETE in forms and buttons.
 import "phoenix_html"
 // Establish Phoenix Socket and LiveView configuration.
@@ -79,12 +79,11 @@ import "phoenix_html"
 <%= @live_comment %>
 <%= @live_comment %>    window.liveReloader = reloader
 <%= @live_comment %>  })
-<%= @live_comment %>}
+<%= @live_comment %>}<%= if not @live do %>
 
-<%= if not @live do %>
 // Handle flash close
 document.querySelectorAll("[role=alert][data-flash]").forEach((el) => {
   el.addEventListener("click", () => {
     el.setAttribute("hidden", "")
   })
-})<% end %><% end %>
+})<% end %><% end %>

installer/templates/phx_gettext/errors.pot.eex

@@ -6,8 +6,9 @@
 ##
 ## Run `mix gettext.extract` to bring this file up to
 ## date. Leave `msgstr`s empty as changing them here has no
-## effect: edit them in PO (`.po`) files instead.
-<%= if @ecto do %>## From Ecto.Changeset.cast/4
+## effect: edit them in PO (`.po`) files instead.<%= if @ecto do %>
+
+## From Ecto.Changeset.cast/4
 msgid "can't be blank"
 msgstr ""
 

installer/templates/phx_single/gitignore.eex

@@ -23,8 +23,8 @@ erl_crash.dump
 /tmp/
 
 # Ignore package tarball (built via "mix hex.build").
-<%= @app_name %>-*.tar
-<%= if @javascript or @css do %>
+<%= @app_name %>-*.tar<%= if @javascript or @css do %>
+
 # Ignore assets that are produced by build tools.
 /priv/static/assets/
 
@@ -33,9 +33,8 @@ erl_crash.dump
 
 # In case you use Node.js/npm, you want to ignore these.
 npm-debug.log
-/assets/node_modules/
-<% end %><%= if @adapter_app == :ecto_sqlite3 do %>
+/assets/node_modules/<% end %><%= if @adapter_app == :ecto_sqlite3 do %>
+
 # Database files
 *.db
-*.db-*
-<% end %>
+*.db-*<% end %>

installer/templates/phx_static/default.css

@@ -2587,4 +2587,3 @@
     transform: rotate(360deg);
   }
 }
-

installer/templates/phx_umbrella/apps/app_name_web/gitignore.eex

@@ -23,8 +23,8 @@ erl_crash.dump
 /tmp/
 
 # Ignore package tarball (built via "mix hex.build").
-<%= @web_app_name %>-*.tar
-<%= if @javascript or @css do %>
+<%= @web_app_name %>-*.tar<%= if @javascript or @css do %>
+
 # Ignore assets that are produced by build tools.
 /priv/static/assets/
 
@@ -33,9 +33,8 @@ erl_crash.dump
 
 # In case you use Node.js/npm, you want to ignore these.
 npm-debug.log
-/assets/node_modules/
-<% end %><%= if @adapter_app == :ecto_sqlite3 do %>
+/assets/node_modules/<% end %><%= if @adapter_app == :ecto_sqlite3 do %>
+
 # Database files
 *.db
-*.db-*
-<% end %>
+*.db-*<% end %>

installer/templates/phx_umbrella/config/test.exs.eex

@@ -16,7 +16,7 @@ config :phoenix, :plug_init_mode, :runtime<%= if @html do %>
 # Enable helpful, but potentially expensive runtime checks
 config :phoenix_live_view,
   enable_expensive_runtime_checks: true<% end %>
-  
+
 # Sort query params output of verified routes for robust url comparisons
 config :phoenix,
-  sort_verified_routes_query_params: true
+  sort_verified_routes_query_params: true

installer/templates/phx_umbrella/gitignore.eex

@@ -20,10 +20,8 @@ erl_crash.dump
 *.ez
 
 # Temporary files, for example, from tests.
-/tmp/
+/tmp/<%= if @adapter_app == :ecto_sqlite3 do %>
 
-<%= if @adapter_app == :ecto_sqlite3 do %>
 # Database files
 *.db
-*.db-*
-<% end %>
+*.db-*<% end %>

installer/test/mix_helper.exs

@@ -96,8 +96,21 @@ defmodule MixHelper do
 
   def assert_file(file) do
     assert File.regular?(file), "Expected #{file} to exist, but does not"
+    content = File.read!(file)
+
+    if not binary_file?(file) do
+      # \S\n\z matches non-whitespace followed by a single newline at EOF
+      assert content == "" or content =~ ~r/\S\n\z/,
+             "Expected #{file} to end with a single trailing newline"
+
+      refute content =~ "\n\n\n", "Expected #{file} to not contain consecutive blank lines"
+    end
+
+    content
   end
 
+  defp binary_file?(file), do: Path.extname(file) in ~w(.ico .pem .png)
+
   def refute_file(file) do
     refute File.regular?(file), "Expected #{file} to not exist, but it does"
   end
@@ -111,8 +124,8 @@ defmodule MixHelper do
         assert_file(file, &assert(&1 =~ match))
 
       is_function(match, 1) ->
-        assert_file(file)
-        match.(File.read!(file))
+        content = assert_file(file)
+        match.(content)
 
       true ->
         raise inspect({file, match})

installer/test/phx_new_test.exs

@@ -147,7 +147,6 @@ defmodule Mix.Tasks.Phx.NewTest do
       assert_file("phx_blog/.gitignore", fn file ->
         assert file =~ "/priv/static/assets/"
         assert file =~ "phx_blog-*.tar"
-        assert file =~ ~r/\n$/
       end)
 
       assert_file("phx_blog/config/dev.exs", fn file ->
@@ -324,7 +323,6 @@ defmodule Mix.Tasks.Phx.NewTest do
       # No assets
       assert_file("phx_blog/.gitignore", fn file ->
         refute file =~ "/priv/static/assets/"
-        assert file =~ ~r/\n$/
       end)
 
       refute File.exists?("phx_blog/priv/static/images/logo.svg")
@@ -531,8 +529,6 @@ defmodule Mix.Tasks.Phx.NewTest do
         refute file =~ "/priv/static/assets/"
       end)
 
-      assert_file("phx_blog/.gitignore")
-      assert_file("phx_blog/.gitignore", ~r/\n$/)
       assert_file("phx_blog/priv/static/assets/css/app.css")
       assert_file("phx_blog/priv/static/assets/js/app.js")
       assert_file("phx_blog/priv/static/favicon.ico")
@@ -592,7 +588,6 @@ defmodule Mix.Tasks.Phx.NewTest do
       Mix.Tasks.Phx.New.run([project_path, "--app", @app_name, "--module", "PhoteuxBlog"])
 
       assert_file("custom_path/.gitignore")
-      assert_file("custom_path/.gitignore", ~r/\n$/)
       assert_file("custom_path/mix.exs", ~r/app: :phx_blog/)
       assert_file("custom_path/lib/phx_blog_web/endpoint.ex", ~r/app: :phx_blog/)
       assert_file("custom_path/config/config.exs", ~r/namespace: PhoteuxBlog/)