1 files changed, 48 insertions, 26 deletions

diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb
index 4a344f71af..177f586c0e 100644
--- a/actionpack/lib/action_dispatch/journey/formatter.rb
+++ b/actionpack/lib/action_dispatch/journey/formatter.rb
@@ -1,7 +1,10 @@
+require 'action_controller/metal/exceptions'
+require 'active_support/deprecation'
+
 module ActionDispatch
   module Journey
     # The Formatter class is used for formatting URLs. For example, parameters
-    # passed to +url_for+ in rails will eventually call Formatter#generate.
+    # passed to +url_for+ in Rails will eventually call Formatter#generate.
     class Formatter # :nodoc:
       attr_reader :routes
 
@@ -10,13 +13,17 @@ module ActionDispatch
         @cache  = nil
       end
 
-      def generate(type, name, options, recall = {}, parameterize = nil)
-        constraints = recall.merge(options)
+      def generate(name, options, path_parameters, parameterize = nil)
+        constraints = path_parameters.merge(options)
         missing_keys = []
 
         match_route(name, constraints) do |route|
-          parameterized_parts = extract_parameterized_parts(route, options, recall, parameterize)
-          next if !name && route.requirements.empty? && route.parts.empty?
+          parameterized_parts = extract_parameterized_parts(route, options, path_parameters, parameterize)
+
+          # Skip this route unless a name has been provided or it is a
+          # standard Rails route since we can't determine whether an options
+          # hash passed to url_for matches a Rack application or a redirect.
+          next unless name || route.dispatcher?
 
           missing_keys = missing_keys(route, parameterized_parts)
           next unless missing_keys.empty?
@@ -24,10 +31,19 @@ module ActionDispatch
             parameterized_parts.key?(key) || route.defaults.key?(key)
           end
 
+          defaults       = route.defaults
+          required_parts = route.required_parts
+          parameterized_parts.delete_if do |key, value|
+            value.to_s == defaults[key].to_s && !required_parts.include?(key)
+          end
+
           return [route.format(parameterized_parts), params]
         end
 
-        raise Router::RoutingError.new "missing required keys: #{missing_keys}"
+        message = "No route matches #{Hash[constraints.sort].inspect}"
+        message << " missing required keys: #{missing_keys.sort.inspect}" unless missing_keys.empty?
+
+        raise ActionController::UrlGenerationError, message
       end
 
       def clear
@@ -37,26 +53,23 @@ module ActionDispatch
       private
 
         def extract_parameterized_parts(route, options, recall, parameterize = nil)
-          constraints = recall.merge(options)
-          data = constraints.dup
+          parameterized_parts = recall.merge(options)
 
           keys_to_keep = route.parts.reverse.drop_while { |part|
             !options.key?(part) || (options[part] || recall[part]).nil?
           } | route.required_parts
 
-          (data.keys - keys_to_keep).each do |bad_key|
-            data.delete(bad_key)
+          (parameterized_parts.keys - keys_to_keep).each do |bad_key|
+            parameterized_parts.delete(bad_key)
           end
 
-          parameterized_parts = data.dup
-
           if parameterize
             parameterized_parts.each do |k, v|
               parameterized_parts[k] = parameterize.call(k, v)
             end
           end
 
-          parameterized_parts.keep_if { |_, v| v  }
+          parameterized_parts.keep_if { |_, v| v }
           parameterized_parts
         end
 
@@ -68,14 +81,28 @@ module ActionDispatch
           if named_routes.key?(name)
             yield named_routes[name]
           else
-            routes = non_recursive(cache, options.to_a)
+            # Make sure we don't show the deprecation warning more than once
+            warned = false
+
+            routes = non_recursive(cache, options)
 
             hash = routes.group_by { |_, r| r.score(options) }
 
             hash.keys.sort.reverse_each do |score|
-              next if score < 0
+              break if score < 0
 
               hash[score].sort_by { |i, _| i }.each do |_, route|
+                if name && !warned
+                  ActiveSupport::Deprecation.warn <<-MSG.squish
+                    You are trying to generate the URL for a named route called
+                    #{name.inspect} but no such route was found. In the future,
+                    this will result in an `ActionController::UrlGenerationError`
+                    exception.
+                  MSG
+
+                  warned = true
+                end
+
                 yield route
               end
             end
@@ -84,14 +111,14 @@ module ActionDispatch
 
         def non_recursive(cache, options)
           routes = []
-          stack  = [cache]
+          queue  = [cache]
 
-          while stack.any?
-            c = stack.shift
+          while queue.any?
+            c = queue.shift
             routes.concat(c[:___routes]) if c.key?(:___routes)
 
             options.each do |pair|
-              stack << c[pair] if c.key?(pair)
+              queue << c[pair] if c.key?(pair)
             end
           end
 
@@ -115,14 +142,9 @@ module ActionDispatch
         def possibles(cache, options, depth = 0)
           cache.fetch(:___routes) { [] } + options.find_all { |pair|
             cache.key?(pair)
-          }.map { |pair|
+          }.flat_map { |pair|
             possibles(cache[pair], options, depth + 1)
-          }.flatten(1)
-        end
-
-        # Returns +true+ if no missing keys are present, otherwise +false+.
-        def verify_required_parts!(route, parts)
-          missing_keys(route, parts).empty?
+          }
         end
 
         def build_cache