Merge branch 'master' of git://github.com/rails/rails

5 files changed, 199 insertions, 205 deletions

diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb
index cd59b13c42..db1e3198b3 100644
--- a/actionpack/lib/action_dispatch/routing/mapper.rb
+++ b/actionpack/lib/action_dispatch/routing/mapper.rb
@@ -16,7 +16,7 @@ module ActionDispatch
           end
         end
 
-        attr_reader :app
+        attr_reader :app, :constraints
 
         def initialize(app, constraints, request)
           @app, @constraints, @request = app, constraints, request
@@ -49,7 +49,7 @@ module ActionDispatch
       class Mapping #:nodoc:
         IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix]
         ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z}
-        SHORTHAND_REGEX = %r{^/[\w/]+$}
+        SHORTHAND_REGEX = %r{/[\w/]+$}
         WILDCARD_PATH = %r{\*([^/\)]+)\)?$}
 
         def initialize(set, scope, path, options)
@@ -70,7 +70,7 @@ module ActionDispatch
 
             if using_match_shorthand?(path_without_format, @options)
               to_shorthand    = @options[:to].blank?
-              @options[:to] ||= path_without_format[1..-1].sub(%r{/([^/]*)$}, '#\1')
+              @options[:to] ||= path_without_format.gsub(/\(.*\)/, "")[1..-1].sub(%r{/([^/]*)$}, '#\1')
             end
 
             @options.merge!(default_controller_and_action(to_shorthand))
@@ -90,7 +90,7 @@ module ActionDispatch
 
           # match "account/overview"
           def using_match_shorthand?(path, options)
-            path && options.except(:via, :anchor, :to, :as).empty? && path =~ SHORTHAND_REGEX
+            path && (options[:to] || options[:action]).nil? && path =~ SHORTHAND_REGEX
           end
 
           def normalize_path(path)
@@ -285,7 +285,7 @@ module ActionDispatch
         # A pattern can also point to a +Rack+ endpoint i.e. anything that
         # responds to +call+:
         #
-        #   match 'photos/:id' => lambda {|hash| [200, {}, "Coming soon" }
+        #   match 'photos/:id' => lambda {|hash| [200, {}, "Coming soon"] }
         #   match 'photos/:id' => PhotoRackApp
         #   # Yes, controller actions are just rack endpoints
         #   match 'photos/:id' => PhotosController.action(:show)
@@ -374,10 +374,6 @@ module ActionDispatch
         #     # Matches any request starting with 'path'
         #     match 'path' => 'c#a', :anchor => false
         def match(path, options=nil)
-          mapping = Mapping.new(@set, @scope, path, options || {})
-          app, conditions, requirements, defaults, as, anchor = mapping.to_route
-          @set.add_route(app, conditions, requirements, defaults, as, anchor)
-          self
         end
 
         # Mount a Rack-based application to be used within the application.
@@ -696,7 +692,7 @@ module ActionDispatch
         # Allows you to constrain the nested routes based on a set of rules.
         # For instance, in order to change the routes to allow for a dot character in the +id+ parameter:
         #
-        #   constraints(:id => /\d+\.\d+) do
+        #   constraints(:id => /\d+\.\d+/) do
         #     resources :posts
         #   end
         #
@@ -706,7 +702,7 @@ module ActionDispatch
         # You may use this to also restrict other parameters:
         #
         #   resources :posts do
-        #     constraints(:post_id => /\d+\.\d+) do
+        #     constraints(:post_id => /\d+\.\d+/) do
         #       resources :comments
         #     end
         #   end
@@ -735,7 +731,7 @@ module ActionDispatch
         # if the user should be given access to that route, or +false+ if the user should not.
         #
         #    class Iphone
-        #      def self.matches(request)
+        #      def self.matches?(request)
         #        request.env["HTTP_USER_AGENT"] =~ /iPhone/
         #      end
         #    end
@@ -867,8 +863,6 @@ module ActionDispatch
         CANONICAL_ACTIONS = %w(index create new show update destroy)
 
         class Resource #:nodoc:
-          DEFAULT_ACTIONS = [:index, :create, :new, :show, :update, :destroy, :edit]
-
           attr_reader :controller, :path, :options
 
           def initialize(entities, options = {})
@@ -880,7 +874,7 @@ module ActionDispatch
           end
 
           def default_actions
