Skip to content

Preserve positional options for named params - #2852

Open
cyphercodes wants to merge 1 commit into
ruby-grape:masterfrom
cyphercodes:fix-use-positional-options-hash
Open

Preserve positional options for named params#2852
cyphercodes wants to merge 1 commit into
ruby-grape:masterfrom
cyphercodes:fix-use-positional-options-hash

Conversation

@cyphercodes

Copy link
Copy Markdown

Summary

Grape::DSL::Parameters#use still accepts keyword options, but Ruby 3 no longer turns a trailing positional Hash into keywords. After #2618, calls like use :pagination, { max_per_page: 100 } treated the Hash as another named params key instead of as options.

This restores the old extract_options! behavior for the multi-argument case by extracting a trailing positional Hash when no keyword options were supplied, while preserving single-argument named param lookup.

Fixes #2851.

Test plan

  • Added a regression example for use :params_group, options; verified it fails before the fix.
  • bundle exec rspec spec/grape/dsl/parameters_spec.rb (Ruby 3.4 Docker): 24 examples, 0 failures.
  • bundle exec rubocop lib/grape/dsl/parameters.rb spec/grape/dsl/parameters_spec.rb: 2 files inspected, no offenses detected.
  • bundle exec rake (Ruby 3.4 Docker): RuboCop 338 files inspected, no offenses detected; RSpec 2551 examples, 0 failures.
  • CI green.

🤖 AI-assisted with Hermes Agent; changes were reviewed and verified locally.

@cyphercodes
cyphercodes force-pushed the fix-use-positional-options-hash branch from 82550c8 to 8be56b2 Compare August 5, 2026 03:14
@OuYangJinTing

Copy link
Copy Markdown
Contributor

Thanks for the quick turnaround on this @cyphercodes!

While the use fix looks correct for the case described in #2851, the underlying problem from #2618 (extract_options! semantics not being equivalent to a literal *args, **options / *names, **options signature) is broader than just Grape::DSL::Parameters#use. The following methods introduced/changed by #2618 have the same pattern and are, as far as I can tell, still affected on master:

  • Grape::DSL::Entity#present
  • Grape::DSL::Parameters#requires
  • Grape::DSL::Parameters#optional
  • Grape::DSL::RequestResponse#rescue_from
  • Grape::DSL::Routing#version
  • Grape::API.override_all_methods!
  • Grape::API.replay_step_on
  • Grape::API.skip_immediate_run?

So this PR fixes one instance of the symptom, but the current fix does not fully resolve the problem — the same class of bug is reachable through several other entry points.

To recap the case already covered by #2851/this PR:

Steps to reproduce

require 'grape'

class API < Grape::API
  params do
    use :pagination, { max_per_page: 100 }
  end
  get '/items' do
    params
  end
end

Expected behavior

The options hash passed positionally is extracted and forwarded to the named param block, same as before #2618.

Actual behavior

The positional hash is treated as another name in *names instead of being merged into **options, so Params :{max_per_page: 100} not found! is raised (or, more generally, the options are silently dropped).

Here's a case this PR does not fix, on present:

Steps to reproduce

require 'grape'

class API < Grape::API
  get 'version' do
    present version: '1.0.0'
  end
end

Expected behavior

The response body should be { version: '1.0.0' }.

Actual behavior

The response body is null.

present(*args, root: nil, with: nil, **options) binds the bare version: '1.0.0' keyword to **options instead of leaving it as the positional Hash object to represent (which is what happened before #2618, when present(*args) had no explicit keyword parameters and Ruby collapsed the trailing keywords into a Hash appended to args). Since args ends up empty, object is nil, and the response body becomes null.

Given how many call sites share this pattern, it might be worth addressing this class of bug more systematically (e.g. reverting to extract_options!-style handling, or auditing/fixing each affected method individually) rather than patching them one at a time as they're reported.

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