Skip to content

Commit 753143d

Browse files
dmealingclaude
dmealing
and
claude
committed
fix(codegen-kotlin): collision-scoped payload class naming (ADR-0044, #219 stage 3b, #220)
The Kotlin payload generator named every nested payload data class after the VO's bare short name and wrote it via KotlinPoet `FileSpec(outPkg, <Short>Payload)`. Dedupe is FQN-keyed (target.name is the FQN), so two `object.value` `Note`s in different packages both reached emission — but both were named `NotePayload` and written to the same path, so the second silently CLOBBERED the first (last-wins), binding the wrong shape. Ports ADR-0044's collision-scoped naming (mirroring the TS/C#/Python/Java reference), adapted for Kotlin's one-class-per-file emit — the collision domain is the OUTPUT (prompts) PACKAGE. `computePayloadNameMap` walks each template's nested-payload closure (`collectNestedClosure` / `nestedTargetOf`, the same two edge types the emitter resolves), assigns each nested VO to its output package, then per (package, short-name) group emits `<Short>Payload` when unique (byte-identical to before) or every colliding member as `<PkgSegmentsPascal><Short>Payload` (`AcmeAlphaNotePayload`). A still-colliding derived name fails loud with ERR_PAYLOAD_NAME_COLLISION. The map threads through the emit chain and drives the class name (declaration, KotlinPoet file, and every reference). Upgrades the xpkg-collision conformance test (#220) from "a render-helper file exists" to running the PAYLOAD generator, asserting two DISTINCT package-qualified classes (never a clobbered NotePayload.kt), asserting each carries its own field, and COMPILING the generated output — the stronger gate the ADR asks for. Non- colliding output byte-identical: full codegen-kotlin suite green (295). This lands the Kotlin half of ADR-0044 stage 3. The ERR_PAYLOAD_NAME_COLLISION shared-ledger promotion and the coordinated PyPI + Maven release follow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeGSV3StPCcJGZNNJ4ZfAb
1 parent 8760128 commit 753143d

2 files changed

Lines changed: 187 additions & 11 deletions

File tree

‎server/java/codegen-kotlin/src/main/kotlin/com/metaobjects/generator/kotlin/KotlinPayloadGenerator.kt‎

Lines changed: 162 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package com.metaobjects.generator.kotlin
33
import com.metaobjects.field.EnumField
44
import com.metaobjects.field.MetaField
55
import com.metaobjects.field.ObjectField
6+
import com.metaobjects.generator.GeneratorException
67
import com.metaobjects.generator.GeneratorIOWriter
78
import com.metaobjects.generator.direct.MultiFileDirectGeneratorBase
89
import com.metaobjects.loader.MetaDataLoader
@@ -59,6 +60,14 @@ import java.nio.file.Paths
5960
*/
6061
open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
6162

63+
companion object {
64+
// ADR-0044 backstop error code — a codegen-time (not loader) error, peer of the
65+
// render tier's ERR_VAR_NOT_ON_PAYLOAD. Declared LOCALLY (as in the TS/C#/Python/
66+
// Java ports) rather than in the shared cross-port ledger; promoted to the ledger
67+
// once the coordinated follow-up lands, so no port reddens on a code it doesn't emit.
68+
const val ERR_PAYLOAD_NAME_COLLISION = "ERR_PAYLOAD_NAME_COLLISION"
69+
}
70+
6271
override fun getFilterClass(): Class<MetaObject> = MetaObject::class.java
6372

6473
override fun execute(loader: MetaDataLoader) {
@@ -73,9 +82,142 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
7382
// abstract enum super collapse onto ONE emitted file.
7483
val emittedEnumFqns = mutableSetOf<String>()
7584
// ADR-0039: root-scan discipline — resolving children accessor.
76-
for (md in loader.root.getChildren(MetaTemplate::class.java, true)) {
77-
emit(md, loader, outRoot, emittedNestedFqns, emittedEnumFqns)
85+
val templates = loader.root.getChildren(MetaTemplate::class.java, true)
86+
.sortedBy { it.name }
87+
// ADR-0044 — collision-scoped nested-payload class names, a pure function of the
88+
// loaded templates (order-independent). Keyed by VO FQN.
89+
val nameMap = computePayloadNameMap(templates, loader)
90+
for (md in templates) {
91+
emit(md, loader, outRoot, emittedNestedFqns, emittedEnumFqns, nameMap)
92+
}
93+
}
94+
95+
/**
96+
* ADR-0044 pass 1/2 — the run's nested-payload name map, keyed by value-object FQN
97+
* (`MetaObject.name`), scoped per OUTPUT PACKAGE. Kotlin is a one-class-per-file
98+
* emitter (KotlinPoet `FileSpec(outPkg, className)`), so its collision domain is the
99+
* output prompts package: two value-objects sharing a bare short name written into
100+
* the same package would clobber one `NotePayload.kt`. A nested VO whose bare short
101+
* name is UNIQUE in its output package emits `<Short>Payload` (byte-identical to
102+
* pre-ADR-0044 output); a COLLISION emits every member under its package-qualified
103+
* derived name (`acme::alpha::Note` -> `AcmeAlphaNotePayload`). A still-colliding
104+
* derived name fails loud with [ERR_PAYLOAD_NAME_COLLISION]. Pure function of the
105+
* templates — never of emission order.
106+
*/
107+
protected open fun computePayloadNameMap(
108+
templates: List<MetaTemplate>,
109+
loader: MetaDataLoader,
110+
): Map<String, String> {
111+
// FQN -> output package (first reaching template in sorted order wins, matching
112+
// the run-wide dedupe). The primary VO is template-named, so excluded.
113+
val voOutPkg = LinkedHashMap<String, String>()
114+
val orderedFqns = ArrayList<String>()
115+
for (tmpl in templates) {
116+
val payloadRef = tmpl.payloadRef ?: continue
117+
val vo = resolveViewObject(loader, payloadRef) ?: continue
118+
val nestedPkg = KotlinNaming.promptsPackage(PackageMapping.splitFqn(tmpl.name).first)
119+
collectNestedClosure(vo, loader, nestedPkg, voOutPkg, orderedFqns, mutableSetOf(vo.name))
120+
}
121+
// Group by (output package, bare short name).
122+
val byPkgShort = LinkedHashMap<String, MutableList<String>>()
123+
for (fqn in orderedFqns) {
124+
val key = voOutPkg[fqn] + " " + PackageMapping.splitFqn(fqn).second
125+
byPkgShort.getOrPut(key) { ArrayList() }.add(fqn)
126+
}
127+
val nameMap = LinkedHashMap<String, String>()
128+
for (fqns in byPkgShort.values) {
129+
if (fqns.size == 1) {
130+
val fqn = fqns[0]
131+
nameMap[fqn] = KotlinNaming.payloadName(PackageMapping.splitFqn(fqn).second)
132+
} else {
133+
for (fqn in fqns) {
134+
val (pkg, short) = PackageMapping.splitFqn(fqn)
135+
nameMap[fqn] = KotlinNaming.payloadName(packageQualifiedName(pkg, short))
136+
}
137+
}
138+
}
139+
// Backstop — per output package, two DISTINCT FQNs deriving the same class name.
140+
// Sorted so the named pair (and whether any collision fires) is order-independent.
141+
val ownerByPkgName = HashMap<String, String>()
142+
for (fqn in nameMap.keys.sorted()) {
143+
val pkgName = voOutPkg[fqn] + " " + nameMap[fqn]
144+
val prev = ownerByPkgName.putIfAbsent(pkgName, fqn)
145+
if (prev != null && prev != fqn) {
146+
throw GeneratorException(
147+
"$ERR_PAYLOAD_NAME_COLLISION: payload record name collision: \"${nameMap[fqn]}\" " +
148+
"derives from both \"$prev\" and \"$fqn\" — rename one value-object or move " +
149+
"it to a package that derives a distinct name"
150+
)
151+
}
152+
}
153+
return nameMap
154+
}
155+
156+
/**
157+
* ADR-0044 pass 1 — walk [vo]'s transitive nested-payload closure (plain
158+
* `field.object @objectRef` + `origin.collection @via` edges), assigning each
159+
* not-yet-seen target VO to [outPkg] (first reaching template wins) and recording it
160+
* in [orderedFqns]. [seen] is seeded with the primary VO's FQN and is the cycle guard.
161+
*/
162+
protected fun collectNestedClosure(
163+
vo: MetaObject,
164+
loader: MetaDataLoader,
165+
outPkg: String,
166+
voOutPkg: MutableMap<String, String>,
167+
orderedFqns: MutableList<String>,
168+
seen: MutableSet<String>,
169+
) {
170+
for (field in vo.metaFields) {
171+
val target = nestedTargetOf(field, loader) ?: continue
172+
val fqn = target.name
173+
if (!seen.add(fqn)) continue
174+
if (!voOutPkg.containsKey(fqn)) {
175+
voOutPkg[fqn] = outPkg
176+
orderedFqns.add(fqn)
177+
}
178+
collectNestedClosure(target, loader, outPkg, voOutPkg, orderedFqns, seen)
179+
}
180+
}
181+
182+
/**
183+
* The nested-payload target VO a [field] contributes to the closure, or `null` when
184+
* it contributes no nested class. Mirrors the resolution in [resolveObjectFieldType]
185+
* (plain `field.object @objectRef`) and [resolveCollectionType] (`origin.collection
186+
* @via`) EXACTLY, so the closure walk and the emission walk agree. Passthrough /
187+
* aggregate / computed / first origins yield scalar types (no nested class).
188+
*/
189+
protected fun nestedTargetOf(field: MetaField<*>, loader: MetaDataLoader): MetaObject? {
190+
val origin = field.children.filterIsInstance<MetaOrigin>().firstOrNull()
191+
if (origin is CollectionOrigin) {
192+
val via = origin.via ?: return null
193+
val (parentName, relName) = KotlinGenUtil.splitDottedRef(via) ?: return null
194+
val parent = KotlinGenUtil.resolveObjectByShortOrFqn(loader, parentName) ?: return null
195+
val rel = parent.relationships
196+
.firstOrNull { it.name == relName || it.name.substringAfterLast("::") == relName }
197+
?: return null
198+
val targetRef = rel.objectRef ?: return null
199+
return KotlinGenUtil.resolveObjectByShortOrFqn(loader, targetRef)
200+
}
201+
if (origin != null) return null // passthrough / aggregate / computed / first -> scalar
202+
if (field is ObjectField) {
203+
val target = try { field.objectRef } catch (e: RuntimeException) { null } ?: return null
204+
if (target.subType != MetaObject.SUBTYPE_VALUE) return null
205+
return target
78206
}
207+
return null
208+
}
209+
210+
/**
211+
* ADR-0044 — PascalCase each dotted segment of [kotlinPkg] (already `::`->`.`
212+
* converted by [PackageMapping.splitFqn]), concatenate, append the bare [shortName]
213+
* (`"acme.alpha"` + `"Note"` -> `"AcmeAlphaNote"`). A root-level (empty-package) node
214+
* keeps its bare short name.
215+
*/
216+
protected fun packageQualifiedName(kotlinPkg: String, shortName: String): String {
217+
if (kotlinPkg.isEmpty()) return shortName
218+
return kotlinPkg.split(".")
219+
.filter { it.isNotEmpty() }
220+
.joinToString("") { it.replaceFirstChar { c -> c.uppercaseChar() } } + shortName
79221
}
80222

81223
protected open fun emit(
@@ -84,6 +226,7 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
84226
outRoot: Path,
85227
emittedNestedFqns: MutableSet<String>,
86228
emittedEnumFqns: MutableSet<String>,
229+
nameMap: Map<String, String>,
87230
) {
88231
val payloadRef = template.payloadRef ?: return
89232
val payloadVo = resolveViewObject(loader, payloadRef) ?: return
@@ -101,6 +244,7 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
101244
outRoot = outRoot,
102245
emittedNestedFqns = emittedNestedFqns,
103246
emittedEnumFqns = emittedEnumFqns,
247+
nameMap = nameMap,
104248
)
105249
}
106250

@@ -119,6 +263,7 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
119263
outRoot: Path,
120264
emittedNestedFqns: MutableSet<String>,
121265
emittedEnumFqns: MutableSet<String>,
266+
nameMap: Map<String, String>,
122267
) {
123268
val serializable = ClassName("kotlinx.serialization", "Serializable")
124269

@@ -129,7 +274,7 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
129274

130275
val ctorBuilder = FunSpec.constructorBuilder()
131276
for (field in voObject.metaFields) {
132-
val type = resolveFieldType(field, voObject, loader, outPkg, outRoot, emittedNestedFqns, emittedEnumFqns)
277+
val type = resolveFieldType(field, voObject, loader, outPkg, outRoot, emittedNestedFqns, emittedEnumFqns, nameMap)
133278
ctorBuilder.addParameter(ParameterSpec.builder(field.name, type).build())
134279
typeBuilder.addProperty(
135280
PropertySpec.builder(field.name, type).initializer(field.name).build()
@@ -156,6 +301,7 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
156301
outRoot: Path,
157302
emittedNestedFqns: MutableSet<String>,
158303
emittedEnumFqns: MutableSet<String>,
304+
nameMap: Map<String, String>,
159305
): TypeName {
160306
// ADR-0039/ADR-0029: origin.* NEVER inherits — a derived field's origin is
161307
// declared-here, so read OWN children (field.children), not resolving.
@@ -166,7 +312,7 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
166312
is PassthroughOrigin -> resolvePassthroughType(origin, loader, field)
167313
is AggregateOrigin -> resolveAggregateType(origin, loader, field)
168314
is CollectionOrigin -> resolveCollectionType(
169-
origin, loader, nestedPkg, outRoot, emittedNestedFqns, emittedEnumFqns, field
315+
origin, loader, nestedPkg, outRoot, emittedNestedFqns, emittedEnumFqns, field, nameMap
170316
)
171317
// #195 origin.computed: a row-level value; its type is the field's own declared
172318
// subType (validation pins the inferred root type == field subType). Conservative
@@ -200,7 +346,7 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
200346
// and return its type (single, or List<TargetPayload> when isArray). Mirrors the
201347
// Spring port's resolveObjectFieldType — needed so a nested-object payload compiles.
202348
if (field is ObjectField) {
203-
return resolveObjectFieldType(field, loader, nestedPkg, outRoot, emittedNestedFqns, emittedEnumFqns)
349+
return resolveObjectFieldType(field, loader, nestedPkg, outRoot, emittedNestedFqns, emittedEnumFqns, nameMap)
204350
}
205351

206352
// Scalar array (`isArray: true` on a non-object field): model as List<ElementType> in the
@@ -227,6 +373,7 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
227373
outRoot: Path,
228374
emittedNestedFqns: MutableSet<String>,
229375
emittedEnumFqns: MutableSet<String>,
376+
nameMap: Map<String, String>,
230377
): TypeName {
231378
val fallbackType = { KotlinTypeMapper.payloadTypeName(field) }
232379
val target = try {
@@ -236,7 +383,10 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
236383
} ?: return fallbackType()
237384
if (target.subType != MetaObject.SUBTYPE_VALUE) return fallbackType()
238385

239-
val nestedClassName = PackageMapping.splitFqn(target.name).second + "Payload"
386+
// ADR-0044 — class name (declaration, file, reference) from the collision-scoped
387+
// name map (bare when unique in the output package, package-qualified on collision).
388+
val nestedClassName = nameMap[target.name]
389+
?: (PackageMapping.splitFqn(target.name).second + "Payload")
240390
if (emittedNestedFqns.add(target.name)) {
241391
emitPayloadClass(
242392
outPkg = nestedPkg,
@@ -247,6 +397,7 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
247397
outRoot = outRoot,
248398
emittedNestedFqns = emittedNestedFqns,
249399
emittedEnumFqns = emittedEnumFqns,
400+
nameMap = nameMap,
250401
)
251402
}
252403

@@ -344,6 +495,7 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
344495
emittedNestedFqns: MutableSet<String>,
345496
emittedEnumFqns: MutableSet<String>,
346497
fallbackField: MetaField<*>,
498+
nameMap: Map<String, String>,
347499
): TypeName {
348500
val fallbackType = { KotlinTypeMapper.payloadTypeName(fallbackField) }
349501
val via = origin.via ?: return fallbackType()
@@ -356,7 +508,9 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
356508
?: return fallbackType()
357509
val targetRef = relationship.objectRef ?: return fallbackType()
358510
val target = KotlinGenUtil.resolveObjectByShortOrFqn(loader, targetRef) ?: return fallbackType()
359-
val nestedClassName = PackageMapping.splitFqn(target.name).second + "Payload"
511+
// ADR-0044 — collision-scoped class name (see resolveObjectFieldType).
512+
val nestedClassName = nameMap[target.name]
513+
?: (PackageMapping.splitFqn(target.name).second + "Payload")
360514

361515
if (emittedNestedFqns.add(target.name)) {
362516
emitPayloadClass(
@@ -368,6 +522,7 @@ open class KotlinPayloadGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
368522
outRoot = outRoot,
369523
emittedNestedFqns = emittedNestedFqns,
370524
emittedEnumFqns = emittedEnumFqns,
525+
nameMap = nameMap,
371526
)
372527
}
373528

‎server/java/codegen-kotlin/src/test/kotlin/com/metaobjects/generator/kotlin/KotlinRenderHelperConformanceTest.kt‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -225,14 +225,35 @@ class KotlinRenderHelperConformanceTest {
225225
))
226226
loader.register()
227227
// Must NOT throw: the FQN refs resolve to their own package's Note.
228+
KotlinPayloadGenerator().apply { setArgs(mapOf("outputDir" to outDir.toString())) }.execute(loader)
228229
KotlinRenderHelperGenerator().apply {
229230
setArgs(mapOf("outputDir" to outDir.toString(), "templateRoot" to templates.toString()))
230231
}.execute(loader)
232+
231233
val produced = Files.walk(outDir).filter { it.isRegularFile() }.toList()
232-
assertTrue(
233-
produced.any { it.fileName.toString() == "DigestDocRenderHelper.kt" },
234-
"DigestDocRenderHelper.kt must be generated; files=$produced",
235-
)
234+
val names = produced.map { it.fileName.toString() }.toSet()
235+
assertTrue("DigestDocRenderHelper.kt" in names,
236+
"DigestDocRenderHelper.kt must be generated; files=$produced")
237+
// ADR-0044 — the two colliding Notes emit as DISTINCT package-qualified data
238+
// classes, never one clobbered NotePayload.kt (the pre-fix bug wrote both to
239+
// the same path via KotlinPoet, last-wins, dropping the alpha shape).
240+
assertTrue("AcmeAlphaNotePayload.kt" in names, "expected AcmeAlphaNotePayload.kt; files=$names")
241+
assertTrue("AcmeBetaNotePayload.kt" in names, "expected AcmeBetaNotePayload.kt; files=$names")
242+
assertTrue("NotePayload.kt" !in names,
243+
"must NOT emit a clobbered bare NotePayload.kt; files=$names")
244+
val alphaSrc = produced.first { it.fileName.toString() == "AcmeAlphaNotePayload.kt" }.readText()
245+
val betaSrc = produced.first { it.fileName.toString() == "AcmeBetaNotePayload.kt" }.readText()
246+
assertTrue("alphaText" in alphaSrc, "AcmeAlphaNotePayload must carry alphaText")
247+
assertTrue("betaText" in betaSrc, "AcmeBetaNotePayload must carry betaText")
248+
val digestSrc = produced.first { it.fileName.toString() == "DigestDocPayload.kt" }.readText()
249+
assertTrue("AcmeAlphaNotePayload" in digestSrc,
250+
"DigestDocPayload.fromAlpha must type AcmeAlphaNotePayload; saw:\n$digestSrc")
251+
assertTrue("AcmeBetaNotePayload" in digestSrc,
252+
"DigestDocPayload.fromBeta must type AcmeBetaNotePayload; saw:\n$digestSrc")
253+
// The generated output COMPILES — proves the two distinct classes are real,
254+
// valid Kotlin (the prior test could only assert a file existed).
255+
val result = compile(outDir)
256+
assertEquals(KotlinCompilation.ExitCode.OK, result.exitCode, result.messages)
236257
} finally {
237258
outDir.toFile().deleteRecursively()
238259
}

0 commit comments

Comments
 (0)