From d4ec70fdd3380e7113e9e3c23cef2bea50dbf2e7 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Tue, 14 Jul 2020 05:51:52 +0100 Subject: [PATCH] Fix C# optional field reflection when there are regular fields too Previous tests didn't spot this as each message was either "all optional" or "all non-optional", at which point there isn't a problem. --- csharp/generate_protos.sh | 1 + csharp/protos/unittest_issues.proto | 5 + .../UnittestIssues.cs | 233 +++++++++++++++++- .../Proto3OptionalTest.cs | 10 + csharp/src/Google.Protobuf.Test/testprotos.pb | Bin 338156 -> 338443 bytes .../Reflection/OneofDescriptor.cs | 2 +- 6 files changed, 244 insertions(+), 7 deletions(-) diff --git a/csharp/generate_protos.sh b/csharp/generate_protos.sh index 49a5a42665cd..b663138d1087 100755 --- a/csharp/generate_protos.sh +++ b/csharp/generate_protos.sh @@ -45,6 +45,7 @@ $PROTOC -Isrc --csharp_out=csharp/src/Google.Protobuf \ # and old_extensions2.proto, which are generated with an older version # of protoc. $PROTOC -Isrc -Icsharp/protos \ + --experimental_allow_proto3_optional \ --csharp_out=csharp/src/Google.Protobuf.Test.TestProtos \ --descriptor_set_out=csharp/src/Google.Protobuf.Test/testprotos.pb \ --include_source_info \ diff --git a/csharp/protos/unittest_issues.proto b/csharp/protos/unittest_issues.proto index ef40d75fce20..388998f0a0ea 100644 --- a/csharp/protos/unittest_issues.proto +++ b/csharp/protos/unittest_issues.proto @@ -151,3 +151,8 @@ message NullValueOutsideStruct { message NullValueNotInOneof { google.protobuf.NullValue null_value = 2; } + +message MixedRegularAndOptional { + string regular_field = 1; + optional string optional_field = 2; +} diff --git a/csharp/src/Google.Protobuf.Test.TestProtos/UnittestIssues.cs b/csharp/src/Google.Protobuf.Test.TestProtos/UnittestIssues.cs index e09191a65cec..4377cf3a19cf 100644 --- a/csharp/src/Google.Protobuf.Test.TestProtos/UnittestIssues.cs +++ b/csharp/src/Google.Protobuf.Test.TestProtos/UnittestIssues.cs @@ -52,11 +52,13 @@ public static partial class UnittestIssuesReflection { "bmdfdmFsdWUYASABKAlIABIwCgpudWxsX3ZhbHVlGAIgASgOMhouZ29vZ2xl", "LnByb3RvYnVmLk51bGxWYWx1ZUgAQgcKBXZhbHVlIkUKE051bGxWYWx1ZU5v", "dEluT25lb2YSLgoKbnVsbF92YWx1ZRgCIAEoDjIaLmdvb2dsZS5wcm90b2J1", - "Zi5OdWxsVmFsdWUqVQoMTmVnYXRpdmVFbnVtEhYKEk5FR0FUSVZFX0VOVU1f", - "WkVSTxAAEhYKCUZpdmVCZWxvdxD7//////////8BEhUKCE1pbnVzT25lEP//", - "/////////wEqLgoORGVwcmVjYXRlZEVudW0SEwoPREVQUkVDQVRFRF9aRVJP", - "EAASBwoDb25lEAFCHaoCGlVuaXRUZXN0Lklzc3Vlcy5UZXN0UHJvdG9zYgZw", - "cm90bzM=")); + "Zi5OdWxsVmFsdWUiYAoXTWl4ZWRSZWd1bGFyQW5kT3B0aW9uYWwSFQoNcmVn", + "dWxhcl9maWVsZBgBIAEoCRIbCg5vcHRpb25hbF9maWVsZBgCIAEoCUgAiAEB", + "QhEKD19vcHRpb25hbF9maWVsZCpVCgxOZWdhdGl2ZUVudW0SFgoSTkVHQVRJ", + "VkVfRU5VTV9aRVJPEAASFgoJRml2ZUJlbG93EPv//////////wESFQoITWlu", + "dXNPbmUQ////////////ASouCg5EZXByZWNhdGVkRW51bRITCg9ERVBSRUNB", + "VEVEX1pFUk8QABIHCgNvbmUQAUIdqgIaVW5pdFRlc3QuSXNzdWVzLlRlc3RQ", + "cm90b3NiBnByb3RvMw==")); descriptor = pbr::FileDescriptor.FromGeneratedCode(descriptorData, new pbr::FileDescriptor[] { global::Google.Protobuf.WellKnownTypes.StructReflection.Descriptor, }, new pbr::GeneratedClrTypeInfo(new[] {typeof(global::UnitTest.Issues.TestProtos.NegativeEnum), typeof(global::UnitTest.Issues.TestProtos.DeprecatedEnum), }, null, new pbr::GeneratedClrTypeInfo[] { @@ -70,7 +72,8 @@ public static partial class UnittestIssuesReflection { new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.TestJsonName), global::UnitTest.Issues.TestProtos.TestJsonName.Parser, new[]{ "Name", "Description", "Guid" }, null, null, null, null), new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.OneofMerging), global::UnitTest.Issues.TestProtos.OneofMerging.Parser, new[]{ "Text", "Nested" }, new[]{ "Value" }, null, 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, null)}), new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.NullValueOutsideStruct), global::UnitTest.Issues.TestProtos.NullValueOutsideStruct.Parser, new[]{ "StringValue", "NullValue" }, new[]{ "Value" }, null, null, null), - new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.NullValueNotInOneof), global::UnitTest.Issues.TestProtos.NullValueNotInOneof.Parser, new[]{ "NullValue" }, null, null, null, null) + new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.NullValueNotInOneof), global::UnitTest.Issues.TestProtos.NullValueNotInOneof.Parser, new[]{ "NullValue" }, null, null, null, null), + new pbr::GeneratedClrTypeInfo(typeof(global::UnitTest.Issues.TestProtos.MixedRegularAndOptional), global::UnitTest.Issues.TestProtos.MixedRegularAndOptional.Parser, new[]{ "RegularField", "OptionalField" }, new[]{ "OptionalField" }, null, null, null) })); } #endregion @@ -3304,6 +3307,224 @@ public sealed partial class NullValueNotInOneof : pb::IMessage + #if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE + , pb::IBufferMessage + #endif + { + private static readonly pb::MessageParser _parser = new pb::MessageParser(() => new MixedRegularAndOptional()); + private pb::UnknownFieldSet _unknownFields; + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public static pb::MessageParser Parser { get { return _parser; } } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public static pbr::MessageDescriptor Descriptor { + get { return global::UnitTest.Issues.TestProtos.UnittestIssuesReflection.Descriptor.MessageTypes[11]; } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + pbr::MessageDescriptor pb::IMessage.Descriptor { + get { return Descriptor; } + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public MixedRegularAndOptional() { + OnConstruction(); + } + + partial void OnConstruction(); + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public MixedRegularAndOptional(MixedRegularAndOptional other) : this() { + regularField_ = other.regularField_; + optionalField_ = other.optionalField_; + _unknownFields = pb::UnknownFieldSet.Clone(other._unknownFields); + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public MixedRegularAndOptional Clone() { + return new MixedRegularAndOptional(this); + } + + /// Field number for the "regular_field" field. + public const int RegularFieldFieldNumber = 1; + private string regularField_ = ""; + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public string RegularField { + get { return regularField_; } + set { + regularField_ = pb::ProtoPreconditions.CheckNotNull(value, "value"); + } + } + + /// Field number for the "optional_field" field. + public const int OptionalFieldFieldNumber = 2; + private string optionalField_; + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public string OptionalField { + get { return optionalField_ ?? ""; } + set { + optionalField_ = pb::ProtoPreconditions.CheckNotNull(value, "value"); + } + } + /// Gets whether the "optional_field" field is set + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public bool HasOptionalField { + get { return optionalField_ != null; } + } + /// Clears the value of the "optional_field" field + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public void ClearOptionalField() { + optionalField_ = null; + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public override bool Equals(object other) { + return Equals(other as MixedRegularAndOptional); + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public bool Equals(MixedRegularAndOptional other) { + if (ReferenceEquals(other, null)) { + return false; + } + if (ReferenceEquals(other, this)) { + return true; + } + if (RegularField != other.RegularField) return false; + if (OptionalField != other.OptionalField) return false; + return Equals(_unknownFields, other._unknownFields); + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public override int GetHashCode() { + int hash = 1; + if (RegularField.Length != 0) hash ^= RegularField.GetHashCode(); + if (HasOptionalField) hash ^= OptionalField.GetHashCode(); + if (_unknownFields != null) { + hash ^= _unknownFields.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 !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE + output.WriteRawMessage(this); + #else + if (RegularField.Length != 0) { + output.WriteRawTag(10); + output.WriteString(RegularField); + } + if (HasOptionalField) { + output.WriteRawTag(18); + output.WriteString(OptionalField); + } + if (_unknownFields != null) { + _unknownFields.WriteTo(output); + } + #endif + } + + #if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + void pb::IBufferMessage.InternalWriteTo(ref pb::WriteContext output) { + if (RegularField.Length != 0) { + output.WriteRawTag(10); + output.WriteString(RegularField); + } + if (HasOptionalField) { + output.WriteRawTag(18); + output.WriteString(OptionalField); + } + if (_unknownFields != null) { + _unknownFields.WriteTo(ref output); + } + } + #endif + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public int CalculateSize() { + int size = 0; + if (RegularField.Length != 0) { + size += 1 + pb::CodedOutputStream.ComputeStringSize(RegularField); + } + if (HasOptionalField) { + size += 1 + pb::CodedOutputStream.ComputeStringSize(OptionalField); + } + if (_unknownFields != null) { + size += _unknownFields.CalculateSize(); + } + return size; + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public void MergeFrom(MixedRegularAndOptional other) { + if (other == null) { + return; + } + if (other.RegularField.Length != 0) { + RegularField = other.RegularField; + } + if (other.HasOptionalField) { + OptionalField = other.OptionalField; + } + _unknownFields = pb::UnknownFieldSet.MergeFrom(_unknownFields, other._unknownFields); + } + + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + public void MergeFrom(pb::CodedInputStream input) { + #if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE + input.ReadRawMessage(this); + #else + uint tag; + while ((tag = input.ReadTag()) != 0) { + switch(tag) { + default: + _unknownFields = pb::UnknownFieldSet.MergeFieldFrom(_unknownFields, input); + break; + case 10: { + RegularField = input.ReadString(); + break; + } + case 18: { + OptionalField = input.ReadString(); + break; + } + } + } + #endif + } + + #if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE + [global::System.Diagnostics.DebuggerNonUserCodeAttribute] + void pb::IBufferMessage.InternalMergeFrom(ref pb::ParseContext input) { + uint tag; + while ((tag = input.ReadTag()) != 0) { + switch(tag) { + default: + _unknownFields = pb::UnknownFieldSet.MergeFieldFrom(_unknownFields, ref input); + break; + case 10: { + RegularField = input.ReadString(); + break; + } + case 18: { + OptionalField = input.ReadString(); + break; + } + } + } + } + #endif + + } + #endregion } diff --git a/csharp/src/Google.Protobuf.Test/Proto3OptionalTest.cs b/csharp/src/Google.Protobuf.Test/Proto3OptionalTest.cs index 20c1846da442..46a8c57a78ab 100644 --- a/csharp/src/Google.Protobuf.Test/Proto3OptionalTest.cs +++ b/csharp/src/Google.Protobuf.Test/Proto3OptionalTest.cs @@ -34,6 +34,7 @@ using ProtobufUnittest; using System; using System.IO; +using UnitTest.Issues.TestProtos; namespace Google.Protobuf.Test { @@ -139,5 +140,14 @@ public void Equality_IgnoresPresence() Assert.IsTrue(message1.Equals(message2)); message1.ClearOptionalInt32(); } + + [Test] + public void MixedFields() + { + var descriptor = MixedRegularAndOptional.Descriptor; + Assert.AreEqual(1, descriptor.Oneofs.Count); + Assert.AreEqual(0, descriptor.RealOneofCount); + Assert.True(descriptor.Oneofs[0].IsSynthetic); + } } } diff --git a/csharp/src/Google.Protobuf.Test/testprotos.pb b/csharp/src/Google.Protobuf.Test/testprotos.pb index 8aedcdb0e3c4c177b7ecca3a1507b41321f3ee4d..b02594242a02c34d1209b4045dc939c8d5ced72a 100644 GIT binary patch delta 347 zcmaEJQKb8d$c6)Jn3_#CA6oO5wO*-~OWZfJA~hu_HN7+^vB)tm#lN5=Ge0jeM@X5A zw+JE{pO%@LlOn;Wz^K6)!~+#{1BnS~aq;CtH9%D{fmC@g1o6Ve!3sMV8Jz^V_~T)M z5XF=CZ(7cIO_hsXh?Rk1?&kc>`a<vsd;z~+EwE`vQFfyq^B^g=3TrL<$c6)Jn4TDJKD6d9>twsl%Q?5HaIp)qGBC{8{BE fieldProto.OneofIndex == index); + var firstFieldInOneof = parent.Proto.Field.FirstOrDefault(fieldProto => fieldProto.HasOneofIndex && fieldProto.OneofIndex == index); IsSynthetic = firstFieldInOneof?.Proto3Optional ?? false; accessor = CreateAccessor(clrName);