Deprecate positional options Hash in Auth DSL; use keyword arguments

CHANGELOG.md

@@ -41,6 +41,7 @@
 * [#2720](https://github.com/ruby-grape/grape/pull/2720): Move declaration-coherence checks into `Grape::Validations::ValidationsSpec` - [@ericproulx](https://github.com/ericproulx).
 * [#2725](https://github.com/ruby-grape/grape/pull/2725): Encapsulate `Grape::Validations::Validators::Base` state behind readers; add `required?`/`allow_blank?` predicates - [@ericproulx](https://github.com/ericproulx).
 * [#2726](https://github.com/ruby-grape/grape/pull/2726): Reuse one `AttributesIterator` per validator and drop the unused `Enumerable` mixin - [@ericproulx](https://github.com/ericproulx).
+* [#2728](https://github.com/ruby-grape/grape/pull/2728): Deprecate passing a positional options Hash to `auth`/`http_basic`/`http_digest`; pass keyword arguments instead - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes

UPGRADING.md

@@ -3,6 +3,30 @@ Upgrading Grape
 
 ### Upgrading to >= 3.3
 
+#### `auth`, `http_basic` and `http_digest` now take keyword arguments
+
+`Grape::Middleware::Auth::DSL#auth`, `#http_basic` and `#http_digest` now accept their options as keyword arguments instead of a positional `Hash`. Calls using bare keyword syntax or a block are unaffected:
+
+```ruby
+http_basic realm: 'API' do |u, p|
+  # ...
+end
+
+auth :http_digest, realm: 'API', opaque: 'secret', &proc
+```
+
+Passing a positional options `Hash` still works but is deprecated and will be removed in a future release:
+
+```ruby
+# deprecated
+http_basic({ realm: 'API' })
+auth :http_digest, { realm: 'API', opaque: 'secret' }
+
+# preferred
+http_basic(realm: 'API')
+auth :http_digest, realm: 'API', opaque: 'secret'
+```
+
 #### `Grape::Middleware::Globals` removed
 
 `Grape::Middleware::Globals` and the three env constants it set (`Grape::Env::GRAPE_REQUEST`, `Grape::Env::GRAPE_REQUEST_HEADERS`, `Grape::Env::GRAPE_REQUEST_PARAMS`) have been deleted. The middleware was introduced in 2013 (commit `9987090b`) but never mounted by Grape's own stack — the `Grape::Request` it built is now constructed directly inside `Grape::Endpoint`. Nothing in `lib/` read those env keys.

lib/grape/middleware/auth/dsl.rb

@@ -4,24 +4,27 @@ module Grape
   module Middleware
     module Auth
       module DSL
-        def auth(type = nil, options = {}, &block)
+        def auth(type = nil, *legacy_options, **options, &block)
           namespace_inheritable = inheritable_setting.namespace_inheritable
           return namespace_inheritable[:auth] unless type
 
+          options = merge_legacy_auth_options(:auth, legacy_options, options)
           namespace_inheritable[:auth] = options.reverse_merge(type: type.to_sym, proc: block)
           use Grape::Middleware::Auth::Base, namespace_inheritable[:auth]
         end
 
         # Add HTTP Basic authorization to the API.
         #
-        # @param [Hash] options A hash of options.
-        # @option options [String] :realm "API Authorization" The HTTP Basic realm.
-        def http_basic(options = {}, &)
+        # @param options [Hash] a hash of options
+        # @option options [String] :realm "API Authorization" the HTTP Basic realm
+        def http_basic(*legacy_options, **options, &)
+          options = merge_legacy_auth_options(:http_basic, legacy_options, options)
           options[:realm] ||= 'API Authorization'
-          auth(:http_basic, options, &)
+          auth(:http_basic, **options, &)
         end
 
-        def http_digest(options = {}, &)
+        def http_digest(*legacy_options, **options, &)
+          options = merge_legacy_auth_options(:http_digest, legacy_options, options)
           options[:realm] ||= 'API Authorization'
 
           if options[:realm].respond_to?(:values_at)
@@ -30,7 +33,19 @@ def http_digest(options = {}, &)
             options[:opaque] ||= 'secret'
           end
 
-          auth(:http_digest, options, &)
+          auth(:http_digest, **options, &)
+        end
+
+        private
+
+        # @deprecated Passing a positional options Hash is deprecated; pass
+        #   keyword arguments instead. Kept so downstream callers keep working
+        #   through the deprecation cycle.
+        def merge_legacy_auth_options(method_name, legacy_options, options)
+          return options if legacy_options.empty?
+
+          Grape.deprecator.warn("Passing a positional options Hash to `#{method_name}` is deprecated. Pass keyword arguments instead.")
+          legacy_options.first.merge(options)
         end
       end
     end

spec/grape/middleware/auth/dsl_spec.rb

@@ -57,4 +57,30 @@
       end
     end
   end
+
+  describe 'deprecated positional options Hash' do
+    it 'deprecates a positional Hash for `auth` but still works when silenced' do
+      expect { subject.auth :http_digest, { realm: 'r', opaque: 'o' }, &block }
+        .to raise_error(ActiveSupport::DeprecationException, /positional options Hash to `auth`/)
+
+      Grape.deprecator.silence { subject.auth :http_digest, { realm: 'r', opaque: 'o' }, &block }
+      expect(subject.auth).to eq(realm: 'r', opaque: 'o', type: :http_digest, proc: block)
+    end
+
+    it 'deprecates a positional Hash for `http_basic` but still works when silenced' do
+      expect { subject.http_basic({ realm: 'my_realm' }, &block) }
+        .to raise_error(ActiveSupport::DeprecationException, /positional options Hash to `http_basic`/)
+
+      Grape.deprecator.silence { subject.http_basic({ realm: 'my_realm' }, &block) }
+      expect(subject.auth).to eq(realm: 'my_realm', type: :http_basic, proc: block)
+    end
+
+    it 'deprecates a positional Hash for `http_digest` but still works when silenced' do
+      expect { subject.http_digest({ realm: 'my_realm' }, &block) }
+        .to raise_error(ActiveSupport::DeprecationException, /positional options Hash to `http_digest`/)
+
+      Grape.deprecator.silence { subject.http_digest({ realm: 'my_realm' }, &block) }
+      expect(subject.auth).to eq(realm: 'my_realm', opaque: 'secret', type: :http_digest, proc: block)
+    end
+  end
 end