Repository: phoenixframework/phoenix
Pull request #5857 (2024-06-28 07:01:27)

Remove noop Tailwind class 'transform'
Not needed since Tailwind v2.2, see:
- https://github.com/tailwindlabs/tailwindcss/blob/3.4/CHANGELOG.md#220---2021-06-17

rhcarvalho: Nice catch from integration tests wrt `mix format` on the generate code from templates!

Patch:
diff --git a/installer/templates/phx_web/components/core_components.ex b/installer/templates/phx_web/components/core_components.ex
index 586ca1b30..ee349533a 100644
--- a/installer/templates/phx_web/components/core_components.ex
+++ b/installer/templates/phx_web/components/core_components.ex
@@ -604,7 +604,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
       to: selector,
       time: 300,
       transition:
-        {"transition-all transform ease-out duration-300",
+        {"transition-all ease-out duration-300",
          "opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95",
          "opacity-100 translate-y-0 sm:scale-100"}
     )
@@ -615,8 +615,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
       to: selector,
       time: 200,
       transition:
-        {"transition-all transform ease-in duration-200",
-         "opacity-100 translate-y-0 sm:scale-100",
+        {"transition-all ease-in duration-200", "opacity-100 translate-y-0 sm:scale-100",
          "opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95"}
     )
   end
@@ -627,7 +626,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
     |> JS.show(
       to: "##{id}-bg",
       time: 300,
-      transition: {"transition-all transform ease-out duration-300", "opacity-0", "opacity-100"}
+      transition: {"transition-all ease-out duration-300", "opacity-0", "opacity-100"}
     )
     |> show("##{id}-container")
     |> JS.add_class("overflow-hidden", to: "body")
@@ -638,7 +637,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
     js
     |> JS.hide(
       to: "##{id}-bg",
-      transition: {"transition-all transform ease-in duration-200", "opacity-100", "opacity-0"}
+      transition: {"transition-all ease-in duration-200", "opacity-100", "opacity-0"}
     )
     |> hide("##{id}-container")
     |> JS.hide(to: "##{id}", transition: {"block", "block", "hidden"})
diff --git a/lib/mix/tasks/phx.routes.ex b/lib/mix/tasks/phx.routes.ex
index 9af49e333..ab8717a6c 100644
--- a/lib/mix/tasks/phx.routes.ex
+++ b/lib/mix/tasks/phx.routes.ex
@@ -97,10 +97,9 @@ defmodule Mix.Tasks.Phx.Routes do
     %{plug: plug, plug_opts: plug_opts} = meta
 
     {module, func_name} =
-      if log_mod = meta[:log_module] do
-        {log_mod, meta[:log_function]}
-      else
-        {plug, plug_opts}
+      case meta[:mfa] do
+        {mod, fun, _} -> {mod, fun}
+        _ -> {plug, plug_opts}
       end
 
     Mix.shell().info("Module: #{inspect(module)}")
diff --git a/lib/phoenix/logger.ex b/lib/phoenix/logger.ex
index 97f7fea7a..6a19b6a53 100644
--- a/lib/phoenix/logger.ex
+++ b/lib/phoenix/logger.ex
@@ -273,7 +273,6 @@ defmodule Phoenix.Logger do
   def phoenix_router_dispatch_start(_, _, metadata, _) do
     %{log: level, conn: conn, plug: plug} = metadata
     level = log_level(level, conn)
