codegen-kotlin: Generate toString() methods consistent with data classes (#793)

Motivation:
codegen-kotlin generates a mix of data classes and regular classes.
For regular classes, toString() methods are also generated.
However, the output of generated toString() methods
differs from the output of default toString() methods of data classes.

Changes:
Generate toString() methods that produce the same output
as default toString() methods of data classes.

Also: rename KotlinCodegenOptions to KotlinCodeGeneratorOptions

The new name is consistent with existing names KotlinCodeGenerator and CliKotlinCodeGeneratorOptions.
Backward compatibility is ensured by turning KotlinCodegenOptions into a (deprecated) type alias.

pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/CliKotlinCodeGeneratorOptions.kt

@@ -47,8 +47,8 @@ data class CliKotlinCodeGeneratorOptions(
    */
   val renames: Map<String, String> = emptyMap()
 ) {
-  fun toKotlinCodegenOptions(): KotlinCodegenOptions =
+  fun toKotlinCodegenOptions(): KotlinCodeGeneratorOptions =
-    KotlinCodegenOptions(
+    KotlinCodeGeneratorOptions(
       indent,
       generateKdoc,
       generateSpringBootConfig,

pkl-codegen-kotlin/src/main/kotlin/org/pkl/codegen/kotlin/KotlinCodeGenerator.kt

@@ -25,7 +25,10 @@ import org.pkl.core.*
 import org.pkl.core.util.CodeGeneratorUtils
 import org.pkl.core.util.IoUtils
 
-data class KotlinCodegenOptions(
+@Deprecated("renamed to KotlinCodeGeneratorOptions", ReplaceWith("KotlinCodeGeneratorOptions"))
+typealias KotlinCodegenOptions = KotlinCodeGeneratorOptions
+
+data class KotlinCodeGeneratorOptions(
   /** The characters to use for indenting generated Kotlin code. */
   val indent: String = "  ",
 
@@ -55,22 +58,9 @@ class KotlinCodeGenerator(
   private val moduleSchema: ModuleSchema,
 
   /** The options to use for the code generator */
-  private val options: KotlinCodegenOptions,
+  private val options: KotlinCodeGeneratorOptions,
 ) {
   companion object {
-    // Prevent class name from being replaced with shaded name
-    // when pkl-codegen-kotlin is shaded and embedded in pkl-tools
-    // (requires circumventing kotlinc constant folding).
-    private val KOTLIN_TEXT_PACKAGE_NAME = buildString {
-      append("kot")
-      append("lin.")
-      append("text")
-    }
-
-    // `StringBuilder::class.asClassName()` generates "java.lang.StringBuilder",
-    // apparently because `StringBuilder` is an `expect class`.
-    private val STRING_BUILDER = ClassName(KOTLIN_TEXT_PACKAGE_NAME, "StringBuilder")
-
     private val STRING = String::class.asClassName()
     private val ANY_NULL = ANY.copy(nullable = true)
     private val NOTHING = Nothing::class.asClassName()
@@ -151,36 +141,6 @@ class KotlinCodeGenerator(
           TypeSpec.companionObjectBuilder()
         }
 
-        // generate append method for module classes w/o parent class;
-        // reuse in subclasses and nested classes
-        val isGenerateAppendPropertyMethod =
-          isModuleType &&
-            // check if we inherit another module's append method
-            pModuleClass.superclass!!.info == PClassInfo.Module &&
-            // check if anyone is (potentially) going to use our append method
-            (pModuleClass.isOpen ||
-              pModuleClass.isAbstract ||
-              (isGenerateModuleClass && !builder.modifiers.contains(KModifier.DATA)) ||
-              builder.typeSpecs.any { !it.modifiers.contains(KModifier.DATA) })
-
-        if (isGenerateAppendPropertyMethod) {
-          val appendPropertyMethodModifier =
-            if (pModuleClass.isOpen || pModuleClass.isAbstract) {
-              // alternative is `@JvmStatic protected`
-              // (`protected` alone isn't sufficient as of Kotlin 1.6)
-              KModifier.PUBLIC
-            } else KModifier.PRIVATE
-          if (isGenerateModuleClass) {
-            companionObjectBuilder.value.addFunction(
-              appendPropertyMethod().addModifiers(appendPropertyMethodModifier).build()
-            )
-          } else { // kotlin object
-            builder.addFunction(
-              appendPropertyMethod().addModifiers(appendPropertyMethodModifier).build()
-            )
-          }
-        }
-
         // generate serialization code
         if (
           options.implementSerializable &&
@@ -296,14 +256,8 @@ class KotlinCodeGenerator(
       }
 
       val codeBuilder = CodeBlock.builder().add("return %T(", kotlinPoetClassName)
-      var firstProperty = true
+      for ((index, name) in allProperties.keys.withIndex()) {
-      for (name in allProperties.keys) {
+        codeBuilder.add(if (index == 0) "%N" else ", %N", name)
-        if (firstProperty) {
-          codeBuilder.add("%N", name)
-          firstProperty = false
-        } else {
-          codeBuilder.add(", %N", name)
-        }
       }
       codeBuilder.add(")\n")
 
@@ -384,35 +338,27 @@ class KotlinCodeGenerator(
       return builder.build()
     }
 
+    // produce same output as default toString() method of data classes
     fun generateToStringMethod(): FunSpec {
-      val builder = FunSpec.builder("toString").addModifiers(KModifier.OVERRIDE).returns(STRING)
+      return FunSpec.builder("toString")
-
+        .addModifiers(KModifier.OVERRIDE)
-      var builderSize = 50
+        .returns(STRING)
-      val appendBuilder = CodeBlock.builder()
-      for (propertyName in allProperties.keys) {
-        builderSize += 50
-        appendBuilder.addStatement(
-          "appendProperty(builder, %S, this.%N)",
-          propertyName,
-          propertyName
-        )
-      }
-
-      builder
-        .addStatement("val builder = %T(%L)", STRING_BUILDER, builderSize)
         .addStatement(
-          // generate `::class.java.simpleName` instead of `::class.simpleName`
+          "return %P",
-          // to avoid making user code depend on kotlin-reflect
+          CodeBlock.builder()
-          "builder.append(%T::class.java.simpleName).append(\" {\")",
+            .apply {
-          kotlinPoetClassName
+              add("%L", pClass.toKotlinPoetName().simpleName)
+              add("(")
+              for ((index, propertyName) in allProperties.keys.withIndex()) {
+                add(if (index == 0) "%L" else ", %L", propertyName)
+                add("=$")
+                add("%N", propertyName)
+              }
+              add(")")
+            }
+            .build()
         )
-        .addCode(appendBuilder.build())
+        .build()
-        // not using %S here because it generates `"\n" + "{"`
-        // with a line break in the generated code after `+`
-        .addStatement("builder.append(\"\\n}\")")
-        .addStatement("return builder.toString()")
-
-      return builder.build()
     }
 
     fun generateDeprecation(
@@ -633,18 +579,6 @@ class KotlinCodeGenerator(
   // generating idiomatic KDoc would require parsing doc comments, converting member links, etc.
   private fun renderAsKdoc(docComment: String): String = docComment
 
-  private fun appendPropertyMethod() =
-    FunSpec.builder("appendProperty")
-      .addParameter("builder", STRING_BUILDER)
-      .addParameter("name", STRING)
-      .addParameter("value", ANY_NULL)
-      .addStatement("builder.append(\"\\n  \").append(name).append(\" = \")")
-      .addStatement("val lines = value.toString().split(\"\\n\")")
-      .addStatement("builder.append(lines[0])")
-      .beginControlFlow("for (i in 1..lines.lastIndex)")
-      .addStatement("builder.append(\"\\n  \").append(lines[i])")
-      .endControlFlow()
-
   private fun PClass.toKotlinPoetName(): ClassName {
     val (packageName, moduleTypeName) = nameMapper.map(moduleName)
     return if (isModuleClass) {

pkl-codegen-kotlin/src/test/kotlin/org/pkl/codegen/kotlin/KotlinCodeGeneratorTest.kt

@@ -147,7 +147,7 @@ class KotlinCodeGeneratorTest {
       val generator =
         KotlinCodeGenerator(
           module,
-          KotlinCodegenOptions(
+          KotlinCodeGeneratorOptions(
             generateKdoc = generateKdoc,
             generateSpringBootConfig = generateSpringBootConfig,
             implementSerializable = implementSerializable
@@ -195,65 +195,52 @@ class KotlinCodeGeneratorTest {
 
     assertThat(propertyTypes.toString())
       .isEqualTo(
+        """PropertyTypes(boolean=true, int=42, float=42.3, string=string, duration=5.min, """ +
+          """durationUnit=min, dataSize=3.gb, dataSizeUnit=gb, nullable=idea, nullable2=null, """ +
+          """pair=(1, 2), pair2=(pigeon, Other(name=pigeon)), coll=[1, 2], """ +
+          """coll2=[Other(name=pigeon), Other(name=pigeon)], list=[1, 2], """ +
+          """list2=[Other(name=pigeon), Other(name=pigeon)], set=[1, 2], """ +
+          """set2=[Other(name=pigeon)], map={1=one, 2=two}, map2={one=Other(name=pigeon), """ +
+          """two=Other(name=pigeon)}, container={1=one, 2=two}, container2={one=Other(name=pigeon), """ +
+          """two=Other(name=pigeon)}, other=Other(name=pigeon), regex=(i?)\w*, any=Other(name=pigeon), """ +
+          """nonNull=Other(name=pigeon), enum=north)"""
+      )
+  }
+
+  @Test
+  fun `quoted identifiers`() {
+    val kotlinCode =
+      generateKotlinCode(
         """
-        PropertyTypes {
+      open class `A Person` {
-          boolean = true
+        `first name`: String
-          int = 42
+      }
-          float = 42.3
+    """
-          string = string
-          duration = 5.min
-          durationUnit = min
-          dataSize = 3.gb
-          dataSizeUnit = gb
-          nullable = idea
-          nullable2 = null
-          pair = (1, 2)
-          pair2 = (pigeon, Other {
-            name = pigeon
-          })
-          coll = [1, 2]
-          coll2 = [Other {
-            name = pigeon
-          }, Other {
-            name = pigeon
-          }]
-          list = [1, 2]
-          list2 = [Other {
-            name = pigeon
-          }, Other {
-            name = pigeon
-          }]
-          set = [1, 2]
-          set2 = [Other {
-            name = pigeon
-          }]
-          map = {1=one, 2=two}
-          map2 = {one=Other {
-            name = pigeon
-          }, two=Other {
-            name = pigeon
-          }}
-          container = {1=one, 2=two}
-          container2 = {one=Other {
-            name = pigeon
-          }, two=Other {
-            name = pigeon
-          }}
-          other = Other {
-            name = pigeon
-          }
-          regex = (i?)\w*
-          any = Other {
-            name = pigeon
-          }
-          nonNull = Other {
-            name = pigeon
-          }
-          enum = north
-        }
-      """
           .trimIndent()
       )
+
+    assertThat(kotlinCode)
+      .compilesSuccessfully()
+      .contains(
+        """
+        |  open class `A Person`(
+        |    open val `first name`: String
+        |  )
+      """
+          .trimMargin()
+      )
+      .contains(
+        """
+          |  override fun toString(): String = ""${'"'}A Person(first name=${'$'}`first name`)""${'"'}
+        """
+          .trimMargin()
+      )
+      .contains(
+        """
+          |  open fun copy(`first name`: String = this.`first name`): `A Person` = `A Person`(`first name`)
+        """
+          .trimMargin()
+      )
   }
 
   @Test
@@ -1816,7 +1803,7 @@ class KotlinCodeGeneratorTest {
   @Test
   fun `override names in a standalone module`() {
     val files =
-      KotlinCodegenOptions(
+      KotlinCodeGeneratorOptions(
           renames = mapOf("a.b.c" to "x.y.z", "d.e.f.AnotherModule" to "u.v.w.RenamedModule")
         )
         .generateFiles(
@@ -1851,7 +1838,7 @@ class KotlinCodeGeneratorTest {
   @Test
   fun `override names based on the longest prefix`() {
     val files =
-      KotlinCodegenOptions(
+      KotlinCodeGeneratorOptions(
           renames = mapOf("com.foo.bar." to "x.", "com.foo." to "y.", "com." to "z.", "" to "w.")
         )
         .generateFiles(
@@ -1897,7 +1884,7 @@ class KotlinCodeGeneratorTest {
   @Test
   fun `override names in multiple modules using each other`() {
     val files =
-      KotlinCodegenOptions(
+      KotlinCodeGeneratorOptions(
           renames =
             mapOf(
               "org.foo" to "com.foo.x",
@@ -1980,7 +1967,7 @@ class KotlinCodeGeneratorTest {
   @Test
   fun `do not capitalize names of renamed classes`() {
     val files =
-      KotlinCodegenOptions(
+      KotlinCodeGeneratorOptions(
           renames = mapOf("a.b.c.MyModule" to "x.y.z.renamed_module", "d.e.f." to "u.v.w.")
         )
         .generateFiles(
@@ -2024,7 +2011,7 @@ class KotlinCodeGeneratorTest {
     assertThat(files).isEmpty()
   }
 
-  private fun KotlinCodegenOptions.generateFiles(
+  private fun KotlinCodeGeneratorOptions.generateFiles(
     vararg pklModules: PklModule
   ): Map<String, String> {
     val pklFiles = pklModules.map { it.writeToDisk(tempDir.resolve("pkl/${it.name}.pkl")) }
@@ -2036,13 +2023,13 @@ class KotlinCodeGeneratorTest {
     }
   }
 
-  private fun KotlinCodegenOptions.generateFiles(
+  private fun KotlinCodeGeneratorOptions.generateFiles(
     vararg pklModules: kotlin.Pair<String, String>
   ): Map<String, String> =
     generateFiles(*pklModules.map { (name, text) -> PklModule(name, text) }.toTypedArray())
 
   private fun generateFiles(vararg pklModules: PklModule): Map<String, KotlinSourceCode> =
-    KotlinCodegenOptions().generateFiles(*pklModules).mapValues { KotlinSourceCode(it.value) }
+    KotlinCodeGeneratorOptions().generateFiles(*pklModules).mapValues { KotlinSourceCode(it.value) }
 
   private fun instantiateOtherAndPropertyTypes(): kotlin.Pair<Any, Any> {
     val otherCtor = propertyTypesClasses.getValue("Other").constructors.first()

pkl-codegen-kotlin/src/test/resources/org/pkl/codegen/kotlin/Inheritance.kotlin

@@ -6,23 +6,9 @@ import kotlin.Boolean
 import kotlin.Int
 import kotlin.Long
 import kotlin.String
-import kotlin.text.StringBuilder
 import org.pkl.core.Duration
 
 object Mod {
-  private fun appendProperty(
-    builder: StringBuilder,
-    name: String,
-    value: Any?
-  ) {
-    builder.append("\n  ").append(name).append(" = ")
-    val lines = value.toString().split("\n")
-    builder.append(lines[0])
-    for (i in 1..lines.lastIndex) {
-      builder.append("\n  ").append(lines[i])
-    }
-  }
-
   open class Foo(
     open val one: Long
   ) {
@@ -42,13 +28,7 @@ object Mod {
       return result
     }
 
-    override fun toString(): String {
+    override fun toString(): String = """Foo(one=$one)"""
-      val builder = StringBuilder(100)
-      builder.append(Foo::class.java.simpleName).append(" {")
-      appendProperty(builder, "one", this.one)
-      builder.append("\n}")
-      return builder.toString()
-    }
   }
 
   open class None(
@@ -70,13 +50,7 @@ object Mod {
       return result
     }
 
-    override fun toString(): String {
+    override fun toString(): String = """None(one=$one)"""
-      val builder = StringBuilder(100)
-      builder.append(None::class.java.simpleName).append(" {")
-      appendProperty(builder, "one", this.one)
-      builder.append("\n}")
-      return builder.toString()
-    }
   }
 
   open class Bar(
@@ -103,14 +77,7 @@ object Mod {
       return result
     }
 
-    override fun toString(): String {
+    override fun toString(): String = """Bar(one=$one, two=$two)"""
-      val builder = StringBuilder(150)
-      builder.append(Bar::class.java.simpleName).append(" {")
-      appendProperty(builder, "one", this.one)
-      appendProperty(builder, "two", this.two)
-      builder.append("\n}")
-      return builder.toString()
-    }
   }
 
   class Baz(
@@ -146,14 +113,6 @@ object Mod {
       return result
     }
 
-    override fun toString(): String {
+    override fun toString(): String = """Baz(one=$one, two=$two, three=$three)"""
-      val builder = StringBuilder(200)
-      builder.append(Baz::class.java.simpleName).append(" {")
-      appendProperty(builder, "one", this.one)
-      appendProperty(builder, "two", this.two)
-      appendProperty(builder, "three", this.three)
-      builder.append("\n}")
-      return builder.toString()
-    }
   }
 }

pkl-codegen-kotlin/src/test/resources/org/pkl/codegen/kotlin/Kdoc.kotlin

@@ -5,7 +5,6 @@ import kotlin.Any
 import kotlin.Boolean
 import kotlin.Int
 import kotlin.String
-import kotlin.text.StringBuilder
 
 /**
  * type alias comment.
@@ -51,13 +50,7 @@ data class Mod(
       return result
     }
 
-    override fun toString(): String {
+    override fun toString(): String = """Product(price=$price)"""
-      val builder = StringBuilder(100)
-      builder.append(Product::class.java.simpleName).append(" {")
-      appendProperty(builder, "price", this.price)
-      builder.append("\n}")
-      return builder.toString()
-    }
   }
 
   /**
@@ -71,19 +64,4 @@ data class Mod(
      */
     val name: String
   )
-
-  companion object {
-    private fun appendProperty(
-      builder: StringBuilder,
-      name: String,
-      value: Any?
-    ) {
-      builder.append("\n  ").append(name).append(" = ")
-      val lines = value.toString().split("\n")
-      builder.append(lines[0])
-      for (i in 1..lines.lastIndex) {
-        builder.append("\n  ").append(lines[i])
-      }
-    }
-  }
 }

pkl-codegen-kotlin/src/test/resources/org/pkl/codegen/kotlin/PropertyTypes.kotlin

@@ -13,26 +13,12 @@ import kotlin.collections.List
 import kotlin.collections.Map
 import kotlin.collections.Set
 import kotlin.text.Regex
-import kotlin.text.StringBuilder
 import org.pkl.core.DataSize
 import org.pkl.core.DataSizeUnit
 import org.pkl.core.Duration
 import org.pkl.core.DurationUnit
 
 object Mod {
-  private fun appendProperty(
-    builder: StringBuilder,
-    name: String,
-    value: Any?
-  ) {
-    builder.append("\n  ").append(name).append(" = ")
-    val lines = value.toString().split("\n")
-    builder.append(lines[0])
-    for (i in 1..lines.lastIndex) {
-      builder.append("\n  ").append(lines[i])
-    }
-  }
-
   open class PropertyTypes(
     open val boolean: Boolean,
     open val int: Long,
@@ -160,39 +146,8 @@ object Mod {
       return result
     }
 
-    override fun toString(): String {
+    override fun toString(): String =
-      val builder = StringBuilder(1400)
+        """PropertyTypes(boolean=$boolean, int=$int, float=$float, string=$string, duration=$duration, durationUnit=$durationUnit, dataSize=$dataSize, dataSizeUnit=$dataSizeUnit, nullable=$nullable, nullable2=$nullable2, pair=$pair, pair2=$pair2, coll=$coll, coll2=$coll2, list=$list, list2=$list2, set=$set, set2=$set2, map=$map, map2=$map2, container=$container, container2=$container2, other=$other, regex=$regex, any=$any, nonNull=$nonNull, enum=$enum)"""
-      builder.append(PropertyTypes::class.java.simpleName).append(" {")
-      appendProperty(builder, "boolean", this.boolean)
-      appendProperty(builder, "int", this.int)
-      appendProperty(builder, "float", this.float)
-      appendProperty(builder, "string", this.string)
-      appendProperty(builder, "duration", this.duration)
-      appendProperty(builder, "durationUnit", this.durationUnit)
-      appendProperty(builder, "dataSize", this.dataSize)
-      appendProperty(builder, "dataSizeUnit", this.dataSizeUnit)
-      appendProperty(builder, "nullable", this.nullable)
-      appendProperty(builder, "nullable2", this.nullable2)
-      appendProperty(builder, "pair", this.pair)
-      appendProperty(builder, "pair2", this.pair2)
-      appendProperty(builder, "coll", this.coll)
-      appendProperty(builder, "coll2", this.coll2)
-      appendProperty(builder, "list", this.list)
-      appendProperty(builder, "list2", this.list2)
-      appendProperty(builder, "set", this.set)
-      appendProperty(builder, "set2", this.set2)
-      appendProperty(builder, "map", this.map)
-      appendProperty(builder, "map2", this.map2)
-      appendProperty(builder, "container", this.container)
-      appendProperty(builder, "container2", this.container2)
-      appendProperty(builder, "other", this.other)
-      appendProperty(builder, "regex", this.regex)
-      appendProperty(builder, "any", this.any)
-      appendProperty(builder, "nonNull", this.nonNull)
-      appendProperty(builder, "enum", this.enum)
-      builder.append("\n}")
-      return builder.toString()
-    }
   }
 
   open class Other(
@@ -214,13 +169,7 @@ object Mod {
       return result
     }
 
-    override fun toString(): String {
+    override fun toString(): String = """Other(name=$name)"""
-      val builder = StringBuilder(100)
-      builder.append(Other::class.java.simpleName).append(" {")
-      appendProperty(builder, "name", this.name)
-      builder.append("\n}")
-      return builder.toString()
-    }
   }
 
   enum class Direction(