-            self.class::DEFAULT_ACTIONS
+            [:index, :create, :new, :show, :update, :destroy, :edit]
           end
 
           def actions
@@ -934,16 +928,17 @@ module ActionDispatch
         end
 
         class SingletonResource < Resource #:nodoc:
-          DEFAULT_ACTIONS = [:show, :create, :update, :destroy, :new, :edit]
-
           def initialize(entities, options)
             super
-
             @as         = nil
             @controller = (options[:controller] || plural).to_s
             @as         = options[:as]
           end
 
+          def default_actions
+            [:show, :create, :update, :destroy, :new, :edit]
+          end
+
           def plural
             @plural ||= name.to_s.pluralize
           end
@@ -991,7 +986,7 @@ module ActionDispatch
             return self
           end
 
-          resource_scope(SingletonResource.new(resources.pop, options)) do
+          resource_scope(:resource, SingletonResource.new(resources.pop, options)) do
             yield if block_given?
 
             collection do
@@ -1023,6 +1018,7 @@ module ActionDispatch
         # creates seven different routes in your application, all mapping to
         # the +Photos+ controller:
         #
+        #   GET     /photos
         #   GET     /photos/new
         #   POST    /photos
         #   GET     /photos/:id
@@ -1038,6 +1034,7 @@ module ActionDispatch
         #
         # This generates the following comments routes:
         #
+        #   GET     /photos/:photo_id/comments
         #   GET     /photos/:photo_id/comments/new
         #   POST    /photos/:photo_id/comments
         #   GET     /photos/:photo_id/comments/:id
@@ -1049,13 +1046,20 @@ module ActionDispatch
         # Takes same options as <tt>Base#match</tt> as well as:
         #
         # [:path_names]
-        #   Allows you to change the paths of the seven default actions.
-        #   Paths not specified are not changed.
+        #   Allows you to change the segment component of the +edit+ and +new+ actions.
+        #   Actions not specified are not changed.
         #
         #     resources :posts, :path_names => { :new => "brand_new" }
         #
         #   The above example will now change /posts/new to /posts/brand_new
         #
+        # [:path]
+        #   Allows you to change the path prefix for the resource.
+        #
+        #     resources :posts, :path => 'postings'
+        #
+        #   The resource and all segments will now route to /postings instead of /posts
+        #
         # [:only]
         #   Only generate routes for the given actions.
         #
@@ -1120,7 +1124,7 @@ module ActionDispatch
             return self
           end
 
-          resource_scope(Resource.new(resources.pop, options)) do
+          resource_scope(:resources, Resource.new(resources.pop, options)) do
             yield if block_given?
 
             collection do
@@ -1243,32 +1247,44 @@ module ActionDispatch
           parent_resource.instance_of?(Resource) && @scope[:shallow]
         end
 
-        def match(*args)
-          options = args.extract_options!.dup
-          options[:anchor] = true unless options.key?(:anchor)
-
-          if args.length > 1
-            args.each { |path| match(path, options.dup) }
-            return self
+        def match(path, *rest)
+          if rest.empty? && Hash === path
+            options  = path
+            path, to = options.find { |name, value| name.is_a?(String) }
+            options[:to] = to
+            options.delete(path)
+            paths = [path]
+          else
+            options = rest.pop || {}
+            paths = [path] + rest
           end
 
-          on = options.delete(:on)
-          if VALID_ON_OPTIONS.include?(on)
-            args.push(options)
-            return send(on){ match(*args) }
-          elsif on
+          options[:anchor] = true unless options.key?(:anchor)
+
+          if options[:on] && !VALID_ON_OPTIONS.include?(options[:on])
             raise ArgumentError, "Unknown scope #{on.inspect} given to :on"
           end
 
-          if @scope[:scope_level] == :resources
-            args.push(options)
-            return nested { match(*args) }
-          elsif @scope[:scope_level] == :resource
-            args.push(options)
-            return member { match(*args) }
+          paths.each { |_path| decomposed_match(_path, options.dup) }
+          self
+        end
+
+        def decomposed_match(path, options) # :nodoc:
+          if on = options.delete(:on)
+            send(on) { decomposed_match(path, options) }
+          else
+            case @scope[:scope_level]
+            when :resources
+              nested { decomposed_match(path, options) }
+            when :resource
+              member { decomposed_match(path, options) }
+            else
+              add_route(path, options)
+            end
           end
