From 450022de9939f5eaf9f33696c17005ee7609eecc Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 28 Feb 2024 15:21:06 -0800 Subject: [PATCH] Runtime support for Protobuf Editions in C#. PiperOrigin-RevId: 611246775 --- csharp/generate_protos.sh | 2 + .../Reflection/DescriptorsTest.cs | 62 +++ .../Reflection/FeatureInheritanceTest.cs | 373 ++++++++++++++++++ .../Reflection/FeatureSetDescriptorTest.cs | 37 ++ .../src/Google.Protobuf/MessageExtensions.cs | 6 +- .../Reflection/DescriptorBase.cs | 14 +- .../Reflection/EnumDescriptor.cs | 15 +- .../Reflection/EnumValueDescriptor.cs | 16 +- .../Reflection/FeatureSetDescriptor.cs | 118 ++++++ .../Reflection/FieldDescriptor.cs | 94 +++-- .../Reflection/FileDescriptor.cs | 63 ++- .../Reflection/MessageDescriptor.cs | 16 +- .../Reflection/MethodDescriptor.cs | 17 +- .../Reflection/OneofDescriptor.cs | 17 +- .../Reflection/ServiceDescriptor.cs | 16 +- .../Reflection/SingleFieldAccessor.cs | 10 +- 16 files changed, 809 insertions(+), 67 deletions(-) create mode 100644 csharp/src/Google.Protobuf.Test/Reflection/FeatureInheritanceTest.cs create mode 100644 csharp/src/Google.Protobuf.Test/Reflection/FeatureSetDescriptorTest.cs create mode 100644 csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.cs diff --git a/csharp/generate_protos.sh b/csharp/generate_protos.sh index d747bff501..2acec5c02e 100755 --- a/csharp/generate_protos.sh +++ b/csharp/generate_protos.sh @@ -73,6 +73,8 @@ $PROTOC -Isrc -I. \ src/google/protobuf/unittest_well_known_types.proto \ src/google/protobuf/test_messages_proto3.proto \ src/google/protobuf/test_messages_proto2.proto \ + src/google/protobuf/unittest_features.proto \ + src/google/protobuf/unittest_legacy_features.proto \ src/google/protobuf/unittest_proto3_optional.proto \ src/google/protobuf/unittest_retention.proto diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs index ba31634524..4bd857aacb 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs @@ -8,12 +8,15 @@ #endregion using Google.Protobuf.TestProtos; +using LegacyFeaturesUnittest; using NUnit.Framework; using ProtobufUnittest; using System; using System.Collections.Generic; using System.Linq; using UnitTest.Issues.TestProtos; +using static Google.Protobuf.Reflection.FeatureSet.Types; +using proto2 = Google.Protobuf.TestProtos.Proto2; namespace Google.Protobuf.Reflection { @@ -454,6 +457,65 @@ namespace Google.Protobuf.Reflection UnittestRetentionExtensions.SourceRetentionOption)); } + [Test] + public void GetOptionsStripsFeatures() + { + var messageDescriptor = TestEditionsMessage.Descriptor; + var fieldDescriptor = messageDescriptor.FindFieldByName("required_field"); + // Note: ideally we'd test GetOptions() for other descriptor types as well, but that requires + // non-fields with features applied. + Assert.Null(fieldDescriptor.GetOptions().Features); + } + + [Test] + public void LegacyRequiredTransform() + { + var messageDescriptor = TestEditionsMessage.Descriptor; + var fieldDescriptor = messageDescriptor.FindFieldByName("required_field"); + Assert.True(fieldDescriptor.IsRequired); + } + + [Test] + public void LegacyGroupTransform() + { + var messageDescriptor = TestEditionsMessage.Descriptor; + var fieldDescriptor = messageDescriptor.FindFieldByName("delimited_field"); + Assert.AreEqual(FieldType.Group, fieldDescriptor.FieldType); + } + + [Test] + public void LegacyInferRequired() + { + var messageDescriptor = proto2::TestRequired.Descriptor; + var fieldDescriptor = messageDescriptor.FindFieldByName("a"); + Assert.AreEqual(FieldPresence.LegacyRequired, fieldDescriptor.Features.FieldPresence); + } + + [Test] + public void LegacyInferGroup() + { + var messageDescriptor = proto2::TestAllTypes.Descriptor; + var fieldDescriptor = messageDescriptor.FindFieldByName("optionalgroup"); + Assert.AreEqual(MessageEncoding.Delimited, fieldDescriptor.Features.MessageEncoding); + } + + [Test] + public void LegacyInferProto2Packed() + { + var messageDescriptor = proto2::TestPackedTypes.Descriptor; + var fieldDescriptor = messageDescriptor.FindFieldByName("packed_int32"); + Assert.AreEqual(RepeatedFieldEncoding.Packed, fieldDescriptor.Features.RepeatedFieldEncoding); + } + + [Test] + public void LegacyInferProto3Expanded() + { + var messageDescriptor = TestUnpackedTypes.Descriptor; + var fieldDescriptor = messageDescriptor.FindFieldByName("unpacked_int32"); + Assert.NotNull(fieldDescriptor); + Assert.AreEqual(RepeatedFieldEncoding.Expanded, fieldDescriptor.Features.RepeatedFieldEncoding); + } + private static void TestDescriptorToProto(Func toProtoFunction, IMessage expectedProto) { var clone1 = toProtoFunction(); diff --git a/csharp/src/Google.Protobuf.Test/Reflection/FeatureInheritanceTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/FeatureInheritanceTest.cs new file mode 100644 index 0000000000..33458f6321 --- /dev/null +++ b/csharp/src/Google.Protobuf.Test/Reflection/FeatureInheritanceTest.cs @@ -0,0 +1,373 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd +#endregion + +using Google.Protobuf.Reflection; +using NUnit.Framework; +using Pb; +using static Google.Protobuf.Reflection.FieldDescriptorProto.Types; +using Type = Google.Protobuf.Reflection.FieldDescriptorProto.Types.Type; + +namespace Google.Protobuf.Test.Reflection; + +public class FeatureInheritanceTest +{ + // Note: there's no test for file defaults, as we don't have the same access to modify the + // global defaults in C# that exists in Java. + + [Test] + public void FileOverrides() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto, 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.Features)); + } + + [Test] + public void FileMessageInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto, 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.MessageTypes[0].Features)); + } + + [Test] + public void FileMessageOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto, 3); + SetTestFeature(fileProto.MessageType[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.MessageTypes[0].Features)); + } + + [Test] + public void FileEnumInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto, 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.EnumTypes[0].Features)); + } + + [Test] + public void FileEnumOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto, 3); + SetTestFeature(fileProto.EnumType[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.EnumTypes[0].Features)); + } + + [Test] + public void FileExtensionInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto, 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.EnumTypes[0].Features)); + } + + [Test] + public void FileExtensionOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto, 3); + SetTestFeature(fileProto.Extension[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.Extensions.UnorderedExtensions[0].Features)); + } + + [Test] + public void FileServiceInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto, 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.Services[0].Features)); + } + + [Test] + public void FileServiceOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto, 3); + SetTestFeature(fileProto.Service[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.Services[0].Features)); + } + + [Test] + public void MessageFieldInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.MessageTypes[0].Fields.InFieldNumberOrder()[0].Features)); + } + + [Test] + public void MessageFieldOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + SetTestFeature(fileProto.MessageType[0].Field[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.MessageTypes[0].Fields.InFieldNumberOrder()[0].Features)); + } + + [Test] + public void MessageEnumInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.MessageTypes[0].EnumTypes[0].Features)); + } + + [Test] + public void MessageEnumOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + SetTestFeature(fileProto.MessageType[0].EnumType[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.MessageTypes[0].EnumTypes[0].Features)); + } + + [Test] + public void MessageMessageInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.MessageTypes[0].NestedTypes[0].Features)); + } + + [Test] + public void MessageMessageOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + SetTestFeature(fileProto.MessageType[0].NestedType[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.MessageTypes[0].NestedTypes[0].Features)); + } + + [Test] + public void MessageExtensionInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.MessageTypes[0].Extensions.UnorderedExtensions[0].Features)); + } + + [Test] + public void MessageExtensionOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + SetTestFeature(fileProto.MessageType[0].Extension[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.MessageTypes[0].Extensions.UnorderedExtensions[0].Features)); + } + + [Test] + public void MessageOneofInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.MessageTypes[0].Oneofs[0].Features)); + } + + [Test] + public void MessageOneofOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + SetTestFeature(fileProto.MessageType[0].OneofDecl[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.MessageTypes[0].Oneofs[0].Fields[0].Features)); + } + + [Test] + public void OneofFieldInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.MessageTypes[0].Oneofs[0].Fields[0].Features)); + } + + [Test] + public void OneofFieldOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + SetTestFeature(fileProto.MessageType[0].OneofDecl[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.MessageTypes[0].Oneofs[0].Features)); + } + + [Test] + public void EnumValueInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.EnumType[0], 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.EnumTypes[0].Values[0].Features)); + } + + [Test] + public void EnumValueOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.MessageType[0], 3); + SetTestFeature(fileProto.EnumType[0].Value[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.EnumTypes[0].Values[0].Features)); + } + + [Test] + public void ServiceMethodInherit() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.Service[0], 3); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(3, GetTestFeature(fileDescriptor.Services[0].Methods[0].Features)); + } + + [Test] + public void ServiceMethodOverride() + { + var fileProto = CreateFileProto(); + SetTestFeature(fileProto.Service[0], 3); + SetTestFeature(fileProto.Service[0].Method[0], 5); + var fileDescriptor = Build(fileProto); + Assert.AreEqual(5, GetTestFeature(fileDescriptor.Services[0].Methods[0].Features)); + } + + private static int GetTestFeature(FeatureSetDescriptor features) => + (features.Proto.GetExtension(UnittestFeaturesExtensions.Test) ?? new TestFeatures()).IntMultipleFeature; + + private static void SetTestFeature(FileDescriptorProto proto, int value) + { + proto.Options ??= new FileOptions(); + proto.Options.Features ??= new FeatureSet(); + SetTestFeature(proto.Options.Features, value); + } + + private static void SetTestFeature(DescriptorProto proto, int value) + { + proto.Options ??= new MessageOptions(); + proto.Options.Features ??= new FeatureSet(); + SetTestFeature(proto.Options.Features, value); + } + + private static void SetTestFeature(EnumDescriptorProto proto, int value) + { + proto.Options ??= new EnumOptions(); + proto.Options.Features ??= new FeatureSet(); + SetTestFeature(proto.Options.Features, value); + } + + private static void SetTestFeature(EnumValueDescriptorProto proto, int value) + { + proto.Options ??= new EnumValueOptions(); + proto.Options.Features ??= new FeatureSet(); + SetTestFeature(proto.Options.Features, value); + } + + private static void SetTestFeature(FieldDescriptorProto proto, int value) + { + proto.Options ??= new FieldOptions(); + proto.Options.Features ??= new FeatureSet(); + SetTestFeature(proto.Options.Features, value); + } + + private static void SetTestFeature(ServiceDescriptorProto proto, int value) + { + proto.Options ??= new ServiceOptions(); + proto.Options.Features ??= new FeatureSet(); + SetTestFeature(proto.Options.Features, value); + } + + private static void SetTestFeature(OneofDescriptorProto proto, int value) + { + proto.Options ??= new OneofOptions(); + proto.Options.Features ??= new FeatureSet(); + SetTestFeature(proto.Options.Features, value); + } + + private static void SetTestFeature(MethodDescriptorProto proto, int value) + { + proto.Options ??= new MethodOptions(); + proto.Options.Features ??= new FeatureSet(); + SetTestFeature(proto.Options.Features, value); + } + + private static void SetTestFeature(FeatureSet features, int value) => + features.SetExtension(UnittestFeaturesExtensions.Test, new TestFeatures { IntMultipleFeature = value }); + + private static FileDescriptor Build(FileDescriptorProto fileProto) => + FileDescriptor.BuildFromByteStrings(new[] { fileProto.ToByteString() }, new ExtensionRegistry { UnittestFeaturesExtensions.Test })[0]; + + private static FileDescriptorProto CreateFileProto() => new FileDescriptorProto + { + Name = "some/filename/some.proto", + Package = "proto2_unittest", + Edition = Edition._2023, + Syntax = "editions", + Extension = + { + new FieldDescriptorProto { Name = "top_extension", Number = 10, Type = Type.Int32, Label = Label.Optional, Extendee = ".proto2_unittest.TopMessage" } + }, + EnumType = + { + new EnumDescriptorProto { Name = "TopEnum", Value = { new EnumValueDescriptorProto { Name = "TOP_VALUE", Number = 0 } } } + }, + MessageType = + { + new DescriptorProto + { + Name = "TopMessage", + Field = + { + new FieldDescriptorProto { Name = "field", Number = 1, Type = Type.Int32, Label = Label.Optional }, + new FieldDescriptorProto { Name = "oneof_field", Number = 2, Type = Type.Int32, Label = Label.Optional, OneofIndex = 0 } + }, + Extension = + { + new FieldDescriptorProto { Name = "nested_extension", Number = 11, Type = Type.Int32, Label = Label.Optional, Extendee = ".proto2_unittest.TopMessage" } + }, + NestedType = + { + new DescriptorProto { Name = "NestedMessage" }, + }, + EnumType = + { + new EnumDescriptorProto { Name = "NestedEnum", Value = { new EnumValueDescriptorProto { Name = "NESTED_VALUE", Number = 0 } } } + }, + OneofDecl = { new OneofDescriptorProto { Name = "Oneof" } } + } + }, + Service = + { + new ServiceDescriptorProto + { + Name = "TestService", + Method = { new MethodDescriptorProto { Name = "CallMethod", InputType = ".proto2_unittest.TopMessage", OutputType = ".proto2_unittest.TopMessage" } } + } + } + }; +} diff --git a/csharp/src/Google.Protobuf.Test/Reflection/FeatureSetDescriptorTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/FeatureSetDescriptorTest.cs new file mode 100644 index 0000000000..fcc24346fc --- /dev/null +++ b/csharp/src/Google.Protobuf.Test/Reflection/FeatureSetDescriptorTest.cs @@ -0,0 +1,37 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd +#endregion + +using Google.Protobuf.Reflection; +using NUnit.Framework; +using System; +using System.Linq; + +namespace Google.Protobuf.Test.Reflection; + +public class FeatureSetDescriptorTest +{ + // Canonical serialized form of the edition defaults, generated by embed_edition_defaults. + // TODO: Update this automatically. + private const string DefaultsBase64 = "CrEBEqsBCAEQAhgCIAMoATAC6vAEMQj+//////////8BEAEYASABKAEwATgBQAFIAVABWABlzcyMP2oAcAB4AYIBBDIwMjPy8AQxCP7//////////wEQARgBIAEoATABOAFAAUgBUAFYAGXNzIw/agBwAHgBggEEMjAyM/rwBDEI/v//////////ARABGAEgASgBMAE4AUABSAFQAVgAZc3MjD9qAHAAeAGCAQQyMDIzGOYHCrEBEqsBCAIQARgBIAIoATAB6vAEMQj9//////////8BEAEYASABKAEwATgBQAFIAVABWABlzcyMP2oAcAB4AYIBBDIwMjPy8AQxCP3//////////wEQARgBIAEoATABOAFAAUgBUAFYAGXNzIw/agBwAHgBggEEMjAyM/rwBDEI/f//////////ARABGAEgASgBMAE4AUABSAFQAVgAZc3MjD9qAHAAeAGCAQQyMDIzGOcHCsMBEr0BCAEQARgBIAIoATAB6vAENwgBEAEYASABKAEwATgBQAFIAVABWABlzcyMP2oPCAEQAR0AAMA/IgQyMDIzcAF4AYIBBDIwMjPy8AQ3CAEQARgBIAEoATABOAFAAUgBUAFYAGXNzIw/ag8IARABHQAAwD8iBDIwMjNwAXgBggEEMjAyM/rwBDcIARABGAEgASgBMAE4AUABSAFQAVgAZc3MjD9qDwgBEAEdAADAPyIEMjAyM3ABeAGCAQQyMDIzGOgHIOgHKOgH"; + + [Test] + [TestCase(Edition.Proto2)] + [TestCase(Edition.Proto3)] + [TestCase(Edition._2023)] + public void DefaultsMatchCanonicalSerializedForm(Edition edition) + { + var canonicalDefaults = FeatureSetDefaults.Parser + .WithDiscardUnknownFields(true) // Discard language-specific extensions. + .ParseFrom(Convert.FromBase64String(DefaultsBase64)); + var canonicalEditionDefaults = canonicalDefaults.Defaults.Single(def => def.Edition == edition).Features; + var candidateEditionDefaults = FeatureSetDescriptor.GetEditionDefaults(edition).Proto; + + Assert.AreEqual(canonicalEditionDefaults, candidateEditionDefaults); + } +} diff --git a/csharp/src/Google.Protobuf/MessageExtensions.cs b/csharp/src/Google.Protobuf/MessageExtensions.cs index 00d57567f2..910c40ff96 100644 --- a/csharp/src/Google.Protobuf/MessageExtensions.cs +++ b/csharp/src/Google.Protobuf/MessageExtensions.cs @@ -1,4 +1,4 @@ -#region Copyright notice and license +#region Copyright notice and license // Protocol Buffers - Google's data interchange format // Copyright 2015 Google Inc. All rights reserved. // @@ -175,11 +175,11 @@ namespace Google.Protobuf } /// - /// Checks if all required fields in a message have values set. For proto3 messages, this returns true + /// Checks if all required fields in a message have values set. For proto3 messages, this returns true. /// public static bool IsInitialized(this IMessage message) { - if (message.Descriptor.File.Syntax == Syntax.Proto3) + if (message.Descriptor.File.Edition == Edition.Proto3) { return true; } diff --git a/csharp/src/Google.Protobuf/Reflection/DescriptorBase.cs b/csharp/src/Google.Protobuf/Reflection/DescriptorBase.cs index e70b23ad1f..f55740ab5c 100644 --- a/csharp/src/Google.Protobuf/Reflection/DescriptorBase.cs +++ b/csharp/src/Google.Protobuf/Reflection/DescriptorBase.cs @@ -12,19 +12,31 @@ using System.Diagnostics; namespace Google.Protobuf.Reflection { + // Implementation note: The descriptors which don't derive from this class are FileDescriptor + // and FeatureSetDescriptor - the latter of which isn't a descriptor in exactly the same way + // that the others are anyway. + /// /// Base class for nearly all descriptors, providing common functionality. /// [DebuggerDisplay("Type = {GetType().Name,nq}, FullName = {FullName}")] public abstract class DescriptorBase : IDescriptor { - internal DescriptorBase(FileDescriptor file, string fullName, int index) + internal DescriptorBase(FileDescriptor file, string fullName, int index, FeatureSetDescriptor features) { File = file; FullName = fullName; Index = index; + Features = features; } + /// + /// The feature set for this descriptor, including inherited features. + /// This is internal as external users should use the properties on individual + /// descriptor types (e.g. FieldDescriptor.IsPacked) rather than querying features directly. + /// + internal FeatureSetDescriptor Features { get; } + /// /// The index of this descriptor within its parent descriptor. /// diff --git a/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs index c13a65e5b6..293fbb24ec 100644 --- a/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs @@ -19,7 +19,7 @@ namespace Google.Protobuf.Reflection public sealed class EnumDescriptor : DescriptorBase { internal EnumDescriptor(EnumDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index, Type clrType) - : base(file, file.ComputeFullName(parent, proto.Name), index) + : base(file, file.ComputeFullName(parent, proto.Name), index, (parent?.Features ?? file.Features).MergedWith(proto.Options?.Features)) { Proto = proto; ClrType = clrType; @@ -107,7 +107,18 @@ namespace Google.Protobuf.Reflection /// Custom options can be retrieved as extensions of the returned message. /// NOTE: A defensive copy is created each time this property is retrieved. /// - public EnumOptions GetOptions() => Proto.Options?.Clone(); + public EnumOptions GetOptions() + { + var clone = Proto.Options?.Clone(); + if (clone is null) + { + return null; + } + // Clients should be using feature accessor methods, not accessing features on the + // options proto. + clone.Features = null; + return clone; + } /// /// Gets a single value enum option for this descriptor diff --git a/csharp/src/Google.Protobuf/Reflection/EnumValueDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/EnumValueDescriptor.cs index 37a40a991e..ef427c6933 100644 --- a/csharp/src/Google.Protobuf/Reflection/EnumValueDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/EnumValueDescriptor.cs @@ -19,7 +19,7 @@ namespace Google.Protobuf.Reflection { internal EnumValueDescriptor(EnumValueDescriptorProto proto, FileDescriptor file, EnumDescriptor parent, int index) - : base(file, parent.FullName + "." + proto.Name, index) + : base(file, parent.FullName + "." + proto.Name, index, parent.Features.MergedWith(proto.Options?.Features)) { Proto = proto; EnumDescriptor = parent; @@ -64,7 +64,18 @@ namespace Google.Protobuf.Reflection /// Custom options can be retrieved as extensions of the returned message. /// NOTE: A defensive copy is created each time this property is retrieved. /// - public EnumValueOptions GetOptions() => Proto.Options?.Clone(); + public EnumValueOptions GetOptions() + { + var clone = Proto.Options?.Clone(); + if (clone is null) + { + return null; + } + // Clients should be using feature accessor methods, not accessing features on the + // options proto. + clone.Features = null; + return clone; + } /// /// Gets a single value enum value option for this descriptor @@ -86,4 +97,3 @@ namespace Google.Protobuf.Reflection } } } - \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.cs new file mode 100644 index 0000000000..13e922336f --- /dev/null +++ b/csharp/src/Google.Protobuf/Reflection/FeatureSetDescriptor.cs @@ -0,0 +1,118 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file or at +// https://developers.google.com/open-source/licenses/bsd +#endregion + +using System; +using System.Collections.Concurrent; +using static Google.Protobuf.Reflection.FeatureSet.Types; + +namespace Google.Protobuf.Reflection; + +/// +/// A resolved set of features for a file, message etc. +/// +/// +/// Only features supported by the C# runtime are exposed; currently +/// all enums in C# are open, and we never perform UTF-8 validation. +/// If either of those features are ever implemented in this runtime, +/// the feature settings will be exposed as properties in this class. +/// +internal sealed class FeatureSetDescriptor +{ + private static readonly ConcurrentDictionary cache = new(); + + // Note: this approach is deliberately chosen to circumvent bootstrapping issues. + // This can still be tested using the binary representation. + // TODO: Generate this code (as a partial class) from the binary representation. + private static readonly FeatureSetDescriptor edition2023Defaults = new FeatureSetDescriptor( + new FeatureSet + { + EnumType = EnumType.Open, + FieldPresence = FieldPresence.Explicit, + JsonFormat = JsonFormat.Allow, + MessageEncoding = MessageEncoding.LengthPrefixed, + RepeatedFieldEncoding = RepeatedFieldEncoding.Packed, + Utf8Validation = Utf8Validation.Verify, + }); + private static readonly FeatureSetDescriptor proto2Defaults = new FeatureSetDescriptor( + new FeatureSet + { + EnumType = EnumType.Closed, + FieldPresence = FieldPresence.Explicit, + JsonFormat = JsonFormat.LegacyBestEffort, + MessageEncoding = MessageEncoding.LengthPrefixed, + RepeatedFieldEncoding = RepeatedFieldEncoding.Expanded, + Utf8Validation = Utf8Validation.None, + }); + private static readonly FeatureSetDescriptor proto3Defaults = new FeatureSetDescriptor( + new FeatureSet + { + EnumType = EnumType.Open, + FieldPresence = FieldPresence.Implicit, + JsonFormat = JsonFormat.Allow, + MessageEncoding = MessageEncoding.LengthPrefixed, + RepeatedFieldEncoding = RepeatedFieldEncoding.Packed, + Utf8Validation = Utf8Validation.Verify, + }); + + internal static FeatureSetDescriptor GetEditionDefaults(Edition edition) => + edition switch + { + Edition.Proto2 => proto2Defaults, + Edition.Proto3 => proto3Defaults, + Edition._2023 => edition2023Defaults, + _ => throw new ArgumentOutOfRangeException($"Unsupported edition: {edition}") + }; + + // Visible for testing. The underlying feature set proto, usually derived during + // feature resolution. + internal FeatureSet Proto { get; } + + /// + /// Only relevant to fields. Indicates if a field has explicit presence. + /// + internal FieldPresence FieldPresence => Proto.FieldPresence; + + /// + /// Only relevant to fields. Indicates how a repeated field should be encoded. + /// + internal RepeatedFieldEncoding RepeatedFieldEncoding => Proto.RepeatedFieldEncoding; + + /// + /// Only relevant to fields. Indicates how a message-valued field should be encoded. + /// + internal MessageEncoding MessageEncoding => Proto.MessageEncoding; + + private FeatureSetDescriptor(FeatureSet proto) + { + Proto = proto; + } + + /// + /// Returns a new descriptor based on this one, with the specified overrides. + /// Multiple calls to this method that produce equivalent feature sets will return + /// the same instance. + /// + /// The proto representation of the "child" feature set to merge with this + /// one. May be null, in which case this descriptor is returned. + /// A descriptor based on the current one, with the given set of overrides. + public FeatureSetDescriptor MergedWith(FeatureSet overrides) + { + if (overrides is null) + { + return this; + } + + // Note: It would be nice if we could avoid cloning unless + // there are actual changes, but this won't happen that often; + // it'll be temporary garbage. + var clone = Proto.Clone(); + clone.MergeFrom(overrides); + return cache.GetOrAdd(clone, clone => new FeatureSetDescriptor(clone)); + } +} diff --git a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs index 56378bed99..c2c7239461 100644 --- a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs @@ -62,9 +62,10 @@ namespace Google.Protobuf.Reflection : IsRepeated ? false : IsMap ? false : FieldType == FieldType.Message ? true + : FieldType == FieldType.Group ? true // This covers "real oneof members" and "proto3 optional fields" : ContainingOneof != null ? true - : File.Syntax == Syntax.Proto2; + : Features.FieldPresence != FeatureSet.Types.FieldPresence.Implicit; internal FieldDescriptorProto Proto { get; } @@ -83,10 +84,11 @@ namespace Google.Protobuf.Reflection internal FieldDescriptor(FieldDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index, string propertyName, Extension extension) - : base(file, file.ComputeFullName(parent, proto.Name), index) + : base(file, file.ComputeFullName(parent, proto.Name), index, + GetDirectParentFeatures(proto, file, parent).MergedWith(InferFeatures(file, proto)).MergedWith(proto.Options?.Features)) { Proto = proto; - if (proto.Type != 0) + if (proto.HasType) { fieldType = GetFieldTypeFromProtoType(proto.Type); } @@ -117,6 +119,52 @@ namespace Google.Protobuf.Reflection JsonName = Proto.JsonName == "" ? JsonFormatter.ToJsonName(Proto.Name) : Proto.JsonName; } + /// + /// Returns the features from the direct parent: + /// - The file for top-level extensions + /// - The oneof for one-of fields + /// - Otherwise the message + /// + private static FeatureSetDescriptor GetDirectParentFeatures(FieldDescriptorProto proto, FileDescriptor file, MessageDescriptor parent) => + parent is null ? file.Features + // Ignore invalid oneof indexes here; they'll be validated later anyway. + : proto.OneofIndex >= 0 && proto.OneofIndex < parent.Proto.OneofDecl.Count ? parent.Oneofs[proto.OneofIndex].Features + : parent.Features; + + /// + /// Returns a feature set with inferred features for the given field, or null if no features + /// need to be inferred. + /// + private static FeatureSet InferFeatures(FileDescriptor file, FieldDescriptorProto proto) + { + if ((int) file.Edition >= (int) Edition._2023) + { + return null; + } + // This is lazily initialized, as most fields won't need it. + FeatureSet features = null; + if (proto.Label == FieldDescriptorProto.Types.Label.Required) + { + features ??= new FeatureSet(); + features.FieldPresence = FeatureSet.Types.FieldPresence.LegacyRequired; + } + if (proto.Type == FieldDescriptorProto.Types.Type.Group) + { + features ??= new FeatureSet(); + features.MessageEncoding = FeatureSet.Types.MessageEncoding.Delimited; + } + if (file.Edition == Edition.Proto2 && (proto.Options?.Packed ?? false)) + { + features ??= new FeatureSet(); + features.RepeatedFieldEncoding = FeatureSet.Types.RepeatedFieldEncoding.Packed; + } + if (file.Edition == Edition.Proto3 && !(proto.Options?.Packed ?? true)) + { + features ??= new FeatureSet(); + features.RepeatedFieldEncoding = FeatureSet.Types.RepeatedFieldEncoding.Expanded; + } + return features; + } /// /// The brief name of the descriptor's target. @@ -185,7 +233,7 @@ namespace Google.Protobuf.Reflection /// /// Returns true if this field is a required field; false otherwise. /// - public bool IsRequired => Proto.Label == FieldDescriptorProto.Types.Label.Required; + public bool IsRequired => Features.FieldPresence == FeatureSet.Types.FieldPresence.LegacyRequired; /// /// Returns true if this field is a map field; false otherwise. @@ -195,21 +243,7 @@ namespace Google.Protobuf.Reflection /// /// Returns true if this field is a packed, repeated field; false otherwise. /// - public bool IsPacked - { - get - { - if (File.Syntax != Syntax.Proto3) - { - return Proto.Options?.Packed ?? false; - } - else - { - // Packed by default with proto3 - return Proto.Options == null || !Proto.Options.HasPacked || Proto.Options.Packed; - } - } - } + public bool IsPacked => Features.RepeatedFieldEncoding == FeatureSet.Types.RepeatedFieldEncoding.Packed; /// /// Returns true if this field extends another message type; false otherwise. @@ -219,7 +253,8 @@ namespace Google.Protobuf.Reflection /// /// Returns the type of the field. /// - public FieldType FieldType => fieldType; + public FieldType FieldType => fieldType == FieldType.Message && Features.MessageEncoding == FeatureSet.Types.MessageEncoding.Delimited + ? FieldType.Group : fieldType; /// /// Returns the field number declared in the proto file. @@ -299,12 +334,23 @@ namespace Google.Protobuf.Reflection /// Custom options can be retrieved as extensions of the returned message. /// NOTE: A defensive copy is created each time this property is retrieved. /// - public FieldOptions GetOptions() => Proto.Options?.Clone(); + public FieldOptions GetOptions() + { + var clone = Proto.Options?.Clone(); + if (clone is null) + { + return null; + } + // Clients should be using feature accessor methods, not accessing features on the + // options proto. + clone.Features = null; + return clone; + } /// /// Gets a single value field option for this descriptor /// - [Obsolete("GetOption is obsolete. Use the GetOptions() method.")] + [Obsolete("GetOption is obsolete. Use the GetOptions() method.")] public T GetOption(Extension extension) { var value = Proto.Options.GetExtension(extension); @@ -330,7 +376,8 @@ namespace Google.Protobuf.Reflection IDescriptor typeDescriptor = File.DescriptorPool.LookupSymbol(Proto.TypeName, this); - if (Proto.HasType) + // TODO: See how much of this is actually required. + if (!Proto.HasType) { // Choose field type based on symbol. if (typeDescriptor is MessageDescriptor) @@ -423,4 +470,3 @@ namespace Google.Protobuf.Reflection } } } - \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs index 8f257f42ca..c7b307b1c8 100644 --- a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs @@ -21,6 +21,7 @@ namespace Google.Protobuf.Reflection /// /// The syntax of a .proto file /// + [Obsolete("Use features instead")] public enum Syntax { /// @@ -32,6 +33,10 @@ namespace Google.Protobuf.Reflection /// Proto3, /// + /// Editions syntax + /// + Editions, + /// /// An unknown declared syntax /// Unknown @@ -47,7 +52,9 @@ namespace Google.Protobuf.Reflection // Prevent linker failures when using IL2CPP with the well-known types. static FileDescriptor() { +#pragma warning disable CS0618 // Type or member is obsolete ForceReflectionInitialization(); +#pragma warning restore CS0618 // Type or member is obsolete ForceReflectionInitialization(); ForceReflectionInitialization(); ForceReflectionInitialization(); @@ -61,6 +68,9 @@ namespace Google.Protobuf.Reflection SerializedData = descriptorData; DescriptorPool = pool; Proto = proto; + // Note: the Edition property relies on the proto being set first, so this line + // has to come after Proto = proto. + Features = FeatureSetDescriptor.GetEditionDefaults(Edition).MergedWith(proto.Options?.Features); Dependencies = new ReadOnlyCollection(dependencies.ToList()); PublicDependencies = DeterminePublicDependencies(this, proto, dependencies, allowUnknownDependencies); @@ -82,19 +92,6 @@ namespace Google.Protobuf.Reflection Extensions = new ExtensionCollection(this, generatedCodeInfo?.Extensions); declarations = new Lazy>(CreateDeclarationMap, LazyThreadSafetyMode.ExecutionAndPublication); - - if (!proto.HasSyntax || proto.Syntax == "proto2") - { - Syntax = Syntax.Proto2; - } - else if (proto.Syntax == "proto3") - { - Syntax = Syntax.Proto3; - } - else - { - Syntax = Syntax.Unknown; - } } private Dictionary CreateDeclarationMap() @@ -226,9 +223,32 @@ namespace Google.Protobuf.Reflection public FileDescriptorProto ToProto() => Proto.Clone(); /// - /// The syntax of the file + /// The feature set for this file, including inherited features. /// - public Syntax Syntax { get; } + internal FeatureSetDescriptor Features { get; } + + /// + /// Returns the edition of the file descriptor. + /// + internal Edition Edition => Proto.Syntax switch + { + "editions" => Proto.Edition, + "proto3" => Edition.Proto3, + _ => Edition.Proto2 + }; + + /// + /// The syntax of the file. + /// + [Obsolete("Use features instead of proto syntax.")] + public Syntax Syntax => Proto.HasEdition ? Syntax.Editions + : Proto.Syntax switch + { + "proto3" => Syntax.Proto3, + "proto2" => Syntax.Proto2, + "" => Syntax.Proto2, + _ => throw new InvalidOperationException("No edition or known syntax present") + }; /// /// The file name. @@ -546,7 +566,18 @@ namespace Google.Protobuf.Reflection /// Custom options can be retrieved as extensions of the returned message. /// NOTE: A defensive copy is created each time this property is retrieved. /// - public FileOptions GetOptions() => Proto.Options?.Clone(); + public FileOptions GetOptions() + { + var clone = Proto.Options?.Clone(); + if (clone is null) + { + return null; + } + // Clients should be using feature accessor methods, not accessing features on the + // options proto. + clone.Features = null; + return clone; + } /// /// Gets a single value file option for this descriptor diff --git a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs index 37ca0f152e..911bef56ca 100644 --- a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs @@ -13,7 +13,6 @@ using System.Collections.ObjectModel; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; -using System.Reflection; namespace Google.Protobuf.Reflection { @@ -42,7 +41,7 @@ namespace Google.Protobuf.Reflection private Func extensionSetIsInitialized; internal MessageDescriptor(DescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int typeIndex, GeneratedClrTypeInfo generatedCodeInfo) - : base(file, file.ComputeFullName(parent, proto.Name), typeIndex) + : base(file, file.ComputeFullName(parent, proto.Name), typeIndex, (parent?.Features ?? file.Features).MergedWith(proto.Options?.Features)) { Proto = proto; Parser = generatedCodeInfo?.Parser; @@ -280,7 +279,18 @@ namespace Google.Protobuf.Reflection /// Custom options can be retrieved as extensions of the returned message. /// NOTE: A defensive copy is created each time this property is retrieved. /// - public MessageOptions GetOptions() => Proto.Options?.Clone(); + public MessageOptions GetOptions() + { + var clone = Proto.Options?.Clone(); + if (clone is null) + { + return null; + } + // Clients should be using feature accessor methods, not accessing features on the + // options proto. + clone.Features = null; + return clone; + } /// /// Gets a single value message option for this descriptor diff --git a/csharp/src/Google.Protobuf/Reflection/MethodDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MethodDescriptor.cs index 42084cb933..4139a43225 100644 --- a/csharp/src/Google.Protobuf/Reflection/MethodDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MethodDescriptor.cs @@ -17,7 +17,6 @@ namespace Google.Protobuf.Reflection /// public sealed class MethodDescriptor : DescriptorBase { - /// /// The service this method belongs to. /// @@ -55,7 +54,18 @@ namespace Google.Protobuf.Reflection /// Custom options can be retrieved as extensions of the returned message. /// NOTE: A defensive copy is created each time this property is retrieved. /// - public MethodOptions GetOptions() => Proto.Options?.Clone(); + public MethodOptions GetOptions() + { + var clone = Proto.Options?.Clone(); + if (clone is null) + { + return null; + } + // Clients should be using feature accessor methods, not accessing features on the + // options proto. + clone.Features = null; + return clone; + } /// /// Gets a single value method option for this descriptor @@ -78,7 +88,7 @@ namespace Google.Protobuf.Reflection internal MethodDescriptor(MethodDescriptorProto proto, FileDescriptor file, ServiceDescriptor parent, int index) - : base(file, parent.FullName + "." + proto.Name, index) + : base(file, parent.FullName + "." + proto.Name, index, parent.Features.MergedWith(proto.Options?.Features)) { Proto = proto; Service = parent; @@ -118,4 +128,3 @@ namespace Google.Protobuf.Reflection } } } - \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs index f138642984..33b3c943c2 100644 --- a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs @@ -1,4 +1,4 @@ -#region Copyright notice and license +#region Copyright notice and license // Protocol Buffers - Google's data interchange format // Copyright 2015 Google Inc. All rights reserved. // @@ -27,7 +27,7 @@ namespace Google.Protobuf.Reflection private readonly OneofAccessor accessor; internal OneofDescriptor(OneofDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index, string clrName) - : base(file, file.ComputeFullName(parent, proto.Name), index) + : base(file, file.ComputeFullName(parent, proto.Name), index, parent.Features.MergedWith(proto.Options?.Features)) { this.Proto = proto; containingType = parent; @@ -113,7 +113,18 @@ namespace Google.Protobuf.Reflection /// Custom options can be retrieved as extensions of the returned message. /// NOTE: A defensive copy is created each time this property is retrieved. /// - public OneofOptions GetOptions() => Proto.Options?.Clone(); + public OneofOptions GetOptions() + { + var clone = Proto.Options?.Clone(); + if (clone is null) + { + return null; + } + // Clients should be using feature accessor methods, not accessing features on the + // options proto. + clone.Features = null; + return clone; + } /// /// Gets a single value oneof option for this descriptor diff --git a/csharp/src/Google.Protobuf/Reflection/ServiceDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/ServiceDescriptor.cs index a0947e4467..77cc394e97 100644 --- a/csharp/src/Google.Protobuf/Reflection/ServiceDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/ServiceDescriptor.cs @@ -19,7 +19,7 @@ namespace Google.Protobuf.Reflection public sealed class ServiceDescriptor : DescriptorBase { internal ServiceDescriptor(ServiceDescriptorProto proto, FileDescriptor file, int index) - : base(file, file.ComputeFullName(null, proto.Name), index) + : base(file, file.ComputeFullName(null, proto.Name), index, file.Features.MergedWith(proto.Options?.Features)) { Proto = proto; Methods = DescriptorUtil.ConvertAndMakeReadOnly(proto.Method, @@ -75,7 +75,18 @@ namespace Google.Protobuf.Reflection /// Custom options can be retrieved as extensions of the returned message. /// NOTE: A defensive copy is created each time this property is retrieved. /// - public ServiceOptions GetOptions() => Proto.Options?.Clone(); + public ServiceOptions GetOptions() + { + var clone = Proto.Options?.Clone(); + if (clone is null) + { + return null; + } + // Clients should be using feature accessor methods, not accessing features on the + // options proto. + clone.Features = null; + return clone; + } /// /// Gets a single value service option for this descriptor @@ -105,4 +116,3 @@ namespace Google.Protobuf.Reflection } } } - \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs b/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs index fbf9c9fac5..8d6854c8f0 100644 --- a/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs +++ b/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs @@ -1,4 +1,4 @@ -#region Copyright notice and license +#region Copyright notice and license // Protocol Buffers - Google's data interchange format // Copyright 2015 Google Inc. All rights reserved. // @@ -64,8 +64,9 @@ namespace Google.Protobuf.Reflection } }; } - // Primitive fields always support presence in proto2, and support presence in proto3 for optional fields. - else if (descriptor.File.Syntax == Syntax.Proto2 || descriptor.Proto.Proto3Optional) + // Anything else that supports presence should have a "HasXyz" property and a "ClearXyz" + // method. + else if (descriptor.HasPresence) { MethodInfo hasMethod = messageType.GetRuntimeProperty("Has" + property.Name).GetMethod; if (hasMethod == null) @@ -80,8 +81,7 @@ namespace Google.Protobuf.Reflection } clearDelegate = ReflectionUtil.CreateActionIMessage(clearMethod); } - // What's left? - // Primitive proto3 fields without the optional keyword, which aren't in oneofs. + // Otherwise, we don't support presence. else { hasDelegate = message => throw new InvalidOperationException("Presence is not implemented for this field");