From 84f4ec863c587fd0f39d1fbed101b8fb0ac69a9a Mon Sep 17 00:00:00 2001 From: translatenix <119817707+translatenix@users.noreply.github.com> Date: Sat, 19 Oct 2024 20:32:21 -0700 Subject: [PATCH] codegen-kotlin: Fix generation of copy() methods (#705) - don't generate copy methods for abstract classes - precisely determine if copy method needs override modifier (tricky) Fixes #569 --- .../pkl/codegen/kotlin/KotlinCodeGenerator.kt | 13 +- .../codegen/kotlin/KotlinCodeGeneratorTest.kt | 205 ++++++++++++++++++ 2 files changed, 217 insertions(+), 1 deletion(-) diff --git a/pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/KotlinCodeGenerator.kt b/pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/KotlinCodeGenerator.kt index f1533331..e259f9d8 100644 --- a/pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/KotlinCodeGenerator.kt +++ b/pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/KotlinCodeGenerator.kt @@ -274,9 +274,20 @@ class KotlinCodeGenerator( return methodBuilder.addCode(codeBuilder.build()).build() } + fun inheritsCopyMethodWithSameArity(): Boolean { + val nearestNonAbstractAncestor = + generateSequence(pClass.superclass) { it.superclass }.firstOrNull { !it.isAbstract } + ?: return false + return nearestNonAbstractAncestor.allProperties.values.count { !it.isHidden } == + allProperties.size + } + // besides generating copy method for current class, // override copy methods inherited from parent classes fun generateCopyMethods(typeBuilder: TypeSpec.Builder) { + // copy methods don't make sense for abstract classes + if (pClass.isAbstract) return + var prevParameterCount = Int.MAX_VALUE for (currClass in generateSequence(pClass) { it.superclass }) { if (currClass.isAbstract) continue @@ -285,7 +296,7 @@ class KotlinCodeGenerator( // avoid generating multiple methods with same no. of parameters if (currParameters.size < prevParameterCount) { - val isOverride = currClass !== pClass || superclass != null && properties.isEmpty() + val isOverride = currClass !== pClass || inheritsCopyMethodWithSameArity() typeBuilder.addFunction(generateCopyMethod(currParameters, isOverride)) prevParameterCount = currParameters.size } diff --git a/pkl-codegen-kotlin/src/test/kotlin/org/pkl/codegen/kotlin/KotlinCodeGeneratorTest.kt b/pkl-codegen-kotlin/src/test/kotlin/org/pkl/codegen/kotlin/KotlinCodeGeneratorTest.kt index e3ad786f..b53a1971 100644 --- a/pkl-codegen-kotlin/src/test/kotlin/org/pkl/codegen/kotlin/KotlinCodeGeneratorTest.kt +++ b/pkl-codegen-kotlin/src/test/kotlin/org/pkl/codegen/kotlin/KotlinCodeGeneratorTest.kt @@ -661,6 +661,211 @@ class KotlinCodeGeneratorTest { assertCompilesSuccessfully(kotlinCode) } + // https://github.com/apple/pkl/issues/569 + @Test + fun `abstract classes`() { + val kotlinCode = + generateKotlinCode( + """ + module my.mod + + abstract class Foo { one: Int } + abstract class Bar extends Foo { two: String } + class Baz extends Bar { three: Duration } + class Qux extends Bar {} + """ + ) + + assertContains( + """ + | abstract class Foo( + | open val one: Long + | ) { + """ + .trimMargin(), + kotlinCode + ) + + assertContains( + """ + | abstract class Bar( + | one: Long, + | open val two: String + | ) : Foo(one) { + """ + .trimMargin(), + kotlinCode + ) + + assertContains( + """ + | class Baz( + | one: Long, + | two: String, + | val three: Duration + | ) : Bar(one, two) { + | fun copy( + | one: Long = this.one, + | two: String = this.two, + | three: Duration = this.three + | ): Baz = Baz(one, two, three) + """ + .trimMargin(), + kotlinCode + ) + + assertContains( + """ + | class Qux( + | one: Long, + | two: String + | ) : Bar(one, two) { + | fun copy(one: Long = this.one, two: String = this.two): Qux = Qux(one, two) + """ + .trimMargin(), + kotlinCode + ) + + assertCompilesSuccessfully(kotlinCode) + } + + // https://github.com/apple/pkl/issues/569 + @Test + fun `abstract class that extends open class`() { + val kotlinCode = + generateKotlinCode( + """ + module my.mod + + open class Foo { one: Int } + abstract class Bar extends Foo { two: String } + class Baz extends Bar { three: Duration } + class Qux extends Bar {} + """ + ) + + assertContains( + """ + | open class Foo( + | open val one: Long + | ) { + | open fun copy(one: Long = this.one): Foo = Foo(one) + """ + .trimMargin(), + kotlinCode + ) + + assertContains( + """ + | abstract class Bar( + | one: Long, + | open val two: String + | ) : Foo(one) { + """ + .trimMargin(), + kotlinCode + ) + + assertContains( + """ + | class Baz( + | one: Long, + | two: String, + | val three: Duration + | ) : Bar(one, two) { + | fun copy( + | one: Long = this.one, + | two: String = this.two, + | three: Duration = this.three + | ): Baz = Baz(one, two, three) + | + | override fun copy(one: Long): Baz = Baz(one, two, three) + """ + .trimMargin(), + kotlinCode + ) + + assertContains( + """ + | class Qux( + | one: Long, + | two: String + | ) : Bar(one, two) { + | fun copy(one: Long = this.one, two: String = this.two): Qux = Qux(one, two) + | + | override fun copy(one: Long): Qux = Qux(one, two) + """ + .trimMargin(), + kotlinCode + ) + + assertCompilesSuccessfully(kotlinCode) + } + + // https://github.com/apple/pkl/issues/569 + @Test + fun `abstract class that extends open class without adding properties`() { + val kotlinCode = + generateKotlinCode( + """ + module my.mod + + open class Foo { one: Int } + abstract class Bar extends Foo {} + class Baz extends Bar { two: Duration } + class Qux extends Bar {} + """ + ) + + assertContains( + """ + | open class Foo( + | open val one: Long + | ) { + | open fun copy(one: Long = this.one): Foo = Foo(one) + """ + .trimMargin(), + kotlinCode + ) + + assertContains( + """ + | abstract class Bar( + | one: Long + | ) : Foo(one) { + """ + .trimMargin(), + kotlinCode + ) + + assertContains( + """ + | class Baz( + | one: Long, + | val two: Duration + | ) : Bar(one) { + | fun copy(one: Long = this.one, two: Duration = this.two): Baz = Baz(one, two) + | + | override fun copy(one: Long): Baz = Baz(one, two) + """ + .trimMargin(), + kotlinCode + ) + + assertContains( + """ + | class Qux( + | one: Long + | ) : Bar(one) { + | override fun copy(one: Long): Qux = Qux(one) + """ + .trimMargin(), + kotlinCode + ) + + assertCompilesSuccessfully(kotlinCode) + } + @Test fun keywords() { val props = kotlinKeywords.joinToString("\n") { "`$it`: Int" }