From 9da4ca0c33baa857445c88e04373b69d58f9171a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@dashbit.co>
Date: Sat, 18 Sep 2021 09:59:23 +0200
Subject: [PATCH] Do not generate conflicting helpers in router

---
 lib/phoenix/router.ex         |  8 ++++---
 lib/phoenix/router/helpers.ex | 43 +++++++++++++++++++++++++----------
 2 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/lib/phoenix/router.ex b/lib/phoenix/router.ex
index a0315742a9..0fb7aea86d 100644
--- a/lib/phoenix/router.ex
+++ b/lib/phoenix/router.ex
@@ -433,9 +433,11 @@ defmodule Phoenix.Router do
 
     # @anno is used here to avoid warnings if forwarding to root path
     match_404 =
-      quote @anno do
-        def __match_route__(_method, _path_info, _host) do
-          :error
+      unless Enum.any?(routes, &(&1.kind == :forward and &1.path == "/")) do
+        quote @anno do
+          def __match_route__(_method, _path_info, _host) do
+            :error
+          end
         end
       end
 
diff --git a/lib/phoenix/router/helpers.ex b/lib/phoenix/router/helpers.ex
index d0cc374a0c..1f7d9b8782 100644
--- a/lib/phoenix/router/helpers.ex
+++ b/lib/phoenix/router/helpers.ex
@@ -109,12 +109,14 @@ defmodule Phoenix.Router.Helpers do
       end)
 
     trailing_slash? = Enum.any?(routes, fn {route, _} -> route.trailing_slash? end)
-
     groups = Enum.group_by(routes, fn {route, _exprs} -> route.helper end)
 
     impls =
-      for {_helper, group} <- groups,
-          {route, exprs} <- Enum.sort_by(group, fn {_, exprs} -> length(exprs.binding) end),
+      for {_helper, helper_routes} <- groups,
+          {_, [{route, exprs} | _]} <-
+            helper_routes
+            |> Enum.group_by(fn {route, exprs} -> [length(exprs.binding) | route.plug_opts] end)
+            |> Enum.sort(),
           do: defhelper(route, exprs)
 
     catch_all = Enum.map(groups, &defhelper_catch_all/1)
@@ -143,8 +145,8 @@ defmodule Phoenix.Router.Helpers do
     end
 
     defcatch_all = quote @anno do
-      defcatch_all = fn helper, lengths, routes ->
-	      for length <- lengths do
+      defcatch_all = fn helper, binding_lengths, params_lengths, routes ->
+	      for length <- binding_lengths do
 	        binding = List.duplicate({:_, [], nil}, length)
 	        arity = length + 2
 
@@ -153,15 +155,20 @@ defmodule Phoenix.Router.Helpers do
             raise_route_error(unquote(helper), :path, unquote(arity), action, [])
           end
 
-          def unquote(:"#{helper}_path")(conn_or_endpoint, action, unquote_splicing(binding), params) do
-            path(conn_or_endpoint, "/")
-            raise_route_error(unquote(helper), :path, unquote(arity + 1), action, params)
-          end
-
           def unquote(:"#{helper}_url")(conn_or_endpoint, action, unquote_splicing(binding)) do
             url(conn_or_endpoint)
             raise_route_error(unquote(helper), :url, unquote(arity), action, [])
           end
+        end
+
+        for length <- params_lengths do
+          binding = List.duplicate({:_, [], nil}, length)
+          arity = length + 2
+
+          def unquote(:"#{helper}_path")(conn_or_endpoint, action, unquote_splicing(binding), params) do
+            path(conn_or_endpoint, "/")
+            raise_route_error(unquote(helper), :path, unquote(arity + 1), action, params)
+          end
 
           def unquote(:"#{helper}_url")(conn_or_endpoint, action, unquote_splicing(binding), params) do
             url(conn_or_endpoint)
@@ -332,15 +339,27 @@ defmodule Phoenix.Router.Helpers do
       |> Enum.map(fn {routes, exprs} -> {routes.plug_opts, Enum.map(exprs.binding, &elem(&1, 0))} end)
       |> Enum.sort()
 
-    lengths =
+    params_lengths =
 	    routes
 	    |> Enum.map(fn {_, bindings} -> length(bindings) end)
 	    |> Enum.uniq()
 
+    # Each helper defines catch alls like this:
+    #
+    #     def helper_path(context, action, ...binding)
+    #     def helper_path(context, action, ...binding, params)
+    #
+    # Given the helpers are ordered by binding length, the additional
+    # helper with param for a helper_path/n will always override the
+    # binding for helper_path/n+1, so we skip those here to avoid warnings.
+    binding_lengths =
+      Enum.reject(params_lengths, &(&1 - 1 in params_lengths))
+
     quote do
       defcatch_all.(
         unquote(helper),
-        unquote(lengths),
+        unquote(binding_lengths),
+        unquote(params_lengths),
         unquote(Macro.escape(routes))
       )
     end