Fix kotlin date/time CI failures: klaxon crash, jackson round-trip, union guards#1
Merged
Merged
Conversation
The "original" style returned names verbatim, skipping combineWords entirely, so JSON keys that aren't valid identifiers produced uncompilable C++ (e.g. "some-header" emitted as a member name, or a leading-digit key like "9lives"). Route "original" through combineWords as a single unsplit word instead: casing and legal characters (including underscores) are preserved exactly, while illegal characters are stripped, invalid start characters get the usual "the" prefix, and empty names are handled. This matches the semantics of the existing "--acronym-style original" option. Reserved words like "class" were already handled by the renderer's forbidden-names machinery and still are. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Added support for original field and type naming for C++ language.
Bump the build compiler from ~5.8.3 to ~7.0.2 (the native compiler) in the root and all workspace manifests. Exactly one hoisted copy still resolves; typescript-json-schema keeps its own nested TypeScript for the TS-input path, which is deliberately decoupled from the build compiler. Migration fallout, all trivial: - tsconfigs: TS 7 removed `baseUrl` and requires an explicit `rootDir`; replace `baseUrl: "src"` with `rootDir: "src"` (no non-relative imports relied on baseUrl). - JSONSchemaRenderer: TS 7 no longer infers the Schema return type across matchTypeExhaustive's arms; pass the type argument explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
No runtime code imports the typescript package (verified by grep over all runtime sources): the CLI's TypeScript-input path deliberately uses the TypeScript instance nested under typescript-json-schema. The compiler is only needed to build, so shipping it as a dependency just bloats `npm i -g quicktype` installs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
packages/quicktype-typescript-input/src/EncodedDefaultTypeScriptLibrary.ts is a gzipped, base64-encoded lib.d.ts from 2018 (TypeScript ~2.8 era), left over from when quicktype bundled its own default lib for browser use. Nothing references it anymore; the default libs come from the TypeScript installation typescript-json-schema resolves on disk. Also remove script/make-encoded-ts-lib.sh, which existed only to regenerate it and copies from a ../data/lib.d.ts that no longer exists in the repo. (script/make-encoded-markov-chain.sh is an unrelated mechanism and stays.) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
typescript-json-schema declares "typescript": "~5.9.3", so the compiler that parses TypeScript *input* — and with it the input syntax quicktype accepts — drifted with whatever 5.9.x patch npm resolved at install time. Add an exact "typescript": "5.9.3" to quicktype-typescript-input's own dependencies. It satisfies typescript-json-schema's range, so npm dedupes the two to a single exact copy — both in this repo and for npm consumers of the published package (a root "overrides" entry would only pin this repo). The wrapper still uses the instance re-exported by typescript-json-schema at runtime; this dependency exists to pin what that instance resolves to. The build compiler (7.0.2, hoisted at the root) stays deliberately independent of the input compiler. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Register strict ISO converters on the generated ObjectMapper for OffsetDateTime, LocalDate and OffsetTime via the existing convert() infrastructure instead of requiring jackson-datatype-jsr310: the module's serializers don't round-trip faithfully (OffsetTime pads "23:20:50.52Z" to "23:20:50.520Z"), while the ISO formatters do, and this way generated code needs no extra dependency. Also fix union deserialization when several members are guarded by the same node type: date, time, date-time and enum values are all TextNode, so the previous output had duplicate 'is TextNode ->' branches of which only the first could ever match. Members sharing a guard now parse in a single branch that tries each member in sequence, strictest first (transformed strings, then enums, then plain strings). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The klaxon renderer's matchType calls were missing the transformedStringType matcher the base renderer maps to java.time types, so any schema with date/time formats in a union crashed generation with "Unsupported type date-time in non-exhaustive match". Add the matchers, register Klaxon converters that parse the java.time types from ISO strings and re-serialize them via the ISO formatters (exact round-trip), and give union members that share the 'is String' guard the same sequential-parse treatment as the Jackson renderer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…va.time
With date/time inference now mapping to java.time types, two groups of
misc inputs fail the round-trip comparison: files with non-RFC3339
date-times (space-separated, no offset) that OffsetDateTime.parse
rejects, and files whose fractional seconds have trailing zeros that
the ISO formatters legitimately trim (".000Z" -> "Z"). Skip them for
both Kotlin variants, mirroring what Go already does for the first
group.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TypeScript 7's JS API no longer provides ts.sys, which crashes ts-node
on startup ("Cannot read properties of undefined (reading 'fileExists')"),
breaking script/test, the typescript fixture, and the release helper.
tsx transforms with esbuild and doesn't use the compiler API, so it is
independent of the installed TypeScript version. The typescript-zod and
typescript-effect-schema fixtures already run via tsx.
Since tsx doesn't type-check, the typescript fixture gets an explicit
"tsc -p ." compile step so type errors in generated code still fail the
fixture; its tsconfig now lists main.ts explicitly (TS 7 errors on
CLI files when a tsconfig is present, and "files" never supported the
"*.ts" glob anyway).
script/test no longer passes --tsconfig: tsx propagates it to child
processes via TSX_TSCONFIG_PATH, which broke the fixture's "tsx main.ts"
in the run directory - the same problem the old TS_NODE_PROJECT= unset
in the run command worked around, so that workaround is gone too.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Under tsx, cluster workers calling the "exit" package's stdio-draining exit never terminate; the primary later SIGTERMs them, the exit event reports a null code, and fixture failures were swallowed (the harness exited 0 on failing fixtures with CPUs > 1). process.exit propagates failures correctly; verified by injecting a failing compile step. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
TypeScript dependency cleanups: latest compiler, devDeps, exact input pin, dead artifact removal
The default DateTimeRecognizer, vendored from ajv, accepted two forms
that RFC 3339 does not: a space as the date/time separator, and times
without a timezone offset. As a result, JSON samples containing strings
like "2013-06-15 21:10:28" were inferred as date-times, and languages
with strict date parsers (Go, Swift, java.time, ...) generated code
that could not parse the very samples the types were inferred from.
Several languages carried test-skip lists to paper over this.
The separator must now be "T" (or "t"), and times must carry an offset
("Z"/"z" or "±hh:mm"), for both isDateTime and standalone isTime
(RFC 3339 full-time requires the offset). Non-conforming strings are
inferred as plain strings, which every target handles faithfully.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins the strict RFC 3339 behavior: rejects space separators and missing timezone offsets, accepts lowercase t/z and fractional seconds, and validates date/time field ranges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Now that non-RFC 3339 strings ("2013-06-15 21:10:28") are no longer
inferred as date-times, the fixtures that contain them work for
languages with strict date parsers:
- golang: all 8 skipJSON entries round-trip (verified locally with go)
- swift: the 8 "date-time issues" skipDiffViaSchema entries pass -- the
divergence between SwiftDateTimeRecognizer (direct path) and the
lenient default (schema path) is gone for these files
- typescript/javascript/flow/typescript-zod: 7681c.json contains only
space-separated date-times, so no field is Date-typed anymore and the
"year 0 is out of range" runtime failure cannot occur (verified by
round-tripping the typescript, javascript, and zod fixtures locally;
flow left to CI)
Also removes entries that an audit showed were already stale
(passing even before this change): cplusplus and elm space-class
skipDiffViaSchema entries and csharp's 437e7.json (both variants).
Kept deliberately: python/rust skipDiffViaSchema entries (they still
diff, due to schema-path type naming, not date-times), all skips for
RFC 3339-valid files with fractional-second normalization issues
(0a358, 32d5c, 54d32, 5eae5, 77392, 80aff, 9ac3b, a0496, b4865, d23d5,
337ed), and the zod/effect-schema naming-related lists.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ence Infer date-times only for strict RFC 3339 strings
…erence With the default recognizer now requiring RFC 3339 (T separator and timezone offset), space-separated timestamps like "2013-06-15 21:10:28" are inferred as plain strings, so these eight misc fixtures round-trip for both kotlin variants without skips (verified per file; f6a65.json also verified end-to-end with kotlinc). The nine remaining skips are unchanged: their values are RFC 3339-valid but carry trailing-zero fractional seconds (e.g. ".000Z") that java.time does not re-serialize identically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Author
|
Simplified after upstream glideapps#2916 (strict RFC 3339 date-time inference) merged: the 8 space-separated-timestamp skips are gone — those fixtures now pass on merit — leaving only the 9 trailing-zero-fraction skips. The branch also now includes current upstream master (conflict-free merge). |
schani
added a commit
that referenced
this pull request
Jul 19, 2026
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 glideapps#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>
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.
This PR fixes the CI failures on glideapps/quicktype#2845. It targets your
masterbranch, so merging it here will update the upstream PR and should turn its kotlin CI jobs green. If you'd rather cherry-pick, the commits are self-contained; or tell us on the upstream PR and we can push directly.The three defects fixed
1. Klaxon: generation-time crash (currently masked in CI)
quicktype --lang kotlin --src-lang schema test/inputs/schema/date-time.schemacrashes withInternal error: Unsupported type date-time in non-exhaustive match— the twomatchTypecalls inKotlinKlaxonRenderer.tswere missing thetransformedStringTypematcher that the base and Jackson renderers got. CI doesn't show this yet because the harness aborts on the Jackson failure first.Fix: added the matchers, mapping date/time/date-time to the same
java.timetypes as the base renderer, plus Klaxon converters that parse/format strict ISO strings.2. Jackson:
java.timedeserialization fails at runtimeCI error on
date-time.1.json:The generated mapper never registered anything that can handle
java.timetypes.Why not
JavaTimeModule/jsr310: we implemented that first (including addingjackson-datatype-jsr310-2.9.7.jarto the test fixture) and it fails round-trip verification — jsr310'sOffsetTimeserializer pads fractional seconds ("23:20:50.52Z"→"23:20:50.520Z"), so the harness's byte-wise JSON comparison breaks. Instead, the generated code now registers strict ISO converters through its existingconvert()infrastructure (OffsetDateTime.parse(...)/DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(...), etc.). Bonus: generated code needs no extra dependency for end users, and the test fixture needs no new jar.3. Both renderers: duplicate union type-guards (first branch always wins)
Union deserialization emitted e.g.
so
date-time.schema's union arrays could never parse dates/times even with working converters. Union members sharing a guard now parse in one branch via a strictest-first try/catch cascade (transformed strings → enums → plain strings), in both Jackson and Klaxon output.Fixture skips
9 misc JSON fixtures are added to
skipJSONfor both kotlin variants: their values are RFC 3339-valid but carry trailing-zero fractional seconds (".000Z"), whichjava.timeISO formatting legitimately trims, breaking byte-wise round-trip.(An earlier revision of this branch also skipped 8 fixtures with space-separated non-RFC3339 date-times like
"2013-03-21 03:59:02". Upstream glideapps/quicktype#2916 has since made default date-time inference strictly RFC 3339, so those values are now inferred as plain strings and the fixtures pass without skips — verified per file, and end-to-end with kotlinc for one of them.)Branch freshness
The branch now includes current upstream
master(through glideapps#2916), which the upstream PR needs anyway; the merge was conflict-free.Verification
Locally replicated the CI harness (kotlinc 2.0.21, the fixture jars, and the same build/run/
jq -Sdiff steps):date-time.schema+date-time.1.json: round-trips byte-identical for both klaxon and jacksondate-time.1.fail.date-time.json: correctly rejected by both (DateTimeParseException)pokedexoutput: byte-identical before/after (no regression for non-date code)🤖 Generated with Claude Code