Rename a nested type named Builder to avoid a collision

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
 ---------------------
 

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,14 +466,28 @@ private static void putAll(
       Map<ProtoType, TypeName> wireToJava,
       String javaPackage,
       ClassName enclosingClassName,
+      boolean enclosingTypeEmitsBuilder,
       List<Type> 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 (enclosingTypeEmitsBuilder) nameAllocator.newName("Builder");
     for (Type type : types) {
+      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(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());
+      putAll(
+          wireToJava, javaPackage, className, type instanceof MessageType, type.getNestedTypes());
     }
   }
 

wire-java-generator/src/test/java/com/squareup/wire/java/JavaGeneratorTest.java

@@ -102,6 +102,95 @@ 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 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<Builder_, Builder_.Builder>");
+    assertThat(javaOutput)
+        .doesNotContain("class Builder extends Message<Builder, Builder.Builder>");
+  }
+
+  @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<Builder_, Builder_.Builder>");
+    assertThat(javaOutput)
+        .doesNotContain("class Builder extends Message<Builder, Builder.Builder>");
+  }
+
+  @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 =

wire-kotlin-generator/src/main/java/com/squareup/wire/kotlin/KotlinGenerator.kt

@@ -3515,20 +3515,43 @@ class KotlinGenerator private constructor(
       val typeToKotlinName = mutableMapOf<ProtoType, TypeName>()
       val memberToKotlinName = mutableMapOf<ProtoMember, TypeName>()
 
-      fun putAll(kotlinPackage: String, enclosingClassName: ClassName?, types: List<Type>) {
+      fun putAll(
+        kotlinPackage: String,
+        enclosingClassName: ClassName?,
+        enclosingTypeEmitsBuilder: Boolean,
+        types: List<Type>,
+      ) {
+        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 (enclosingTypeEmitsBuilder) nameAllocator.newName("Builder")
         for (type in types) {
           val simpleName = type.type.simpleName
-          val name = 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)

wire-kotlin-generator/src/test/java/com/squareup/wire/kotlin/KotlinGeneratorTest.kt

@@ -68,6 +68,110 @@ 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 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(