diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 158b22c0cce82af62f82ebe17017db68cc24aee0..7bca373920f89f8b104521c028390ca890e1d5b4 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,9 @@ +* Deprecate use of string keys in URL helpers. + + Use symbols instead. Fixes #16958. + + *Byron Bischoff & Melanie Gilman* + * Deprecate the `only_path` option on `*_path` helpers. In cases where this option is set to `true`, the option is redundant and can diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index d2ae2a496f4a0435d6a2d1038cdb10c5931bd8b3..dcfb81990663a33dcd10c0e820ec92f1111f21d5 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -271,7 +271,7 @@ def call(t, args, inner_options) controller_options = t.url_options options = controller_options.merge @options hash = handle_positional_args(controller_options, - inner_options || {}, + deprecate_string_options(inner_options) || {}, args, options, @segment_keys) @@ -293,6 +293,22 @@ def handle_positional_args(controller_options, inner_options, args, result, path result.merge!(inner_options) end + + DEPRECATED_STRING_OPTIONS = %w[controller action] + + def deprecate_string_options(options) + options ||= {} + deprecated_string_options = options.keys & DEPRECATED_STRING_OPTIONS + if deprecated_string_options.any? + msg = "Calling URL helpers with string keys #{deprecated_string_options.join(", ")} is deprecated. Use symbols instead." + ActiveSupport::Deprecation.warn(msg) + deprecated_string_options.each do |option| + value = options.delete(option) + options[option.to_sym] = value + end + end + options + end end private diff --git a/actionpack/test/dispatch/routing/route_set_test.rb b/actionpack/test/dispatch/routing/route_set_test.rb index a7acc0de412429f9e2cdf77480f8873d4e44bd9a..5a3911944655245b9978ac10a56787b5e07dbf26 100644 --- a/actionpack/test/dispatch/routing/route_set_test.rb +++ b/actionpack/test/dispatch/routing/route_set_test.rb @@ -160,6 +160,26 @@ def call(env) assert_equal '/foo/1/bar/2', url_helpers.foo_bar_path(2, foo_id: 1) end + test "stringified controller and action keys are properly symbolized" do + draw do + root 'foo#bar' + end + + assert_deprecated do + assert_equal '/', url_helpers.root_path('controller' => 'foo', 'action' => 'bar') + end + end + + test "mix of string and symbol keys are properly symbolized" do + draw do + root 'foo#bar' + end + + assert_deprecated do + assert_equal '/', url_helpers.root_path('controller' => 'foo', :action => 'bar') + end + end + private def draw(&block) @set.draw(&block)