Ruby 3.4 and refactor swagger documentation modules

[moskvin]

• Aligns lint configuration with Ruby 3.4 target.

[github-actions]

Danger Report

No issues found.

View run

[moskvin]

I've to drop support the old grape version to make CI happy

[moskvin]

I would suggest to dropping ruby 3.1 from CI

[moskvin]

@numbata please have a look again - I've removed some weird changes

[numbata]

@moskvin Thx for contribution!
I'll review the PR later today. Hope it is okay for you.

[numbata]

Thanks for the contribution! Very good job 👍

Main things before this can merge: the CI matrix now starts at Grape 2.4.0 but the gemspec still says >= 1.7, so either the gemspec needs updating or we should keep at least one older entry. And SwaggerRouting/SwaggerDocumentationAdder sitting at the top level is a collision risk, as now that they have their own files it's a good moment to nest them under GrapeSwagger::.

The triple lookup and test issues are in the inline comments. The dead-code comment and each_key signature are minor but easy to fix while you're already touching those lines.

[numbata]

Now that the module lives in its own file, this is the natural moment to fix the top-level namespace pollution and nest it under GrapeSwagger::. Same applies to SwaggerDocumentationAdder in the sibling file. If you want to keep the old constant reachable for any out-of-tree code that referenced it, you can SwaggerRouting = GrapeSwagger::SwaggerRouting with a deprecation warning, but the canonical name should be namespaced.

[numbata]

How about:

  alt = param.is_a?(Symbol) ? param.to_s : param.to_sym
  path_options = path_params[param] || path_params[alt] || {}

or normalize keys once in build_path_params so callers don't need to think about it.

[numbata]

The gemspec declares grape >= 1.7, but this PR replaces all matrix entries for Grape 1.8.0–2.2.0 with 2.4.0+. As the minimum requirement is being raised to 2.4.0, could you update the gemspec and README?

[numbata]

Hash#each_key yields exactly one value (the key). The _ parameter silently receives nil. Pre-existing code, but now extracted into a permanent home, so maybe let's do such small fix as well?

Suggested change

	namespaces.each_key do |name, _|
	namespaces.each_key do |name|

[numbata]

probably should be removed as a dead-code with comment?

[numbata]

This context passes symbol-keyed path_params ({ id: { ... } }), but fetch_inherited_params always produces string keys via space.to_s.gsub(':', ''), so path_params[param.to_sym] can never find anything in production.

The real fix scenario is in symbol-keyed route.params + string-keyed path_params isn't yet covered. The existing 'when inherited namespace stackable values contain path params across levels' context uses string-keyed route.params, so path_params[param] hits directly and the param.to_s fallback is never reached.

Suggest replacing this context with a #parse integration test that exercises the actual fix path:

context 'when route.params has symbol keys but namespace options use string keys' do
  let(:stackable) { Grape::Util::StackableValues.new }
  let(:inheritable_setting) { instance_double('inheritable_setting', namespace_stackable: stackable) }
  let(:app) { instance_double('app', inheritable_setting: inheritable_setting) }

  before do
    stackable[:namespace] = instance_double('namespace', space: ':id', options: { required: true, type: 'Integer' })
    allow(route).to receive(:app).and_return(app)
    allow(route).to receive(:params).and_return(id: {})
  end

  it 'merges namespace options into the symbol-keyed route param' do
    expect(parse_request_params).to eq(id: { required: true, type: 'Integer' })
  end
end

wdyt?