From fdf867f7818187218109bab926ca74e29642fcc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BCp=20Can=20Akman?= Date: Mon, 29 Jun 2026 00:52:55 +0300 Subject: [PATCH 1/2] Rename a nested type named `Builder` to avoid a collision Fixes #593 --- CHANGELOG.md | 8 ++++ .../com/squareup/wire/java/JavaGenerator.java | 9 +++- .../squareup/wire/java/JavaGeneratorTest.java | 22 ++++++++++ .../squareup/wire/kotlin/KotlinGenerator.kt | 7 +++- .../wire/kotlin/KotlinGeneratorTest.kt | 41 +++++++++++++++++++ 5 files changed, 84 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f7c4ba12f..905d1d9f92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ Change Log Unreleased ---------- +### Java + +* Fix a naming collision when a message has a nested type named `Builder` (#593) + +### Kotlin + +* Fix a naming collision when a message has a nested type named `Builder` (#593) + Version 6.4.4 --------------------- diff --git a/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java b/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java index e979995b90..252bccfec6 100644 --- a/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java +++ b/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java @@ -467,11 +467,16 @@ private static void putAll( String javaPackage, ClassName enclosingClassName, List types) { + NameAllocator nameAllocator = new NameAllocator(); + // A message always emits a nested Builder. Reserve that name so a nested proto type + // named Builder is renamed Builder_ instead of clashing with the generated builder. + if (enclosingClassName != null) nameAllocator.newName("Builder"); for (Type type : types) { + String simpleName = nameAllocator.newName(type.getType().getSimpleName()); ClassName className = enclosingClassName != null - ? enclosingClassName.nestedClass(type.getType().getSimpleName()) - : ClassName.get(javaPackage, type.getType().getSimpleName()); + ? enclosingClassName.nestedClass(simpleName) + : ClassName.get(javaPackage, simpleName); wireToJava.put(type.getType(), className); putAll(wireToJava, javaPackage, className, type.getNestedTypes()); } diff --git a/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java b/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java index 78d5787a75..b77c28644a 100644 --- a/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java +++ b/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java @@ -102,6 +102,28 @@ public void generatedFieldSanitizesJavadocUnicodeEscapes() throws Exception { assertThat(javaOutput).doesNotContain("\\u002a\\u002f class Cheeky"); } + @Test + public void nestedTypeNamedBuilderIsRenamed() throws Exception { + Schema schema = + new SchemaBuilder() + .add( + Path.get("message.proto"), + "" + + "syntax = \"proto2\";\n" + + "message Foo {\n" + + " optional string name = 1;\n" + + " message Builder {\n" + + " optional string value = 1;\n" + + " }\n" + + "}\n") + .build(); + + String javaOutput = new JavaWithProfilesGenerator(schema).generateJava("Foo"); + + assertThat(javaOutput).contains("class Builder_"); + assertThat(javaOutput).doesNotContain("class Builder extends Message<"); + } + @Test public void enclosingTypeSanitizesJavadoc() throws Exception { Schema schema = 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 edf555ddf7..0523f5cfa1 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 @@ -3516,9 +3516,14 @@ class KotlinGenerator private constructor( val memberToKotlinName = mutableMapOf() fun putAll(kotlinPackage: String, enclosingClassName: ClassName?, types: List) { + val nameAllocator = NameAllocator(preallocateKeywords = false) + // A message emits a nested `Builder` in javaInterop or buildersOnly mode. Reserve that + // name so a nested proto type named `Builder` is renamed `Builder_` instead of clashing. + if (enclosingClassName != null && (javaInterop || buildersOnly)) nameAllocator.newName("Builder") for (type in types) { val simpleName = type.type.simpleName - val name = if (mutableTypes && type !is EnumType) "Mutable$simpleName" else simpleName + val candidate = if (mutableTypes && type !is EnumType) "Mutable$simpleName" else simpleName + val name = nameAllocator.newName(candidate) val className = enclosingClassName?.nestedClass(name) ?: ClassName(kotlinPackage, name) typeToKotlinName[type.type] = className 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 58fa85c7c4..8a7b625bec 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 @@ -68,6 +68,47 @@ class KotlinGeneratorTest { assertThat(code).contains("WORK(1),") } + @Test fun nestedTypeNamedBuilderIsRenamed() { + val schema = buildSchema { + add( + "message.proto".toPath(), + """ + |syntax = "proto2"; + |message Foo { + | optional string name = 1; + | message Builder { + | optional string value = 1; + | } + |} + """.trimMargin(), + ) + } + val code = KotlinWithProfilesGenerator(schema).generateKotlin("Foo", javaInterop = true) + assertThat(code).contains("public class Builder_") + assertThat(code).doesNotContain("public class Builder(") + } + + @Test fun nestedTypeNamedBuilderIsRenamedInBuildersOnlyMode() { + val schema = buildSchema { + add( + "message.proto".toPath(), + """ + |syntax = "proto2"; + |message Foo { + | optional string name = 1; + | message Builder { + | optional string value = 1; + | } + |} + """.trimMargin(), + ) + } + val code = KotlinWithProfilesGenerator(schema) + .generateKotlin("Foo", buildersOnly = true, javaInterop = false) + assertThat(code).contains("public class Builder_") + assertThat(code).doesNotContain("public class Builder(") + } + @Test fun generateSealedClassEnumForProto2() { val schema = buildSchema { add( From 41418e794fba74eddac0cafaf4c6d6d74ac3ca73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Quenaudon?= Date: Mon, 29 Jun 2026 13:08:54 +0100 Subject: [PATCH 2/2] Only rename 'Builder' when the outer message is gonna generate one --- .../com/squareup/wire/java/JavaGenerator.java | 17 +++-- .../squareup/wire/java/JavaGeneratorTest.java | 67 +++++++++++++++++++ .../squareup/wire/kotlin/KotlinGenerator.kt | 28 ++++++-- .../wire/kotlin/KotlinGeneratorTest.kt | 63 +++++++++++++++++ 4 files changed, 166 insertions(+), 9 deletions(-) diff --git a/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java b/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java index 252bccfec6..c87049e75e 100644 --- a/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java +++ b/wire-java-generator/src/main/java/com/squareup/wire/java/JavaGenerator.java @@ -385,7 +385,7 @@ public static JavaGenerator get(Schema schema) { for (ProtoFile protoFile : schema.getProtoFiles()) { String javaPackage = javaPackage(protoFile); - putAll(nameToJavaName, javaPackage, null, protoFile.getTypes()); + putAll(nameToJavaName, javaPackage, null, false, protoFile.getTypes()); for (Service service : protoFile.getServices()) { ClassName className = ClassName.get(javaPackage, service.type().getSimpleName()); @@ -466,19 +466,28 @@ private static void putAll( Map wireToJava, String javaPackage, ClassName enclosingClassName, + boolean enclosingTypeEmitsBuilder, List types) { NameAllocator nameAllocator = new NameAllocator(); // A message always emits a nested Builder. Reserve that name so a nested proto type // named Builder is renamed Builder_ instead of clashing with the generated builder. - if (enclosingClassName != null) nameAllocator.newName("Builder"); + if (enclosingTypeEmitsBuilder) nameAllocator.newName("Builder"); for (Type type : types) { - String simpleName = nameAllocator.newName(type.getType().getSimpleName()); + String candidateName = type.getType().getSimpleName(); + // A message named Builder also collides with its own generated nested Builder. + if (!enclosingTypeEmitsBuilder + && type instanceof MessageType + && candidateName.equals("Builder")) { + candidateName = "Builder_"; + } + String simpleName = nameAllocator.newName(candidateName); ClassName className = enclosingClassName != null ? enclosingClassName.nestedClass(simpleName) : ClassName.get(javaPackage, simpleName); wireToJava.put(type.getType(), className); - putAll(wireToJava, javaPackage, className, type.getNestedTypes()); + putAll( + wireToJava, javaPackage, className, type instanceof MessageType, type.getNestedTypes()); } } diff --git a/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java b/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java index b77c28644a..41a96c345c 100644 --- a/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java +++ b/wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java @@ -124,6 +124,73 @@ public void nestedTypeNamedBuilderIsRenamed() throws Exception { assertThat(javaOutput).doesNotContain("class Builder extends Message<"); } + @Test + public void topLevelMessageNamedBuilderIsRenamed() throws Exception { + Schema schema = + new SchemaBuilder() + .add( + Path.get("message.proto"), + "" + + "syntax = \"proto2\";\n" + + "message Builder {\n" + + " optional string value = 1;\n" + + "}\n") + .build(); + + String javaOutput = new JavaWithProfilesGenerator(schema).generateJava("Builder"); + + assertThat(javaOutput).contains("class Builder_ extends Message"); + assertThat(javaOutput) + .doesNotContain("class Builder extends Message"); + } + + @Test + public void prunedNestedMessageNamedBuilderIsRenamed() throws Exception { + Schema schema = + new SchemaBuilder() + .add( + Path.get("message.proto"), + "" + + "syntax = \"proto2\";\n" + + "message A {\n" + + " message Builder {\n" + + " optional string value = 1;\n" + + " }\n" + + "}\n") + .build(); + Schema pruned = schema.prune(new PruningRules.Builder().addRoot("A.Builder").build()); + JavaGenerator javaGenerator = JavaGenerator.get(pruned); + TypeSpec typeSpec = javaGenerator.generateType(pruned.getType("A")); + String javaOutput = JavaFile.builder("", typeSpec).build().toString(); + + assertThat(javaOutput).contains("class Builder_ extends Message"); + assertThat(javaOutput) + .doesNotContain("class Builder extends Message"); + } + + @Test + public void prunedNestedEnumNamedBuilderIsNotRenamed() throws Exception { + Schema schema = + new SchemaBuilder() + .add( + Path.get("message.proto"), + "" + + "syntax = \"proto2\";\n" + + "message A {\n" + + " enum Builder {\n" + + " VALUE = 0;\n" + + " }\n" + + "}\n") + .build(); + Schema pruned = schema.prune(new PruningRules.Builder().addRoot("A.Builder").build()); + JavaGenerator javaGenerator = JavaGenerator.get(pruned); + TypeSpec typeSpec = javaGenerator.generateType(pruned.getType("A")); + String javaOutput = JavaFile.builder("", typeSpec).build().toString(); + + assertThat(javaOutput).contains("enum Builder implements WireEnum"); + assertThat(javaOutput).doesNotContain("enum Builder_ implements WireEnum"); + } + @Test public void enclosingTypeSanitizesJavadoc() throws Exception { Schema schema = 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 0523f5cfa1..e6f525e03b 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 @@ -3515,25 +3515,43 @@ class KotlinGenerator private constructor( val typeToKotlinName = mutableMapOf() val memberToKotlinName = mutableMapOf() - fun putAll(kotlinPackage: String, enclosingClassName: ClassName?, types: List) { + fun putAll( + kotlinPackage: String, + enclosingClassName: ClassName?, + enclosingTypeEmitsBuilder: Boolean, + types: List, + ) { val nameAllocator = NameAllocator(preallocateKeywords = false) // A message emits a nested `Builder` in javaInterop or buildersOnly mode. Reserve that // name so a nested proto type named `Builder` is renamed `Builder_` instead of clashing. - if (enclosingClassName != null && (javaInterop || buildersOnly)) nameAllocator.newName("Builder") + if (enclosingTypeEmitsBuilder) nameAllocator.newName("Builder") for (type in types) { val simpleName = type.type.simpleName - val candidate = if (mutableTypes && type !is EnumType) "Mutable$simpleName" else simpleName + var candidate = if (mutableTypes && type !is EnumType) "Mutable$simpleName" else simpleName + // A message named Builder also collides with its own generated nested Builder. + if (!enclosingTypeEmitsBuilder && + (javaInterop || buildersOnly) && + type is MessageType && + candidate == "Builder" + ) { + candidate = "Builder_" + } val name = nameAllocator.newName(candidate) val className = enclosingClassName?.nestedClass(name) ?: ClassName(kotlinPackage, name) typeToKotlinName[type.type] = className - putAll(kotlinPackage, className, type.nestedTypes) + putAll( + kotlinPackage, + className, + enclosingTypeEmitsBuilder = (javaInterop || buildersOnly) && type is MessageType, + type.nestedTypes, + ) } } for (protoFile in schema.protoFiles) { val kotlinPackage = javaPackage(protoFile) - putAll(kotlinPackage, null, protoFile.types) + putAll(kotlinPackage, null, enclosingTypeEmitsBuilder = false, protoFile.types) for (service in protoFile.services) { val className = ClassName(kotlinPackage, service.type.simpleName) 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 8a7b625bec..9634708b46 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 @@ -109,6 +109,69 @@ class KotlinGeneratorTest { assertThat(code).doesNotContain("public class Builder(") } + @Test fun topLevelMessageNamedBuilderIsRenamed() { + val schema = buildSchema { + add( + "message.proto".toPath(), + """ + |syntax = "proto2"; + |message Builder { + | optional string value = 1; + |} + """.trimMargin(), + ) + } + val code = KotlinWithProfilesGenerator(schema).generateKotlin("Builder", javaInterop = true) + assertThat(code).contains("public class Builder_") + assertThat(code).doesNotContain("public class Builder(") + } + + @Test fun prunedNestedMessageNamedBuilderIsRenamed() { + val schema = buildSchema { + add( + "message.proto".toPath(), + """ + |syntax = "proto2"; + |message A { + | message Builder { + | optional string value = 1; + | } + |} + """.trimMargin(), + ) + } + val pruned = schema.prune(PruningRules.Builder().addRoot("A.Builder").build()) + val kotlinGenerator = KotlinGenerator.invoke(pruned, javaInterop = true) + val typeSpec = kotlinGenerator.generateType(pruned.getType("A")!!) + val code = FileSpec.get("", typeSpec).toString() + + assertThat(code).contains("public class Builder_") + assertThat(code).doesNotContain("public class Builder(") + } + + @Test fun prunedNestedEnumNamedBuilderIsNotRenamed() { + val schema = buildSchema { + add( + "message.proto".toPath(), + """ + |syntax = "proto2"; + |message A { + | enum Builder { + | VALUE = 0; + | } + |} + """.trimMargin(), + ) + } + val pruned = schema.prune(PruningRules.Builder().addRoot("A.Builder").build()) + val kotlinGenerator = KotlinGenerator.invoke(pruned, javaInterop = true) + val typeSpec = kotlinGenerator.generateType(pruned.getType("A")!!) + val code = FileSpec.get("", typeSpec).toString() + + assertThat(code).contains("public enum class Builder(") + assertThat(code).doesNotContain("public enum class Builder_(") + } + @Test fun generateSealedClassEnumForProto2() { val schema = buildSchema { add(