-    log_module = metadata[:log_module] || plug
 
     Logger.log(level, fn ->
       %{
@@ -281,10 +280,16 @@ defmodule Phoenix.Logger do
         plug_opts: plug_opts
       } = metadata
 
+      log_mfa =
+        case metadata[:mfa] do
+          {mod, fun, arity} -> mfa(mod, fun, arity)
+          _ when is_atom(plug_opts) -> mfa(plug, plug_opts, 2)
+          _ -> inspect(plug)
+        end
+
       [
         "Processing with ",
-        inspect(log_module),
-        maybe_action(plug_opts),
+        log_mfa,
         ?\n,
         "  Parameters: ",
         params(conn.params),
@@ -295,8 +300,8 @@ defmodule Phoenix.Logger do
     end)
   end
 
-  defp maybe_action(action) when is_atom(action), do: [?., Atom.to_string(action), ?/, ?2]
-  defp maybe_action(_), do: []
+  defp mfa(mod, fun, arity),
+    do: [inspect(mod), ?., Atom.to_string(fun), ?/, arity + ?0]
 
   defp params(%Plug.Conn.Unfetched{}), do: "[UNFETCHED]"
   defp params(params), do: params |> filter_values() |> inspect()
diff --git a/lib/phoenix/router.ex b/lib/phoenix/router.ex
index 3a9c01dbb..9725b1a21 100644
--- a/lib/phoenix/router.ex
+++ b/lib/phoenix/router.ex
@@ -601,9 +601,12 @@ defmodule Phoenix.Router do
 
     checks =
       routes
-      |> Enum.uniq_by(&{&1.line, &1.plug})
-      |> Enum.map(fn %{line: line, plug: plug} ->
-        quote line: line, do: _ = &unquote(plug).init/1
+      |> Enum.map(fn %{line: line, metadata: metadata, plug: plug} ->
+        {line, Map.get(metadata, :mfa, {plug, :init, 1})}
+      end)
+      |> Enum.uniq()
+      |> Enum.map(fn {line, {module, function, arity}} ->
+        quote line: line, do: _ = &(unquote(module).unquote(function) / unquote(arity))
       end)
 
     keys = [:verb, :path, :plug, :plug_opts, :helper, :metadata]
@@ -752,7 +755,8 @@ defmodule Phoenix.Router do
       when a route matches
     * `:assigns` - a map of data to merge into the connection when a route matches
     * `:metadata` - a map of metadata used by the telemetry events and returned by
-      `route_info/4`
+      `route_info/4`. The `:mfa` field is used by telemetry to print logs and by the
+      router to emit compile time checks. Custom fields may be added.
     * `:warn_on_verify` - the boolean for whether matches to this route trigger
       an unmatched route warning for `Phoenix.VerifiedRoutes`. It is useful to ignore
       an otherwise catch-all route definition from being matched when verifying routes.
diff --git a/lib/phoenix/router/console_formatter.ex b/lib/phoenix/router/console_formatter.ex
index 017219e1b..58f852818 100644
--- a/lib/phoenix/router/console_formatter.ex
+++ b/lib/phoenix/router/console_formatter.ex
@@ -19,45 +19,57 @@ defmodule Phoenix.Router.ConsoleFormatter do
   end
 
   defp format_endpoint(nil, _router, _), do: ""
+
   defp format_endpoint(endpoint, router, widths) do
     case endpoint.__sockets__() do
-      [] -> ""
+      [] ->
+        ""
+
       sockets ->
         Enum.map_join(sockets, "", fn socket ->
           format_websocket(socket, router, widths) <>
-          format_longpoll(socket, router, widths)
+            format_longpoll(socket, router, widths)
         end)
-      end
+    end
   end
 
   defp format_websocket({_path, Phoenix.LiveReloader.Socket, _opts}, _router, _), do: ""
+
   defp format_websocket({path, module, opts}, router, widths) do
     if opts[:websocket] != false do
       prefix = if router.__helpers__(), do: "websocket", else: ""
       {verb_len, path_len, route_name_len} = widths
 
-      String.pad_leading(prefix, route_name_len) <> "  " <>
-      String.pad_trailing(@socket_verb, verb_len) <> "  " <>
-      String.pad_trailing(path <> "/websocket", path_len) <> "  " <>
-      inspect(module) <>
-      "\n"
+      String.pad_leading(prefix, route_name_len) <>
+        "  " <>
+        String.pad_trailing(@socket_verb, verb_len) <>
+        "  " <>
+        String.pad_trailing(path <> "/websocket", path_len) <>
+        "  " <>
+        inspect(module) <>
+        "\n"
     else
       ""
     end
   end
 
   defp format_longpoll({_path, Phoenix.LiveReloader.Socket, _opts}, _router, _), do: ""
+
   defp format_longpoll({path, module, opts}, router, widths) do
     if opts[:longpoll] != false do
       prefix = if router.__helpers__(), do: "longpoll", else: ""
+
       for method <- @longpoll_verbs, into: "" do
         {verb_len, path_len, route_name_len} = widths
 
-        String.pad_leading(prefix, route_name_len) <> "  " <>
-        String.pad_trailing(method, verb_len) <> "  " <>
-        String.pad_trailing(path <> "/longpoll", path_len) <> "  " <>
-        inspect(module) <>
-        "\n"
+        String.pad_leading(prefix, route_name_len) <>
+          "  " <>
+          String.pad_trailing(method, verb_len) <>
+          "  " <>
+          String.pad_trailing(path <> "/longpoll", path_len) <>
+          "  " <>
+          inspect(module) <>
+          "\n"
       end
     else
       ""
@@ -65,7 +77,7 @@ defmodule Phoenix.Router.ConsoleFormatter do
   end
 
   defp calculate_column_widths(router, routes, endpoint) do
-    sockets = endpoint && endpoint.__sockets__() || []
+    sockets = (endpoint && endpoint.__sockets__()) || []
 
     widths =
       Enum.reduce(routes, {0, 0, 0}, fn route, acc ->
@@ -73,36 +85,54 @@ defmodule Phoenix.Router.ConsoleFormatter do
         verb = verb_name(verb)
         {verb_len, path_len, route_name_len} = acc
         route_name = route_name(router, helper)
-        {max(verb_len, String.length(verb)),
-        max(path_len, String.length(path)),
-        max(route_name_len, String.length(route_name))}
+
+        {max(verb_len, String.length(verb)), max(path_len, String.length(path)),
+         max(route_name_len, String.length(route_name))}
       end)
 
     Enum.reduce(sockets, widths, fn {path, _mod, opts}, acc ->
       {verb_len, path_len, route_name_len} = acc
       prefix = if router.__helpers__(), do: "websocket", else: ""
-      verb_length = socket_verbs(opts) |> Enum.map(&String.length/1) |> Enum.max(&>=/2, fn -> 0 end)
 
-      {max(verb_len, verb_length),
-      max(path_len, String.length(path <> "/websocket")),
-      max(route_name_len, String.length(prefix))}
+      verb_length =
+        socket_verbs(opts) |> Enum.map(&String.length/1) |> Enum.max(&>=/2, fn -> 0 end)
+
+      {max(verb_len, verb_length), max(path_len, String.length(path <> "/websocket")),
+       max(route_name_len, String.length(prefix))}
     end)
   end
 
   defp format_route(route, router, column_widths) do
-    %{verb: verb, path: path, plug: plug, metadata: metadata, plug_opts: plug_opts, helper: helper} = route
+    %{
+      verb: verb,
+      path: path,
+      plug: plug,
+      metadata: metadata,
+      plug_opts: plug_opts,
+      helper: helper
+    } = route
+
     verb = verb_name(verb)
     route_name = route_name(router, helper)
     {verb_len, path_len, route_name_len} = column_widths
-    log_module = metadata[:log_module] || plug
 
-    String.pad_leading(route_name, route_name_len) <> "  " <>
-    String.pad_trailing(verb, verb_len) <> "  " <>
-    String.pad_trailing(path, path_len) <> "  " <>
-    "#{inspect(log_module)} #{inspect(plug_opts)}\n"
+    log_module =
+      case metadata[:mfa] do
+        {mod, _fun, _arity} -> mod
+        _ -> plug
+      end
+
+    String.pad_leading(route_name, route_name_len) <>
+      "  " <>
+      String.pad_trailing(verb, verb_len) <>
+      "  " <>
+      String.pad_trailing(path, path_len) <>
+      "  " <>
+      "#{inspect(log_module)} #{inspect(plug_opts)}\n"
   end
 
-  defp route_name(_router, nil),  do: ""
+  defp route_name(_router, nil), do: ""
+
   defp route_name(router, name) do
     if router.__helpers__() do
       name <> "_path"
diff --git a/priv/templates/phx.gen.live/core_components.ex b/priv/templates/phx.gen.live/core_components.ex
index 586ca1b30..ee349533a 100644
--- a/priv/templates/phx.gen.live/core_components.ex
+++ b/priv/templates/phx.gen.live/core_components.ex
@@ -604,7 +604,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
       to: selector,
       time: 300,
       transition:
-        {"transition-all transform ease-out duration-300",
+        {"transition-all ease-out duration-300",
          "opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95",
          "opacity-100 translate-y-0 sm:scale-100"}
     )
@@ -615,8 +615,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
       to: selector,
       time: 200,
       transition:
-        {"transition-all transform ease-in duration-200",
-         "opacity-100 translate-y-0 sm:scale-100",
+        {"transition-all ease-in duration-200", "opacity-100 translate-y-0 sm:scale-100",
          "opacity-0 translate-y-4 sm:translate-y-0 sm:scale-95"}
     )
   end
@@ -627,7 +626,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
     |> JS.show(
       to: "##{id}-bg",
       time: 300,
-      transition: {"transition-all transform ease-out duration-300", "opacity-0", "opacity-100"}
+      transition: {"transition-all ease-out duration-300", "opacity-0", "opacity-100"}
     )
     |> show("##{id}-container")
     |> JS.add_class("overflow-hidden", to: "body")
@@ -638,7 +637,7 @@ defmodule <%= @web_namespace %>.CoreComponents do
     js
     |> JS.hide(
       to: "##{id}-bg",
-      transition: {"transition-all transform ease-in duration-200", "opacity-100", "opacity-0"}
+      transition: {"transition-all ease-in duration-200", "opacity-100", "opacity-0"}
     )
     |> hide("##{id}-container")
     |> JS.hide(to: "##{id}", transition: {"block", "block", "hidden"})


Test patch:
diff --git a/test/mix/tasks/phx.routes_test.exs b/test/mix/tasks/phx.routes_test.exs
index 7db1f46a9..e044fc868 100644
--- a/test/mix/tasks/phx.routes_test.exs
+++ b/test/mix/tasks/phx.routes_test.exs
@@ -1,8 +1,12 @@
-Code.require_file "../../../installer/test/mix_helper.exs", __DIR__
+Code.require_file("../../../installer/test/mix_helper.exs", __DIR__)
 
 defmodule PageController do
   def init(opts), do: opts
   def call(conn, _opts), do: conn
+
+  defmodule Live do
+    def init(opts), do: opts
+  end
 end
 
 defmodule PhoenixTestWeb.Router do
@@ -17,7 +21,7 @@ end
 
 defmodule PhoenixTestLiveWeb.Router do
   use Phoenix.Router
-  get "/", PageController, :index, metadata: %{log_module: PageLive.Index}
+  get "/", PageController, :index, metadata: %{mfa: {PageController.Live, :init, 1}}
 end
 
 defmodule Mix.Tasks.Phx.RoutesTest do
@@ -30,9 +34,11 @@ defmodule Mix.Tasks.Phx.RoutesTest do
   end
 
   test "prints error when explicit router cannot be found" do
-    assert_raise Mix.Error, "the provided router, Foo.UnknownBar.CantFindBaz, does not exist", fn ->
-      Mix.Tasks.Phx.Routes.run(["Foo.UnknownBar.CantFindBaz", "--no-compile"])
-    end
+    assert_raise Mix.Error,
+                 "the provided router, Foo.UnknownBar.CantFindBaz, does not exist",
+                 fn ->
+                   Mix.Tasks.Phx.Routes.run(["Foo.UnknownBar.CantFindBaz", "--no-compile"])
+                 end
   end
 
   test "prints error when implicit router cannot be found" do
@@ -53,9 +59,9 @@ defmodule Mix.Tasks.Phx.RoutesTest do
     assert routes =~ "page_path  GET  /old  PageController :index"
   end
 
-  test "overrides module name for route with :log_module metadata" do
+  test "overrides module name for route with :mfa metadata" do
     Mix.Tasks.Phx.Routes.run(["PhoenixTestLiveWeb.Router", "--no-compile"])
     assert_received {:mix_shell, :info, [routes]}
-    assert routes =~ "page_path  GET  /  PageLive.Index :index"
+    assert routes =~ "page_path  GET  /  PageController.Live :index"
   end
 end
diff --git a/test/phoenix/router/routing_test.exs b/test/phoenix/router/routing_test.exs
index 58ab37ada..fa015fc8c 100644
--- a/test/phoenix/router/routing_test.exs
+++ b/test/phoenix/router/routing_test.exs
@@ -67,7 +67,7 @@ defmodule Phoenix.Router.RoutingTest do
 
     get "/no_log", SomePlug, [], log: false
     get "/fun_log", SomePlug, [], log: {LogLevel, :log_level, []}
-    get "/override-plug-name", SomePlug, :action, metadata: %{log_module: PlugOverride}
+    get "/override-plug-name", SomePlug, :action, metadata: %{mfa: {LogLevel, :log_level, 1}}
     get "/users/:user_id/files/:id", UserController, :image
 
     scope "/halt-plug" do
@@ -283,7 +283,7 @@ defmodule Phoenix.Router.RoutingTest do
 
     test "overrides plug name that processes the route when set in metadata" do
       assert capture_log(fn -> call(Router, :get, "/override-plug-name") end) =~
-               "Processing with PlugOverride"
+               "Processing with Phoenix.Router.RoutingTest.LogLevel.log_level/1"
     end
 
     test "logs custom level when log is set to a 1-arity function" do