Don't qualify oneof type name when it doesn't share a name with its enclosing type (#82)

Author: andrewparmet

protokt-codegen/src/main/kotlin/com/toasttab/protokt/codegen/impl/OneofAnnotator.kt

@@ -91,6 +91,7 @@ private constructor(
             PClass.fromName(fieldType).let {
                 // If a wrapper type is specified and it shares a name with the
                 // oneof, it must be fully qualified.
+                // See testing/options/src/main/proto/com/toasttab/protokt/testing/options/oneof_exercises.proto
                 if (unqualifiedType.simpleName == it.simpleName) {
                     it.possiblyQualify(ctx.pkg).qualifiedName
                 } else {
@@ -109,21 +110,21 @@ private constructor(
         val pClass = f.typePClass
 
         // Cannot strip qualifiers for field type in a different package
+        // See testing/runtime-tests/src/main/proto/com/toasttab/protokt/testing/rt/oneof/oneof_packages.proto
         val requiresQualifiedTypeName = pClass.ppackage != ctx.pkg
 
         return if (requiresQualifiedTypeName) {
             pClass.renderName(ctx.pkg)
         } else {
-            if (oneofFieldTypeName == pClass.nestedName) {
-                // Oneof field name shares name of its type
-                if (oneofFieldTypeName == pClass.simpleName) {
-                    // Oneof field name shares name of its enclosing type
+            // See testing/runtime-tests/src/main/proto/com/toasttab/protokt/testing/rt/oneof/oneof_exercises.proto
+            if (oneofFieldTypeName == pClass.simpleName) {
+                if (oneofFieldTypeName == pClass.nestedName) {
                     pClass.qualifiedName
                 } else {
-                    pClass.simpleName
+                    pClass.nestedName
                 }
             } else {
-                pClass.nestedName
+                pClass.simpleName
             }
         }
     }

testing/options/src/main/proto/com/toasttab/protokt/testing/options/oneof_exercises.proto

@@ -23,27 +23,9 @@ import "protokt/protokt.proto";
 // generated sealed class type names. If the code generator does
 // not handle them properly then compilation will fail.
 
-message OneofExerciseModel {
-  oneof oneof {
-    // Needs partial qualification: OneofExerciseModel.Model
-    Model model = 1;
-  }
-
-  message Model {
-    bytes id = 1;
-  }
-}
-
-message OneofExerciseModel2 {
-  oneof oneof {
-    // Needs full qualification: com.toasttab.model.OneofExerciseModel
-    OneofExerciseModel oneof_exercise_model = 1;
-  }
-}
-
 message OneofExerciseModelWithWrapper {
   oneof oneof {
-    // Needs full qualification: com.toasttab.testing.options.CachingId
+    // Needs full qualification: com.toasttab.protokt.testing.options.CachingId
     bytes caching_id = 1 [
       (.protokt.property).wrap = "CachingId"
     ];

testing/runtime-tests/src/main/proto/com/toasttab/protokt/testing/rt/oneof/oneof_exercises.proto

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2020 Toast Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+syntax = "proto3";
+
+package com.toasttab.protokt.testing.rt.oneof;
+
+// This file exercises oneofs whose names are the same as their
+// generated sealed class type names. If the code generator does
+// not handle them properly then compilation will fail.
+
+message OneofExerciseModel {
+  oneof oneof {
+    // Needs partial qualification: OneofExerciseModel.Model
+    Model model = 1;
+  }
+
+  oneof oneof2 {
+    // Does not need partial qualification: Model
+    // This will still compile if qualified but may show a warning in IDEs
+    Model not_named_model = 2;
+  }
+
+  message Model {
+    bytes id = 1;
+  }
+}
+
+message OneofExerciseModel2 {
+  oneof oneof {
+    // Needs full qualification: com.toasttab.protokt.testing.rt.oneof.OneofExerciseModel
+    OneofExerciseModel oneof_exercise_model = 1;
+  }
+}