fix(schema-input): decode percent-encoded JSON Pointer segments in $ref - #2995
Open
schani wants to merge 5 commits into
Open
fix(schema-input): decode percent-encoded JSON Pointer segments in $ref#2995schani wants to merge 5 commits into
schani wants to merge 5 commits into
Conversation
…ef (#1992) Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…r non-validating langs The getMainFileSymbols fallback in schemaForTypeScriptSources (added for #1992) left the source name empty when it produced a single top-level ref. For a single schema source the JSON Schema input uses that empty name verbatim, so the top-level type was rendered as the synthesized "Empty" instead of its exported name. This broke the json-ts-csharp fixture: the generated C# no longer contained a TopLevel class, and the driver failed to compile with CS0103. When exactly one exported symbol is selected, name it explicitly so the round-trip type keeps its name. Also add percent-encoded-ref.schema to skipsEnumValueValidation: its only negative sample rejects an out-of-range enum value, which languages that do not validate enum values (cjson, crystal, swift, haskell, typescript-zod, typescript-effect-schema) cannot fail, so the expected-failure case was passing unexpectedly. Co-Authored-By: Claude <noreply@anthropic.com>
# Conflicts: # test/languages.ts # test/unit/typescript-input.test.ts
Generated-output differences40 files differ — 0 modified, 40 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.
The bug
quicktype -s typescript -l c++ ...(and-l typescript) fatally errors ongeneric classes instantiated with more than one type argument, e.g.:
No output files are produced at all. This is a regression from the original
report in #1992, where instead of erroring, quicktype silently misnamed the
first generic instantiation's enum (
PurpleStateInfoT) — same root cause,different (now worse) symptom.
Root cause
quicktype-typescript-inputconverts TypeScript generics into monomorphizedJSON Schema
definitions, e.g. a definition literally named"StateInfo<T>",referenced via
"$ref": "#/definitions/StateInfo%3CT%3E"(URI-fragmentpercent-encoded, since
</>aren't valid in a bare JSON Pointer/URIfragment).
quicktype-core'sRef.parsePath(inpackages/quicktype-core/src/input/JSONSchemaInput.ts), which turns a$reffragment into path segments for definition lookup, split the fragment on
/but never percent-decoded the segments. So it looked up the literal key
"StateInfo%3CT%3E"inschema.definitions, which doesn't exist (the realkey is
"StateInfo<T>"), and failed withSchemaKeyNotInObject. Per RFC 6901§6, a URI fragment used as a JSON Pointer must be percent-decoded before its
segments are interpreted — that step was missing.
Separately,
schemaForTypeScriptSourcesinpackages/quicktype-typescript-input/src/index.tsbuilt top-level$refURIs without percent-encoding special characters in the name, and its
fallback for selecting "top-level" types (when no
#TopLevelmarker ispresent) picked up internal helper definitions rather than the program's
actual exported symbols — this fallback path is what produced the original
PurpleStateInfoTmisnaming.The fix
Ref.parsePath: percent-decode each/-separated path segment aftersplitting (so
StateInfo%3CT%3Eresolves to the realStateInfo<T>key).schemaForTypeScriptSources: percent-encode generated top-level definitionURIs to stay consistent with how
$refs are encoded elsewhere, and usegenerator.getMainFileSymbols()(the program's actual exported symbols) asthe top-level type set instead of arbitrary internal definitions when no
#TopLevelmarker is present.Test coverage
test/inputs/schema/percent-encoded-ref.schema(+.1.jsonsample): a newJSON Schema fixture whose
$refs use percent-encoded characters (<,>)in the JSON Pointer, directly exercising
Ref.parsePath. Fails againstunfixed code with the same
SchemaKeyNotInObjecterror; passes after thefix. Runs automatically for every
schema-<language>fixture (verifiedlocally for
schema-typescriptandschema-cplusplus).test/unit/typescript-input.test.ts: new regression test("generic instantiations retain their exported type names") reproducing
the exact ts to C++ class template argument renamed #1992 repro end-to-end through
quicktype()with theTypeScript-source input, asserting the two monomorphized classes get
distinct correct names (
StateInfoXxx,StateInfoWWW) and that neitherPurpleStateInfoTnorStateInfoT1appear.Verification performed locally
npm run buildpasses.npm run test:unit: 164/164 tests pass (25 files).QUICKTEST=true FIXTURE=schema-typescript script/test test/inputs/schema/percent-encoded-ref.schemapasses.QUICKTEST=true FIXTURE=schema-cplusplus script/test test/inputs/schema/percent-encoded-ref.schemapasses (compiled and ran with g++/nlohmann-json).-s typescript -l c++ ...with the samplefrom ts to C++ class template argument renamed #1992, both in original and reversed class-declaration order):
previously fatal, now generates
StateInfoXxx.hpp/StateInfoWww.hpp(and the TypeScript-language equivalent) with correct, order-independent
names and no
PurpleStateInfoT/collision renaming.schema-typescriptfixture run (
test/inputs/schema/vega-lite.schemafails to compile withtscdue to an unrelated index-signature type conflict) reproducesidentically on unmodified master, so it is not caused by this change; CI
will cover the rest of the fixture matrix.
Fixes #1992.
🤖 Generated with Claude Code