fix(rust): stop double-boxing nullable cycle-breaking unions#3040
Merged
Conversation
The Rust renderer's breakCycle wrapped an extra outer Box around a field whose type is a nullable union that is itself a cycle breaker, producing Box<Option<Box<T>>> instead of Option<Box<T>>. The union rendering path already boxes its cycle-breaking member, so the field-level Box was redundant. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
…3040) Co-Authored-By: Claude <noreply@anthropic.com>
The new rust-cycle-breaker-union.schema (a self-referential union member, i.e. a union whose member recursively refers back to the enclosing object) crashes cJSON's multi-source renderer with an internal error during generation. This is a pre-existing cJSON limitation, unrelated to the Rust double-boxing fix this schema was added to cover; all other languages generate valid, round-tripping code. Scope the schema out for cjson. Co-Authored-By: Claude <noreply@anthropic.com>
KotlinX renders unions as sealed classes without serializer wiring, so deserialization of polymorphic union members fails at runtime with "Class discriminator was missing" (a pre-existing, documented KotlinX limitation shared by ~15 other union schemas already skipped in KotlinXLanguage.skipSchema). The new rust-cycle-breaker-union.schema hits the same limitation; scope it out for kotlinx like the others. Co-Authored-By: Claude <noreply@anthropic.com>
PR #898 (merged after #2357) fixed enum case ordering to preserve JSON Schema declaration order instead of the previous incidental alphabetical order. test/unit/cjson-enum-default.test.ts, added by #2357, still asserted the old alphabetical order (CONFIG, HEARTBEAT, STATE) and started failing on master once #898 landed. Update the expectation to the correct, now-declaration-order output (STATE, CONFIG, HEARTBEAT), matching the schema's enum: ["state", "config", "heartbeat"]. Co-Authored-By: Claude <noreply@anthropic.com>
Elm type aliases cannot be recursive (documented at the top of ElmLanguage.skipSchema, and already applied to mutually-recursive.schema, recursive-union-flattening.schema, etc). The generated decoder/encoder for the new rust-cycle-breaker-union.schema forms a genuinely cyclic top-level value definition (`next` depends on `nodeClass` depends on `next`), which elm make rejects with a CYCLIC DEFINITION error since it isn't wrapped in Jdec.lazy. Scope the schema out for elm like the other recursive schemas. Co-Authored-By: Claude <noreply@anthropic.com>
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
1 participant
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
The Rust backend generated redundant boxes for a struct field whose type is a nullable union that is also a cycle breaker, e.g.
Box<Option<Box<ChiangMaiGoose>>>instead ofOption<Box<ChiangMaiGoose>>.Root cause
In
RustRenderer.breakCycle, the field-level call wraps an outerBoxaround any type marked as a cycle breaker. When the field's type is a nullable union rendered asOption<Enum>, and that union is itself the cycle breaker, the union-rendering path (rustType) already boxes the enum inside theOption(Option<Box<Enum>>).breakCyclethen wrapped anotherBoxaround the whole thing, producingBox<Option<Box<Enum>>>.Fix
packages/quicktype-core/src/language/Rust/RustRenderer.ts: inbreakCycle, whentis aUnionTypethat is a cycle breaker, skip the extra outerBoxif the union isn't a simpleT | nullwrapper around a non-cycle-breakingT(i.e.nullableFromUnion(t) === null, the multi-member/enum case) or if the inner nullable type is itself already the cycle breaker — both cases where the necessary box is already emitted byrustType.Test coverage
test/inputs/schema/rust-cycle-breaker-union.schema+.1.json: a new JSON Schema fixture (self-referential object viaanyOfwith a string alternative and null, forcing a named Rustenum) covering the double-boxing scenario end-to-end via theschema-rustfixture.test/unit/rust-cycle-breaker-boxing.test.ts: unit tests assertingOption<Box<Next>>(notBox<Option<Box<Next>>>) for the nullable case, and that the non-nullable cycle-breaking union case still emits a singleBox<Next>(no regression).Verification
npm run buildpasses.npx vitest run test/unit/rust-cycle-breaker-boxing.test.ts— 2/2 pass.node dist/index.js --lang rust --src-lang schema node.schemaon a nullable self-referential union now emitsOption<Box<Next>>instead ofBox<Option<Box<Next>>>.QUICKTEST=true FIXTURE=rust script/test(58 tests × renderer-option variants) — all passed, no regressions.QUICKTEST=true FIXTURE=schema-rust script/test(74 tests, including the newrust-cycle-breaker-union.schema) — all passed, no regressions.Fixes #1112
🤖 Generated with Claude Code