Fix naming conflicts with oneofs

Backport for 0.9.x branch Fixes #695
scalapb · Nov 9, 2019 · 1f56d5d · 1f56d5d
1 parent 3d02f3f
commit 1f56d5d
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 11 deletions.
diff --git a/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala b/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala
@@ -211,9 +211,10 @@ class DescriptorImplicits(params: GeneratorParams, files: Seq[FileDescriptor]) {
         snakeCaseToCamelCase(fieldOptions.getScalaName, true)
       else
         fd.getName match {
-          case "serialized_size" => "_SerializedSize"
-          case "class"           => "_Class"
-          case x                 => getNameWithFallback(x, Case.PascalCase, Appendage.Prefix)
+          case "serialized_size"       => "_SerializedSize"
+          case "class"                 => "_Class"
+          case "empty" if fd.isInOneof => "_Empty"
+          case x                       => getNameWithFallback(x, Case.PascalCase, Appendage.Prefix)
         }
 
     private def getNameWithFallback(x: String, targetCase: Case, appendage: Appendage) = {
@@ -443,9 +444,15 @@ class DescriptorImplicits(params: GeneratorParams, files: Seq[FileDescriptor]) {
     def upperScalaName = {
       val name = oneof.getName match {
         case "ValueType" | "value_type" => "ValueTypeOneof"
-        case n                          => n
+        case n                          => NameUtils.snakeCaseToCamelCase(n, true)
       }
-      NameUtils.snakeCaseToCamelCase(name, true)
+
+      val conflictsWithMessage =
+        oneof.getContainingType.getEnumTypes.asScala.exists(_.name == name) ||
+          oneof.getContainingType.nestedTypes.exists(_.scalaName == name)
+
+      if (conflictsWithMessage) name + "Oneof"
+      else name
     }
 
     def fields: IndexedSeq[FieldDescriptor] =

diff --git a/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala b/compiler-plugin/src/main/scala/scalapb/compiler/ProtobufGenerator.scala
@@ -103,12 +103,6 @@ class ProtobufGenerator(
       e.getContainingType.getEnumTypes.asScala.exists(_.name == e.upperScalaName) ||
         e.getContainingType.nestedTypes.exists(_.scalaName == e.upperScalaName)
 
-    if (possiblyConflictingName) {
-      throw new GeneratorException(
-        s"${e.getFile.getName}: The sealed trait generated for the oneof '${e.getName}' conflicts with " +
-          s"another message name '${e.upperScalaName}'."
-      )
-    }
     printer
       .add(s"sealed trait ${e.upperScalaName} extends ${e.baseClasses.mkString(" with ")} {")
       .indent

diff --git a/e2e/src/main/protobuf/issue695.proto b/e2e/src/main/protobuf/issue695.proto
@@ -0,0 +1,46 @@
+syntax = "proto3";
+
+package com.thesamet.proto.e2e;
+
+message OneOfFieldNamedEmpty {
+
+  oneof content {
+    Foo   foo   = 1;
+    Bar   empty = 2; // tests that we avoid a conflict with the `Empty` case object
+    Empty baz   = 3; // Message called `Empty` is fine.
+  }
+
+  message Foo {}
+  message Bar {}
+  message Empty {}
+}
+
+message OneOfNameOnSameLevel {
+
+  Options options = 1;
+
+  message Options {
+
+    // sealed trait will be named FooOptionsOneof to avoid conflict with
+    // FooOptions
+    oneof foo_options {
+      FooOptions for = 1;
+      BarOptions bar = 2;
+    }
+
+    message FooOptions { }
+
+    message BarOptions {
+      // Works fine when using same name as
+      // the enclosing message.
+      oneof bar_options {
+        uint64 baz1 = 1;
+        Empty  bar2 = 2;
+      }
+
+    }
+
+  }
+
+  message Empty {}
+}