+        end
 
-          action = args.first
+        def add_route(action, options) # :nodoc:
           path = path_for_action(action, options.delete(:path))
 
           if action.to_s =~ /^[\w\/]+$/
@@ -1277,13 +1293,15 @@ module ActionDispatch
             action = nil
           end
 
-          if options.key?(:as) && !options[:as]
+          if !options.fetch(:as, true)
             options.delete(:as)
           else
             options[:as] = name_for_action(options[:as], action)
           end
 
-          super(path, options)
+          mapping = Mapping.new(@set, @scope, path, options)
+          app, conditions, requirements, defaults, as, anchor = mapping.to_route
+          @set.add_route(app, conditions, requirements, defaults, as, anchor)
         end
 
         def root(options={})
@@ -1339,7 +1357,7 @@ module ActionDispatch
           end
 
           def scope_action_options? #:nodoc:
-            @scope[:options].is_a?(Hash) && (@scope[:options][:only] || @scope[:options][:except])
+            @scope[:options] && (@scope[:options][:only] || @scope[:options][:except])
           end
 
           def scope_action_options #:nodoc:
@@ -1347,11 +1365,11 @@ module ActionDispatch
           end
 
           def resource_scope? #:nodoc:
-            @scope[:scope_level].in?([:resource, :resources])
+            [:resource, :resources].include? @scope[:scope_level]
           end
 
           def resource_method_scope? #:nodoc:
-            @scope[:scope_level].in?([:collection, :member, :new])
+            [:collection, :member, :new].include? @scope[:scope_level]
           end
 
           def with_exclusive_scope
@@ -1376,8 +1394,8 @@ module ActionDispatch
             @scope[:scope_level_resource] = old_resource
           end
 
-          def resource_scope(resource) #:nodoc:
-            with_scope_level(resource.is_a?(SingletonResource) ? :resource : :resources, resource) do
+          def resource_scope(kind, resource) #:nodoc:
+            with_scope_level(kind, resource) do
               scope(parent_resource.resource_scope) do
                 yield
               end
@@ -1385,10 +1403,12 @@ module ActionDispatch
           end
 
           def nested_options #:nodoc:
-            {}.tap do |options|
-              options[:as] = parent_resource.member_name
-              options[:constraints] = { "#{parent_resource.singular}_id".to_sym => id_constraint } if id_constraint?
-            end
+            options = { :as => parent_resource.member_name }
+            options[:constraints] = {
+              :"#{parent_resource.singular}_id" => id_constraint
+            } if id_constraint?
+
+            options
           end
 
           def id_constraint? #:nodoc:
@@ -1419,8 +1439,7 @@ module ActionDispatch
           end
 
           def action_path(name, path = nil) #:nodoc:
-            # Ruby 1.8 can't transform empty strings to symbols
-            name = name.to_sym if name.is_a?(String) && !name.empty?
+            name = name.to_sym if name.is_a?(String)
             path || @scope[:path_names][name] || name.to_s
           end
 
@@ -1459,22 +1478,17 @@ module ActionDispatch
               [name_prefix, member_name, prefix]
             end
 
-            candidate = name.select(&:present?).join("_").presence
-            candidate unless as.nil? && @set.routes.find { |r| r.name == candidate }
-          end
-      end
-
-      module Shorthand #:nodoc:
-        def match(path, *rest)
-          if rest.empty? && Hash === path
-            options  = path
-            path, to = options.find { |name, value| name.is_a?(String) }
-            options.merge!(:to => to).delete(path)
-            super(path, options)
-          else
-            super
+            if candidate = name.select(&:present?).join("_").presence
+              # If a name was not explicitly given, we check if it is valid
+              # and return nil in case it isn't. Otherwise, we pass the invalid name
+              # forward so the underlying router engine treats it and raises an exception.
+              if as.nil?
+                candidate unless @set.routes.find { |r| r.name == candidate } || candidate !~ /\A[_a-z]/i
+              else
+                candidate
+              end
+            end
           end
-        end
       end
 
       def initialize(set) #:nodoc:
@@ -1487,7 +1501,6 @@ module ActionDispatch
       include Redirection
       include Scoping
       include Resources
-      include Shorthand
     end
   end
 end
diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb
index e989a38d8b..013cf93dbc 100644
--- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb
+++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb
@@ -165,7 +165,7 @@ module ActionDispatch
               if parent.is_a?(Symbol) || parent.is_a?(String)
                 parent
               else
-                ActiveModel::Naming.route_key(parent).singularize
+                ActiveModel::Naming.singular_route_key(parent)
               end
             end
           else
@@ -176,9 +176,11 @@ module ActionDispatch
           if record.is_a?(Symbol) || record.is_a?(String)
             route << record
           elsif record
-            route << ActiveModel::Naming.route_key(record)
-            route = [route.join("_").singularize] if inflection == :singular
-            route << "index" if ActiveModel::Naming.uncountable?(record) && inflection == :plural
+            if inflection == :singular
+              route << ActiveModel::Naming.singular_route_key(record)
+            else
+              route << ActiveModel::Naming.route_key(record)
+            end
           else
             raise ArgumentError, "Nil location provided. Can't build URI."
           end
diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb
index 804991ad5f..617b24b46a 100644
--- a/actionpack/lib/action_dispatch/routing/redirection.rb
+++ b/actionpack/lib/action_dispatch/routing/redirection.rb
@@ -2,6 +2,54 @@ require 'action_dispatch/http/request'
 
 module ActionDispatch
   module Routing
+    class Redirect # :nodoc:
+      attr_reader :status, :block
+
+      def initialize(status, block)
+        @status = status
+        @block  = block
+      end
+
+      def call(env)
+        req = Request.new(env)
+
+        uri = URI.parse(path(req.symbolized_path_parameters, req))
+        uri.scheme ||= req.scheme
+        uri.host   ||= req.host
+        uri.port   ||= req.port unless req.standard_port?
+
+        body = %(<html><body>You are being <a href="#{ERB::Util.h(uri.to_s)}">redirected</a>.</body></html>)
+
+        headers = {
+          'Location' => uri.to_s,
+          'Content-Type' => 'text/html',
+          'Content-Length' => body.length.to_s
+        }
+
+        [ status, headers, [body] ]
+      end
+
+      def path(params, request)
+        block.call params, request
+      end
+    end
+
+    class OptionRedirect < Redirect # :nodoc:
+      alias :options :block
+
+      def path(params, request)
+        url_options = {
+          :protocol => request.protocol,
+          :host     => request.host,
+          :port     => request.optional_port,
+          :path     => request.path,
+          :params   => request.query_parameters
+        }.merge options
+
+        ActionDispatch::Http::URL.url_for url_options
+      end
+    end
+
     module Redirection
 
       # Redirect any path to another path:
@@ -40,71 +88,18 @@ module ActionDispatch
         options = args.last.is_a?(Hash) ? args.pop : {}
         status  = options.delete(:status) || 301
 
-        path = args.shift
+        return OptionRedirect.new(status, options) if options.any?
 
-        path_proc = if path.is_a?(String)
-          proc { |params| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) }
-        elsif options.any?
-          options_proc(options)
-        elsif path.respond_to?(:call)
-          proc { |params, request| path.call(params, request) }
-        elsif block
-          block
-        else
-          raise ArgumentError, "redirection argument not supported"
-        end
-
-        redirection_proc(status, path_proc)
-      end
+        path = args.shift
 
-      private
-
-        def options_proc(options)
-          proc do |params, request|
-            path = if options[:path].nil?
-              request.path
-            elsif params.empty? || !options[:path].match(/%\{\w*\}/)
-              options.delete(:path)
-            else
-              (options.delete(:path) % params)
-            end
-
-            default_options = {
-              :protocol => request.protocol,
-              :host => request.host,
-              :port => request.optional_port,
-              :path => path,
-              :params => request.query_parameters
-            }
-
-            ActionDispatch::Http::URL.url_for(options.reverse_merge(default_options))
-          end
-        end
-
-        def redirection_proc(status, path_proc)
-          lambda do |env|
-            req = Request.new(env)
-
-            params = [req.symbolized_path_parameters]
-            params << req if path_proc.arity > 1
-
-            uri = URI.parse(path_proc.call(*params))
-            uri.scheme ||= req.scheme
-            uri.host   ||= req.host
-            uri.port   ||= req.port unless req.standard_port?
-
-            body = %(<html><body>You are being <a href="#{ERB::Util.h(uri.to_s)}">redirected</a>.</body></html>)
-
-            headers = {
-              'Location' => uri.to_s,
-              'Content-Type' => 'text/html',
-              'Content-Length' => body.length.to_s
-            }
-
-            [ status, headers, [body] ]
-          end
-        end
+        block = lambda { |params, request|
+          (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params)
+        } if String === path
 
