Fix Array/Set custom-type coercer dropping symbolized hash keys#2699
Open
ericproulx wants to merge 1 commit intomasterfrom
Open
Fix Array/Set custom-type coercer dropping symbolized hash keys#2699ericproulx wants to merge 1 commit intomasterfrom
ericproulx wants to merge 1 commit intomasterfrom
Conversation
The Array/Set branch of `enforce_symbolized_keys` built a symbolized copy via non-mutating `deep_symbolize_keys` and discarded it, so the returned collection still carried string-keyed hashes. Switch to `map!` so the symbolized values replace the originals, matching the Hash branch's contract. While here, refactor the class for readability: hoist the `[Array, Set]` and `[:coerced?, :parsed?]` arrays to private frozen constants, lift trailing `if/else` chains into guard clauses, extract `hash_symbolizer` / `collection_symbolizer` / `enumerable_type_check` helpers, collapse the `infer_coercion_method` + symbolize-wrap chain into a single `build_coercion_method` entry point, and reorder the private methods to follow call order from `initialize`. Add `spec/grape/validations/types/custom_type_coercer_spec.rb` covering the Array and Set symbolization paths through the public API. The Array case is the regression test for the bug above. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5f67d67 to
051b7da
Compare
Danger ReportNo issues found. |
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
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::Validations::Types::CustomTypeCoercer#enforce_symbolized_keys, theArray/Setbranch built a symbolized copy via non-mutatingdeep_symbolize_keysand discarded it, returning the original string-keyed hashes. Swapped the inner block tomap!so the symbolized values replace the originals (matching theHashbranch's contract).[Array, Set]and[:coerced?, :parsed?]to private frozen constants (COLLECTION_TYPES,TYPE_CHECK_METHODS) — one allocation at load time, named intent.if/elsif/elsechains ininfer_type_check,recursive_type_check, and the newbuild_coercion_methodinto guard clauses; the simple default sits at top indentation.hash_symbolizer,collection_symbolizer,symbolize_if_hash, andenumerable_type_checkhelpers so the dispatch lists read uniformly and the per-item logic has a name.infer_coercion_method+ symbolize-wrap chain ininitializeinto a singlebuild_coercion_methodentry point.initialize.spec/grape/validations/types/custom_type_coercer_spec.rbcovers Array and Set symbolization paths through the public API. The Array case is the regression test for the bug above; before the fix it returned[{"foo" => "bar"}]instead of[{foo: "bar"}].Test plan
bundle exec rspec spec/grape/validations/types/custom_type_coercer_spec.rb— both examples passbundle exec rspec— full suite green (2,256 examples)bundle exec rubocop lib/grape/validations/types/custom_type_coercer.rb spec/grape/validations/types/custom_type_coercer_spec.rb— clean🤖 Generated with Claude Code