fix: update kotlin codegen for empty data structures (#794)

fixes #768
block · Jan 17, 2024 · d5b358e · d5b358e
1 parent 0f25aa6
commit d5b358e
Show file tree

Hide file tree

Showing 6 changed files with 86 additions and 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
@@ -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
@@ -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
@@ -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
@@ -99,13 +99,6 @@
             <groupId>io.gitlab.arturbosch.detekt</groupId>
             <artifactId>detekt-test</artifactId>
             <version>${detekt.version}</version>
-            <exclusions>
-                <!-- Exclude Kotlin Main KTS to fix SLF4J diamond dependency problem. -->
-                <exclusion>
-                    <groupId>org.jetbrains.kotlin</groupId>
-                    <artifactId>kotlin-main-kts</artifactId>
-                </exclusion>
-            </exclusions>
             <scope>test</scope>
         </dependency>
         <dependency>

diff --git a/...in-runtime/ftl-runtime/src/main/kotlin/xyz/block/ftl/schemaextractor/ExtractSchemaRule.kt b/...in-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\((?<fn>[^,]+),\s*(?<req>[^,]+?)\s*\(""".toRegex(RegexOption.IGNORE_CASE)
+      return """${ctxVarName}\.call\((?<fn>[^,]+),\s*(?<req>[^,]+?)\s*[()]""".toRegex(RegexOption.IGNORE_CASE)
     }
 
     private fun KotlinType.toClassDescriptor(): ClassDescriptor =

diff --git a/...untime/ftl-runtime/src/test/kotlin/xyz/block/ftl/schemaextractor/ExtractSchemaRuleTest.kt b/...untime/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)
             }
         }
         """