Convert route params array into object

This PR converts the route params array into an object and moves the
error generation code into its own object as well. This is a refactoring
of internal code to make it easier to change and simplier for internals
to interface with. We have two new objects:

1) `RouteWithParams`

Previously `#generate` returned an array of parameterized parts for a
route. We have now turned this into an object with a `path` and a
`params methods` to be able to access the parts we need.

2) `MissingRoute`

`#generate` was also responsible for constructing the message for the
error. We've moved this code into the new `MissingRoute` object so it
does that work instead.

This change makes these methods reusable throughout the code and easier
for future refactoring we plan to do.

Co-authored-by: Aaron Patterson <aaron.patterson@gmail.com>

actionpack/lib/action_dispatch/journey/formatter.rb

@@ -15,7 +15,48 @@ module ActionDispatch
         @cache  = nil
       end
 
-      def generate(name, options, path_parameters, method_name = nil)
+      class RouteWithParams
+        attr_reader :params
+
+        def initialize(route, parameterized_parts, params)
+          @route = route
+          @parameterized_parts = parameterized_parts
+          @params = params
+        end
+
+        def path(_)
+          @route.format(@parameterized_parts)
+        end
+      end
+
+      class MissingRoute
+        attr_reader :routes, :name, :constraints, :missing_keys, :unmatched_keys
+
+        def initialize(constraints, missing_keys, unmatched_keys, routes, name)
+          @constraints = constraints
+          @missing_keys = missing_keys
+          @unmatched_keys = unmatched_keys
+          @routes = routes
+          @name = name
+        end
+
+        def path(method_name)
+          raise ActionController::UrlGenerationError.new(message, routes, name, method_name)
+        end
+
+        def params
+          path("unknown")
+        end
+
+        def message
+          message = +"No route matches #{Hash[constraints.sort_by { |k, v| k.to_s }].inspect}"
+          message << ", missing required keys: #{missing_keys.sort.inspect}" if missing_keys && !missing_keys.empty?
+          message << ", possible unmatched constraints: #{unmatched_keys.sort.inspect}" if unmatched_keys && !unmatched_keys.empty?
+          message
+        end
+      end
+
+      def generate(name, options, path_parameters)
         constraints = path_parameters.merge(options)
         missing_keys = nil
 
@@ -44,17 +85,13 @@ module ActionDispatch
             parameterized_parts.delete(key)
           end
 
-          return [route.format(parameterized_parts), params]
+          return RouteWithParams.new(route, parameterized_parts, params)
         end
 
         unmatched_keys = (missing_keys || []) & constraints.keys
         missing_keys = (missing_keys || []) - unmatched_keys
 
-        message = +"No route matches #{Hash[constraints.sort_by { |k, v| k.to_s }].inspect}"
+        MissingRoute.new(constraints, missing_keys, unmatched_keys, routes, name)
-        message << ", missing required keys: #{missing_keys.sort.inspect}" if missing_keys && !missing_keys.empty?
-        message << ", possible unmatched constraints: #{unmatched_keys.sort.inspect}" if unmatched_keys && !unmatched_keys.empty?
-
-        raise ActionController::UrlGenerationError.new(message, routes, name, method_name)
       end
 
       def clear

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -733,10 +733,10 @@ module ActionDispatch
           end
         end
 
-        # Generates a path from routes, returns [path, params].
+        # Generates a path from routes, returns a RouteWithParams or MissingRoute.
-        # If no route is generated the formatter will raise ActionController::UrlGenerationError
+        # MissingRoute will raise ActionController::UrlGenerationError.
-        def generate(method_name)
+        def generate
-          @set.formatter.generate(named_route, options, recall, method_name)
+          @set.formatter.generate(named_route, options, recall)
         end
 
         def different_controller?
@@ -761,13 +761,20 @@ module ActionDispatch
       end
 
       def generate_extras(options, recall = {})
-        route_key = options.delete :use_route
+        if recall
-        path, params = generate(route_key, options, recall)
+          options = options.merge(_recall: recall)
-        return path, params.keys
         end
 
-      def generate(route_key, options, recall = {}, method_name = nil)
+        route_name = options.delete :use_route
-        Generator.new(route_key, options, recall, self).generate(method_name)
+        path = path_for(options, route_name)
+
+        uri = URI.parse(path)
+        params = Rack::Utils.parse_nested_query(uri.query).symbolize_keys
+        [uri.path, params.keys]
+      end
+
+      def generate(route_name, options, recall = {}, method_name = nil)
+        Generator.new(route_name, options, recall, self).generate
       end
       private :generate
 
@@ -814,7 +821,9 @@ module ActionDispatch
         path_options = options.dup
         RESERVED_OPTIONS.each { |ro| path_options.delete ro }
 
-        path, params = generate(route_name, path_options, recall, method_name)
+        route_with_params = generate(route_name, path_options, recall)
+        path = route_with_params.path(method_name)
+        params = route_with_params.params
 
         if options.key? :params
           params.merge! options[:params]

actionpack/test/controller/routing_test.rb

@@ -2076,11 +2076,11 @@ class RackMountIntegrationTests < ActiveSupport::TestCase
   def test_extras
     params = { controller: "people" }
     assert_equal [], @routes.extra_keys(params)
-    assert_equal({ controller: "people", action: "index" }, params)
+    assert_equal({ controller: "people" }, params)
 
     params = { controller: "people", foo: "bar" }
     assert_equal [:foo], @routes.extra_keys(params)
-    assert_equal({ controller: "people", action: "index", foo: "bar" }, params)
+    assert_equal({ controller: "people", foo: "bar" }, params)
 
     params = { controller: "people", action: "create", person: { name: "Josh" } }
     assert_equal [:person], @routes.extra_keys(params)

actionpack/test/journey/router_test.rb

@@ -61,7 +61,7 @@ module ActionDispatch
         get "/foo/:id", id: /\d/, anchor: false, to: "foo#bar"
 
         assert_raises(ActionController::UrlGenerationError) do
-          _generate(nil, { controller: "foo", action: "bar", id: "10" }, {})
+          @route_set.url_for({ controller: "foo", action: "bar", id: "10" }, nil)
         end
       end