+        block = path if path.respond_to? :call
+        raise ArgumentError, "redirection argument not supported" unless block
+        Redirect.new status, block
+      end
     end
   end
-end
\ No newline at end of file
+end
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index e921269331..10b3e212e6 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -1,4 +1,4 @@
-require 'journey/router'
+require 'journey'
 require 'forwardable'
 require 'active_support/core_ext/object/blank'
 require 'active_support/core_ext/object/to_query'
@@ -37,7 +37,7 @@ module ActionDispatch
 
         # If this is a default_controller (i.e. a controller specified by the user)
         # we should raise an error in case it's not found, because it usually means
-        # an user error. However, if the controller was retrieved through a dynamic
+        # a user error. However, if the controller was retrieved through a dynamic
         # segment, as in :controller(/:action), we should simply return nil and
         # delegate the control back to Rack cascade. Besides, if this is not a default
         # controller, it means we should respect the @scope[:module] parameter.
@@ -83,7 +83,9 @@ module ActionDispatch
         attr_reader :routes, :helpers, :module
 
         def initialize
-          clear!
+          @routes  = {}
+          @helpers = []
+          @module  = Module.new
         end
 
         def helper_names
@@ -91,12 +93,8 @@ module ActionDispatch
         end
 
         def clear!
-          @routes = {}
-          @helpers = []
-
-          @module ||= Module.new do
-            instance_methods.each { |selector| remove_method(selector) }
-          end
+          @routes.clear
+          @helpers.clear
         end
 
         def add(name, route)
@@ -125,19 +123,9 @@ module ActionDispatch
           routes.length
         end
 
-        def reset!
-          old_routes = routes.dup
-          clear!
-          old_routes.each do |name, route|
-            add(name, route)
-          end
-        end
-
-        def install(destinations = [ActionController::Base, ActionView::Base], regenerate = false)
-          reset! if regenerate
-          Array(destinations).each do |dest|
-            dest.__send__(:include, @module)
-          end
+        def install(destinations = [ActionController::Base, ActionView::Base])
+          helper = @module
+          destinations.each { |d| d.module_eval { include helper } }
         end
 
         private
@@ -160,22 +148,23 @@ module ActionDispatch
           def define_hash_access(route, name, kind, options)
             selector = hash_access_name(name, kind)
 
-            # We use module_eval to avoid leaks
-            @module.module_eval <<-END_EVAL, __FILE__, __LINE__ + 1
-              remove_possible_method :#{selector}
-              def #{selector}(*args)
-                options = args.extract_options!
-                result = #{options.inspect}
+            @module.module_eval do
+              remove_possible_method selector
+
+              define_method(selector) do |*args|
+                inner_options = args.extract_options!
+                result = options.dup
 
                 if args.any?
                   result[:_positional_args] = args
-                  result[:_positional_keys] = #{route.segment_keys.inspect}
+                  result[:_positional_keys] = route.segment_keys
                 end
 
-                result.merge(options)
+                result.merge(inner_options)
               end
-              protected :#{selector}
-            END_EVAL
+
+              protected selector
+            end
             helpers << selector
           end
 
@@ -287,9 +276,9 @@ module ActionDispatch
         @prepend.each { |blk| eval_block(blk) }
       end
 
-      def install_helpers(destinations = [ActionController::Base, ActionView::Base], regenerate_code = false)
-        Array(destinations).each { |d| d.module_eval { include Helpers } }
-        named_routes.install(destinations, regenerate_code)
+      def install_helpers(destinations)
+        destinations.each { |d| d.module_eval { include Helpers } }
+        named_routes.install(destinations)
       end
 
       module MountedHelpers
@@ -317,32 +306,29 @@ module ActionDispatch
       end
 
       def url_helpers
-        @url_helpers ||= begin
-          routes = self
+        routes = self
 
