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/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..d9968367c9 --- /dev/null +++ b/test/unit/kotlin-klaxon-empty-object-map.test.ts @@ -0,0 +1,82 @@ +// 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"); +});