Skip to content

Make ParamsScope#validates pure via ValidationsSpec (closes #2517)#2706

Open
ericproulx wants to merge 1 commit intomasterfrom
refactor/validates-pure-helpers
Open

Make ParamsScope#validates pure via ValidationsSpec (closes #2517)#2706
ericproulx wants to merge 1 commit intomasterfrom
refactor/validates-pure-helpers

Conversation

@ericproulx
Copy link
Copy Markdown
Contributor

@ericproulx ericproulx commented May 9, 2026

Refactors Grape::Validations::ParamsScope#validates so that the validations hash supplied by the DSL is never mutated. The caller's hash flows through the pipeline untouched; passing a frozen hash works.

Resolves the project memory note project_validations_param_mutation.md — both Phase 1 (dup-at-boundary) and Phase 2 (pure helpers) in one shot, since the spec object makes Phase 1's defensive dup unnecessary.

Closes #2517 — incidentally. With :message now in SPEC_CONSUMED_KEYS, optional :foo, message: 'oops' no longer raises UnknownValidator (it used to crash because :message survived dispatch when :presence was absent). Regression spec added.

Before

The hash was destructively consumed across six helpers:

Site Mutation
infer_coercion sets :coerce, :coerce_message; deletes :type, :types, sometimes :coerce_with
derive_validator_options deletes :fail_fast
validates_presence deletes :presence, :message
validates itself validations.extract!(:coerce, :coerce_with, :coerce_message)
document_params validations.except!(:desc, :description, :documentation) when do_not_document
extract_details deletes :desc, :description, :documentation

The contract was implicit: "validates eats the hash."

After

A small frozen Grape::Validations::ValidationsSpec parses the raw hash once and exposes named accessors:

spec = ValidationsSpec.from(validations)
spec.coerce_type        # parsed type
spec.coerce_options     # { type:, method:, message: }
spec.required?          # presence-derived
spec.shared_opts        # frozen { allow_blank:, fail_fast: }
spec.values             # resolves `{ value: ..., message: ... }` form
spec.except_values
spec.default
spec.validator_entries  # filtered hash for the dispatch loop
spec.raw                # the original, untouched

validates shrinks to a small dispatcher:

def validates(attrs, validations)
  spec = ValidationsSpec.from(validations)

  check_incompatible_option_values(spec.default, spec.values, spec.except_values)
  validate_value_coercion(spec.coerce_type, spec.values, spec.except_values)
  document_params(attrs, spec)

  validate_presence(spec, attrs)
  validate_coerce(spec, attrs)

  spec.validator_entries.each do |type, options|
    validate(type, options, attrs, spec.required?, spec.shared_opts)
  end
end

The caller can pass validations.freeze and nothing crashes.

Helpers retired

Helper Replaced by
infer_coercion(validations) ValidationsSpec#parse_coerce (private)
derive_validator_options(validations) ValidationsSpec#shared_opts
validates_presence(validations, ...) ParamsScope#validate_presence(spec, ...)
coerce_type(validations, ...) ParamsScope#validate_coerce(spec, ...)
guess_coerce_type private helper on the spec
options_key? unused — replaced by ValidationsSpec#resolve_value

Behaviour preserved

  • Validator dispatch order: presence → coerce → other entries (some validators rely on the typed value).
  • :message belongs to presence: it's in SPEC_CONSUMED_KEYS, so it never appears in validator_entries (no risk of being re-dispatched as a MessageValidator). This is also what fixes Optional parameter with a message attribute exception #2517.
  • :allow_blank is dual-purpose: read into shared_opts and still appears in validator_entries (so AllowBlankValidator runs).
  • :length is dual-purpose: read for documentation and dispatched to LengthValidator.
  • check_coerce_with still raises ArgumentError at definition time when coerce_with: is supplied without a type or with JSON.
  • do_not_document short-circuit: still returns early; just no longer mutates.

