Skip to content

Commit

Permalink
Fix C# optional field reflection when there are regular fields too
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jskeet
jskeet committed Jul 14, 2020
1 parent ffb2b53 commit d4ec70f
Show file tree
Hide file tree
Showing 6 changed files with 244 additions and 7 deletions.
1 change: 1 addition & 0 deletions csharp/generate_protos.sh
View file
Expand Up @@ -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 \
Expand Down
5 changes: 5 additions & 0 deletions csharp/protos/unittest_issues.proto
View file
Expand Up @@ -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;
}
233 changes: 227 additions & 6 deletions csharp/src/Google.Protobuf.Test.TestProtos/UnittestIssues.cs
View file
Expand Up @@ -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[] {
Expand All @@ -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
Expand Down Expand Up @@ -3304,6 +3307,224 @@ public sealed partial class NullValueNotInOneof : pb::IMessage<NullValueNotInOne

}

public sealed partial class MixedRegularAndOptional : pb::IMessage<MixedRegularAndOptional>
#if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE
, pb::IBufferMessage
#endif
{
private static readonly pb::MessageParser<MixedRegularAndOptional> _parser = new pb::MessageParser<MixedRegularAndOptional>(() => new MixedRegularAndOptional());
private pb::UnknownFieldSet _unknownFields;
[global::System.Diagnostics.DebuggerNonUserCodeAttribute]
public static pb::MessageParser<MixedRegularAndOptional> 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);
}

/// <summary>Field number for the "regular_field" field.</summary>
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");
}
}

/// <summary>Field number for the "optional_field" field.</summary>
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");
}
}
/// <summary>Gets whether the "optional_field" field is set</summary>
[global::System.Diagnostics.DebuggerNonUserCodeAttribute]
public bool HasOptionalField {
get { return optionalField_ != null; }
}
/// <summary>Clears the value of the "optional_field" field</summary>
[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

}
Expand Down
10 changes: 10 additions & 0 deletions csharp/src/Google.Protobuf.Test/Proto3OptionalTest.cs
View file
Expand Up @@ -34,6 +34,7 @@
using ProtobufUnittest;
using System;
using System.IO;
using UnitTest.Issues.TestProtos;

namespace Google.Protobuf.Test
{
Expand Down Expand Up @@ -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);
}
}
}
Binary file modified csharp/src/Google.Protobuf.Test/testprotos.pb
View file
Binary file not shown.
2 changes: 1 addition & 1 deletion csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs
View file
Expand Up @@ -59,7 +59,7 @@ internal OneofDescriptor(OneofDescriptorProto proto, FileDescriptor file, Messag

// It's useful to determine whether or not this is a synthetic oneof before cross-linking. That means
// diving into the proto directly rather than using FieldDescriptor, but that's okay.
var firstFieldInOneof = parent.Proto.Field.FirstOrDefault(fieldProto => fieldProto.OneofIndex == index);
var firstFieldInOneof = parent.Proto.Field.FirstOrDefault(fieldProto => fieldProto.HasOneofIndex && fieldProto.OneofIndex == index);
IsSynthetic = firstFieldInOneof?.Proto3Optional ?? false;

accessor = CreateAccessor(clrName);
Expand Down

0 comments on commit d4ec70f

Please sign in to comment.