Fix merging with message-valued oneof

If messages A and B have the same oneof case, which is a message
type, and we merge B into A, those sub-messages should be merged.

Fixes #3200.

Note that I haven't regenerated all the code, as some of the protos
have been changed, breaking generation.

csharp/protos/unittest_issues.proto

@@ -124,3 +124,18 @@ message TestJsonName {
   string description = 2 [json_name = "desc"];
   string guid = 3 [json_name = "exid"];
 }
+
+// Issue 3200: When merging two messages which use the same
+// oneof case, which is itself a message type, the submessages should
+// be merged.
+message OneofMerging {
+  message Nested {
+    int32 x = 1;
+    int32 y = 2;
+  }
+
+  oneof value {
+    string text = 1;
+    Nested nested = 2;
+  }
+}

csharp/src/Google.Protobuf.Test/IssuesTest.cs

@@ -33,7 +33,7 @@
 using Google.Protobuf.Reflection;
 using UnitTest.Issues.TestProtos;
 using NUnit.Framework;
-
+using static UnitTest.Issues.TestProtos.OneofMerging.Types;
 
 namespace Google.Protobuf
 {
@@ -78,5 +78,17 @@ namespace Google.Protobuf
             Assert.AreEqual("{ \"name\": \"test\", \"desc\": \"test2\", \"exid\": \"test3\" }",
                 JsonFormatter.Default.Format(message));
         }
+
+        [Test]
+        public void OneofMerging()
+        {
+            var message1 = new OneofMerging { Nested = new Nested { X = 10 } };
+            var message2 = new OneofMerging { Nested = new Nested { Y = 20 } };
+            var expected = new OneofMerging { Nested = new Nested { X = 10, Y = 20 } };
+
+            var merged = message1.Clone();
+            merged.MergeFrom(message2);
+            Assert.AreEqual(expected, merged);
+        }
     }
 }

csharp/src/Google.Protobuf.Test/TestProtos/UnittestIssues.cs

@@ -42,11 +42,14 @@ namespace UnitTest.Issues.TestProtos {
             "MV9pbnQzMhgFIAEoBUgAEhQKDHBsYWluX3N0cmluZxgBIAEoCRISCghvMl9p",
             "bnQzMhgGIAEoBUgBEhMKCW8yX3N0cmluZxgDIAEoCUgBQgQKAm8xQgQKAm8y",
             "IksKDFRlc3RKc29uTmFtZRIMCgRuYW1lGAEgASgJEhkKC2Rlc2NyaXB0aW9u",
-            "GAIgASgJUgRkZXNjEhIKBGd1aWQYAyABKAlSBGV4aWQqVQoMTmVnYXRpdmVF",
-            "bnVtEhYKEk5FR0FUSVZFX0VOVU1fWkVSTxAAEhYKCUZpdmVCZWxvdxD7////",
-            "//////8BEhUKCE1pbnVzT25lEP///////////wEqLgoORGVwcmVjYXRlZEVu",
-            "dW0SEwoPREVQUkVDQVRFRF9aRVJPEAASBwoDb25lEAFCH0gBqgIaVW5pdFRl",
-            "c3QuSXNzdWVzLlRlc3RQcm90b3NiBnByb3RvMw=="));
+            "GAIgASgJUgRkZXNjEhIKBGd1aWQYAyABKAlSBGV4aWQifwoMT25lb2ZNZXJn",
+            "aW5nEg4KBHRleHQYASABKAlIABI2CgZuZXN0ZWQYAiABKAsyJC51bml0dGVz",
+            "dF9pc3N1ZXMuT25lb2ZNZXJnaW5nLk5lc3RlZEgAGh4KBk5lc3RlZBIJCgF4",
+            "GAEgASgFEgkKAXkYAiABKAVCBwoFdmFsdWUqVQoMTmVnYXRpdmVFbnVtEhYK",
+            "Ek5FR0FUSVZFX0VOVU1fWkVSTxAAEhYKCUZpdmVCZWxvdxD7//////////8B",
+            "EhUKCE1pbnVzT25lEP///////////wEqLgoORGVwcmVjYXRlZEVudW0SEwoP",
+            "REVQUkVDQVRFRF9aRVJPEAASBwoDb25lEAFCH0gBqgIaVW5pdFRlc3QuSXNz",
+            "dWVzLlRlc3RQcm90b3NiBnByb3RvMw=="));
       descriptor = pbr::FileDescriptor.FromGeneratedCode(descriptorData,
           new pbr::FileDescriptor[] { },
           new pbr::GeneratedClrTypeInfo(new[] {typeof(global::UnitTest.Issues.TestProtos.NegativeEnum), typeof(global::UnitTest.Issues.TestProtos.DeprecatedEnum), }, new pbr::GeneratedClrTypeInfo[] {
@@ -57,7 +60,8 @@ namespace UnitTest.Issues.TestProtos {
             new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.ItemField), global::UnitTest.Issues.TestProtos.ItemField.Parser, new[]{ "Item" }, null, null, null),
             new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.ReservedNames), global::UnitTest.Issues.TestProtos.ReservedNames.Parser, new[]{ "Types_", "Descriptor_" }, null, null, new pbr::GeneratedClrTypeInfo[] { new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.ReservedNames.Types.SomeNestedType), global::UnitTest.Issues.TestProtos.ReservedNames.Types.SomeNestedType.Parser, null, null, null, null)}),
             new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.TestJsonFieldOrdering), global::UnitTest.Issues.TestProtos.TestJsonFieldOrdering.Parser, new[]{ "PlainInt32", "O1String", "O1Int32", "PlainString", "O2Int32", "O2String" }, new[]{ "O1", "O2" }, null, null),
