From 80a0aec41bf96d0a2599fd96a87cabd062f13152 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Tue, 14 Jul 2026 13:48:54 -0400 Subject: [PATCH 1/2] test(kotlin): failing tests for Klaxon maps of empty objects (#2881) Maps of empty objects render as Map with typealias X = JsonObject, but Klaxon's reflective deserializer never consults custom converters for map values, so parsing fails with "Couldn't find a suitable constructor for class JsonObject to initialize with {}". Adds unit tests expecting such fields to be handled by a field-level converter, and re-enables the bug2037.json fixture for Kotlin/Klaxon that #2877 had to skip. The unit tests currently fail; the fix follows in the next commit. Co-Authored-By: Claude Fable 5 --- test/languages.ts | 2 - .../kotlin-klaxon-empty-object-map.test.ts | 84 +++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 test/unit/kotlin-klaxon-empty-object-map.test.ts diff --git a/test/languages.ts b/test/languages.ts index 5d971b30ca..517e5fc520 100644 --- a/test/languages.ts +++ b/test/languages.ts @@ -1141,8 +1141,6 @@ export const KotlinLanguage: Language = { "nst-test-suite.json", // Klaxon does not support top-level primitives "no-classes.json", - // Klaxon cannot deserialize empty object map values as JsonObject: #2881 - "bug2037.json", // These should be enabled "nbl-stats.json", // TODO Investigate these diff --git a/test/unit/kotlin-klaxon-empty-object-map.test.ts b/test/unit/kotlin-klaxon-empty-object-map.test.ts new file mode 100644 index 0000000000..0b4f42a01c --- /dev/null +++ b/test/unit/kotlin-klaxon-empty-object-map.test.ts @@ -0,0 +1,84 @@ +// Maps of empty objects render as `Map` with `typealias X = +// JsonObject` in Kotlin/Klaxon. Klaxon's reflective deserializer never +// consults custom converters for map values, so it tries to construct the +// JsonObject values via reflection and fails with "Couldn't find a suitable +// constructor for class JsonObject to initialize with {}". Fields holding +// such maps must instead be handled by a field-level converter, which Klaxon +// does consult (https://github.com/glideapps/quicktype/issues/2881). +import { expect, test } from "vitest"; + +import { + InputData, + jsonInputForTargetLanguage, + quicktype, +} from "../../packages/quicktype-core/src/index.js"; + +async function kotlinKlaxonForSamples( + samples: string[], +): Promise { + const jsonInput = jsonInputForTargetLanguage("kotlin"); + await jsonInput.addSource({ name: "TopLevel", samples }); + + const inputData = new InputData(); + inputData.addInput(jsonInput); + + const result = await quicktype({ + inputData, + lang: "kotlin", + rendererOptions: { framework: "klaxon" }, + }); + return result.lines.join("\n"); +} + +// The repro from issue #2881 (minimized from #2037's fixture): sibling maps +// where one has empty-object entries and the other is empty. +const bug2881 = JSON.stringify({ + mission_specs: { + "1": { objectives: { "2": { rewards: { "3": {}, "5": {} } } } }, + "4": { objectives: { "6": { rewards: {} } } }, + }, +}); + +test("map-of-empty-object fields get a Klaxon field converter", async () => { + const output = await kotlinKlaxonForSamples([bug2881]); + + // Empty objects still render as a JsonObject typealias. + expect(output).toContain("typealias Reward = JsonObject"); + + // The field-level converter must be declared and registered... + expect(output).toContain("@Target(AnnotationTarget.FIELD)"); + expect(output).toContain("private annotation class KlaxonJsonObjectMap"); + expect(output).toContain( + ".fieldConverter(KlaxonJsonObjectMap::class, jsonObjectMapConverter)", + ); + + // ...and the map-of-empty-object property must be annotated with it. + expect(output).toContain( + "@KlaxonJsonObjectMap\n val rewards: Map", + ); +}); + +test("maps nested inside maps of empty objects are annotated", async () => { + const nested = JSON.stringify({ + outer: { "1": { "2": {}, "3": {} }, "4": { "5": {} } }, + }); + const output = await kotlinKlaxonForSamples([nested]); + + expect(output).toContain( + "@KlaxonJsonObjectMap\n val outer: Map>", + ); +}); + +test("plain classes and maps of non-empty objects are not annotated", async () => { + const sample = JSON.stringify({ + plain: { x: 1 }, + widgets: { "1": { x: 1 }, "4": { x: 2 } }, + }); + const output = await kotlinKlaxonForSamples([sample]); + + // `widgets` renders as `Map`, which Klaxon deserializes + // fine on its own, so none of the workaround machinery should appear. + expect(output).toContain("val widgets: Map"); + expect(output).not.toContain("KlaxonJsonObjectMap"); + expect(output).not.toContain("fieldConverter"); +}); From 9fcf37c9ecba4f966237a7c7f357b02c89b7a6f5 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Tue, 14 Jul 2026 13:55:08 -0400 Subject: [PATCH 2/2] fix(kotlin): Klaxon field converter for maps of empty objects Empty object types render as `typealias X = JsonObject`, and a custom converter registered for JsonObject::class handles them. But Klaxon's DefaultConverter looks up converters for map values with `findConverterFromClass(typeValue.javaClass, ...)`, where `typeValue.javaClass` is `java.lang.Class` rather than the value type, so custom converters are never consulted for map values. Klaxon then falls back to constructing JsonObject reflectively and fails with "Couldn't find a suitable constructor for class JsonObject to initialize with {}". Klaxon does consult field-level converters, so properties whose type is a map (possibly nested in further maps) with empty-object values now get a generated `@KlaxonJsonObjectMap` field annotation, backed by a registered field converter that returns the parsed JsonObject (which already is a `Map` at runtime) as-is. Fixes #2881 Co-Authored-By: Claude Fable 5 --- .../language/Kotlin/KotlinKlaxonRenderer.ts | 93 ++++++++++++++++++- .../src/language/Kotlin/KotlinRenderer.ts | 9 +- .../kotlin-klaxon-empty-object-map.test.ts | 4 +- 3 files changed, 101 insertions(+), 5 deletions(-) diff --git a/packages/quicktype-core/src/language/Kotlin/KotlinKlaxonRenderer.ts b/packages/quicktype-core/src/language/Kotlin/KotlinKlaxonRenderer.ts index cb803f3e6d..ac46686358 100644 --- a/packages/quicktype-core/src/language/Kotlin/KotlinKlaxonRenderer.ts +++ b/packages/quicktype-core/src/language/Kotlin/KotlinKlaxonRenderer.ts @@ -6,9 +6,10 @@ import { camelCase } from "../../support/Strings.js"; import { mustNotHappen } from "../../support/Support.js"; import { type ArrayType, + type ClassProperty, ClassType, type EnumType, - type MapType, + MapType, type PrimitiveType, type Type, UnionType, @@ -77,6 +78,51 @@ export class KotlinKlaxonRenderer extends KotlinRenderer { ); } + // Empty object types render as a typealias for JsonObject. Klaxon's + // reflective deserializer never consults custom converters for map + // values, so a `Map` property fails to parse: + // https://github.com/glideapps/quicktype/issues/2881 + // Properties holding such maps (directly or nested inside other maps) + // are annotated so that a field-level converter — which Klaxon does + // consult — handles them instead. + private isEmptyObjectType(t: Type): boolean { + if (t instanceof UnionType) { + const nullable = nullableFromUnion(t); + return nullable !== null && this.isEmptyObjectType(nullable); + } + + return t instanceof ClassType && t.getProperties().size === 0; + } + + private needsJsonObjectMapAnnotation(t: Type): boolean { + if (t instanceof UnionType) { + const nullable = nullableFromUnion(t); + return ( + nullable !== null && this.needsJsonObjectMapAnnotation(nullable) + ); + } + + if (!(t instanceof MapType)) { + return false; + } + + return ( + this.isEmptyObjectType(t.values) || + this.needsJsonObjectMapAnnotation(t.values) + ); + } + + private hasJsonObjectMaps(): boolean { + return iterableSome( + this.typeGraph.allNamedTypes(), + (t) => + t instanceof ClassType && + iterableSome(t.getProperties().values(), (p) => + this.needsJsonObjectMapAnnotation(p.type), + ), + ); + } + protected emitUsageHeader(): void { this.emitLine("// To parse the JSON, install Klaxon and do:"); this.emitLine("//"); @@ -108,6 +154,11 @@ export class KotlinKlaxonRenderer extends KotlinRenderer { this.emitGenericConverter(); } + const hasJsonObjectMaps = this.hasJsonObjectMaps(); + if (hasJsonObjectMaps) { + this.emitJsonObjectMapConverter(); + } + const converters: Sourcelike[][] = []; if (hasEmptyObjects) { converters.push([ @@ -137,6 +188,14 @@ export class KotlinKlaxonRenderer extends KotlinRenderer { if (converters.length > 0) { this.indent(() => this.emitTable(converters)); } + + if (hasJsonObjectMaps) { + this.indent(() => + this.emitLine( + ".fieldConverter(KlaxonJsonObjectMap::class, jsonObjectMapConverter)", + ), + ); + } } protected emitTopLevelArray(t: ArrayType, name: Name): void { @@ -259,7 +318,12 @@ export class KotlinKlaxonRenderer extends KotlinRenderer { jsonName: string, _required: boolean, meta: Array<() => void>, + p: ClassProperty, ): void { + if (this.needsJsonObjectMapAnnotation(p.type)) { + meta.push(() => this.emitLine("@KlaxonJsonObjectMap")); + } + const rename = this.klaxonRenameAttribute(name, jsonName); if (rename !== undefined) { meta.push(() => this.emitLine(rename)); @@ -333,6 +397,33 @@ export class KotlinKlaxonRenderer extends KotlinRenderer { }); } + private emitJsonObjectMapConverter(): void { + this.ensureBlankLine(); + this.emitLine( + "// Klaxon cannot deserialize map values typed as JsonObject, so fields", + ); + this.emitLine( + "// holding such maps are converted at the field level instead.", + ); + this.emitLine("@Target(AnnotationTarget.FIELD)"); + this.emitLine("private annotation class KlaxonJsonObjectMap"); + this.ensureBlankLine(); + this.emitLine( + "private val jsonObjectMapConverter: Converter = object : Converter {", + ); + this.indent(() => { + this.emitTable([ + ["override fun canConvert(cls: Class<*>)", " = true"], + ["override fun fromJson(jv: JsonValue)", " = jv.obj!!"], + [ + "override fun toJson(value: Any)", + " = klaxon.toJsonString(value)", + ], + ]); + }); + this.emitLine("}"); + } + protected emitUnionDefinitionMethods( u: UnionType, nonNulls: ReadonlySet, diff --git a/packages/quicktype-core/src/language/Kotlin/KotlinRenderer.ts b/packages/quicktype-core/src/language/Kotlin/KotlinRenderer.ts index ff9c487347..ba1f7c843c 100644 --- a/packages/quicktype-core/src/language/Kotlin/KotlinRenderer.ts +++ b/packages/quicktype-core/src/language/Kotlin/KotlinRenderer.ts @@ -277,7 +277,13 @@ export class KotlinRenderer extends ConvenienceRenderer { meta.push(() => this.emitDescription(description)); } - this.renameAttribute(name, jsonName, !nullableOrOptional, meta); + this.renameAttribute( + name, + jsonName, + !nullableOrOptional, + meta, + p, + ); if (meta.length > 0 && !first) { this.ensureBlankLine(); @@ -323,6 +329,7 @@ export class KotlinRenderer extends ConvenienceRenderer { _jsonName: string, _required: boolean, _meta: Array<() => void>, + _p: ClassProperty, ): void { // to be overridden } diff --git a/test/unit/kotlin-klaxon-empty-object-map.test.ts b/test/unit/kotlin-klaxon-empty-object-map.test.ts index 0b4f42a01c..d9968367c9 100644 --- a/test/unit/kotlin-klaxon-empty-object-map.test.ts +++ b/test/unit/kotlin-klaxon-empty-object-map.test.ts @@ -13,9 +13,7 @@ import { quicktype, } from "../../packages/quicktype-core/src/index.js"; -async function kotlinKlaxonForSamples( - samples: string[], -): Promise { +async function kotlinKlaxonForSamples(samples: string[]): Promise { const jsonInput = jsonInputForTargetLanguage("kotlin"); await jsonInput.addSource({ name: "TopLevel", samples });