Document the definition-time rejection of uncoercible collection types

CHANGELOG.md

@@ -39,6 +39,7 @@
 * [#2831](https://github.com/ruby-grape/grape/pull/2831): Document and test open (beginless/endless) `Range`s as a `values:`/`except_values:` alternative to a dedicated numericality validator - [@dblock](https://github.com/dblock).
 * [#2830](https://github.com/ruby-grape/grape/pull/2830): Remove `Grape::Util::InheritableValues`, resolving inheritable settings by walking `Grape::Util::InheritableSetting#parent` instead of layering a second chain of stores, and keep namespace settings in a plain Hash - [@ericproulx](https://github.com/ericproulx).
 * [#2833](https://github.com/ruby-grape/grape/pull/2833): Fix `Grape::API::Instance.to_s` returning an empty string instead of the class name when the instance has no base (regression from #2581) - [@ericproulx](https://github.com/ericproulx).
+* [#2837](https://github.com/ruby-grape/grape/pull/2837): Document the definition-time rejection of an `Array`/`Set` of an uncoercible element type introduced by #2817, and pin it with specs - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 #### Fixes
@@ -47,7 +48,7 @@
 * [#2816](https://github.com/ruby-grape/grape/pull/2816): Guard `length`, `same_as` and `oneof` validators against non-hash params (500 → 400) - [@ericproulx](https://github.com/ericproulx).
 * [#2814](https://github.com/ruby-grape/grape/pull/2814): Fix `type: [A, B]` combined with `values:`/`except_values:` raising `IncompatibleOptionValues` at definition time - [@ericproulx](https://github.com/ericproulx).
 * [#2815](https://github.com/ruby-grape/grape/pull/2815): Fix line-anchored regexes: `type: JSON` payloads containing a blank line were silently coerced to `nil` - [@ericproulx](https://github.com/ericproulx).
-* [#2817](https://github.com/ruby-grape/grape/pull/2817): Remove request-time mutable state from Array and custom-type coercers - [@ericproulx](https://github.com/ericproulx).
+* [#2817](https://github.com/ruby-grape/grape/pull/2817): Remove request-time mutable state from Array and custom-type coercers, which also makes an `Array`/`Set` of an uncoercible element type raise when the API is defined instead of on the first request that supplies it (see UPGRADING) - [@ericproulx](https://github.com/ericproulx).
 * [#2819](https://github.com/ruby-grape/grape/pull/2819): Freeze coercers at construction, synchronize `Grape::Util::Cache` lookups, and assign `Route#regexp_capture_index` eagerly - [@ericproulx](https://github.com/ericproulx).
 * [#2824](https://github.com/ruby-grape/grape/pull/2824): Fix cascaded routes (`X-Cascade: pass`) leaking `route_info` and path captures into the next matched route's `route` and `params` - [@ericproulx](https://github.com/ericproulx).
 * [#2825](https://github.com/ruby-grape/grape/pull/2825): Stop `present` entity autodetection from picking up a top-level `::Entity` constant - [@ericproulx](https://github.com/ericproulx).

UPGRADING.md

@@ -3,6 +3,63 @@ Upgrading Grape
 
 ### Upgrading to >= 4.0.0
 
+#### `Array`/`Set` of an unsupported type is rejected when the API is defined
+
+Declaring a collection whose element type Grape cannot coerce — `type: Array[Foo]` or `type: Set[Foo]` where `Foo` is neither a primitive, a structure, nor a valid custom type — now raises as soon as the `params` block is evaluated, i.e. while the API class is being loaded:
+
+```
+ArgumentError: type Foo should support coercion via `[]`
+```
+
+This is a **load-time** failure. An application that boots today can fail to boot on 4.0 without any request being made.
+
+A valid custom type is one that implements a class-level `parse` taking exactly one argument (`Grape::Validations::Types.custom?`). A class that implements neither that nor `[]` was never coercible, so such a declaration was always a misconfiguration — but it used to surface much later, and much less clearly.
+
+**What changed.** Nothing about which types are supported; only *when* the element coercer is built. [#2817](https://github.com/ruby-grape/grape/pull/2817) made `ArrayCoercer` build it eagerly in the constructor rather than memoizing it on first use, because coercers are shared across requests and must not create state at request time. Building it eagerly means the unsupported element type is discovered while the route is being defined.
+
+Previously the coercer was built on the first request that actually supplied the parameter. An API that declared the parameter but was never sent one — a documentation-only declaration, for instance — never built it and never raised. When a request did supply it, the same `ArgumentError` was raised inside the coercer and swallowed by the coercion validator into a generic `400`:
+
+```json
+{ "error": "foo is invalid" }
+```
+
+so the misconfiguration presented as a puzzling per-request validation failure rather than as a broken declaration.
+
+Note that the non-collection form has always raised at definition time:
+
+| declaration | before | 4.0 |
+| --- | --- | --- |
+| `type: Foo` | `ArgumentError` when defined | unchanged |
+| `type: Array[Foo]` | accepted; `400 "is invalid"` per request | `ArgumentError` when defined |
+
+The collection form's leniency was an accident of the lazy build, not a supported behavior. The two are now consistent.
+
+**Fixing a declaration that now raises.** Give the type a one-argument `parse`, which is what makes it a custom type:
+
+```ruby
+class Foo
+  def self.parse(value)
+    new(value)
+  end
+end
+
+params { requires :foos, type: Array[Foo] }
+```
+
+Or coerce the collection yourself, in which case Grape does not build an element coercer at all:
+
+```ruby
+params { requires :foos, type: Array[Foo], coerce_with: ->(value) { Array(value).map { |v| Foo.new(v) } } }
+```
+
+Or, if the class is only there to describe the parameter and never to coerce it — the common case behind this break — drop `type:` and document it instead:
+
+```ruby
+params { requires :foos, documentation: { type: Foo } }
+```
+
+Defining `self.[]` on the class silences the load-time error, because that is the escape hatch for `dry-types` objects, but it does **not** make the type coercible: such a parameter still fails with `400 "is invalid"` on every request. Prefer `parse` or `coerce_with`.
+
 #### The `cascade` getter returns the configured value
 
 Calling `cascade` with no argument used to report whether cascading had been *configured at all* (`cascade false` still read back as `true`, contradicting the actual runtime behavior, which was correctly disabled). It now returns the configured value itself — `true`/`false` as set, or `true` when never set. Code that used the getter to detect "was `cascade` called" rather than "does this API cascade" must track that separately.

spec/grape/validations/types_spec.rb

@@ -98,6 +98,48 @@ def self.parse; end
       expect(a_coercer.object_id).to eq(b_coercer.object_id)
     end
 
+    # The element coercer is built eagerly (see ArrayCoercer), so a collection
+    # of a type Grape cannot coerce is rejected here -- while the params block
+    # is evaluated -- rather than on the first request that supplies the
+    # parameter, where it used to surface as a generic 400. See UPGRADING.
+    describe 'a collection of an unsupported element type' do
+      before { stub_const('UncoercibleType', Class.new) }
+
+      it 'raises for an Array of it' do
+        expect { described_class.build_coercer(Array[UncoercibleType]) }
+          .to raise_error(ArgumentError, 'type UncoercibleType should support coercion via `[]`')
+      end
+
+      it 'raises for a Set of it' do
+        expect { described_class.build_coercer(Set[UncoercibleType]) }
+          .to raise_error(ArgumentError, 'type UncoercibleType should support coercion via `[]`')
+      end
+
+      it 'raises while the params block is evaluated' do
+        expect do
+          Class.new(Grape::API) do
+            params { requires :foos, type: Array[UncoercibleType] }
+            get('/foos') { 'never reached' }
+          end
+        end.to raise_error(ArgumentError, 'type UncoercibleType should support coercion via `[]`')
+      end
+
+      # A one-argument `parse` makes it a custom type, which ::custom? accepts
+      # and which routes to CustomTypeCollectionCoercer instead of dry-types.
+      it 'is accepted once the element type implements a one-argument parse' do
+        stub_const('CoercibleType', Class.new { def self.parse(value) = value })
+
+        expect { described_class.build_coercer(Array[CoercibleType]) }.not_to raise_error
+      end
+
+      # coerce_with takes over the whole collection, so no element coercer is
+      # built and the element type is never looked up.
+      it 'is accepted when coerce_with supplies the coercion' do
+        expect { described_class.build_coercer(Array[UncoercibleType], method: ->(v) { Array(v) }) }
+          .not_to raise_error
+      end
+    end
+
     # Coercer instances are shared across requests, so they must be frozen at
     # construction — a request-time lazy ivar write would be a data race and
     # now raises FrozenError instead.