Kotlin/Klaxon: fix deserialization of maps with empty-object values

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<String, JsonObject>` 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<Type>,

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
     }

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

test/unit/kotlin-klaxon-empty-object-map.test.ts

@@ -0,0 +1,82 @@
+// Maps of empty objects render as `Map<String, X>` 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<string> {
+    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<String, Reward>",
+    );
+});
+
+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<String, Map<String, Outer>>",
+    );
+});
+
+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<String, Plain>`, which Klaxon deserializes
+    // fine on its own, so none of the workaround machinery should appear.
+    expect(output).toContain("val widgets: Map<String, Plain>");
+    expect(output).not.toContain("KlaxonJsonObjectMap");
+    expect(output).not.toContain("fieldConverter");
+});