Skip to content

Commit bc591b7

Browse files
dmealingclaude
andcommitted
feat(codegen-kotlin): a generated entity/value object is now constructible from Java
Kotlin default arguments are a COMPILER feature, not a bytecode one. A generated data class whose properties are all defaulted offered Java exactly three constructors — the full N-arg one, a synthetic bitmask one Java cannot call, and (because every parameter is defaulted) a no-arg one yielding an all-null instance of an IMMUTABLE class. Nothing in between, and no setters to fill in afterwards. So a Java caller setting 3 of 14 properties had to pass 14 arguments with 11 nulls. An adopter converting an untyped jsonb bag to an `object.value` per the authoring skill's ladder found the generated VO replacing a hand-written Lombok `@Builder` made four Java call sites strictly worse, and reverted the conversion. The ladder's promise is a better handle than the cast; here the handle was worse. Emits a nested `Builder` plus a `@JvmStatic builder()`: ItemEffect.builder().name("Restore HP").value(10).build(); A builder rather than `@JvmOverloads`: overloads are positional, so they only help a caller whose omissions are all TRAILING, and a 14-member class would emit 15 constructors to say so. Deliberately NOT Lombok — generated code must not force a third-party annotation processor on an adopter — so it is a plain nested class with no new dependency. Every backing field is nullable even where the property is not: a builder fills incrementally and cannot hold the constructor's non-null guarantee. `build()` restores it with `requireNotNull`, naming the property. The data-class constructor remains the real enforcement point. NOT on `object.projection`. A projection is a derived read-only representation — it arrives from a view query and nothing constructs one, so a builder would advertise something callers must not do. That exclusion was FOUND, not reasoned about: KotlinProjectionCompileTest requires projection data classes to be immutable and my `var` backing fields tripped it. The stored-proc fixture now demonstrates the split cleanly — `OrderReportArgs` (object.value, the input a caller builds) gets a builder; `OrderReport` (object.projection, the result) does not. The new test is the point of the change as much as the generator is: every other test in this module is Kotlin-side, where named arguments hide the problem completely, which is exactly why the Java-interop face of this output was free to be bad. KotlinValueObjectJavaConstructionTest compiles real JAVA against the generated class through kotlin-compile-testing and sets 2 of 5 properties. Verified by reverting the generator: `error: cannot find symbol`. 13 snapshots re-cut. The only removed line in that whole diff is `)` becoming `) {` — everything else is additive. No vocabulary change, so metamodelVersion does not move (`check-metamodel-version`: none change since v1.0.1). 373/373 in codegen-kotlin; java+kotlin conformance green; gates lane green. codegen-spring emits Java `record`s, which have the same all-args problem and no defaults at all — tracked in #365 as a follow-on, deliberately out of scope here. Closes #365 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NTcEKXTQMYt84fAjuw5A2M
1 parent a4e6f8c commit bc591b7

16 files changed

Lines changed: 774 additions & 14 deletions

File tree

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

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,8 +238,12 @@ open class KotlinEntityGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
238238
typeBuilder.addProperty(PropertySpec.builder(propName, propType).initializer(propName).build())
239239
}
240240

241+
val primaryCtor = ctorBuilder.build()
242+
typeBuilder.primaryConstructor(primaryCtor)
243+
addJavaBuilder(obj, typeBuilder, ClassName(pkg, shortName), primaryCtor.parameters)
244+
241245
val fileSpec = FileSpec.builder(pkg, shortName)
242-
.addType(typeBuilder.primaryConstructor(ctorBuilder.build()).build())
246+
.addType(typeBuilder.build())
243247
.build()
244248