-            new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.TestJsonName), global::UnitTest.Issues.TestProtos.TestJsonName.Parser, new[]{ "Name", "Description", "Guid" }, null, null, null)
+            new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.TestJsonName), global::UnitTest.Issues.TestProtos.TestJsonName.Parser, new[]{ "Name", "Description", "Guid" }, null, null, null),
+            new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.OneofMerging), global::UnitTest.Issues.TestProtos.OneofMerging.Parser, new[]{ "Text", "Nested" }, new[]{ "Value" }, null, new pbr::GeneratedClrTypeInfo[] { new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested), global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested.Parser, new[]{ "X", "Y" }, null, null, null)})
           }));
     }
     #endregion
@@ -1729,6 +1733,347 @@ namespace UnitTest.Issues.TestProtos {
 
   }
 
+  /// <summary>
+  /// Issue 3200: When merging two messages which use the same
+  /// oneof case, which is itself a message type, the submessages should
+  /// be merged.
+  /// </summary>
+  public sealed partial class OneofMerging : pb::IMessage<OneofMerging> {
+    private static readonly pb::MessageParser<OneofMerging> _parser = new pb::MessageParser<OneofMerging>(() => new OneofMerging());
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public static pb::MessageParser<OneofMerging> Parser { get { return _parser; } }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public static pbr::MessageDescriptor Descriptor {
+      get { return global::UnitTest.Issues.TestProtos.UnittestIssuesReflection.Descriptor.MessageTypes[8]; }
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    pbr::MessageDescriptor pb::IMessage.Descriptor {
+      get { return Descriptor; }
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public OneofMerging() {
+      OnConstruction();
+    }
+
+    partial void OnConstruction();
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public OneofMerging(OneofMerging other) : this() {
+      switch (other.ValueCase) {
+        case ValueOneofCase.Text:
+          Text = other.Text;
+          break;
+        case ValueOneofCase.Nested:
+          Nested = other.Nested.Clone();
+          break;
+      }
+
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public OneofMerging Clone() {
+      return new OneofMerging(this);
+    }
+
+    /// <summary>Field number for the "text" field.</summary>
+    public const int TextFieldNumber = 1;
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public string Text {
+      get { return valueCase_ == ValueOneofCase.Text ? (string) value_ : ""; }
+      set {
+        value_ = pb::ProtoPreconditions.CheckNotNull(value, "value");
+        valueCase_ = ValueOneofCase.Text;
+      }
+    }
+
+    /// <summary>Field number for the "nested" field.</summary>
+    public const int NestedFieldNumber = 2;
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested Nested {
+      get { return valueCase_ == ValueOneofCase.Nested ? (global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested) value_ : null; }
+      set {
+        value_ = value;
+        valueCase_ = value == null ? ValueOneofCase.None : ValueOneofCase.Nested;
+      }
+    }
+
+    private object value_;
+    /// <summary>Enum of possible cases for the "value" oneof.</summary>
+    public enum ValueOneofCase {
+      None = 0,
+      Text = 1,
+      Nested = 2,
+    }
+    private ValueOneofCase valueCase_ = ValueOneofCase.None;
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public ValueOneofCase ValueCase {
+      get { return valueCase_; }
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public void ClearValue() {
+      valueCase_ = ValueOneofCase.None;
+      value_ = null;
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public override bool Equals(object other) {
+      return Equals(other as OneofMerging);
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public bool Equals(OneofMerging other) {
+      if (ReferenceEquals(other, null)) {
+        return false;
+      }
+      if (ReferenceEquals(other, this)) {
+        return true;
+      }
+      if (Text != other.Text) return false;
+      if (!object.Equals(Nested, other.Nested)) return false;
+      if (ValueCase != other.ValueCase) return false;
+      return true;
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public override int GetHashCode() {
+      int hash = 1;
+      if (valueCase_ == ValueOneofCase.Text) hash ^= Text.GetHashCode();
+      if (valueCase_ == ValueOneofCase.Nested) hash ^= Nested.GetHashCode();
+      hash ^= (int) valueCase_;
+      return hash;
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public override string ToString() {
+      return pb::JsonFormatter.ToDiagnosticString(this);
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public void WriteTo(pb::CodedOutputStream output) {
+      if (valueCase_ == ValueOneofCase.Text) {
+        output.WriteRawTag(10);
+        output.WriteString(Text);
+      }
+      if (valueCase_ == ValueOneofCase.Nested) {
+        output.WriteRawTag(18);
+        output.WriteMessage(Nested);
+      }
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public int CalculateSize() {
+      int size = 0;
+      if (valueCase_ == ValueOneofCase.Text) {
+        size += 1 + pb::CodedOutputStream.ComputeStringSize(Text);
+      }
+      if (valueCase_ == ValueOneofCase.Nested) {
+        size += 1 + pb::CodedOutputStream.ComputeMessageSize(Nested);
+      }
+      return size;
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public void MergeFrom(OneofMerging other) {
+      if (other == null) {
+        return;
+      }
+      switch (other.ValueCase) {
+        case ValueOneofCase.Text:
+          Text = other.Text;
+          break;
+        case ValueOneofCase.Nested:
+          if (Nested == null) {
+            Nested = new global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested();
+          }
+          Nested.MergeFrom(other.Nested);
+          break;
+      }
+
+    }
+
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public void MergeFrom(pb::CodedInputStream input) {
+      uint tag;
+      while ((tag = input.ReadTag()) != 0) {
+        switch(tag) {
+          default:
+            input.SkipLastField();
+            break;
+          case 10: {
+            Text = input.ReadString();
+            break;
+          }
+          case 18: {
+            global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested subBuilder = new global::UnitTest.Issues.TestProtos.OneofMerging.Types.Nested();
+            if (valueCase_ == ValueOneofCase.Nested) {
+              subBuilder.MergeFrom(Nested);
+            }
+            input.ReadMessage(subBuilder);
+            Nested = subBuilder;
+            break;
+          }
+        }
+      }
+    }
+
+    #region Nested types
+    /// <summary>Container for nested types declared in the OneofMerging message type.</summary>
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+    public static partial class Types {
+      public sealed partial class Nested : pb::IMessage<Nested> {
+        private static readonly pb::MessageParser<Nested> _parser = new pb::MessageParser<Nested>(() => new Nested());
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public static pb::MessageParser<Nested> Parser { get { return _parser; } }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public static pbr::MessageDescriptor Descriptor {
+          get { return global::UnitTest.Issues.TestProtos.OneofMerging.Descriptor.NestedTypes[0]; }
+        }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        pbr::MessageDescriptor pb::IMessage.Descriptor {
+          get { return Descriptor; }
+        }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public Nested() {
+          OnConstruction();
+        }
+
+        partial void OnConstruction();
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public Nested(Nested other) : this() {
+          x_ = other.x_;
+          y_ = other.y_;
+        }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public Nested Clone() {
+          return new Nested(this);
+        }
+
+        /// <summary>Field number for the "x" field.</summary>
+        public const int XFieldNumber = 1;
+        private int x_;
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public int X {
+          get { return x_; }
+          set {
+            x_ = value;
+          }
+        }
+
+        /// <summary>Field number for the "y" field.</summary>
+        public const int YFieldNumber = 2;
+        private int y_;
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public int Y {
+          get { return y_; }
+          set {
+            y_ = value;
+          }
+        }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public override bool Equals(object other) {
+          return Equals(other as Nested);
+        }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public bool Equals(Nested other) {
+          if (ReferenceEquals(other, null)) {
+            return false;
+          }
+          if (ReferenceEquals(other, this)) {
+            return true;
+          }
+          if (X != other.X) return false;
+          if (Y != other.Y) return false;
+          return true;
+        }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public override int GetHashCode() {
+          int hash = 1;
+          if (X != 0) hash ^= X.GetHashCode();
+          if (Y != 0) hash ^= Y.GetHashCode();
+          return hash;
+        }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public override string ToString() {
+          return pb::JsonFormatter.ToDiagnosticString(this);
+        }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public void WriteTo(pb::CodedOutputStream output) {
+          if (X != 0) {
+            output.WriteRawTag(8);
+            output.WriteInt32(X);
+          }
+          if (Y != 0) {
+            output.WriteRawTag(16);
+            output.WriteInt32(Y);
+          }
+        }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public int CalculateSize() {
+          int size = 0;
+          if (X != 0) {
+            size += 1 + pb::CodedOutputStream.ComputeInt32Size(X);
+          }
+          if (Y != 0) {
+            size += 1 + pb::CodedOutputStream.ComputeInt32Size(Y);
+          }
+          return size;
+        }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public void MergeFrom(Nested other) {
+          if (other == null) {
+            return;
+          }
+          if (other.X != 0) {
+            X = other.X;
+          }
+          if (other.Y != 0) {
+            Y = other.Y;
+          }
+        }
+
+        [global::System.Diagnostics.DebuggerNonUserCodeAttribute]
+        public void MergeFrom(pb::CodedInputStream input) {
+          uint tag;
+          while ((tag = input.ReadTag()) != 0) {
+            switch(tag) {
+              default:
+                input.SkipLastField();
+                break;
+              case 8: {
+                X = input.ReadInt32();
+                break;
+              }
+              case 16: {
+                Y = input.ReadInt32();
+                break;
+              }
+            }
+          }
+        }
+
+      }
+
+    }
+    #endregion
+
+  }
+
   #endregion
 
 }

csharp/src/Google.Protobuf/WellKnownTypes/Struct.cs

@@ -466,10 +466,16 @@ namespace Google.Protobuf.WellKnownTypes {
           BoolValue = other.BoolValue;
           break;
         case KindOneofCase.StructValue:
-          StructValue = other.StructValue;
+          if (StructValue == null) {
+            StructValue = new global::Google.Protobuf.WellKnownTypes.Struct();
+          }
+          StructValue.MergeFrom(other.StructValue);
           break;
         case KindOneofCase.ListValue:
-          ListValue = other.ListValue;
+          if (ListValue == null) {
+            ListValue = new global::Google.Protobuf.WellKnownTypes.ListValue();
+          }
+          ListValue.MergeFrom(other.ListValue);
           break;
       }
 

src/google/protobuf/compiler/csharp/csharp_enum_field.cc

@@ -89,6 +89,10 @@ EnumOneofFieldGenerator::EnumOneofFieldGenerator(
 EnumOneofFieldGenerator::~EnumOneofFieldGenerator() {
 }
 
+void EnumOneofFieldGenerator::GenerateMergingCode(io::Printer* printer) {
+  printer->Print(variables_, "$property_name$ = other.$property_name$;\n");
+}
+
 void EnumOneofFieldGenerator::GenerateParsingCode(io::Printer* printer) {
   // TODO(jonskeet): What about if we read the default value?
   printer->Print(

src/google/protobuf/compiler/csharp/csharp_enum_field.h

@@ -64,6 +64,7 @@ class EnumOneofFieldGenerator : public PrimitiveOneofFieldGenerator {
                           const Options *options);
   ~EnumOneofFieldGenerator();
 
+  virtual void GenerateMergingCode(io::Printer* printer);
   virtual void GenerateParsingCode(io::Printer* printer);
   virtual void GenerateSerializationCode(io::Printer* printer);
   virtual void GenerateSerializedSizeCode(io::Printer* printer);

src/google/protobuf/compiler/csharp/csharp_message.cc

@@ -463,9 +463,12 @@ void MessageGenerator::GenerateMergingMethods(io::Printer* printer) {
       vars["field_property_name"] = GetPropertyName(field);
       printer->Print(
         vars,
-        "case $property_name$OneofCase.$field_property_name$:\n"
-        "  $field_property_name$ = other.$field_property_name$;\n"
-        "  break;\n");
+        "case $property_name$OneofCase.$field_property_name$:\n");
+      printer->Indent();
+      scoped_ptr<FieldGeneratorBase> generator(CreateFieldGeneratorInternal(field));
+      generator->GenerateMergingCode(printer);
+      printer->Print("break;\n");
+      printer->Outdent();
     }
     printer->Outdent();
     printer->Print("}\n\n");

src/google/protobuf/compiler/csharp/csharp_message_field.cc

@@ -171,6 +171,14 @@ void MessageOneofFieldGenerator::GenerateMembers(io::Printer* printer) {
     "}\n");
 }
 
+void MessageOneofFieldGenerator::GenerateMergingCode(io::Printer* printer) {
+  printer->Print(variables_, 
+    "if ($property_name$ == null) {\n"
+    "  $property_name$ = new $type_name$();\n"
+    "}\n"
+    "$property_name$.MergeFrom(other.$property_name$);\n");
+}
+
 void MessageOneofFieldGenerator::GenerateParsingCode(io::Printer* printer) {
   // TODO(jonskeet): We may be able to do better than this
   printer->Print(

src/google/protobuf/compiler/csharp/csharp_message_field.h

@@ -74,6 +74,7 @@ class MessageOneofFieldGenerator : public MessageFieldGenerator {
 
   virtual void GenerateCloningCode(io::Printer* printer);
   virtual void GenerateMembers(io::Printer* printer);
+  virtual void GenerateMergingCode(io::Printer* printer);
   virtual void WriteToString(io::Printer* printer);
   virtual void GenerateParsingCode(io::Printer* printer);
 

src/google/protobuf/compiler/csharp/csharp_primitive_field.cc

@@ -196,6 +196,10 @@ void PrimitiveOneofFieldGenerator::GenerateMembers(io::Printer* printer) {
       "}\n");
 }
 
+void PrimitiveOneofFieldGenerator::GenerateMergingCode(io::Printer* printer) {
+  printer->Print(variables_, "$property_name$ = other.$property_name$;\n");
+}
+
 void PrimitiveOneofFieldGenerator::WriteToString(io::Printer* printer) {
   printer->Print(variables_,
     "PrintField(\"$descriptor_name$\", $has_property_check$, $oneof_name$_, writer);\n");

src/google/protobuf/compiler/csharp/csharp_primitive_field.h

@@ -78,6 +78,7 @@ class PrimitiveOneofFieldGenerator : public PrimitiveFieldGenerator {
 
   virtual void GenerateCloningCode(io::Printer* printer);
   virtual void GenerateMembers(io::Printer* printer);
+  virtual void GenerateMergingCode(io::Printer* printer);
   virtual void WriteToString(io::Printer* printer);
   virtual void GenerateParsingCode(io::Printer* printer);
 

src/google/protobuf/compiler/csharp/csharp_wrapper_field.cc

@@ -181,6 +181,10 @@ void WrapperOneofFieldGenerator::GenerateMembers(io::Printer* printer) {
     "}\n");
 }
 
+void WrapperOneofFieldGenerator::GenerateMergingCode(io::Printer* printer) {
+  printer->Print(variables_, "$property_name$ = other.$property_name$;\n");
+}
+
 void WrapperOneofFieldGenerator::GenerateParsingCode(io::Printer* printer) {
   printer->Print(
     variables_,

src/google/protobuf/compiler/csharp/csharp_wrapper_field.h

@@ -75,6 +75,7 @@ class WrapperOneofFieldGenerator : public WrapperFieldGenerator {
   ~WrapperOneofFieldGenerator();
 
   virtual void GenerateMembers(io::Printer* printer);
+  virtual void GenerateMergingCode(io::Printer* printer);
   virtual void GenerateParsingCode(io::Printer* printer);
   virtual void GenerateSerializationCode(io::Printer* printer);
   virtual void GenerateSerializedSizeCode(io::Printer* printer);