Address comments in the PR.

* `newBuilder()` throws an `AssertionError` instead. Thanks @oldergod. * Updated the `KotlinGeneratorTest` to use the renamed option in the `KotlinGenerator`. * Update goldens to include the new error message.
square · Dec 18, 2024 · 7946e6e · 7946e6e
1 parent 83c75b8
commit 7946e6e
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 8 deletions.
diff --git a/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutableHeader.kt b/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutableHeader.kt
@@ -42,7 +42,7 @@ public class MutableHeader(
     message = "Shouldn't be used in Kotlin",
     level = DeprecationLevel.HIDDEN,
   )
-  override fun newBuilder(): Nothing = throw AssertionError("Builders are deprecated and only available in a javaInterop build; see https://square.github.io/wire/wire_compiler/#kotlin")
+  override fun newBuilder(): Nothing = throw AssertionError("newBuilder() is unsupported for Mutable message types")
 
   override fun equals(other: Any?): Boolean {
     if (other !is MutableHeader) return false

diff --git a/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutablePacket.kt b/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutablePacket.kt
@@ -49,7 +49,7 @@ public class MutablePacket(
     message = "Shouldn't be used in Kotlin",
     level = DeprecationLevel.HIDDEN,
   )
-  override fun newBuilder(): Nothing = throw AssertionError("Builders are deprecated and only available in a javaInterop build; see https://square.github.io/wire/wire_compiler/#kotlin")
+  override fun newBuilder(): Nothing = throw AssertionError("newBuilder() is unsupported for Mutable message types")
 
   override fun equals(other: Any?): Boolean {
     if (other !is MutablePacket) return false

diff --git a/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutablePayload.kt b/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutablePayload.kt
@@ -42,7 +42,7 @@ public class MutablePayload(
     message = "Shouldn't be used in Kotlin",
     level = DeprecationLevel.HIDDEN,
   )
-  override fun newBuilder(): Nothing = throw AssertionError("Builders are deprecated and only available in a javaInterop build; see https://square.github.io/wire/wire_compiler/#kotlin")
+  override fun newBuilder(): Nothing = throw AssertionError("newBuilder() is unsupported for Mutable message types")
 
   override fun equals(other: Any?): Boolean {
     if (other !is MutablePayload) return false

diff --git a/wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt b/wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt
@@ -644,6 +644,12 @@ class KotlinGenerator private constructor(
       .addModifiers(OVERRIDE)
 
     if (mutableTypes || !javaInterOp) {
+      val errorMessage = if (mutableTypes) {
+        "newBuilder() is unsupported for Mutable message types"
+      } else {
+        "Builders are deprecated and only available in a javaInterop build; see https://square.github.io/wire/wire_compiler/#kotlin"
+      }
+
       return funBuilder
         .addAnnotation(
           AnnotationSpec.builder(Deprecated::class)
@@ -652,7 +658,7 @@ class KotlinGenerator private constructor(
             .build(),
         )
         .returns(NOTHING)
-        .addStatement("throw %T(%S)", ClassName("kotlin", "AssertionError"), "Builders are deprecated and only available in a javaInterop build; see https://square.github.io/wire/wire_compiler/#kotlin")
+        .addStatement("throw %T(%S)", ClassName("kotlin", "AssertionError"), errorMessage)
         .build()
     }
 
@@ -702,7 +708,7 @@ class KotlinGenerator private constructor(
 
     val body = buildCodeBlock {
       if (!mutableTypes) {
-        // This is true iff the message is not mutable
+        // We cannot rely on referential equality if the message is mutable.
         addStatement("if (%N === this) return·true", otherName)
       }
       addStatement("if (%N !is %T) return·false", otherName, kotlinType)

diff --git a/wire-kotlin-generator/src/test/java/com/squareup/wire/kotlin/KotlinGeneratorTest.kt b/wire-kotlin-generator/src/test/java/com/squareup/wire/kotlin/KotlinGeneratorTest.kt
@@ -2327,14 +2327,17 @@ class KotlinGeneratorTest {
     }
     val code = KotlinWithProfilesGenerator(schema).generateKotlin(
       typeName = "Packet",
-      generateMutableMessages = true,
+      mutableTypes = true,
     )
     assertThat(code).contains("class MutablePacket")
     assertThat(code).contains("public var header_: MutableHeader? = null")
     assertThat(code).contains("public var payload: MutablePayload? = null")
     assertThat(code).contains("MutableHeader#ADAPTER") // should refer to adapters of Mutable message types.
     assertThat(code).contains("MutablePayload#ADAPTER")
     assertThat(code).contains("var result = 0") // hashCode() is no longer calling super.hashCode().
+    assertThat(code).contains(
+      "throw AssertionError(\"newBuilder() is unsupported for Mutable message types\")",
+    )
     assertThat(code).contains(
       "throw UnsupportedOperationException(\"redact() is unsupported for Mutable message types\")",
     )

diff --git a/wire-kotlin-generator/src/test/java/com/squareup/wire/kotlin/KotlinWithProfilesGenerator.kt b/wire-kotlin-generator/src/test/java/com/squareup/wire/kotlin/KotlinWithProfilesGenerator.kt
@@ -54,7 +54,7 @@ internal class KotlinWithProfilesGenerator(private val schema: Schema) {
     buildersOnly: Boolean = false,
     javaInterop: Boolean = false,
     enumMode: EnumMode = EnumMode.ENUM_CLASS,
-    generateMutableMessages: Boolean = false,
+    mutableTypes: Boolean = false,
   ): String {
     val kotlinGenerator = KotlinGenerator(
       schema,
@@ -63,7 +63,7 @@ internal class KotlinWithProfilesGenerator(private val schema: Schema) {
       buildersOnly = buildersOnly,
       javaInterop = javaInterop,
       enumMode = enumMode,
-      generateMutableMessages = generateMutableMessages,
+      mutableTypes = mutableTypes,
     )
     val type = schema.getType(typeName)!!
     val typeSpec = kotlinGenerator.generateType(type)