245249
// Guarded: `<Entity>.kt` is the file an adopter is most likely to want to own, and a
@@ -726,4 +730,90 @@ open class KotlinEntityGenerator : MultiFileDirectGeneratorBase<MetaObject>() {
726730
PackageMapping.splitFqn(md.name).first.replace('.', '/')
727731
override fun getSingleOutputFilename(md: MetaObject): String =
728732
PackageMapping.splitFqn(md.name).second + ".kt"
733+
734+
/**
735+
* Emit a nested `Builder` plus a `@JvmStatic builder()` factory.
736+
*
737+
* Kotlin default arguments are a COMPILER feature, not a bytecode one. A data class whose
738+
* properties are all defaulted exposes only three constructors to Java — the full N-arg one,
739+
* a synthetic bitmask one Java cannot call, and (when every parameter is defaulted) a no-arg
740+
* one that yields an all-null instance of an IMMUTABLE class. So a Java caller setting 3 of
741+
* 14 fields had to pass 14 arguments with 11 nulls, and a generated value object replacing a
742+
* hand-written Lombok `@Builder` made its Java call sites strictly worse (#365). Kotlin
743+
* callers were never affected — named arguments cover it — which is why every existing test
744+
* here, all Kotlin-side, missed it.
745+
*
746+
* A nested builder rather than `@JvmOverloads`: overloads are positional, so they only help a
747+
* caller whose omissions are all TRAILING, and a 14-member class would emit 15 constructors
748+
* to say so. A builder needs no dependency (deliberately NOT Lombok — generated code must not
749+
* force a third-party annotation processor on an adopter) and reads the same from both
750+
* languages.
751+
*
752+
* Every backing field is nullable even where the property is not: a builder is filled
753+
* incrementally, so it cannot hold the constructor's non-null guarantee. `build()` restores it
754+
* with `requireNotNull`, naming the property — the same trade every builder makes, and the
755+
* data-class constructor remains the real enforcement point.
756+
*/
757+
private fun addJavaBuilder(
758+
obj: MetaObject,
759+
typeBuilder: TypeSpec.Builder,
760+
className: ClassName,
761+
params: List<ParameterSpec>,
762+
) {
763+
if (params.isEmpty()) return
764+
// NOT on an object.projection. A projection is a DERIVED read-only representation —
765+
// it arrives from a view query and nothing constructs one, so a builder would be an
766+
// affordance for something callers must not do. It would also put `var` backing fields
767+
// into a type KotlinProjectionCompileTest requires to be immutable, which is how this
768+
// exclusion was found rather than reasoned about.
769+
if (obj.subType == MetaObject.SUBTYPE_PROJECTION) return
770+
771+
val builderClass = ClassName(className.packageName, className.simpleName, "Builder")
772+
val builder = TypeSpec.classBuilder("Builder")
773+
.addKdoc("Fluent builder — lets a JAVA caller set a subset of properties.\n")
774+
775+
for (param in params) {
776+
val backingType = param.type.copy(nullable = true)
777+
builder.addProperty(
778+
PropertySpec.builder(param.name, backingType)
779+
.addModifiers(KModifier.PRIVATE)
780+
.mutable(true)
781+
.initializer("null")
782+
.build()
783+
)
784+
builder.addFunction(
785+
FunSpec.builder(param.name)
786+
.addParameter("v", backingType)
787+
.returns(builderClass)
788+
.addStatement("this.%N = v", param.name)
789+
.addStatement("return this")
790+
.build()
791+
)
792+
}
793+
794+
val args = params.joinToString(",\n ") { p ->
795+
if (p.type.isNullable) "%1N = %1N".replace("%1N", p.name)
796+
else "${p.name} = requireNotNull(${p.name}) { \"${p.name} is required\" }"
797+
}
798+
builder.addFunction(
799+
FunSpec.builder("build")
800+
.returns(className)
801+
.addStatement("return %T(\n $args\n)", className)
802+
.build()
803+
)
804+
805+
typeBuilder.addType(builder.build())
806+
typeBuilder.addType(
807+
TypeSpec.companionObjectBuilder()
808+
.addFunction(
809+
FunSpec.builder("builder")
810+
.addAnnotation(JvmStatic::class)
811+
.returns(builderClass)
812+
.addStatement("return Builder()")
813+
.build()
814+
)
815+
.build()
816+
)
817+
}
818+
729819
}

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

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,4 +313,98 @@ class KotlinEntityGeneratorTest {
313313
outDir.toFile().deleteRecursively()
314314
}
315315
}
316+
317+
@Test fun `emits a Java-callable builder for partial construction`() {
318+
// Kotlin default arguments are a COMPILER feature, not a bytecode one: a data class
319+
// whose properties are all defaulted exposes only a no-arg constructor and the full
320+
// N-arg one to Java, plus a synthetic bitmask ctor Java cannot call. The class is
321+
// immutable, so there are no setters to fill in afterwards either — a Java caller
322+
// setting 3 of 14 fields had to pass 14 arguments with 11 nulls (#365, found when a
323+
// generated value object replaced a hand-written Lombok @Builder and made four Java
324+
// call sites strictly worse). The builder restores partial construction WITHOUT
325+
// adding a dependency: a plain nested class, no Lombok.
326+
val outDir = Files.createTempDirectory("kgen-builder-")
327+
try {
328+
val gen = KotlinEntityGenerator()
329+
gen.setArgs(mapOf("outputDir" to outDir.toString()))
330+
gen.execute(loadString("test", fixture))
331+
332+
val src = Files.readString(outDir.resolve("acme/demo/Author.kt"))
333+
334+
assertTrue("public class Builder" in src, "expected a nested Builder in:\n$src")
335+
// @JvmStatic so Java writes Author.builder(), not Author.Companion.builder().
336+
assertTrue("@JvmStatic" in src, "expected @JvmStatic on builder() in:\n$src")
337+
assertTrue("public fun builder(): Builder" in src, "expected a builder() factory in:\n$src")
338+
// One fluent setter per property, returning Builder so calls chain.
339+
for (prop in listOf("id", "name", "bio")) {
340+
assertTrue("public fun $prop(" in src, "expected a fluent `$prop(...)` setter in:\n$src")
341+
}
342+
assertTrue("public fun build(): Author" in src, "expected build() in:\n$src")
343+
} finally {
344+
outDir.toFile().deleteRecursively()
345+
}
346+
}
347+
348+
@Test fun `the builder covers value objects, not just entities`() {
349+
// Same emit path serves object.entity and object.value, and the reported case was a
350+
// VALUE object — so assert the VO explicitly rather than relying on the shared path.
351+
val voFixture = """{
352+
"metadata.root": { "package": "acme::demo", "children": [
353+
{ "object.value": { "name": "Money", "children": [
354+
{ "field.string": { "name": "currency" } },
355+
{ "field.int": { "name": "amountMinor" } }
356+
] } }
357+
] }
358+
}""".trimIndent()
359+
val outDir = Files.createTempDirectory("kgen-vo-builder-")
360+
try {
361+
val gen = KotlinEntityGenerator()
362+
gen.setArgs(mapOf("outputDir" to outDir.toString()))
363+
gen.execute(loadString("test", voFixture))
364+
365+
val src = Files.readString(outDir.resolve("acme/demo/Money.kt"))
366+
assertTrue("public class Builder" in src, "expected a Builder on the value object in:\n$src")
367+
assertTrue("public fun build(): Money" in src, "expected build(): Money in:\n$src")
368+
assertTrue("public fun currency(" in src, "expected a fluent currency(...) setter in:\n$src")
369+
} finally {
370+
outDir.toFile().deleteRecursively()
371+
}
372+
}
373+
374+
375+
@Test fun `the builder restores the non-null guarantee it cannot hold`() {
376+
// A builder is filled incrementally, so its backing field must be nullable even where
377+
// the property is not. build() puts the guarantee back with requireNotNull, naming the
378+
// property — the data-class constructor stays the real enforcement point.
379+
val requiredFixture = """{
380+
"metadata.root": { "package": "acme::demo", "children": [
381+
{ "object.entity": { "name": "Ticket", "children": [
382+
{ "field.string": { "name": "code", "@required": true } },
383+
{ "field.string": { "name": "note" } }
384+
] } }
385+
] }
386+
}""".trimIndent()
387+
val outDir = Files.createTempDirectory("kgen-required-")
388+
try {
389+
val gen = KotlinEntityGenerator()
390+
gen.setArgs(mapOf("outputDir" to outDir.toString()))
391+
gen.execute(loadString("test", requiredFixture))
392+
393+
val src = Files.readString(outDir.resolve("acme/demo/Ticket.kt"))
394+
// the property itself is non-null...
395+
assertTrue("public val code: String," in src || "public val code: String\n" in src,
396+
"expected a NON-null `code` property in:\n$src")
397+
// ...its builder backing field is nullable, and build() re-asserts it
398+
assertTrue("private var code: String? = null" in src,
399+
"expected a nullable builder field for `code` in:\n$src")
400+
assertTrue("requireNotNull(code)" in src,
401+
"expected build() to requireNotNull(code) in:\n$src")
402+
// a nullable sibling passes straight through, unguarded
403+
assertFalse("requireNotNull(note)" in src,
404+
"a nullable property must NOT be guarded in:\n$src")
405+
} finally {
406+
outDir.toFile().deleteRecursively()
407+
}
408+
}
409+
316410
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package com.metaobjects.generator.kotlin
2+
3+
import com.metaobjects.metadata.ktx.loadString
4+
import com.tschuchort.compiletesting.KotlinCompilation
5+
import com.tschuchort.compiletesting.SourceFile
6+
import java.nio.file.Files
7+
import kotlin.io.path.isRegularFile
8+
import kotlin.io.path.readText
9+
import kotlin.test.Test
10+
import kotlin.test.assertEquals
11+
12+
/**
13+
* The generated shapes must be constructible FROM JAVA, not only from Kotlin (#365).
14+
*
15+
* Kotlin default arguments are a compiler feature, not a bytecode one, so a data class whose
16+
* properties are all defaulted offers Java the full N-arg constructor, a synthetic bitmask one
17+
* it cannot call, and a no-arg one yielding an all-null instance of an immutable class —
18+
* nothing in between. An adopter converting an untyped jsonb bag to an `object.value` found a
19+
* generated 14-member VO needed 14 arguments with 11 nulls where the hand-written class it
20+
* replaced had a Lombok builder.
21+
*
22+
* Every other test in this module is Kotlin-side, where named arguments hide the problem
23+
* completely. This one compiles real JAVA against the generated output, which is the only way
24+
* the guarantee stays honest.
25+
*/
26+
@OptIn(org.jetbrains.kotlin.compiler.plugin.ExperimentalCompilerApi::class)
27+
class KotlinValueObjectJavaConstructionTest {
28+
29+
private val fixture = """{
30+
"metadata.root": { "package": "acme::demo", "children": [
31+
{ "object.value": { "name": "ItemEffect", "children": [
32+
{ "field.string": { "name": "name" } },
33+
{ "field.string": { "name": "targetAttribute" } },
34+
{ "field.int": { "name": "value" } },
35+
{ "field.int": { "name": "duration" } },
36+
{ "field.string": { "name": "damageType" } }
37+
] } }
38+
] }
39+
}""".trimIndent()
40+
41+
@Test fun `java can construct a generated value object setting only some properties`() {
42+
val outDir = Files.createTempDirectory("kgen-java-ctor-")
43+
try {
44+
val gen = KotlinEntityGenerator()
45+
gen.setArgs(mapOf("outputDir" to outDir.toString()))
46+
gen.execute(loadString("test", fixture))
47+
48+
val generated = Files.walk(outDir).filter { it.isRegularFile() }.toList()
49+
.map { SourceFile.kotlin(it.fileName.toString(), it.readText()) }
50+
51+
// Sets 2 of 5 properties. Without a builder this cannot be written at all: the
52+
// class is immutable, so there is no no-arg-then-set path, and the only other
53+
// constructor takes every argument.
54+
val caller = SourceFile.java(
55+
"Caller.java",
56+
"""
57+
package acme.demo;
58+
59+
public class Caller {
60+
public static ItemEffect partial() {
61+
return ItemEffect.builder()
62+
.name("Restore HP")
63+
.value(10)
64+
.build();
65+
}
66+
}
67+
""".trimIndent(),
68+
)
69+
70+
val result = KotlinCompilation().apply {
71+
sources = generated + caller
72+
inheritClassPath = true
73+
messageOutputStream = System.out
74+
}.compile()
75+
76+
assertEquals(KotlinCompilation.ExitCode.OK, result.exitCode,
77+
"Java could not construct the generated value object:\n${result.messages}")
78+
} finally {
79+
outDir.toFile().deleteRecursively()
80+
}
81+
}
82+
}

