fix(ruby): generate from_json!/from_dynamic! helpers for top-level enums#3047
Open
schani wants to merge 6 commits into
Open
fix(ruby): generate from_json!/from_dynamic! helpers for top-level enums#3047schani wants to merge 6 commits into
schani wants to merge 6 commits into
Conversation
…ums (#724) Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
…e-76a-72 # Conflicts: # test/languages.ts
Generated-output differences1 files differ — 0 modified, 1 new, 0 deleted |
Member
Author
|
The PR description says that the problem only happens for top-level enums, but this is emitting those methods for all types, not just enums! Fix that! |
Generated-output differences1 files differ — 0 modified, 1 new, 0 deleted |
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.
Bug
For a top-level JSON Schema enum, quicktype's Ruby target emitted a
leading usage comment claiming:
but
TopLevelEnumwas generated as a plain Rubymoduleof stringconstants with no
from_json!(orfrom_dynamic!) method defined onit. Following the comment's instructions would raise
NoMethodError: undefined method 'from_json!' for TopLevelEnum.Repro:
Root cause
RubyRenderer#emitClassandRubyRenderer#emitUnionboth emit adef self.from_json!(json)(backed byfrom_dynamic!) directly on thegenerated class, so the shared leading comment (emitted once in
emitSourceStructure) is accurate for those.RubyRenderer#emitEnum,however, only emitted the constant table and never defined those
helper methods, so the same comment was misleading for top-level enums.
This was a known gap —
test/languages.tshadtop-level-enum.schemain Ruby's
skipSchemalist with the comment "We don't generate aconvenience method for top-level enums".
Fix
RubyRenderer#emitEnumnow also emitsfrom_dynamic!/from_json!class methods on the enum module (skipped under
--just-types, sameas the class/union renderers), reusing the
Types::<EnumName>dry-typesvalidator that was already being emitted:
This makes the generated code match the leading usage comment for
top-level enums, the same way it already did for classes and unions.
Test coverage
top-level-enum.schemais no longer skipped for the Ruby target intest/languages.ts(theskipSchemaentry and its now-outdatedcomment were removed), so this is now covered end-to-end by the
existing
test/inputs/schema/top-level-enum.schema/.1.jsonJSON Schema fixture viaFIXTURE=schema-ruby, per therepo's fixture-testing conventions.
Verification
npm run buildpasses.npx vitest run test/unit: 170/170 tests pass.QUICKTEST=true FIXTURE=schema-ruby script/test test/inputs/schema/top-level-enum.schemapasses end-to-end (previously skipped, now generates, round-trips, and validates correctly via the Ruby driver).from_dynamic!/from_json!onTopLevelEnum, matching the leading comment.schema-rubyfixture suite: found 3 pre-existing failures (tuple.schema,union-list.schema,union-int-double.schema) unrelated to this change — verified by reproducing the identical failures on unfixed master (sameDry::Types::CoercionError/ "can't convert Symbol into Hash" already documented elsewhere intest/languages.tsas "Ruby union code does not work with new Dry"). No new failures were introduced.schema-rubycase fortop-level-enum.schema.Fixes #724
🤖 Generated with Claude Code