Tests

  • 16 new examples in spec/grape/validations/validations_spec_spec.rb covering parsing, frozen-hash safety, required? semantics, shared opts, validator-entry filtering.
  • spec/grape/validations/params_documentation_spec.rb rewritten to pass a ValidationsSpec instead of a raw hash. The old "removes desc/description/documentation when do_not_document" test (which asserted mutation) is now "does not store any documented params" + "does not mutate the input validations" — capturing the corrected contract.
  • One regression spec for Optional parameter with a message attribute exception #2517 in spec/grape/validations_spec.rb.
  • Full suite: 2,278 examples, 0 failures.

Test plan

  • bundle exec rspec spec/grape/validations/validations_spec_spec.rb — 16/0
  • bundle exec rspec spec/grape/validations/params_documentation_spec.rb — 8/0
  • bundle exec rspec spec/grape/validations_spec.rb -e 'optional accepts a :message option without raising' — 1/0
  • bundle exec rspec — 2,278/0
  • bundle exec rubocop lib/ spec/ — clean

🤖 Generated with Claude Code

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

Danger Report

No issues found.

View run

@ericproulx ericproulx force-pushed the refactor/validates-pure-helpers branch 3 times, most recently from 9154bb1 to 3662c16 Compare May 9, 2026 19:59
@ericproulx ericproulx changed the title Make ParamsScope#validates pure via ValidationsSpec Make ParamsScope#validates pure via ValidationsSpec (closes #2517) May 9, 2026
`Grape::Validations::ParamsScope#validates(attrs, validations)` used
to consume the validations hash destructively across six helpers
(`infer_coercion`, `derive_validator_options`, `validates_presence`,
`coerce_type`, `document_params`, `extract_details`). The caller's
hash was mutated in place; passing a frozen hash would crash the
pipeline.

Introduces `Grape::Validations::ValidationsSpec` — a small frozen
value object that parses the raw hash once and exposes named
accessors. `validates` becomes a thin dispatcher that hands the spec
to the helpers; the raw hash is never written to. Helpers that used
to read/delete are gone:

  * `infer_coercion` → `ValidationsSpec#parse_coerce` (private)
  * `derive_validator_options` → `ValidationsSpec#shared_opts`
  * `validates_presence` → `ParamsScope#validate_presence(spec, ...)`
  * `coerce_type` (the helper) → `ParamsScope#validate_coerce(spec, ...)`
  * `guess_coerce_type` → `ValidationsSpec#guess_coerce_type` (private)
  * `options_key?` → no longer needed; `ValidationsSpec#resolve_value`

`ParamsDocumentation#document_params` now takes the spec instead of
the raw hash and reads via `spec.raw[...]` without deletion. The
`do_not_document` short-circuit no longer mutates either; it just
returns early.

Behaviour preserved:

  * Order of validator dispatch (presence → coerce → others).
  * `:message` belonging to presence is suppressed for later
    validators (it's in `SPEC_CONSUMED_KEYS`).
  * `:allow_blank` and `:length` remain dual-purpose (shared opt + own
    validator entry; doc source + own validator entry).
  * `check_coerce_with` still raises `ArgumentError` at definition
    time when `coerce_with:` is supplied without a type.

Adds:

  * `lib/grape/validations/validations_spec.rb` — value object.
  * `spec/grape/validations/validations_spec_spec.rb` — unit tests
    including a frozen-hash test that asserts `from(frozen_hash)` does
    not raise.

Incidentally fixes #2517: because `:message` is in
`SPEC_CONSUMED_KEYS`, `optional :foo, message: 'oops'` no longer
raises `UnknownValidator`. Regression spec added.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ericproulx ericproulx force-pushed the refactor/validates-pure-helpers branch from 3662c16 to acb86d1 Compare May 9, 2026 20:02
@ericproulx ericproulx requested a review from dblock May 9, 2026 20:11
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.

Optional parameter with a message attribute exception

1 participant