server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-bidirectional-fk/acme/blog/Author.kt

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package acme.blog
33
import jakarta.validation.constraints.Size
44
import kotlin.Long
55
import kotlin.String
6+
import kotlin.jvm.JvmStatic
67

78
/**
89
* GENERATED — do not hand-edit. Regenerated from metadata.
@@ -11,4 +12,33 @@ public data class Author(
1112
public val id: Long? = null,
1213
@field:Size(max = 100)
1314
public val name: String? = null,
14-
)
15+
) {
16+
/**
17+
* Fluent builder — lets a JAVA caller set a subset of properties.
18+
*/
19+
public class Builder {
20+
private var id: Long? = null
21+
22+
private var name: String? = null
23+
24+
public fun id(v: Long?): Builder {
25+
this.id = v
26+
return this
27+
}
28+
29+
public fun name(v: String?): Builder {
30+
this.name = v
31+
return this
32+
}
33+
34+
public fun build(): Author = Author(
35+
id = id,
36+
name = name
37+
)
38+
}
39+
40+
public companion object {
41+
@JvmStatic
42+
public fun builder(): Builder = Builder()
43+
}
44+
}

server/java/codegen-kotlin/src/test/resources/snapshots/entity-with-bidirectional-fk/acme/blog/Post.kt

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package acme.blog
33
import jakarta.validation.constraints.Size
44
import kotlin.Long
55
import kotlin.String
6+
import kotlin.jvm.JvmStatic
67

78
/**
89
* GENERATED — do not hand-edit. Regenerated from metadata.
@@ -11,4 +12,33 @@ public data class Post(
1112
public val id: Long? = null,
1213
@field:Size(max = 200)
1314
public val title: String? = null,
14-
)
15+
) {
16+
/**
17+
* Fluent builder — lets a JAVA caller set a subset of properties.
18+
*/
19+
public class Builder {
20+
private var id: Long? = null
21+
22+
private var title: String? = null
23+
24+
public fun id(v: Long?): Builder {
25+
this.id = v
26+
return this
27+
}
28+
29+
public fun title(v: String?): Builder {
30+
this.title = v
31+
return this
32+
}
33+
34+
public fun build(): Post = Post(
35+
id = id,
36+
title = title
37+
)
38+
}
39+
40+
public companion object {
41+
@JvmStatic
42+
public fun builder(): Builder = Builder()
43+
}
44+
}

0 commit comments

Comments
 (0)