From 5ee73d83965cdc882c805b9b506e16fc7c486968 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Sun, 9 Aug 2026 12:42:00 +0900 Subject: [PATCH] Deprecate the positional options Hash in requires, optional and use #2618 replaced `attrs.extract_options!` with an explicit `**opts` parameter. The two are not equivalent: Ruby only converts a trailing Hash into keyword arguments when it is written without braces, so a braced Hash now stays in the splat and is taken for a parameter name. `optional :id, { type: Integer }` silently lost its coercion, `requires :id, { type: Integer }` made every request fail with `{type: Integer} is missing`, and `use :pg, { foo: 1 }` raised `Params :{foo: 1} not found!`. Restore the old behaviour behind a deprecation warning, following `merge_legacy_auth_options`. The trailing Hash is splatted back as keyword arguments and the method re-invoked, so Ruby redoes the keyword routing: merging into `**opts` in place would strand `:using` and `:except` there while their explicit keyword parameters stayed nil. Fixes #2851 Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + UPGRADING.md | 11 ++++- lib/grape/dsl/parameters.rb | 26 +++++++++- spec/grape/dsl/parameters_spec.rb | 54 +++++++++++++++++++++ spec/grape/validations/params_scope_spec.rb | 16 ++++++ 5 files changed, 105 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13b9d75a3..21a7eea71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ * [#2829](https://github.com/ruby-grape/grape/pull/2829): Fix a cascading route handing over only to the last route registered for the path, making a middle version (3+ mounted versions with a catch-all) answer 406 - [@ericproulx](https://github.com/ericproulx). * [#2826](https://github.com/ruby-grape/grape/pull/2826): Fix `api.version` not being set for the root route of a path-versioned API (`GET /v1`) - [@ericproulx](https://github.com/ericproulx). * [#2842](https://github.com/ruby-grape/grape/pull/2842): Warn at definition time when a `rescue_from` class is already covered by one registered earlier in the same scope, since the later handler never runs - [@ericproulx](https://github.com/ericproulx). +* [#2853](https://github.com/ruby-grape/grape/pull/2853): Restore, behind a deprecation warning, the trailing positional options Hash of `requires`, `optional` and `use`, which #2618 turned into a parameter name - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/UPGRADING.md b/UPGRADING.md index ab4838229..6d940c3e5 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -707,7 +707,16 @@ Grape has been modernized to use Ruby 3+'s preferred argument delegation pattern - Method signatures are now more explicit and follow Ruby 3+ best practices - The `active_support/core_ext/array/extract_options` dependency has been removed -This is a modernization effort that improves code quality while maintaining full backward compatibility. +Passing the options of `requires`, `optional` and `use` as a trailing positional Hash still works, but is deprecated: + +```ruby +params do + requires :id, { type: Integer } # deprecated + requires :id, type: Integer # do this instead +end +``` + +Under `extract_options!` the braces made no difference. They do now: Ruby only turns a trailing Hash into keyword arguments when it is written without braces, so a braced Hash lands in the splat and would otherwise be taken for a parameter name. See [#2618](https://github.com/ruby-grape/grape/pull/2618) for more information. diff --git a/lib/grape/dsl/parameters.rb b/lib/grape/dsl/parameters.rb index 001c4283e..bbacc7f4b 100644 --- a/lib/grape/dsl/parameters.rb +++ b/lib/grape/dsl/parameters.rb @@ -54,6 +54,8 @@ def build_with(build_with) # end # end def use(*names, **options) + return redispatch_legacy_options(:use, names, options) if legacy_options?(names) + named_params = @api.inheritable_setting.named_params || {} names.each do |name| params_block = named_params.fetch(name) do @@ -74,8 +76,8 @@ def use(*names, **options) # # @param attrs list of parameters names, or, if :using is # passed as an option, which keys to include (:all or :none) from - # the :using hash. The last key can be a hash, which specifies - # options for the parameters + # the :using hash. Passing the options as a trailing positional Hash + # is deprecated; pass them as keyword arguments instead. # @option attrs :type [Class] the type to coerce this parameter to before # passing it to the endpoint. See {Grape::Validations::Types} for a list of # types that are supported automatically. Custom classes may be used @@ -123,6 +125,8 @@ def use(*names, **options) # end # end def requires(*attrs, using: nil, except: nil, **opts, &block) + return redispatch_legacy_options(:requires, attrs, { using:, except: }.compact.merge(opts), &block) if legacy_options?(attrs) + opts[:presence] = { value: true, message: opts[:message] } opts = @group.deep_merge(opts) if @group @@ -137,6 +141,8 @@ def requires(*attrs, using: nil, except: nil, **opts, &block) # @param (see #requires) # @option (see #requires) def optional(*attrs, using: nil, except: nil, **opts, &block) + return redispatch_legacy_options(:optional, attrs, { using:, except: }.compact.merge(opts), &block) if legacy_options?(attrs) + type = opts[:type] opts = @group.deep_merge(opts) if @group @@ -210,6 +216,22 @@ def params(params) private + # @deprecated A trailing positional options Hash is deprecated; pass keyword + # arguments instead. Before Ruby 3 keyword separation this Hash was pulled + # off the argument list by `extract_options!`; now it lands in the splat and + # would silently be treated as a parameter name. + def legacy_options?(args) + args.size > 1 && args.last.is_a?(Hash) + end + + # Re-invokes +method_name+ with the trailing Hash splatted as keyword + # arguments, so Ruby routes its keys to the same place they would have + # reached had the caller omitted the braces. + def redispatch_legacy_options(method_name, args, opts, &) + Grape.deprecator.warn("Passing a positional options Hash to `#{method_name}` is deprecated. Pass keyword arguments instead.") + __send__(method_name, *args[0..-2], **args.last.merge(opts), &) + end + def first_hash_key_or_param(parameter) parameter.is_a?(Hash) ? parameter.keys.first : parameter end diff --git a/spec/grape/dsl/parameters_spec.rb b/spec/grape/dsl/parameters_spec.rb index f312ed962..497497c3a 100644 --- a/spec/grape/dsl/parameters_spec.rb +++ b/spec/grape/dsl/parameters_spec.rb @@ -244,6 +244,60 @@ def new_group_scope(group) end end + describe 'deprecated positional options Hash' do + it 'deprecates a positional Hash for `requires` but still works when silenced' do + expect { subject.requires :id, { type: Integer, desc: 'Identity.' } } + .to raise_error(ActiveSupport::DeprecationException, /positional options Hash to `requires`/) + + Grape.deprecator.silence { subject.requires :id, { type: Integer, desc: 'Identity.' } } + expect(subject.validate_attributes_reader).to eq([[:id], { type: Integer, desc: 'Identity.', presence: { value: true, message: nil } }]) + expect(subject.push_declared_params_reader).to eq([:id]) + end + + it 'deprecates a positional Hash for `optional` but still works when silenced' do + expect { subject.optional :id, { type: Integer, desc: 'Identity.' } } + .to raise_error(ActiveSupport::DeprecationException, /positional options Hash to `optional`/) + + Grape.deprecator.silence { subject.optional :id, { type: Integer, desc: 'Identity.' } } + expect(subject.validate_attributes_reader).to eq([[:id], { type: Integer, desc: 'Identity.' }]) + expect(subject.push_declared_params_reader).to eq([:id]) + end + + it 'deprecates a positional Hash for `use` but still works when silenced' do + subject.api = Class.new { include Grape::DSL::Settings }.new + subject.api.inheritable_setting.add_named_params(params_group: proc {}) + + expect { subject.use :params_group, { option: 'value' } } + .to raise_error(ActiveSupport::DeprecationException, /positional options Hash to `use`/) + + expect(subject).to receive(:instance_exec).with(option: 'value').and_yield + Grape.deprecator.silence { subject.use :params_group, { option: 'value' } } + end + + it 'keeps the options of the positional Hash applying to every attribute' do + Grape.deprecator.silence { subject.requires :a, :b, { type: Integer } } + + expect(subject.validate_attributes_reader).to eq([%i[a b], { type: Integer, presence: { value: true, message: nil } }]) + expect(subject.push_declared_params_reader).to eq(%i[a b]) + end + + it 'routes :using out of the positional Hash to the keyword argument' do + documentation = { id: { type: Integer } } + expect(subject).to receive(:require_required_and_optional_fields).with(:all, using: documentation, except: nil) + + Grape.deprecator.silence { subject.requires :all, { using: documentation } } + end + + it 'does not deprecate keyword arguments' do + expect { subject.requires :id, type: Integer }.not_to raise_error + expect { subject.optional :id, type: Integer }.not_to raise_error + end + + it 'does not deprecate a Hash that is the only argument' do + expect { subject.requires(type: Integer) }.not_to raise_error + end + end + describe '#params' do it 'inherits params from parent' do parent_params = { foo: 'bar' } diff --git a/spec/grape/validations/params_scope_spec.rb b/spec/grape/validations/params_scope_spec.rb index a6733a772..454a2779b 100644 --- a/spec/grape/validations/params_scope_spec.rb +++ b/spec/grape/validations/params_scope_spec.rb @@ -9,6 +9,22 @@ def app subject end + context 'when the options are given as a deprecated positional Hash' do + it 'coerces the parameter as if the options had been passed as keyword arguments' do + Grape.deprecator.silence do + subject.params do + requires :id, { type: Integer } + optional :page, { type: Integer, default: 1 } + end + end + subject.get('/legacy') { [params[:id].class, params[:page]].join(',') } + + get '/legacy', id: '5' + expect(last_response.status).to eq(200) + expect(last_response.body).to eq('Integer,1') + end + end + context 'when using custom types' do let(:custom_type) do Class.new do