Skip to content

Fix: VariantCollectionCoercer#to_s returns "Array[Type, ...]" notation#2758

Open
bogdan wants to merge 1 commit into
ruby-grape:masterfrom
bogdan:fix/variant-collection-coercer-to-s
Open

Fix: VariantCollectionCoercer#to_s returns "Array[Type, ...]" notation#2758
bogdan wants to merge 1 commit into
ruby-grape:masterfrom
bogdan:fix/variant-collection-coercer-to-s

Conversation

@bogdan
Copy link
Copy Markdown

@bogdan bogdan commented Jun 2, 2026

Problem

type: [Integer, String] and types: [Integer, String] have different semantics:

  • types: [Integer, String] — the parameter itself can be Integer or String (variant types)
  • type: [Integer, String] / type: Array[Integer, String] — the parameter is always an Array whose members can be Integer or String (variant-member-type collection)

However, VariantCollectionCoercer had no custom to_s, so route.params[:type] for the type: form serialised to a garbage object string ("#<Grape::Validations::Types::VariantCollectionCoercer:0x...>"). Documentation tools such as grape-swagger and grape-oas could not distinguish it from the plain "[Integer, String]" produced by types:, and could not generate a correct schema for either case.

Fix

Add VariantCollectionCoercer#to_s returning the Grape DSL notation:

  • type: [Integer, String]"Array[Integer, String]"
  • type: Set[Integer, String]"Set[Integer, String]"
  • types: [Integer, String]"[Integer, String]" (unchanged)

This matches the notation users write in the DSL and gives documentation tools an unambiguous signal to emit {type: array, items: {oneOf: [...]}} rather than a plain oneOf.

Compatibility

  • declared_params_handler.rb reads route.params[:type] but only matches 'Array', 'Hash', 'Set', and the single-type [Type] pattern — "Array[Integer, String]" does not collide with any of those branches.
  • grape-swagger's type-parsing regexes (/\[(.*)\]$/, /\A\[.*\]\z/) are unaffected.

@dblock dblock requested a review from ericproulx June 2, 2026 12:25
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

Danger Report

No issues found.

View run

@bogdan bogdan force-pushed the fix/variant-collection-coercer-to-s branch from 4cccebe to cf5ef17 Compare June 2, 2026 12:42
Without a custom to_s, type: [Integer, String] serialised to a garbage
object string in route.params[:type], making it indistinguishable from
the VariantCollectionCoercer instance address. Documentation tools
(grape-swagger, grape-oas) could not tell it apart from the plain-array
"[Integer, String]" produced by the types: keyword.

Now type: [Integer, String] produces "Array[Integer, String]" and
type: Set[Integer, String] produces "Set[Integer, String]", matching
the Grape DSL notation and letting tooling generate the correct schema
(array with variant-member items vs oneOf at the parameter level).
@bogdan bogdan force-pushed the fix/variant-collection-coercer-to-s branch from cf5ef17 to 9f06e49 Compare June 2, 2026 12:44
Copy link
Copy Markdown
Contributor

@ericproulx ericproulx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix. Two asks: (1) add a spec for the Set form (type: Set[Integer, String] → "Set[Integer, String]") — it's an untested branch. (2) The compatibility note isn't quite right for Set: "Set[Integer, String]" does match declared_params_handler.rb:111's start_with?('Set'), so a type: Set[…] param now declares Set.new instead of falling through. That looks like the correct/consistent behaviour, but please call it out and add a declared-params test to pin it down.

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.

2 participants