Skip to content

Deprecate the positional options Hash in requires, optional and use - #2853

Open
ericproulx wants to merge 1 commit into
masterfrom
deprecate-positional-options-hash
Open

Deprecate the positional options Hash in requires, optional and use#2853
ericproulx wants to merge 1 commit into
masterfrom
deprecate-positional-options-hash

Conversation

@ericproulx

Copy link
Copy Markdown
Contributor

Fixes #2851. Supersedes #2852.

The bug

#2618 replaced attrs.extract_options! with an explicit **opts parameter in Grape::DSL::Parameters. As @OuYangJinTing pointed out in #2851, the two are not equivalent: Ruby only converts a trailing Hash into keyword arguments when it is written without braces, so a braced Hash stays in the splat — where these methods take it for a parameter name.

UPGRADING.md claimed the change maintained "full backward compatibility". It did not:

call behaviour on master
optional :id, { type: Integer } silent — no coercion, params[:id] is a String
requires :id, { type: Integer } every request 400s with {type: Integer} is missing, and :id loses its coercion
use :pg, { foo: 1 } RuntimeError: Params :{foo: 1} not found!

The optional case is the dangerous one. Nothing raises, nothing warns — a parameter that used to arrive coerced now arrives as a raw String, and the API keeps answering 200.

The fix

The old behaviour is restored behind a deprecation warning, following the merge_legacy_auth_options precedent already used for auth, http_basic, http_digest and desc:

params do
  requires :id, { type: Integer } # warns, still works
  requires :id, type: Integer     # do this instead
end
DEPRECATION WARNING: Passing a positional options Hash to `requires` is deprecated. Pass keyword arguments instead.

Two private helpers in lib/grape/dsl/parameters.rb do the work:

def legacy_options?(args)
  args.size > 1 && args.last.is_a?(Hash)
end

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

Why re-dispatch instead of merging into **opts

requires and optional declare using: and except: as explicit keyword parameters. Merging the legacy Hash into **opts in place would leave :using sitting in opts while the using local stayed nil, so requires :all, { using: Entity.documentation } would quietly build a parameter literally named :all instead of expanding the documentation Hash — one silent bug swapped for another.

Splatting the Hash back at the call site and re-invoking the method lets Ruby redo the keyword routing, and it keeps working if either method gains another explicit keyword later. Keyword arguments win over the legacy Hash on conflict, matching legacy_options.first.merge(options) in Grape::Middleware::Auth::DSL.

Relationship to #2852

@cyphercodes' #2852 fixes the same issue for use. This PR is a superset of it and I think it should land instead, for three reasons:

  1. requires and optional are affected too, and optional fails silently — the worst of the three. Preserve positional options for named params #2852 leaves both untouched.
  2. A deprecation rather than a silent restore. Preserve positional options for named params #2852 restores the old behaviour with no warning, which keeps the legacy form working indefinitely with nothing prompting anyone to migrate. Grape already deprecates exactly this pattern in four other DSL methods; this makes the fifth, sixth and seventh consistent with them.
  3. using: / except: routing, described above, which a positional extraction inside use doesn't need but requires and optional do.

#2852's guard also requires options.empty?, so a mixed call like use :pg, { a: 1 }, b: 2 silently drops { a: 1 }; here the two are merged with the keywords taking precedence, as extract_options! effectively did.

Thanks @cyphercodes for the fix and the regression example — the use spec here is yours, extended.

Scope

Deliberately left alone:

  • given(*attrs, &) must not get this check. It has no **, so keyword arguments legitimately arrive as a trailing Hash in attrs — that is what first_hash_key_or_param exists for. The check only applies to methods that actually declare a double splat.
  • A Hash that is the only argument (requires({ type: Integer })) does not warn; the guard is args.size > 1. With nothing in front of it there is no parameter being declared either way, so there is no prior behaviour to preserve.
  • Grape::DSL::Entity#present has the same latent problem — present(obj, { with: X }) silently drops the options because the args.count == 2 && args.first.is_a?(Symbol) destructuring fails. It is unrelated to named params and out of scope here; happy to open a follow-up.

Docs

  • The @param attrs doc for requires still advertised the trailing Hash ("The last key can be a hash, which specifies options for the parameters"). Updated to point at keyword arguments.
  • UPGRADING.md — the >= 3.0.0 section that introduced this is amended: the "full backward compatibility" claim is now accurate again, with the deprecated form spelled out. No new entry, since nothing breaks.

Test plan

  • spec/grape/dsl/parameters_spec.rb — warns and still works for requires, optional and use; options of a positional Hash apply to every attribute; :using routes out of the Hash to the keyword parameter; the keyword form does not warn; a lone Hash does not warn.
  • spec/grape/validations/params_scope_spec.rb — request-level regression proving the coercion that was silently lost is applied again.
  • bundle exec rspec — 2558 examples, 0 failures.
  • bundle exec rubocop — no offenses.

🤖 Generated with Claude Code

#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 <noreply@anthropic.com>
@ericproulx
ericproulx force-pushed the deprecate-positional-options-hash branch from dd9afb0 to 5ee73d8 Compare August 9, 2026 03:43
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Danger Report

No issues found.

View run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replacing extract_options! with *args, **options in #2618 breaks callers that pass options as a positional Hash

2 participants