Fix nested intersections in union flattening#2944
Merged
Merged
Conversation
Replace the Vitest unit test (which checked Rust output strings for one language) with a standard JSON Schema fixture exercising the same shape: an allOf nested inside a oneOf inside another oneOf, taken from the issue #2913 repro. The fixture runs against every language in CI, round-trips a positive instance, and expects failure on a malformed union member for languages with the "union" feature. Verified end-to-end locally with schema-golang and schema-typescript; all 22 CI target languages generate successfully from the schema. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…e not checked (known limitation) (#2) cjson's generated parser does not validate invalid types in unions, maps, and arrays (documented above this skip list), so it silently coerces the fixture's expected-failure instance (array where an object is required) into an empty object instead of rejecting it. Skip the schema for cjson, exactly like the existing class-with-additional, go-schema-pattern-properties, and multi-type-enum skips. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
These languages cannot pass the fixture's expected-failure sample: - scala3: the test driver prints the circe DecodingFailure and exits 0, so a rejected input is indistinguishable from success. - kotlin / kotlin-jackson: the generated deserializer accepts an array where a union of two classes is expected instead of rejecting it. - haskell: the test driver encodes the Maybe result, so a failed decode prints "null" and exits 0. The scala3 and kotlin legs failed on exactly this in CI; haskell was cancelled by fail-fast but has the same structural limitation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Thanks for doing this 🙏 |
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
2 participants
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.
Credit
The core fix in this PR was authored by @ankrgyl (Ankur Goyal) in #2914. This PR is a maintainer-managed continuation of that work: all four commits were cherry-picked from the original branch with authorship preserved (the fix commit is still authored by Ankur Goyal). Thank you @ankrgyl!
The bug
flattenUnionsonly checked whether the types being unified were themselvesIntersectionTypeinstances. An intersection nested inside aUnionTypemember slipped through tounifyTypes, which then failed with:This is easy to hit with JSON Schema inputs where a
oneOfbranch produces a union whose member is an intersection (e.g.allOfinsideanyOf/oneOf), and it also occurs on large real-world schemas such as the Kestra 0.19.0 JSON Schema (#2941).The fix
flattenUnions(inpackages/quicktype-core/src/rewrites/FlattenUnions.ts) now recursively checks union members for nested intersections and, when one is found, takes the existing reconstitution path (deferring flattening to a later pass) instead of attempting to unify a set that still contains an intersection.Additional commits convert the original unit test into a schema fixture (
test/inputs/schema/nested-intersection-union.schema) and skip that fixture for the cjson and cplusplus test targets, which have known limitations with these inputs.Fixes #2913
Fixes #2941
Verification
npm run buildsucceeds andnpm run test:unitpasses (71 tests, 9 files).quicktype --lang typescript --src-lang schema test/inputs/schema/nested-intersection-union.schemagenerates successfully; on unpatched master the same invocation fails withInternal error: Unknown type intersection.🤖 Generated with Claude Code