From d5b358ea29ce460ba88560b1e54f5356bac8168b Mon Sep 17 00:00:00 2001 From: worstell Date: Wed, 17 Jan 2024 14:22:57 -0500 Subject: [PATCH] fix: update kotlin codegen for empty data structures (#794) fixes #768 --- .../src/main/kotlin/ftl/echo/Echo.kt | 2 +- .../block/ftl/generator/ModuleGenerator.kt | 22 ++-- .../ftl/generator/ModuleGeneratorTest.kt | 12 +- kotlin-runtime/ftl-runtime/pom.xml | 7 -- .../ftl/schemaextractor/ExtractSchemaRule.kt | 2 +- .../schemaextractor/ExtractSchemaRuleTest.kt | 103 ++++++++++++------ 6 files changed, 86 insertions(+), 62 deletions(-) diff --git a/examples/kotlin/ftl-module-echo/src/main/kotlin/ftl/echo/Echo.kt b/examples/kotlin/ftl-module-echo/src/main/kotlin/ftl/echo/Echo.kt index 5b0747a6b9..dc5d453725 100644 --- a/examples/kotlin/ftl-module-echo/src/main/kotlin/ftl/echo/Echo.kt +++ b/examples/kotlin/ftl-module-echo/src/main/kotlin/ftl/echo/Echo.kt @@ -17,7 +17,7 @@ class Echo { @Verb @Ingress(Method.GET, "/echo") fun echo(context: Context, req: EchoRequest): EchoResponse { - val response = context.call(TimeModuleClient::time, TimeRequest()) + val response = context.call(TimeModuleClient::time, TimeRequest) return EchoResponse(message = "Hello, ${req.name ?: "anonymous"}! The time is ${response.time}.") } } diff --git a/kotlin-runtime/ftl-generator/src/main/kotlin/xyz/block/ftl/generator/ModuleGenerator.kt b/kotlin-runtime/ftl-generator/src/main/kotlin/xyz/block/ftl/generator/ModuleGenerator.kt index eccacc00b9..ba27c9e780 100644 --- a/kotlin-runtime/ftl-generator/src/main/kotlin/xyz/block/ftl/generator/ModuleGenerator.kt +++ b/kotlin-runtime/ftl-generator/src/main/kotlin/xyz/block/ftl/generator/ModuleGenerator.kt @@ -46,7 +46,17 @@ class ModuleGenerator() { ) val types = module.decls.mapNotNull { it.data_ } - types.forEach { file.addType(buildDataClass(it, namespace)) } + types.forEach { + if (it.fields.isEmpty()) { + file.addTypeAlias( + TypeAliasSpec.builder(it.name, Unit::class) + .addKdoc(it.comments.joinToString("\n")) + .build() + ) + } else { + file.addType(buildDataClass(it, namespace)) + } + } val verbs = module.decls.mapNotNull { it.verb } verbs.forEach { moduleClass.addFunction(buildVerbFunction(className, namespace, it)) } @@ -76,16 +86,6 @@ class ModuleGenerator() { } } - // Handle empty data classes. - if (type.fields.isEmpty()) { - dataConstructorBuilder.addParameter( - ParameterSpec.builder("_empty", Unit::class).defaultValue("Unit").build() - ) - dataClassBuilder.addProperty( - PropertySpec.builder("_empty", Unit::class).initializer("_empty").build() - ) - } - dataClassBuilder.primaryConstructor(dataConstructorBuilder.build()) return dataClassBuilder.build() diff --git a/kotlin-runtime/ftl-generator/src/test/kotlin/xyz/block/ftl/generator/ModuleGeneratorTest.kt b/kotlin-runtime/ftl-generator/src/test/kotlin/xyz/block/ftl/generator/ModuleGeneratorTest.kt index 7ca9fff614..3e4a83a507 100644 --- a/kotlin-runtime/ftl-generator/src/test/kotlin/xyz/block/ftl/generator/ModuleGeneratorTest.kt +++ b/kotlin-runtime/ftl-generator/src/test/kotlin/xyz/block/ftl/generator/ModuleGeneratorTest.kt @@ -97,9 +97,7 @@ import xyz.block.ftl.Ignore /** * Request comments */ -public data class TestRequest( - public val _empty: Unit = Unit, -) +public typealias TestRequest = Unit /** * Response comments @@ -174,16 +172,12 @@ import xyz.block.ftl.Verb /** * Request comments */ -public data class TestRequest( - public val _empty: Unit = Unit, -) +public typealias TestRequest = Unit /** * Response comments */ -public data class TestResponse( - public val _empty: Unit = Unit, -) +public typealias TestResponse = Unit @Ignore public class TestModule() { diff --git a/kotlin-runtime/ftl-runtime/pom.xml b/kotlin-runtime/ftl-runtime/pom.xml index fae05e2ed5..31828cd466 100644 --- a/kotlin-runtime/ftl-runtime/pom.xml +++ b/kotlin-runtime/ftl-runtime/pom.xml @@ -99,13 +99,6 @@ io.gitlab.arturbosch.detekt detekt-test ${detekt.version} - - - - org.jetbrains.kotlin - kotlin-main-kts - - test diff --git a/kotlin-runtime/ftl-runtime/src/main/kotlin/xyz/block/ftl/schemaextractor/ExtractSchemaRule.kt b/kotlin-runtime/ftl-runtime/src/main/kotlin/xyz/block/ftl/schemaextractor/ExtractSchemaRule.kt index a78df4272a..bcbb325fbb 100644 --- a/kotlin-runtime/ftl-runtime/src/main/kotlin/xyz/block/ftl/schemaextractor/ExtractSchemaRule.kt +++ b/kotlin-runtime/ftl-runtime/src/main/kotlin/xyz/block/ftl/schemaextractor/ExtractSchemaRule.kt @@ -460,7 +460,7 @@ class SchemaExtractor( ) private fun getCallMatcher(ctxVarName: String): Regex { - return """${ctxVarName}\.call\((?[^,]+),\s*(?[^,]+?)\s*\(""".toRegex(RegexOption.IGNORE_CASE) + return """${ctxVarName}\.call\((?[^,]+),\s*(?[^,]+?)\s*[()]""".toRegex(RegexOption.IGNORE_CASE) } private fun KotlinType.toClassDescriptor(): ClassDescriptor = diff --git a/kotlin-runtime/ftl-runtime/src/test/kotlin/xyz/block/ftl/schemaextractor/ExtractSchemaRuleTest.kt b/kotlin-runtime/ftl-runtime/src/test/kotlin/xyz/block/ftl/schemaextractor/ExtractSchemaRuleTest.kt index 1325beb941..5ba27ba718 100644 --- a/kotlin-runtime/ftl-runtime/src/test/kotlin/xyz/block/ftl/schemaextractor/ExtractSchemaRuleTest.kt +++ b/kotlin-runtime/ftl-runtime/src/test/kotlin/xyz/block/ftl/schemaextractor/ExtractSchemaRuleTest.kt @@ -1,6 +1,7 @@ package xyz.block.ftl.schemaextractor import io.gitlab.arturbosch.detekt.api.Config +import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment import org.junit.jupiter.api.Assertions.assertEquals @@ -12,16 +13,8 @@ import xyz.block.ftl.v1.schema.Array import xyz.block.ftl.v1.schema.Map import java.io.File import kotlin.test.AfterTest -import kotlin.test.Ignore - -// TODO(@worstell): -// This test can't run when org.jetbrains.kotlin:kotlin-main-kts is excluded from the -// io.gitlab.arturbosch.detekt:detekt-test dependency, which is necessary to avoid a dependency conflict with -// Logback/SLF4J. When fixed we should uncomment the @KotlinCoreEnvironmentTest annotation amd no longer ignore this -// test. -@Ignore -//@KotlinCoreEnvironmentTest +@KotlinCoreEnvironmentTest internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) { @Test @@ -64,7 +57,7 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) { } fun callTime(context: Context): TimeResponse { - return context.call(TimeModuleClient::time, TimeRequest()) + return context.call(TimeModuleClient::time, TimeRequest) } } """ @@ -80,22 +73,6 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) { */""" ), decls = listOf( - Decl( - data_ = Data( - name = "EchoRequest", - fields = listOf( - Field( - name = "name", - type = Type(string = xyz.block.ftl.v1.schema.String()) - ) - ), - comments = listOf( - """/** - * Request to echo a message. - */""" - ) - ), - ), Decl( data_ = Data( name = "MapValue", @@ -105,6 +82,11 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) { type = Type(string = xyz.block.ftl.v1.schema.String()) ) ), + pos = Position( + filename = "Test.kt", + line = 14, + column = 9, + ), ), ), Decl( @@ -120,11 +102,46 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) { type = Type( map = Map( key = xyz.block.ftl.v1.schema.Type(string = xyz.block.ftl.v1.schema.String()), - value_ = xyz.block.ftl.v1.schema.Type(dataRef = DataRef(name = "MapValue", module = "echo")) + value_ = xyz.block.ftl.v1.schema.Type( + dataRef = DataRef( + name = "MapValue", + pos = Position( + filename = "Test.kt", + line = 15, + column = 67, + ), + ) + ) ) ) ) - ) + ), + pos = Position( + filename = "Test.kt", + line = 15, + column = 9, + ), + ), + ), + Decl( + data_ = Data( + name = "EchoRequest", + fields = listOf( + Field( + name = "name", + type = Type(string = xyz.block.ftl.v1.schema.String()) + ) + ), + comments = listOf( + """/** + * Request to echo a message. + */""" + ), + pos = Position( + filename = "Test.kt", + line = 17, + column = 9, + ), ), ), Decl( @@ -136,12 +153,24 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) { type = Type( array = Array( element = xyz.block.ftl.v1.schema.Type( - dataRef = DataRef(name = "EchoMessage", module = "echo") + dataRef = DataRef( + name = "EchoMessage", + pos = Position( + filename = "Test.kt", + line = 21, + column = 47, + ) + ) ) ) ) ) - ) + ), + pos = Position( + filename = "Test.kt", + line = 21, + column = 9, + ), ), ), Decl( @@ -155,13 +184,21 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) { request = Type( dataRef = DataRef( name = "EchoRequest", - module = "echo" + pos = Position( + filename = "Test.kt", + line = 33, + column = 40, + ), ) ), response = Type( dataRef = DataRef( name = "EchoResponse", - module = "echo" + pos = Position( + filename = "Test.kt", + line = 33, + column = 59, + ), ) ), metadata = listOf( @@ -234,7 +271,7 @@ internal class ExtractSchemaRuleTest(private val env: KotlinCoreEnvironment) { } fun callTime(context: Context): TimeResponse { - return context.call(TimeModuleClient::time, TimeRequest()) + return context.call(TimeModuleClient::time, TimeRequest) } } """