Skip to content

Fix kotlin date/time CI failures: klaxon crash, jackson round-trip, union guards#1

Merged
schani merged 22 commits into
nikhilunni:masterfrom
glideapps:kotlin-datetime-fixes-2845
Jul 19, 2026
Merged

Fix kotlin date/time CI failures: klaxon crash, jackson round-trip, union guards#1
schani merged 22 commits into
nikhilunni:masterfrom
glideapps:kotlin-datetime-fixes-2845

Conversation

@schani

@schani schani commented Jul 12, 2026

Copy link
Copy Markdown

This PR fixes the CI failures on glideapps/quicktype#2845. It targets your master branch, 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.schema crashes with Internal error: Unsupported type date-time in non-exhaustive match — the two matchType calls in KotlinKlaxonRenderer.ts were missing the transformedStringType matcher 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.time types as the base renderer, plus Klaxon converters that parse/format strict ISO strings.

2. Jackson: java.time deserialization fails at runtime

CI error on date-time.1.json:

InvalidDefinitionException: Cannot construct instance of `java.time.LocalDate`
(no Creators, like default construct, exist)

The generated mapper never registered anything that can handle java.time types.

Why not JavaTimeModule/jsr310: we implemented that first (including adding jackson-datatype-jsr310-2.9.7.jar to the test fixture) and it fails round-trip verification — jsr310's OffsetTime serializer 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 existing convert() 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.

when (jn) {
    is TextNode -> DateTimeValue(...)
    is TextNode -> DateValue(...)   // unreachable
    is TextNode -> TimeValue(...)   // unreachable
}

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 skipJSON for both kotlin variants: their values are RFC 3339-valid but carry trailing-zero fractional seconds (".000Z"), which java.time ISO 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 -S diff steps):

  • date-time.schema + date-time.1.json: round-trips byte-identical for both klaxon and jackson
  • date-time.1.fail.date-time.json: correctly rejected by both (DateTimeParseException)
  • pokedex output: byte-identical before/after (no regression for non-date code)
  • Full monorepo build clean; unit tests 59/59

🤖 Generated with Claude Code

bastian-huber-keen and others added 13 commits August 19, 2024 09:27
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>
schani and others added 9 commits July 12, 2026 18:00
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>
@schani

schani commented Jul 13, 2026

Copy link
Copy Markdown
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
schani merged commit c194694 into nikhilunni:master Jul 19, 2026
@schani
schani deleted the kotlin-datetime-fixes-2845 branch July 19, 2026 16:08
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants