fix(schema): unify $ref-to-$id resolution with directly supplied schemas#3025
fix(schema): unify $ref-to-$id resolution with directly supplied schemas#3025schani wants to merge 8 commits into
Conversation
…mas (#1833) When multiple JSON Schema files are passed as separate top-level inputs and one of them is also reached via a $ref to its $id from another file (e.g. a.json and b.json both $ref /schemas/c, and c.json is passed directly as a top-level schema), quicktype generated two separate types for the same schema instead of unifying them, leaving one as dead/unreferenced output. Root cause: in Canonizer.addSchema (JSONSchemaInput.ts), the canonical Location for a schema reached through $id lookup used `Ref.root(address)`, while the Location used for directly supplied top-level schemas is derived via `Ref.parse`. These two forms of the same address didn't compare equal, so the schema store treated them as different schemas. Fix: use `Ref.parse(address)` consistently when registering $id-resolved schema roots, matching how top-level schema references are built. Added test/inputs/schema/issue-1833/ as a new multi-file JSON Schema fixture (extending JSONSchemaFixture.getSamples in test/fixtures.ts to support directories of multiple .schema files as one sample) reproducing the exact scenario from the issue, plus test/unit/json-schema-multiple-sources.test.ts, a targeted unit test that builds a JSONSchemaInput from three named schema sources and asserts the generated output reuses one C interface and never emits a duplicate InElement. The unit test fails on unfixed code and passes with the fix. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…e-76a-85 # Conflicts: # test/fixtures.ts
…p-level languages The issue-1833 fixture's top-level.schema was a bare $ref to /schemas/a, so after unification the 'A' and 'TopLevel' top-levels resolved to the same type. Languages that emit a per-top-level (Un)marshal helper (e.g. Go) generated an UnmarshalA helper referencing a type A that no longer existed, failing to compile. Make top-level.schema its own object type that references /schemas/a and /schemas/b, so TopLevel, A, B and C are four distinct top-levels. This still exercises the fix (C is directly supplied and reached via $ref-to-$id from both A and B, and A/B are reached via $ref-to-$id from TopLevel) while generating code that compiles and round-trips across all fixture languages. Update the positive and no-defaults fail samples to match the new TopLevel shape. Co-Authored-By: Claude <noreply@anthropic.com>
…amed drivers The Elm, Haskell and Objective-C fixture drivers deserialize a single top-level type with a fixed name (QuickType / QTTopLevel) rather than TopLevel. The issue-1833 fixture is a directory of co-equal schema files, so its top-level names come from each file's $id/title (A, B, C, TopLevel) and none matches those drivers' expected name, causing a compile-time naming error. Skip the fixture for those three languages; it still runs its positive and negative cases across the many TopLevel-named languages. Co-Authored-By: Claude <noreply@anthropic.com>
The Java fixture driver deserializes through the generic `Converter.fromJsonString`, which quicktype only emits for a single top-level; multi-top-level generation emits per-type `<Name>FromJsonString` methods instead, so the driver fails to compile against the issue-1833 directory input. Skip it there, as already done for the Elm, Haskell and Objective-C drivers. Co-Authored-By: Claude <noreply@anthropic.com>
The issue-1833 fixture's only negative case is a missing required property. The cJSON, Swift, typescript-zod, typescript-effect-schema and Elixir fixtures already skip required.schema because their generated code does not treat an absent required property as a failure, so the expected-failure sample would not fail for them either. Skip issue-1833 for the same renderers, mirroring the existing required.schema skips. Co-Authored-By: Claude <noreply@anthropic.com>
|
Followed up on the multi-file The original A directory of co-equal schema files is inherently a multi-top-level input, which a few drivers can't consume, so the fixture is skipped for them via the existing
It still runs both a positive and a negative case on the many TopLevel-named renderers (Go, Python, C#, C++, Rust, Kotlin, Dart, Scala 3, PHP, Ruby, TypeScript, and more). Full matrix is green. |
Generated-output differences20 files differ — 0 modified, 20 new, 0 deleted |
Bug
When multiple JSON Schema files are passed as separate top-level inputs and one of them is also reached via a
$refto its$idfrom another file, quicktype generated two separate types for the same schema instead of unifying them into one.Repro (from the issue):
where
a.jsonandb.jsonboth have a property that's{ "$ref": "/schemas/c" }, andc.json(which declares"$id": "/schemas/c") is passed directly alongside them.Before the fix:
After the fix:
Root cause
In
Canonizer.addSchema(packages/quicktype-core/src/input/JSONSchemaInput.ts), the canonicalLocationfor a schema reached through$idlookup was built withRef.root(address), while theLocationfor directly supplied top-level schemas is built viaRef.parse. These two representations of the same address didn't compare as equal, so the schema store treated the$ref-reached schema and the directly-supplied top-level schema as two distinct schemas, producing two separate types.Fix
Use
Ref.parse(address)consistently when registering$id-resolved schema roots, matching how top-level schema references are already built:this.addIDs( schema, - new Location(Ref.root(address), Ref.root(undefined)), + new Location(Ref.parse(address), Ref.root(undefined)), );Test coverage
test/inputs/schema/issue-1833/— a new multi-file JSON Schema fixture (a.schema,b.schema,c.schema,top-level.schema) reproducing the exact scenario from the issue.JSONSchemaFixture.getSamplesintest/fixtures.tswas extended to treat a directory of multiple.schemafiles undertest/inputs/schema/as one combined sample (mirroring how the CLI treats a directory of schema files as multiple co-equal top-level inputs), so this now runs through everyschema-<language>fixture.test/unit/json-schema-multiple-sources.test.ts— a targeted unit test that builds aJSONSchemaInputfrom three named schema sources (mirroring the internal API) and asserts the generated TypeScript output reuses a singleCinterface for bothA.inandB.out, with no duplicateInElementinterface. This is a real regression guard: it fails against the unfixed code and passes with the fix.Verification performed locally
npm run buildpasses cleanly.InElement,Cshared by bothAandB).npx vitest run): 164/164 tests pass.QUICKTEST=true FIXTURE=schema-typescript script/test: only the newissue-1833fixture and pre-existing fixtures ran; the only failure istest/inputs/schema/vega-lite.schemafailing to compile with TypeScript (TS2411errors) — verified this failure is pre-existing on unmodifiedmasteras well and unrelated to this change.npx biome checkon the changed files passes with no issues.Fixes #1833
🤖 Generated with Claude Code