-          helpers = Module.new do
-            extend ActiveSupport::Concern
-            include UrlFor
+        @url_helpers ||= Module.new {
+          extend ActiveSupport::Concern
+          include UrlFor
 
-            @_routes = routes
-            class << self
-              delegate :url_for, :to => '@_routes'
-            end
-            extend routes.named_routes.module
-
-            # ROUTES TODO: install_helpers isn't great... can we make a module with the stuff that
-            # we can include?
-            # Yes plz - JP
-            included do
-              routes.install_helpers(self)
-              singleton_class.send(:redefine_method, :_routes) { routes }
-            end
+          @_routes = routes
+          def self.url_for(options)
+            @_routes.url_for options
+          end
+
+          extend routes.named_routes.module
 
-            define_method(:_routes) { @_routes || routes }
+          # ROUTES TODO: install_helpers isn't great... can we make a module with the stuff that
+          # we can include?
+          # Yes plz - JP
+          included do
+            routes.install_helpers([self])
+            singleton_class.send(:redefine_method, :_routes) { routes }
           end
 
-          helpers
-        end
+          define_method(:_routes) { @_routes || routes }
+        }
       end
 
       def empty?
@@ -356,7 +342,7 @@ module ActionDispatch
         conditions = build_conditions(conditions, valid_conditions, path.names.map { |x| x.to_sym })
 
         route = @set.add_route(app, path, conditions, defaults, name)
-        named_routes[name] = route if name
+        named_routes[name] = route if name && !named_routes[name]
         route
       end
 
@@ -394,10 +380,9 @@ module ActionDispatch
           if name == :controller
             value
           elsif value.is_a?(Array)
-            value.map { |v| Journey::Router::Utils.escape_uri(v.to_param) }.join('/')
-          else
-            return nil unless param = value.to_param
-            param.split('/').map { |v| Journey::Router::Utils.escape_uri(v) }.join("/")
+            value.map { |v| v.to_param }.join('/')
+          elsif param = value.to_param
+            param
           end
         end
 
@@ -558,7 +543,7 @@ module ActionDispatch
         path_addition, params = generate(path_options, path_segments || {})
         path << path_addition
 
-        ActionDispatch::Http::URL.url_for(options.merge({
+        ActionDispatch::Http::URL.url_for(options.merge!({
           :path => path,
           :params => params,
           :user => user,
@@ -585,7 +570,7 @@ module ActionDispatch
         @router.recognize(req) do |route, matches, params|
           params.each do |key, value|
             if value.is_a?(String)
-              value = value.dup.force_encoding(Encoding::BINARY) if value.encoding_aware?
+              value = value.dup.force_encoding(Encoding::BINARY)
               params[key] = URI.parser.unescape(value)
             end
           end
diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb
index 8fc8dc191b..6c2a98ab15 100644
--- a/actionpack/lib/action_dispatch/routing/url_for.rb
+++ b/actionpack/lib/action_dispatch/routing/url_for.rb
@@ -42,7 +42,7 @@ module ActionDispatch
     #   url_for(:controller => 'users',
     #           :action => 'new',
     #           :message => 'Welcome!',
-    #           :host => 'www.example.com')        # Changed this.
+    #           :host => 'www.example.com')
     #   # => "http://www.example.com/users/new?message=Welcome%21"
     #
     # By default, all controllers and views have access to a special version of url_for,
@@ -52,7 +52,7 @@ module ActionDispatch
     #
     # For convenience reasons, mailers provide a shortcut for ActionController::UrlFor#url_for.
     # So within mailers, you only have to type 'url_for' instead of 'ActionController::UrlFor#url_for'
-    # in full. However, mailers don't have hostname information, and what's why you'll still
+    # in full. However, mailers don't have hostname information, and that's why you'll still
     # have to specify the <tt>:host</tt> argument when generating URLs in mailers.
     #
     #
@@ -90,8 +90,7 @@ module ActionDispatch
           if respond_to?(:class_attribute)
             class_attribute :default_url_options
           else
-            mattr_accessor :default_url_options
-            remove_method :default_url_options
+            mattr_writer :default_url_options
           end
 
           self.default_url_options = {}
@@ -145,7 +144,7 @@ module ActionDispatch
         when String
           options
         when nil, Hash
-          _routes.url_for((options || {}).reverse_merge(url_options).symbolize_keys)
+          _routes.url_for((options || {}).symbolize_keys.reverse_merge!(url_options))
         else
           polymorphic_url(options)
         end