Deprecate the `only_path` option on `*_path` helpers.

In cases where this option is set to `true`, the option is redundant and can
be safely removed; otherwise, the corresponding `*_url` helper should be
used instead.

Fixes #17294.

See also #17363.

[Dan Olson, Godfrey Chan]

3 files changed, 116 insertions, 2 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 4626c2650a..158b22c0cc 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,13 @@
+*   Deprecate the `only_path` option on `*_path` helpers.
+
+    In cases where this option is set to `true`, the option is redundant and can
+    be safely removed; otherwise, the corresponding `*_url` helper should be
+    used instead.
+
+    Fixes #17294.
+
+    *Dan Olson*, *Godfrey Chan*
+
 *   Improve Journey compliance to RFC 3986.
 
     The scanner in Journey failed to recognize routes that use literals
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index f51bee3b31..8d4a3c5670 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -134,8 +134,8 @@ module ActionDispatch
             @url_helpers_module.send  :undef_method, url_name
           end
           routes[key] = route
-          define_url_helper @path_helpers_module, route, path_name, route.defaults, name, PATH
-          define_url_helper @url_helpers_module,  route, url_name,  route.defaults, name, FULL
+          define_url_helper @path_helpers_module, route, path_name, route.defaults, name, LEGACY
+          define_url_helper @url_helpers_module,  route, url_name,  route.defaults, name, UNKNOWN
 
           @path_helpers << path_name
           @url_helpers << url_name
@@ -322,6 +322,30 @@ module ActionDispatch
       PATH    = ->(options) { ActionDispatch::Http::URL.path_for(options) }
       FULL    = ->(options) { ActionDispatch::Http::URL.full_url_for(options) }
       UNKNOWN = ->(options) { ActionDispatch::Http::URL.url_for(options) }
+      LEGACY  = ->(options) {
+        if options.key?(:only_path)
+          if options[:only_path]
+            ActiveSupport::Deprecation.warn \
+              "You are calling a `*_path` helper with the `only_path` option " \
+              "expicitly set to `true`. This option will stop working on " \
+              "path helpers in Rails 5. Simply remove the `only_path: true` " \
+              "argument from your call as it is redundant when applied to a " \
+              "path helper."
+
+            PATH.call(options)
+          else
+            ActiveSupport::Deprecation.warn \
+              "You are calling a `*_path` helper with the `only_path` option " \
+              "expicitly set to `false`. This option will stop working on " \
+              "path helpers in Rails 5. Use the corresponding `*_url` helper " \
+              "instead."
+
+            FULL.call(options)
+          end
+        else
+          PATH.call(options)
+        end
+      }
       # :startdoc:
 
       attr_accessor :formatter, :set, :named_routes, :default_scope, :router
diff --git a/actionpack/test/dispatch/routing/route_set_test.rb b/actionpack/test/dispatch/routing/route_set_test.rb
index c465d56bde..a7acc0de41 100644
--- a/actionpack/test/dispatch/routing/route_set_test.rb
+++ b/actionpack/test/dispatch/routing/route_set_test.rb
@@ -69,6 +69,86 @@ module ActionDispatch
         end
       end
 
+      test "only_path: true with *_url and no :host option" do
+        draw do
+          get 'foo', to: SimpleApp.new('foo#index')
+        end
+
+        assert_equal '/foo', url_helpers.foo_url(only_path: true)
+      end
+
+      test "only_path: false with *_url and no :host option" do
+        draw do
+          get 'foo', to: SimpleApp.new('foo#index')
+        end
+
+        assert_raises ArgumentError do
+          assert_equal 'http://example.com/foo', url_helpers.foo_url(only_path: false)
+        end
+      end
+
+      test "only_path: false with *_url and local :host option" do
+        draw do
+          get 'foo', to: SimpleApp.new('foo#index')
+        end
+
+        assert_equal 'http://example.com/foo', url_helpers.foo_url(only_path: false, host: 'example.com')
+      end
+
+      test "only_path: false with *_url and global :host option" do
+        @set.default_url_options = { host: 'example.com' }
+
+        draw do
+          get 'foo', to: SimpleApp.new('foo#index')
+        end
+
+        assert_equal 'http://example.com/foo', url_helpers.foo_url(only_path: false)
+      end
+
+      test "only_path: true with *_path" do
+        draw do
+          get 'foo', to: SimpleApp.new('foo#index')
+        end
+
+        assert_deprecated do
+          assert_equal '/foo', url_helpers.foo_path(only_path: true)
+        end
+      end
+
+      test "only_path: false with *_path with global :host option" do
+        @set.default_url_options = { host: 'example.com' }
+
+        draw do
+          get 'foo', to: SimpleApp.new('foo#index')
+        end
+
+        assert_deprecated do
+          assert_equal 'http://example.com/foo', url_helpers.foo_path(only_path: false)
+        end
+      end
+
+      test "only_path: false with *_path with local :host option" do
+        draw do
+          get 'foo', to: SimpleApp.new('foo#index')
+        end
+
+        assert_deprecated do
+          assert_equal 'http://example.com/foo', url_helpers.foo_path(only_path: false, host: 'example.com')
+        end
+      end
+
+      test "only_path: false with *_path with no :host option" do
+        draw do
+          get 'foo', to: SimpleApp.new('foo#index')
+        end
+
+        assert_deprecated do
+          assert_raises ArgumentError do
+            assert_equal 'http://example.com/foo', url_helpers.foo_path(only_path: false)
+          end
+        end
+      end
+
       test "explicit keys win over implicit keys" do
         draw do
           resources :foo do