Fix Grape 3.2 compatibility: keyword args for desc and custom param types#978
Open
numbata wants to merge 14 commits into
Open
Fix Grape 3.2 compatibility: keyword args for desc and custom param types#978numbata wants to merge 14 commits into
numbata wants to merge 14 commits into
Conversation
Grape 3.2.1 deprecates passing a positional options Hash to `desc` and requires unknown types in params blocks to either be a registered dry-type or implement `parse` (custom type protocol). This change addresses both. - Pass keyword arguments to `desc` in doc_methods.rb so Grape's deprecator does not fire (or raise when behavior = :raise) - Add `self.parse` to the mock `Entities::NestedModule::ApiResponse` so Grape treats it as a custom type instead of attempting a dry-types lookup - Move `type: 'Object'` into the `documentation` hash in params_example_spec so Grape no longer sees an unknown string type; grape-swagger still reads it from the merged settings and emits the expected swagger output
Moving SwaggerRouting and SwaggerDocumentationAdder inside the GrapeSwagger namespace would be a breaking change for dependent gems that reference these constants by their top-level names. Exclude the affected files in .rubocop_todo.yml to keep CI green until that rename ships separately.
Add comments explaining: - 'Object' is a swagger documentation hint, not a valid Grape coercion type; the type lives in documentation: so grape-swagger picks it up via settings merge - ApiResponse.parse is required because Grape 3.2+ validates unknown types via Types.custom? (arity-1 parse check); minimal pass-through is sufficient since tests exercise documentation generation, not request coercion
Representable::Decorator does not implement .parse, so Grape 3.2+ raises ArgumentError when ApiResponse is used as a param type. Same fix as mock_parser.
Danger ReportNo issues found. |
dblock
approved these changes
May 18, 2026
Contributor
Author
|
@dblock FYI, I’ll jump back into this and the other PRs in the my grape-review queue later today. It looks like we have everything we need to release the new minor version of the gem. |
Member
|
CI does need to be fixed |
Grape 1.8.0 and 2.0.0 are broken with Ruby 3.3+ due to a Mustermann private-method incompatibility (named_captures), producing 275 failures on both master and this branch. Supporting these versions is no longer meaningful. Raise the gemspec lower bound from >= 1.7 to >= 2.1 to reflect what actually works, and drop the 1.8.0/2.0.0 rows from the CI matrix so the build result becomes an honest signal.
In Grape >= 3.3 `type: [A, B]` is wrapped in a VariantCollectionCoercer, and Grape's documentation pipeline serialises that wrapper via #to_s before storing it in route.params, which discards the original type list. The previous behaviour exposed the coercer's #inspect string (e.g. "#<Grape::Validations::Types::VariantCollectionCoercer:0x...>") as the swagger type. The live coercer is still reachable via the matching CoerceValidator's @converter, so collect_variant_types walks namespace_stackable[:validations] and builds a name => [types] lookup. When fulfill_params sees the VariantCollectionCoercer signature in the serialised type, it substitutes the real type list and lets the existing Array handling in DataType.parse_multi_type pick the primary type, matching how older Grape versions behaved when they passed the type array through directly.
VariantCollectionCoercer recovery (route.rb) - Build the variant-type map keyed by the fully-qualified param name (e.g. "group[inner]") via the validator's @scope.full_name(attr), so nested-param multi-types are restored instead of being looked up under the wrong key, and same-named outer params at a different scope are not clobbered. - Use the public validator.attrs reader, and normalise the recovered @types to an Array so an order-defined coercer Set cannot leak in. - Cover the inheritance shape of stackable[:validations] with .flatten. desc keyword-args (doc_methods.rb) - Coerce api_documentation / specific_api_documentation keys to symbols via transform_keys before splatting, so string-keyed Hashes (YAML/JSON configs) no longer raise TypeError under `**`. - Accept :description as an alias for :desc. Without this, a user-supplied :description was forwarded into Grape's desc and immediately overwritten with nil because the positional description arg was missing. Tests - Add a regression spec asserting `type: [Integer, Float]` produces "integer" (the only assertion that proves the recovery actually wires the real types through; previously [String, Integer] also "worked" via the broken fallback because String happens to be first). - Cover the nested-namespace case (`group { requires :inner, type: [...] }`) to lock in the scope-aware key. CHANGELOG - Document the >= 2.1 Grape floor bump and the `type: 'Object'` migration pattern under a Breaking changes section, and split the multi-type recovery and desc-kwargs robustness fixes into separate entries.
… scope route.rb - Convert the recovered VariantCollectionCoercer @types via #to_a so a Set-backed coercer yields a deterministic order (insertion order from the user's declaration), matching what `parse_multi_type` will pick via `.first` downstream. - Guard scope.full_name with `respond_to?` so a future CoerceValidator built without @scope (custom subclasses, test doubles) degrades to skipping that validator rather than crashing the whole swagger doc generation with NoMethodError. - Expand the leading comment to note that on Grape < 3.3 the recovery is a no-op (the legacy `"[A, B]"` string is already handled by parse_multi_type's regex branch), so a reader doesn't conclude the defined? guard is silently overriding working behaviour. doc_methods.rb (pop_desc) - Use `key?` instead of `||` so an explicit `desc: nil` is respected rather than silently falling through to `:description`. Document the precedence inline (`:desc` wins when both keys are supplied). specs - Cover the new public-facing `:desc`/`:description`/string-key semantics in api_documentation_spec so a future refactor can't regress them silently.
Member
|
Since this breaks backwards compat, needs a version bump and an UPGRADING note. Check compatibility in README, too. |
Per maintainer feedback on PR #978, this release carries breaking changes (Grape floor bump, `type: 'Object'` migration, custom-type `parse` requirement) and therefore lands as 2.2.0, with a matching UPGRADING entry and README compatibility-matrix row. Docs - README: add a `>= 2.2.0` compatibility row and cap the prior row at `< 2.2.0`. Document the new `:description` alias / string-keyed `api_documentation` affordance and the `:desc` precedence rule. - UPGRADING: new "Upgrading to >= 2.2.0" section covering all three user-facing breaking changes (Grape floor, `type: 'Object'`, custom types needing `.parse`). - CHANGELOG: split bullets into Breaking changes / Features / Fixes with accurate wording. Drop the misleading "matching the pre-keyword -args behaviour" framing — `:description` and string keys are new affordances. Add the custom-type `parse` requirement. Note that the variant-type recovery is a no-op on Grape < 3.3. Code - `pop_desc` is now private; it was an internal helper to `setup` but was leaking onto every `Grape::API` that mixed in `DocMethods`. - `route.rb`: `flatten(1)` instead of unbounded `.flatten`, and skip the merge when the recovered coercer `@types` is empty so we don't overwrite the legacy serialisation with `[]`. Tests - Add regression coverage for the parts of `pop_desc` that the v2 fix was actually for: explicit `desc: nil` must not fall through to `:description`, and `specific_api_documentation: { description: ... }` goes through the second `pop_desc` call site too. Public-API audit: `@converter` / `@types` (on VariantCollectionCoercer) and `@scope` (on Validators::Base) have no public reader on the supported Grape range (`scope` is private on HEAD and absent on 3.2.1), so the `instance_variable_get` reaches remain — there is no stable public alternative across the matrix.
moskvin
approved these changes
May 29, 2026
Contributor
Author
StackableValues#[] already returns a flat Array, so the Array(...).flatten(1) wrapping in collect_variant_types was redundant noise. Replace it with a direct .each and update the comment to match reality. Add an explicit failure-mode note: if Grape ever renames the private ivars (@converter, @types, @scope), the method silently returns {} and swagger degrades to the pre-fix broken output rather than crashing. Split the three-operation desc(pop_desc(...), params: ..., **...) one-liner into named locals for readability.
Grape HEAD introduced Grape::Endpoint::Options (a Data class) that requires a :for keyword (the owner API class). Constructing an Endpoint without it raises ArgumentError: missing keyword: :for.
numbata
force-pushed
the
worktree-fix+issue-977-desc-keyword-args
branch
from
728e002 to
a605502
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Grape 3.2 introduces two breaking changes that cause
add_swagger_documentationto raise whenGrape.deprecator.behavior = :raise(the default in Grape's own CI). First,descemits a deprecation when called with a positional options Hash; grape-swagger called it that way in two places indoc_methods.rb. Second,optional/requiresnow routes:typethrough Grape's dry-types coercion infrastructure, which rejects plain string type names (e.g.'Object') and unknown classes that don't implement.parse. This PR fixes both issues and documents the'Object'type pattern for future maintainers.Changes
lib/grape-swagger/doc_methods.rbdesccalls so Grape's deprecator does not fire (or raise whenbehavior = :raise).spec/support/model_parsers/mock_parser.rbandrepresentable_parser.rbdef self.parse(val) = valtoNestedModule::ApiResponsein both parsers. Grape 3.2+ requires unknown types used inparamsblocks to satisfyTypes.custom?(i.e. respond to.parsewith arity 1), otherwise it attempts a dry-types lookup that raisesArgumentError.Grape::Entityalready implements.parse, soentity_parser.rbneeds no change. The implementation is intentionally minimal — these specs test swagger documentation generation only, not request coercion.spec/swagger_v2/params_example_spec.rbtype: 'Object'from the Grape params declaration intodocumentation: { type: 'Object' }.'Object'is not a valid Grape coercion type; it is a swagger documentation hint. Grape 3.2+ rejects string type names in params blocks. grape-swagger picks up the type from the merged settings hash inParseParams#call, so the swagger output is unchanged. A comment in the spec explains the pattern..rubocop_todo.ymlStyle/OneClassPerFileforlib/grape-swagger.rband two spec files where multiple top-level modules are a pre-existing condition. MovingSwaggerRoutingandSwaggerDocumentationAdderinside theGrapeSwaggernamespace would be a breaking change for dependent gems referencing those constants; that rename will ship separately.Validation — all three MODEL_PARSER values
Tested against Grape 3.2.1 (the version in this worktree's
Gemfile.lock).Closes #977