From 95109afbee0b4bc3c5fcec00bea16f62a12208d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Quenaudon?= Date: Tue, 1 Sep 2026 10:15:51 +0100 Subject: [PATCH] Keep wire-generated Kotlin clean under extra compiler checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kotlin 2.1 added extra compiler checks (-Wextra). Two of them flag wire-generated code, which users cannot edit. VARIABLE_INITIALIZER_IS_REDUNDANT: hashCode() for mutable types initialized the result variable with 0, then the next line overwrote it. The variable now starts with unknownFields.hashCode(). CAN_BE_VAL_DELAYED_INITIALIZATION: encodedSize() for a message with no fields declared a var, never updated it, and returned it. The function now returns value.unknownFields.size directly. The golden files module now compiles with extraWarnings enabled, and these warning families fail the build. REDUNDANT_VISIBILITY_MODIFIER is disabled there: KotlinPoet emits explicit visibility modifiers on purpose, because consumers with explicit API mode need them. Generated code changes shape only. There is no runtime or .api change. Fixes #3700 Fixes #3701 Co-authored-by: Benoît Quenaudon Signed-off-by: Benoît Quenaudon --- wire-golden-files/build.gradle.kts | 17 ++++++++++++++ wire-golden-files/src/main/kotlin/Field.kt | 5 +---- .../squareup/wire/mutable/MutableHeader.kt | 3 +-- .../squareup/wire/mutable/MutablePacket.kt | 3 +-- .../squareup/wire/mutable/MutablePayload.kt | 3 +-- .../squareup/wire/kotlin/KotlinGenerator.kt | 11 +++++++--- .../wire/kotlin/KotlinGeneratorTest.kt | 22 ++++++++++++++++++- 7 files changed, 50 insertions(+), 14 deletions(-) diff --git a/wire-golden-files/build.gradle.kts b/wire-golden-files/build.gradle.kts index a0ca00ecb9..10a50f6295 100644 --- a/wire-golden-files/build.gradle.kts +++ b/wire-golden-files/build.gradle.kts @@ -63,6 +63,23 @@ wire { } } +// The generated code must stay clean under the Kotlin compiler's extra checks. +// See https://github.com/square/wire/issues/3700 and https://github.com/square/wire/issues/3701. +tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile::class.java).configureEach { + compilerOptions { + extraWarnings.set(true) + // KotlinPoet emits explicit visibility modifiers on purpose. Consumers which enable + // explicit API mode need them, so this check does not apply to generated code. + freeCompilerArgs.add("-Xwarning-level=REDUNDANT_VISIBILITY_MODIFIER:disabled") + // Fail the build when these checks flag generated code again. Plain allWarningsAsErrors + // is too broad here: it also fails on repo-wide compiler flag deprecation warnings. + freeCompilerArgs.add("-Xwarning-level=VARIABLE_INITIALIZER_IS_REDUNDANT:error") + freeCompilerArgs.add("-Xwarning-level=CAN_BE_VAL_DELAYED_INITIALIZATION:error") + freeCompilerArgs.add("-Xwarning-level=CAN_BE_VAL:error") + freeCompilerArgs.add("-Xwarning-level=CAN_BE_VAL_LATEINIT:error") + } +} + tasks.getByName("spotlessJava").dependsOn("generateMainProtos") tasks.getByName("spotlessKotlin").dependsOn("generateMainProtos") tasks.getByName("spotlessSwift").dependsOn("generateMainProtos") diff --git a/wire-golden-files/src/main/kotlin/Field.kt b/wire-golden-files/src/main/kotlin/Field.kt index 1d19ecb18b..a7e06c8f7f 100644 --- a/wire-golden-files/src/main/kotlin/Field.kt +++ b/wire-golden-files/src/main/kotlin/Field.kt @@ -57,10 +57,7 @@ public class Field( null, "squareup/wire/hundreds_redacted.proto" ) { - override fun encodedSize(`value`: Field): Int { - var size = value.unknownFields.size - return size - } + override fun encodedSize(`value`: Field): Int = value.unknownFields.size override fun encode(writer: ProtoWriter, `value`: Field) { writer.writeBytes(value.unknownFields) 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 index c50cea2833..bd00e93938 100644 --- a/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutableHeader.kt +++ b/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutableHeader.kt @@ -52,8 +52,7 @@ public class MutableHeader( } override fun hashCode(): Int { - var result = 0 - result = unknownFields.hashCode() + var result = unknownFields.hashCode() result = result * 37 + (id?.hashCode() ?: 0) return result } 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 index 47ff90db6b..7524cdd0eb 100644 --- a/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutablePacket.kt +++ b/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutablePacket.kt @@ -63,8 +63,7 @@ public class MutablePacket( } override fun hashCode(): Int { - var result = 0 - result = unknownFields.hashCode() + var result = unknownFields.hashCode() result = result * 37 + (header_?.hashCode() ?: 0) result = result * 37 + payload.hashCode() return result 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 index 23e86d7549..76ca93e59b 100644 --- a/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutablePayload.kt +++ b/wire-golden-files/src/main/kotlin/squareup/wire/mutable/MutablePayload.kt @@ -79,8 +79,7 @@ public class MutablePayload( } override fun hashCode(): Int { - var result = 0 - result = unknownFields.hashCode() + var result = unknownFields.hashCode() result = result * 37 + (preamble?.hashCode() ?: 0) result = result * 37 + (content?.hashCode() ?: 0) result = result * 37 + (type?.hashCode() ?: 0) 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 index a8bf93652f..2ec9c2023c 100644 --- 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 @@ -889,10 +889,10 @@ class KotlinGenerator private constructor( if (!mutableTypes) { addStatement("var %N = super.hashCode", resultName) beginControlFlow("if (%N == 0)", resultName) + addStatement("%N = unknownFields.hashCode()", resultName) } else { - addStatement("var %N = 0", resultName) + addStatement("var %N = unknownFields.hashCode()", resultName) } - addStatement("%N = unknownFields.hashCode()", resultName) for (fieldOrOneOf in type.fieldsAndFlatOneOfFieldsAndBoxedOneOfs()) { when (fieldOrOneOf) { @@ -1784,8 +1784,13 @@ class KotlinGenerator private constructor( val sizeName = localNameAllocator.newName("size") val body = buildCodeBlock { + val fieldsAndOneOfs = message.fieldsAndFlatOneOfFieldsAndBoxedOneOfs() + if (fieldsAndOneOfs.isEmpty()) { + addStatement("return value.unknownFields.size") + return@buildCodeBlock + } addStatement("var %N = value.unknownFields.size", sizeName) - for (fieldOrOneOf in message.fieldsAndFlatOneOfFieldsAndBoxedOneOfs()) { + for (fieldOrOneOf in fieldsAndOneOfs) { when (fieldOrOneOf) { is Field -> { val fieldName = localNameAllocator[fieldOrOneOf] 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 index 98a7643d54..a53ffd0ea2 100644 --- 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 @@ -2771,6 +2771,25 @@ class KotlinGeneratorTest { assertContains(code, "result = result * 37 + list.hashCode()") } + @Test fun encodedSizeFunctionWithoutFieldsHasNoLocalVariable() { + val schema = buildSchema { + add( + "message.proto".toPath(), + """ + |message NoFields { + |} + """.trimMargin(), + ) + } + val code = KotlinWithProfilesGenerator(schema).generateKotlin("NoFields") + assertThat(code).contains( + """ + | override fun encodedSize(`value`: NoFields): Int = value.unknownFields.size + """.trimMargin(), + ) + assertThat(code).doesNotContain("var size") + } + @Test fun enumConstantConflictingDeclaration() { val schema = buildSchema { @@ -2909,7 +2928,8 @@ class KotlinGeneratorTest { assertThat(code).contains("override var unknownFields: ByteString = ByteString.EMPTY") 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(). + // hashCode() is no longer calling super.hashCode(), and it has no redundant initializer. + assertThat(code).contains("var result = unknownFields.hashCode()") assertThat(code).contains( "throw UnsupportedOperationException(\"newBuilder() is unsupported for mutable message types\")", )