From 53c399a1d65df65e9f83a70b55041a01cf8d7489 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Mon, 20 Jul 2015 19:24:31 +0100 Subject: Revamp to reflection. Changes in brief: 1. Descriptor is now the entry point for all reflection. 2. IReflectedMessage has gone; there's now a Descriptor property in IMessage, which is explicitly implemented (due to the static property). 3. FieldAccessorTable has gone away 4. IFieldAccessor and OneofFieldAccessor still exist; we *could* put the functionality straight into FieldDescriptor and OneofDescriptor... I'm unsure about that. 5. There's a temporary property MessageDescriptor.FieldAccessorsByFieldNumber to make the test changes small - we probably want this to go away 6. Discovery for delegates is now via attributes applied to properties and the Clear method of a oneof I'm happy with 1-3. 4 I'm unsure about - feedback welcome. 5 will go away 6 I'm unsure about, both in design and implementation. Should we have a ProtobufMessageAttribute too? Should we find all the relevant attributes in MessageDescriptor and pass them down, to avoid an O(N^2) scenario? Generated code changes coming in the next commit. --- .../Google.Protobuf.Test/GeneratedMessageTest.cs | 47 ++++++----- .../Reflection/DescriptorsTest.cs | 1 + .../WellKnownTypes/WrappersTest.cs | 6 +- csharp/src/Google.Protobuf/Collections/MapField.cs | 3 + csharp/src/Google.Protobuf/Google.Protobuf.csproj | 3 +- csharp/src/Google.Protobuf/IMessage.cs | 18 ++-- csharp/src/Google.Protobuf/JsonFormatter.cs | 26 +++--- .../Google.Protobuf/Reflection/DescriptorUtil.cs | 5 +- .../Google.Protobuf/Reflection/EnumDescriptor.cs | 10 ++- .../Reflection/FieldAccessorBase.cs | 7 +- .../Reflection/FieldAccessorTable.cs | 97 ---------------------- .../Google.Protobuf/Reflection/FieldDescriptor.cs | 28 +++++++ .../Google.Protobuf/Reflection/FileDescriptor.cs | 36 ++++++-- .../Google.Protobuf/Reflection/MapFieldAccessor.cs | 3 +- .../Reflection/MessageDescriptor.cs | 34 +++++++- .../Google.Protobuf/Reflection/OneofAccessor.cs | 10 +-- .../Google.Protobuf/Reflection/OneofDescriptor.cs | 34 ++++++++ .../Reflection/ProtobufFieldAttribute.cs | 58 +++++++++++++ .../Reflection/ProtobufOneofAttribute.cs | 52 ++++++++++++ .../Google.Protobuf/Reflection/ReflectionUtil.cs | 20 +++++ .../Reflection/RepeatedFieldAccessor.cs | 3 +- .../Reflection/SingleFieldAccessor.cs | 5 +- .../protobuf/compiler/csharp/csharp_field_base.cc | 2 +- .../protobuf/compiler/csharp/csharp_helpers.h | 6 +- .../protobuf/compiler/csharp/csharp_map_field.cc | 1 + .../protobuf/compiler/csharp/csharp_message.cc | 84 ++----------------- .../protobuf/compiler/csharp/csharp_message.h | 2 - .../compiler/csharp/csharp_message_field.cc | 2 + .../compiler/csharp/csharp_primitive_field.cc | 2 + .../compiler/csharp/csharp_repeated_enum_field.cc | 3 +- .../csharp/csharp_repeated_message_field.cc | 1 + .../csharp/csharp_repeated_primitive_field.cc | 1 + .../compiler/csharp/csharp_umbrella_class.cc | 37 ++++++--- .../compiler/csharp/csharp_umbrella_class.h | 1 + .../compiler/csharp/csharp_wrapper_field.cc | 2 + 35 files changed, 372 insertions(+), 278 deletions(-) delete mode 100644 csharp/src/Google.Protobuf/Reflection/FieldAccessorTable.cs create mode 100644 csharp/src/Google.Protobuf/Reflection/ProtobufFieldAttribute.cs create mode 100644 csharp/src/Google.Protobuf/Reflection/ProtobufOneofAttribute.cs diff --git a/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs b/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs index acb20b15..180976d1 100644 --- a/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs +++ b/csharp/src/Google.Protobuf.Test/GeneratedMessageTest.cs @@ -604,7 +604,7 @@ namespace Google.Protobuf public void Reflection_GetValue() { var message = SampleMessages.CreateFullTestAllTypes(); - var fields = ((IReflectedMessage) message).Fields; + var fields = TestAllTypes.Descriptor.FieldAccessorsByFieldNumber; Assert.AreEqual(message.SingleBool, fields[TestAllTypes.SingleBoolFieldNumber].GetValue(message)); Assert.AreEqual(message.SingleBytes, fields[TestAllTypes.SingleBytesFieldNumber].GetValue(message)); Assert.AreEqual(message.SingleDouble, fields[TestAllTypes.SingleDoubleFieldNumber].GetValue(message)); @@ -639,7 +639,7 @@ namespace Google.Protobuf // Just a single map field, for the same reason var mapMessage = new TestMap { MapStringString = { { "key1", "value1" }, { "key2", "value2" } } }; - fields = ((IReflectedMessage) mapMessage).Fields; + fields = TestMap.Descriptor.FieldAccessorsByFieldNumber; var dictionary = (IDictionary) fields[TestMap.MapStringStringFieldNumber].GetValue(mapMessage); Assert.AreEqual(mapMessage.MapStringString, dictionary); Assert.AreEqual("value1", dictionary["key1"]); @@ -648,8 +648,8 @@ namespace Google.Protobuf [Test] public void Reflection_Clear() { - IReflectedMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Fields; + var message = SampleMessages.CreateFullTestAllTypes(); + var fields = TestAllTypes.Descriptor.FieldAccessorsByFieldNumber; fields[TestAllTypes.SingleBoolFieldNumber].Clear(message); fields[TestAllTypes.SingleInt32FieldNumber].Clear(message); fields[TestAllTypes.SingleStringFieldNumber].Clear(message); @@ -673,7 +673,7 @@ namespace Google.Protobuf // Separately, maps. var mapMessage = new TestMap { MapStringString = { { "key1", "value1" }, { "key2", "value2" } } }; - fields = ((IReflectedMessage) mapMessage).Fields; + fields = TestMap.Descriptor.FieldAccessorsByFieldNumber; fields[TestMap.MapStringStringFieldNumber].Clear(mapMessage); Assert.AreEqual(0, mapMessage.MapStringString.Count); } @@ -682,8 +682,8 @@ namespace Google.Protobuf public void Reflection_SetValue_SingleFields() { // Just a sample (primitives, messages, enums, strings, byte strings) - IReflectedMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Fields; + var message = SampleMessages.CreateFullTestAllTypes(); + var fields = TestAllTypes.Descriptor.FieldAccessorsByFieldNumber; fields[TestAllTypes.SingleBoolFieldNumber].SetValue(message, false); fields[TestAllTypes.SingleInt32FieldNumber].SetValue(message, 500); fields[TestAllTypes.SingleStringFieldNumber].SetValue(message, "It's a string"); @@ -709,51 +709,52 @@ namespace Google.Protobuf [Test] public void Reflection_SetValue_SingleFields_WrongType() { - IReflectedMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Fields; + IMessage message = SampleMessages.CreateFullTestAllTypes(); + var fields = message.Descriptor.FieldAccessorsByFieldNumber; Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].SetValue(message, "This isn't a bool")); } [Test] public void Reflection_SetValue_MapFields() { - IReflectedMessage message = new TestMap(); - var fields = message.Fields; + IMessage message = new TestMap(); + var fields = message.Descriptor.FieldAccessorsByFieldNumber; Assert.Throws(() => fields[TestMap.MapStringStringFieldNumber].SetValue(message, new Dictionary())); } [Test] public void Reflection_SetValue_RepeatedFields() { - IReflectedMessage message = SampleMessages.CreateFullTestAllTypes(); - var fields = message.Fields; + IMessage message = SampleMessages.CreateFullTestAllTypes(); + var fields = message.Descriptor.FieldAccessorsByFieldNumber; Assert.Throws(() => fields[TestAllTypes.RepeatedDoubleFieldNumber].SetValue(message, new double[10])); } [Test] public void Reflection_GetValue_IncorrectType() { - IReflectedMessage message = SampleMessages.CreateFullTestAllTypes(); - Assert.Throws(() => message.Fields[TestAllTypes.SingleBoolFieldNumber].GetValue(new TestMap())); + IMessage message = SampleMessages.CreateFullTestAllTypes(); + var fields = message.Descriptor.FieldAccessorsByFieldNumber; + Assert.Throws(() => fields[TestAllTypes.SingleBoolFieldNumber].GetValue(new TestMap())); } [Test] public void Reflection_Oneof() { var message = new TestAllTypes(); - var fields = ((IReflectedMessage) message).Fields; - Assert.AreEqual(1, fields.Oneofs.Count); - var oneof = fields.Oneofs[0]; - Assert.AreEqual("oneof_field", oneof.Descriptor.Name); - Assert.IsNull(oneof.GetCaseFieldDescriptor(message)); + var descriptor = TestAllTypes.Descriptor; + Assert.AreEqual(1, descriptor.Oneofs.Count); + var oneof = descriptor.Oneofs[0]; + Assert.AreEqual("oneof_field", oneof.Name); + Assert.IsNull(oneof.Accessor.GetCaseFieldDescriptor(message)); message.OneofString = "foo"; - Assert.AreSame(fields[TestAllTypes.OneofStringFieldNumber].Descriptor, oneof.GetCaseFieldDescriptor(message)); + Assert.AreSame(descriptor.FieldAccessorsByFieldNumber[TestAllTypes.OneofStringFieldNumber].Descriptor, oneof.Accessor.GetCaseFieldDescriptor(message)); message.OneofUint32 = 10; - Assert.AreSame(fields[TestAllTypes.OneofUint32FieldNumber].Descriptor, oneof.GetCaseFieldDescriptor(message)); + Assert.AreSame(descriptor.FieldAccessorsByFieldNumber[TestAllTypes.OneofUint32FieldNumber].Descriptor, oneof.Accessor.GetCaseFieldDescriptor(message)); - oneof.Clear(message); + oneof.Accessor.Clear(message); Assert.AreEqual(TestAllTypes.OneofFieldOneofCase.None, message.OneofFieldCase); } } diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs index 0db01a5e..0aff0a6c 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs @@ -62,6 +62,7 @@ namespace Google.Protobuf.Reflection Assert.AreEqual(UnittestImportProto3.Descriptor, file.Dependencies[0]); MessageDescriptor messageType = TestAllTypes.Descriptor; + Assert.AreSame(typeof(TestAllTypes), messageType.GeneratedType); Assert.AreEqual(messageType, file.MessageTypes[0]); Assert.AreEqual(messageType, file.FindTypeByName("TestAllTypes")); Assert.Null(file.FindTypeByName("NoSuchType")); diff --git a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs index ad88c4eb..c617db36 100644 --- a/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs +++ b/csharp/src/Google.Protobuf.Test/WellKnownTypes/WrappersTest.cs @@ -192,7 +192,7 @@ namespace Google.Protobuf.WellKnownTypes Uint32Field = 3, Uint64Field = 4 }; - var fields = ((IReflectedMessage) message).Fields; + var fields = TestWellKnownTypes.Descriptor.FieldAccessorsByFieldNumber; Assert.AreEqual("x", fields[TestWellKnownTypes.StringFieldFieldNumber].GetValue(message)); Assert.AreEqual(ByteString.CopyFrom(1, 2, 3), fields[TestWellKnownTypes.BytesFieldFieldNumber].GetValue(message)); @@ -216,7 +216,7 @@ namespace Google.Protobuf.WellKnownTypes { // Just a single example... note that we can't have a null value here var message = new RepeatedWellKnownTypes { Int32Field = { 1, 2 } }; - var fields = ((IReflectedMessage) message).Fields; + var fields = RepeatedWellKnownTypes.Descriptor.FieldAccessorsByFieldNumber; var list = (IList) fields[RepeatedWellKnownTypes.Int32FieldFieldNumber].GetValue(message); CollectionAssert.AreEqual(new[] { 1, 2 }, list); } @@ -226,7 +226,7 @@ namespace Google.Protobuf.WellKnownTypes { // Just a single example... note that we can't have a null value here var message = new MapWellKnownTypes { Int32Field = { { 1, 2 }, { 3, null } } }; - var fields = ((IReflectedMessage) message).Fields; + var fields = MapWellKnownTypes.Descriptor.FieldAccessorsByFieldNumber; var dictionary = (IDictionary) fields[MapWellKnownTypes.Int32FieldFieldNumber].GetValue(message); Assert.AreEqual(2, dictionary[1]); Assert.IsNull(dictionary[3]); diff --git a/csharp/src/Google.Protobuf/Collections/MapField.cs b/csharp/src/Google.Protobuf/Collections/MapField.cs index f5e4fd3a..9ca5104d 100644 --- a/csharp/src/Google.Protobuf/Collections/MapField.cs +++ b/csharp/src/Google.Protobuf/Collections/MapField.cs @@ -30,6 +30,7 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion +using Google.Protobuf.Reflection; using System; using System.Collections; using System.Collections.Generic; @@ -559,6 +560,8 @@ namespace Google.Protobuf.Collections { return codec.keyCodec.CalculateSizeWithTag(Key) + codec.valueCodec.CalculateSizeWithTag(Value); } + + MessageDescriptor IMessage.Descriptor { get { return null; } } } } } diff --git a/csharp/src/Google.Protobuf/Google.Protobuf.csproj b/csharp/src/Google.Protobuf/Google.Protobuf.csproj index 431668fe..50756299 100644 --- a/csharp/src/Google.Protobuf/Google.Protobuf.csproj +++ b/csharp/src/Google.Protobuf/Google.Protobuf.csproj @@ -78,7 +78,6 @@ - @@ -91,6 +90,8 @@ + + diff --git a/csharp/src/Google.Protobuf/IMessage.cs b/csharp/src/Google.Protobuf/IMessage.cs index 8c4a4aaf..d179c386 100644 --- a/csharp/src/Google.Protobuf/IMessage.cs +++ b/csharp/src/Google.Protobuf/IMessage.cs @@ -39,15 +39,6 @@ namespace Google.Protobuf // TODO(jonskeet): Do we want a "weak" (non-generic) version of IReflectedMessage? // TODO(jonskeet): Split these interfaces into separate files when we're happy with them. - /// - /// Reflection support for accessing field values. - /// - public interface IReflectedMessage : IMessage - { - FieldAccessorTable Fields { get; } - // TODO(jonskeet): Descriptor? Or a single property which has "all you need for reflection"? - } - /// /// Interface for a Protocol Buffers message, supporting /// basic operations required for serialization. @@ -73,6 +64,13 @@ namespace Google.Protobuf /// The number of bytes required to write this message /// to a coded output stream. int CalculateSize(); + + /// + /// Descriptor for this message. All instances are expected to return the same descriptor, + /// and for generated types this will be an explicitly-implemented member, returning the + /// same value as the static property declared on the type. + /// + MessageDescriptor Descriptor { get; } } /// @@ -81,7 +79,7 @@ namespace Google.Protobuf /// the implementation class. /// /// The message type. - public interface IMessage : IReflectedMessage, IEquatable, IDeepCloneable, IFreezable where T : IMessage + public interface IMessage : IMessage, IEquatable, IDeepCloneable, IFreezable where T : IMessage { /// /// Merges the given message into this one. diff --git a/csharp/src/Google.Protobuf/JsonFormatter.cs b/csharp/src/Google.Protobuf/JsonFormatter.cs index a06e6545..7f13e33e 100644 --- a/csharp/src/Google.Protobuf/JsonFormatter.cs +++ b/csharp/src/Google.Protobuf/JsonFormatter.cs @@ -118,7 +118,7 @@ namespace Google.Protobuf this.settings = settings; } - public string Format(IReflectedMessage message) + public string Format(IMessage message) { ThrowHelper.ThrowIfNull(message, "message"); StringBuilder builder = new StringBuilder(); @@ -129,7 +129,7 @@ namespace Google.Protobuf return builder.ToString(); } - private void WriteMessage(StringBuilder builder, IReflectedMessage message) + private void WriteMessage(StringBuilder builder, IMessage message) { if (message == null) { @@ -137,15 +137,15 @@ namespace Google.Protobuf return; } builder.Append("{ "); - var fields = message.Fields; + var fields = message.Descriptor.Fields; bool first = true; // First non-oneof fields - foreach (var accessor in fields.Accessors) + foreach (var field in fields) { - var descriptor = accessor.Descriptor; + var accessor = field.Accessor; // Oneofs are written later // TODO: Change to write out fields in order, interleaving oneofs appropriately (as per binary format) - if (descriptor.ContainingOneof != null) + if (field.ContainingOneof != null) { continue; } @@ -156,7 +156,7 @@ namespace Google.Protobuf continue; } // Omit awkward (single) values such as unknown enum values - if (!descriptor.IsRepeated && !descriptor.IsMap && !CanWriteSingleValue(accessor.Descriptor, value)) + if (!field.IsRepeated && !field.IsMap && !CanWriteSingleValue(accessor.Descriptor, value)) { continue; } @@ -173,15 +173,15 @@ namespace Google.Protobuf } // Now oneofs - foreach (var accessor in fields.Oneofs) + foreach (var oneof in message.Descriptor.Oneofs) { + var accessor = oneof.Accessor; var fieldDescriptor = accessor.GetCaseFieldDescriptor(message); if (fieldDescriptor == null) { continue; } - var fieldAccessor = fields[fieldDescriptor.FieldNumber]; - object value = fieldAccessor.GetValue(message); + object value = fieldDescriptor.Accessor.GetValue(message); // Omit awkward (single) values such as unknown enum values if (!fieldDescriptor.IsRepeated && !fieldDescriptor.IsMap && !CanWriteSingleValue(fieldDescriptor, value)) { @@ -194,7 +194,7 @@ namespace Google.Protobuf } WriteString(builder, ToCamelCase(fieldDescriptor.Name)); builder.Append(": "); - WriteValue(builder, fieldAccessor, value); + WriteValue(builder, fieldDescriptor.Accessor, value); first = false; } builder.Append(first ? "}" : " }"); @@ -385,7 +385,7 @@ namespace Google.Protobuf } else { - WriteMessage(builder, (IReflectedMessage) value); + WriteMessage(builder, (IMessage) value); } break; default: @@ -406,7 +406,7 @@ namespace Google.Protobuf WriteSingleValue(builder, descriptor.MessageType.FindFieldByNumber(1), value); return; } - WriteMessage(builder, (IReflectedMessage) value); + WriteMessage(builder, (IMessage) value); } private void WriteList(StringBuilder builder, IFieldAccessor accessor, IList list) diff --git a/csharp/src/Google.Protobuf/Reflection/DescriptorUtil.cs b/csharp/src/Google.Protobuf/Reflection/DescriptorUtil.cs index af31dfb1..f5570fc4 100644 --- a/csharp/src/Google.Protobuf/Reflection/DescriptorUtil.cs +++ b/csharp/src/Google.Protobuf/Reflection/DescriptorUtil.cs @@ -50,9 +50,8 @@ namespace Google.Protobuf.Reflection /// Converts the given array into a read-only list, applying the specified conversion to /// each input element. /// - internal static IList ConvertAndMakeReadOnly(IList input, - IndexedConverter - converter) + internal static IList ConvertAndMakeReadOnly + (IList input, IndexedConverter converter) { TOutput[] array = new TOutput[input.Count]; for (int i = 0; i < array.Length; i++) diff --git a/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs index bf8f8c83..285f26f3 100644 --- a/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs @@ -30,6 +30,7 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion +using System; using System.Collections.Generic; namespace Google.Protobuf.Reflection @@ -42,11 +43,13 @@ namespace Google.Protobuf.Reflection private readonly EnumDescriptorProto proto; private readonly MessageDescriptor containingType; private readonly IList values; + private readonly Type generatedType; - internal EnumDescriptor(EnumDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index) + internal EnumDescriptor(EnumDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index, Type generatedType) : base(file, file.ComputeFullName(parent, proto.Name), index) { this.proto = proto; + this.generatedType = generatedType; containingType = parent; if (proto.Value.Count == 0) @@ -69,6 +72,11 @@ namespace Google.Protobuf.Reflection /// public override string Name { get { return proto.Name; } } + /// + /// The generated type for this enum, or null if the descriptor does not represent a generated type. + /// + public Type GeneratedType { get { return generatedType; } } + /// /// If this is a nested type, get the outer descriptor, otherwise null. /// diff --git a/csharp/src/Google.Protobuf/Reflection/FieldAccessorBase.cs b/csharp/src/Google.Protobuf/Reflection/FieldAccessorBase.cs index 39a63b47..0893dc3d 100644 --- a/csharp/src/Google.Protobuf/Reflection/FieldAccessorBase.cs +++ b/csharp/src/Google.Protobuf/Reflection/FieldAccessorBase.cs @@ -43,13 +43,8 @@ namespace Google.Protobuf.Reflection private readonly Func getValueDelegate; private readonly FieldDescriptor descriptor; - internal FieldAccessorBase(Type type, string propertyName, FieldDescriptor descriptor) + internal FieldAccessorBase(PropertyInfo property, FieldDescriptor descriptor) { - PropertyInfo property = type.GetProperty(propertyName); - if (property == null || !property.CanRead) - { - throw new ArgumentException("Not all required properties/methods available"); - } this.descriptor = descriptor; getValueDelegate = ReflectionUtil.CreateFuncObjectObject(property.GetGetMethod()); } diff --git a/csharp/src/Google.Protobuf/Reflection/FieldAccessorTable.cs b/csharp/src/Google.Protobuf/Reflection/FieldAccessorTable.cs deleted file mode 100644 index 24fcbc64..00000000 --- a/csharp/src/Google.Protobuf/Reflection/FieldAccessorTable.cs +++ /dev/null @@ -1,97 +0,0 @@ -#region Copyright notice and license -// Protocol Buffers - Google's data interchange format -// Copyright 2008 Google Inc. All rights reserved. -// https://developers.google.com/protocol-buffers/ -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are -// met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following disclaimer -// in the documentation and/or other materials provided with the -// distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived from -// this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -#endregion - -using System; -using System.Collections.ObjectModel; - -namespace Google.Protobuf.Reflection -{ - /// - /// Provides access to fields in generated messages via reflection. - /// - public sealed class FieldAccessorTable - { - private readonly ReadOnlyCollection accessors; - private readonly ReadOnlyCollection oneofs; - private readonly MessageDescriptor descriptor; - - /// - /// Constructs a FieldAccessorTable for a particular message class. - /// Only one FieldAccessorTable should be constructed per class. - /// - /// The CLR type for the message. - /// The type's descriptor - /// The Pascal-case names of all the field-based properties in the message. - public FieldAccessorTable(Type type, MessageDescriptor descriptor, string[] propertyNames, string[] oneofPropertyNames) - { - this.descriptor = descriptor; - var accessorsArray = new IFieldAccessor[descriptor.Fields.Count]; - for (int i = 0; i < accessorsArray.Length; i++) - { - var field = descriptor.Fields[i]; - var name = propertyNames[i]; - accessorsArray[i] = - field.IsMap ? new MapFieldAccessor(type, name, field) - : field.IsRepeated ? new RepeatedFieldAccessor(type, name, field) - : (IFieldAccessor) new SingleFieldAccessor(type, name, field); - } - accessors = new ReadOnlyCollection(accessorsArray); - var oneofsArray = new OneofAccessor[descriptor.Oneofs.Count]; - for (int i = 0; i < oneofsArray.Length; i++) - { - var oneof = descriptor.Oneofs[i]; - oneofsArray[i] = new OneofAccessor(type, oneofPropertyNames[i], oneof); - } - oneofs = new ReadOnlyCollection(oneofsArray); - } - - // TODO: Validate the name here... should possibly make this type a more "general reflection access" type, - // bearing in mind the oneof parts to come as well. - /// - /// Returns all of the field accessors for the message type. - /// - public ReadOnlyCollection Accessors { get { return accessors; } } - - public ReadOnlyCollection Oneofs { get { return oneofs; } } - - // TODO: Review this, as it's easy to get confused between FieldNumber and Index. - // Currently only used to get an accessor related to a oneof... maybe just make that simpler? - public IFieldAccessor this[int fieldNumber] - { - get - { - FieldDescriptor field = descriptor.FindFieldByNumber(fieldNumber); - return accessors[field.Index]; - } - } - } -} \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs index 3d9d0d75..57378e4c 100644 --- a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs @@ -31,6 +31,7 @@ #endregion using System; +using System.Linq; namespace Google.Protobuf.Reflection { @@ -45,6 +46,7 @@ namespace Google.Protobuf.Reflection private readonly MessageDescriptor containingType; private readonly OneofDescriptor containingOneof; private FieldType fieldType; + private IFieldAccessor accessor; internal FieldDescriptor(FieldDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index) @@ -82,6 +84,8 @@ namespace Google.Protobuf.Reflection public override string Name { get { return proto.Name; } } internal FieldDescriptorProto Proto { get { return proto; } } + + public IFieldAccessor Accessor { get { return accessor; } } /// /// Maps a field type as included in the .proto file to a FieldType. @@ -287,6 +291,30 @@ namespace Google.Protobuf.Reflection { throw new DescriptorValidationException(this, "MessageSet format is not supported."); } + + accessor = CreateAccessor(); + } + + private IFieldAccessor CreateAccessor() + { + // TODO: Check the performance of this with some large protos. Each message is O(N^2) in the number of fields, + // which isn't great... + if (containingType.GeneratedType == null) + { + return null; + } + var property = containingType + .GeneratedType + .GetProperties() + .FirstOrDefault(p => p.IsDefined(typeof(ProtobufFieldAttribute), false) && + p.GetCustomAttributes(typeof(ProtobufFieldAttribute), false).Cast().Single().Number == FieldNumber); + if (property == null) + { + return null; + } + return IsMap ? new MapFieldAccessor(property, this) + : IsRepeated ? new RepeatedFieldAccessor(property, this) + : (IFieldAccessor) new SingleFieldAccessor(property, this); } } } \ 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 db393480..a10e617b 100644 --- a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs @@ -62,11 +62,12 @@ namespace Google.Protobuf.Reflection get { return proto.Syntax == "proto3" ? ProtoSyntax.Proto3 : ProtoSyntax.Proto2; } } - private FileDescriptor(FileDescriptorProto proto, FileDescriptor[] dependencies, DescriptorPool pool, bool allowUnknownDependencies) + private FileDescriptor(FileDescriptorProto proto, FileDescriptor[] dependencies, DescriptorPool pool, bool allowUnknownDependencies, Type[] generatedTypes) { this.pool = pool; this.proto = proto; this.dependencies = new ReadOnlyCollection((FileDescriptor[]) dependencies.Clone()); + IEnumerator generatedTypeIterator = generatedTypes == null ? null : ((IEnumerable)generatedTypes).GetEnumerator(); publicDependencies = DeterminePublicDependencies(this, proto, dependencies, allowUnknownDependencies); @@ -74,15 +75,21 @@ namespace Google.Protobuf.Reflection messageTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.MessageType, (message, index) => - new MessageDescriptor(message, this, null, index)); + new MessageDescriptor(message, this, null, index, generatedTypeIterator)); enumTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.EnumType, (enumType, index) => - new EnumDescriptor(enumType, this, null, index)); + new EnumDescriptor(enumType, this, null, index, ReflectionUtil.GetNextType(generatedTypeIterator))); services = DescriptorUtil.ConvertAndMakeReadOnly(proto.Service, (service, index) => new ServiceDescriptor(service, this, index)); + + // We should now have consumed all the generated types. + if (generatedTypeIterator != null && generatedTypeIterator.MoveNext()) + { + throw new ArgumentException("More generated types left over after consuming all expected ones", "generatedTypes"); + } } /// @@ -265,7 +272,7 @@ namespace Google.Protobuf.Reflection /// If is not /// a valid descriptor. This can occur for a number of reasons, such as a field /// having an undefined type or because two messages were defined with the same name. - private static FileDescriptor BuildFrom(FileDescriptorProto proto, FileDescriptor[] dependencies, bool allowUnknownDependencies) + private static FileDescriptor BuildFrom(FileDescriptorProto proto, FileDescriptor[] dependencies, bool allowUnknownDependencies, Type[] generatedTypes) { // Building descriptors involves two steps: translating and linking. // In the translation step (implemented by FileDescriptor's @@ -282,7 +289,7 @@ namespace Google.Protobuf.Reflection } DescriptorPool pool = new DescriptorPool(dependencies); - FileDescriptor result = new FileDescriptor(proto, dependencies, pool, allowUnknownDependencies); + FileDescriptor result = new FileDescriptor(proto, dependencies, pool, allowUnknownDependencies, generatedTypes); // TODO(jonskeet): Reinstate these checks, or get rid of them entirely. They aren't in the Java code, // and fail for the CustomOptions test right now. (We get "descriptor.proto" vs "google/protobuf/descriptor.proto".) @@ -319,8 +326,23 @@ namespace Google.Protobuf.Reflection } } + /// + /// Creates an instance for generated code. + /// + /// + /// The parameter should be null for descriptors which don't correspond to + /// generated types. Otherwise, the array should be represent all the generated types in the file: messages then + /// enums. Within each message, there can be nested messages and enums, which must be specified "inline" in the array: + /// containing message, nested messages, nested enums - and of course each nested message may contain *more* nested messages, + /// etc. All messages within the descriptor should be represented, even if they do not have a generated type - any + /// type without a corresponding generated type (such as map entries) should respond to a null element. + /// For example, a file with a messages OuterMessage and InnerMessage, and enums OuterEnum and InnerEnum (where + /// InnerMessage and InnerEnum are nested within InnerMessage) would result in an array of + /// OuterMessage, InnerMessage, InnerEnum, OuterEnum. + /// public static FileDescriptor InternalBuildGeneratedFileFrom(byte[] descriptorData, - FileDescriptor[] dependencies) + FileDescriptor[] dependencies, + Type[] generatedTypes) { FileDescriptorProto proto; try @@ -336,7 +358,7 @@ namespace Google.Protobuf.Reflection { // When building descriptors for generated code, we allow unknown // dependencies by default. - return BuildFrom(proto, dependencies, true); + return BuildFrom(proto, dependencies, true, generatedTypes); } catch (DescriptorValidationException e) { diff --git a/csharp/src/Google.Protobuf/Reflection/MapFieldAccessor.cs b/csharp/src/Google.Protobuf/Reflection/MapFieldAccessor.cs index 317fbd8d..6df4c5f0 100644 --- a/csharp/src/Google.Protobuf/Reflection/MapFieldAccessor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MapFieldAccessor.cs @@ -32,6 +32,7 @@ using System; using System.Collections; +using System.Reflection; namespace Google.Protobuf.Reflection { @@ -40,7 +41,7 @@ namespace Google.Protobuf.Reflection /// internal sealed class MapFieldAccessor : FieldAccessorBase { - internal MapFieldAccessor(Type type, string propertyName, FieldDescriptor descriptor) : base(type, propertyName, descriptor) + internal MapFieldAccessor(PropertyInfo property, FieldDescriptor descriptor) : base(property, descriptor) { } diff --git a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs index 1c22c460..9413cf61 100644 --- a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs @@ -30,8 +30,10 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion +using Google.Protobuf.Collections; using System; using System.Collections.Generic; +using System.Linq; namespace Google.Protobuf.Reflection { @@ -60,11 +62,15 @@ namespace Google.Protobuf.Reflection private readonly IList enumTypes; private readonly IList fields; private readonly IList oneofs; + // CLR representation of the type described by this descriptor, if any. + private readonly Type generatedType; + private IDictionary fieldAccessorsByFieldNumber; - internal MessageDescriptor(DescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int typeIndex) + internal MessageDescriptor(DescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int typeIndex, IEnumerator generatedTypeIterator) : base(file, file.ComputeFullName(parent, proto.Name), typeIndex) { this.proto = proto; + generatedType = ReflectionUtil.GetNextType(generatedTypeIterator); containingType = parent; oneofs = DescriptorUtil.ConvertAndMakeReadOnly(proto.OneofDecl, @@ -73,11 +79,11 @@ namespace Google.Protobuf.Reflection nestedTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.NestedType, (type, index) => - new MessageDescriptor(type, file, this, index)); + new MessageDescriptor(type, file, this, index, generatedTypeIterator)); enumTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.EnumType, (type, index) => - new EnumDescriptor(type, file, this, index)); + new EnumDescriptor(type, file, this, index, ReflectionUtil.GetNextType(generatedTypeIterator))); // TODO(jonskeet): Sort fields first? fields = DescriptorUtil.ConvertAndMakeReadOnly(proto.Field, @@ -85,6 +91,14 @@ namespace Google.Protobuf.Reflection new FieldDescriptor(field, file, this, index)); file.DescriptorPool.AddSymbol(this); } + + /// + /// Returns the total number of nested types and enums, recursively. + /// + private int CountTotalGeneratedTypes() + { + return nestedTypes.Sum(nested => nested.CountTotalGeneratedTypes()) + enumTypes.Count; + } /// /// The brief name of the descriptor's target. @@ -93,6 +107,11 @@ namespace Google.Protobuf.Reflection internal DescriptorProto Proto { get { return proto; } } + /// + /// The generated type for this message, or null if the descriptor does not represent a generated type. + /// + public Type GeneratedType { get { return generatedType; } } + /// /// Returns whether this message is one of the "well known types" which may have runtime/protoc support. /// @@ -141,6 +160,13 @@ namespace Google.Protobuf.Reflection get { return oneofs; } } + /// + /// Returns a map from field number to accessor. + /// TODO: Revisit this. It's mostly in place to make the transition from FieldAccessorTable + /// to descriptor-based reflection simple in terms of tests. Work out what we really want. + /// + public IDictionary FieldAccessorsByFieldNumber { get { return fieldAccessorsByFieldNumber; } } + /// /// Finds a field by field name. /// @@ -192,6 +218,8 @@ namespace Google.Protobuf.Reflection { oneof.CrossLink(); } + + fieldAccessorsByFieldNumber = new ReadOnlyDictionary(fields.ToDictionary(field => field.FieldNumber, field => field.Accessor)); } } } \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/Reflection/OneofAccessor.cs b/csharp/src/Google.Protobuf/Reflection/OneofAccessor.cs index 7a11d36b..20cbea92 100644 --- a/csharp/src/Google.Protobuf/Reflection/OneofAccessor.cs +++ b/csharp/src/Google.Protobuf/Reflection/OneofAccessor.cs @@ -44,18 +44,16 @@ namespace Google.Protobuf.Reflection private readonly Action clearDelegate; private OneofDescriptor descriptor; - internal OneofAccessor(Type type, string propertyName, OneofDescriptor descriptor) + internal OneofAccessor(PropertyInfo caseProperty, MethodInfo clearMethod, OneofDescriptor descriptor) { - PropertyInfo property = type.GetProperty(propertyName + "Case"); - if (property == null || !property.CanRead) + if (!caseProperty.CanRead) { - throw new ArgumentException("Not all required properties/methods available"); + throw new ArgumentException("Cannot read from property"); } this.descriptor = descriptor; - caseDelegate = ReflectionUtil.CreateFuncObjectT(property.GetGetMethod()); + caseDelegate = ReflectionUtil.CreateFuncObjectT(caseProperty.GetGetMethod()); this.descriptor = descriptor; - MethodInfo clearMethod = type.GetMethod("Clear" + propertyName); clearDelegate = ReflectionUtil.CreateActionObject(clearMethod); } diff --git a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs index e92dc8bb..b4cc0791 100644 --- a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs @@ -32,6 +32,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Linq; namespace Google.Protobuf.Reflection { @@ -40,6 +41,7 @@ namespace Google.Protobuf.Reflection private readonly OneofDescriptorProto proto; private MessageDescriptor containingType; private IList fields; + private OneofAccessor accessor; internal OneofDescriptor(OneofDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index) : base(file, file.ComputeFullName(parent, proto.Name), index) @@ -62,6 +64,8 @@ namespace Google.Protobuf.Reflection public IList Fields { get { return fields; } } + public OneofAccessor Accessor { get { return accessor; } } + internal void CrossLink() { List fieldCollection = new List(); @@ -73,6 +77,36 @@ namespace Google.Protobuf.Reflection } } fields = new ReadOnlyCollection(fieldCollection); + accessor = CreateAccessor(); + } + + private OneofAccessor CreateAccessor() + { + if (containingType.GeneratedType == null) + { + return null; + } + var caseProperty = containingType + .GeneratedType + .GetProperties() + .FirstOrDefault(p => p.IsDefined(typeof(ProtobufOneofAttribute), false) && + p.GetCustomAttributes(typeof(ProtobufOneofAttribute), false).Cast().Single().Name == Name); + if (caseProperty == null) + { + return null; + } + + var clearMethod = containingType + .GeneratedType + .GetMethods() + .FirstOrDefault(p => p.IsDefined(typeof(ProtobufOneofAttribute), false) && + p.GetCustomAttributes(typeof(ProtobufOneofAttribute), false).Cast().Single().Name == Name); + if (clearMethod == null) + { + return null; + } + + return new OneofAccessor(caseProperty, clearMethod, this); } } } diff --git a/csharp/src/Google.Protobuf/Reflection/ProtobufFieldAttribute.cs b/csharp/src/Google.Protobuf/Reflection/ProtobufFieldAttribute.cs new file mode 100644 index 00000000..a59caea7 --- /dev/null +++ b/csharp/src/Google.Protobuf/Reflection/ProtobufFieldAttribute.cs @@ -0,0 +1,58 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2015 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#endregion +using System; + +namespace Google.Protobuf.Reflection +{ + /// + /// Attribute applied to a generated property corresponding to a field in a .proto file. + /// + [AttributeUsage(AttributeTargets.Property, AllowMultiple = false)] + public sealed class ProtobufFieldAttribute : Attribute + { + /// + /// The field number in the original .proto file. + /// + public int Number { get; set; } + + /// + /// The field name in the original .proto file. + /// + public string Name { get; set; } + + public ProtobufFieldAttribute(int number, string name) + { + this.Number = number; + this.Name = name; + } + } +} diff --git a/csharp/src/Google.Protobuf/Reflection/ProtobufOneofAttribute.cs b/csharp/src/Google.Protobuf/Reflection/ProtobufOneofAttribute.cs new file mode 100644 index 00000000..74ad8c53 --- /dev/null +++ b/csharp/src/Google.Protobuf/Reflection/ProtobufOneofAttribute.cs @@ -0,0 +1,52 @@ +#region Copyright notice and license +// Protocol Buffers - Google's data interchange format +// Copyright 2015 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +#endregion +using System; + +namespace Google.Protobuf.Reflection +{ + /// + /// Attribute applied to the "case" property or "clear" method corresponding to a oneof in a .proto file. + /// + [AttributeUsage(AttributeTargets.Property | AttributeTargets.Method, AllowMultiple = false)] + public sealed class ProtobufOneofAttribute : Attribute + { + /// + /// The oneof name in the original .proto file. + /// + public string Name { get; set; } + + public ProtobufOneofAttribute(string name) + { + this.Name = name; + } + } +} diff --git a/csharp/src/Google.Protobuf/Reflection/ReflectionUtil.cs b/csharp/src/Google.Protobuf/Reflection/ReflectionUtil.cs index d0dc3e8b..ec222dc1 100644 --- a/csharp/src/Google.Protobuf/Reflection/ReflectionUtil.cs +++ b/csharp/src/Google.Protobuf/Reflection/ReflectionUtil.cs @@ -31,6 +31,7 @@ #endregion using System; +using System.Collections.Generic; using System.Linq.Expressions; using System.Reflection; @@ -102,5 +103,24 @@ namespace Google.Protobuf.Reflection Expression call = Expression.Call(castTarget, method); return Expression.Lambda>(call, targetParameter).Compile(); } + + /// + /// Returns the next type from an iterator of types, unless the iterator is a null reference, + /// in which case null is returned. + /// + internal static Type GetNextType(IEnumerator generatedTypeIterator) + { + if (generatedTypeIterator == null) + { + return null; + } + if (!generatedTypeIterator.MoveNext()) + { + // This parameter name corresponds to any public method supplying the generated types to start with. + throw new ArgumentException("More generated types left over after consuming all expected ones", "generatedTypes"); + } + return generatedTypeIterator.Current; + } + } } \ No newline at end of file diff --git a/csharp/src/Google.Protobuf/Reflection/RepeatedFieldAccessor.cs b/csharp/src/Google.Protobuf/Reflection/RepeatedFieldAccessor.cs index 0ada7567..acb3c8d5 100644 --- a/csharp/src/Google.Protobuf/Reflection/RepeatedFieldAccessor.cs +++ b/csharp/src/Google.Protobuf/Reflection/RepeatedFieldAccessor.cs @@ -32,6 +32,7 @@ using System; using System.Collections; +using System.Reflection; namespace Google.Protobuf.Reflection { @@ -40,7 +41,7 @@ namespace Google.Protobuf.Reflection /// internal sealed class RepeatedFieldAccessor : FieldAccessorBase { - internal RepeatedFieldAccessor(Type type, string propertyName, FieldDescriptor descriptor) : base(type, propertyName, descriptor) + internal RepeatedFieldAccessor(PropertyInfo property, FieldDescriptor descriptor) : base(property, descriptor) { } diff --git a/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs b/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs index 8c24e46e..f00a51ba 100644 --- a/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs +++ b/csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs @@ -48,11 +48,8 @@ namespace Google.Protobuf.Reflection private readonly Action setValueDelegate; private readonly Action clearDelegate; - internal SingleFieldAccessor(Type type, string propertyName, FieldDescriptor descriptor) : base(type, propertyName, descriptor) + internal SingleFieldAccessor(PropertyInfo property, FieldDescriptor descriptor) : base(property, descriptor) { - PropertyInfo property = type.GetProperty(propertyName); - // We know there *is* such a property, or the base class constructor would have thrown. We should be able to write - // to it though. if (!property.CanWrite) { throw new ArgumentException("Not all required properties/methods available"); diff --git a/src/google/protobuf/compiler/csharp/csharp_field_base.cc b/src/google/protobuf/compiler/csharp/csharp_field_base.cc index 89d4eb18..d327e267 100644 --- a/src/google/protobuf/compiler/csharp/csharp_field_base.cc +++ b/src/google/protobuf/compiler/csharp/csharp_field_base.cc @@ -74,6 +74,7 @@ void FieldGeneratorBase::SetCommonFieldVariables( (*variables)["property_name"] = property_name(); (*variables)["type_name"] = type_name(); + (*variables)["original_name"] = descriptor_->name(); (*variables)["name"] = name(); (*variables)["descriptor_name"] = descriptor_->name(); (*variables)["default_value"] = default_value(); @@ -85,7 +86,6 @@ void FieldGeneratorBase::SetCommonFieldVariables( } (*variables)["capitalized_type_name"] = capitalized_type_name(); (*variables)["number"] = number(); - (*variables)["field_ordinal"] = field_ordinal(); (*variables)["has_property_check"] = (*variables)["property_name"] + " != " + (*variables)["default_value"]; (*variables)["other_has_property_check"] = "other." + diff --git a/src/google/protobuf/compiler/csharp/csharp_helpers.h b/src/google/protobuf/compiler/csharp/csharp_helpers.h index 653ff992..7737ffe2 100644 --- a/src/google/protobuf/compiler/csharp/csharp_helpers.h +++ b/src/google/protobuf/compiler/csharp/csharp_helpers.h @@ -77,6 +77,8 @@ std::string GetFullUmbrellaClassName(const FileDescriptor* descriptor); std::string GetQualifiedUmbrellaClassName(const FileDescriptor* descriptor); +std::string GetClassName(const Descriptor* descriptor); + std::string GetClassName(const EnumDescriptor* descriptor); std::string GetFieldName(const FieldDescriptor* descriptor); @@ -119,10 +121,6 @@ inline bool IsDescriptorProto(const FileDescriptor* descriptor) { return descriptor->name() == "google/protobuf/descriptor_proto_file.proto"; } -inline bool IsMapEntry(const Descriptor* descriptor) { - return descriptor->options().map_entry(); -} - inline bool IsWrapperType(const FieldDescriptor* descriptor) { return descriptor->type() == FieldDescriptor::TYPE_MESSAGE && descriptor->message_type()->file()->name() == "google/protobuf/wrappers.proto"; diff --git a/src/google/protobuf/compiler/csharp/csharp_map_field.cc b/src/google/protobuf/compiler/csharp/csharp_map_field.cc index cba24a59..bdbfd92b 100644 --- a/src/google/protobuf/compiler/csharp/csharp_map_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_map_field.cc @@ -79,6 +79,7 @@ void MapFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ pbc::MapField<$key_type_name$, $value_type_name$> $property_name$ {\n" " get { return $name$_; }\n" "}\n"); diff --git a/src/google/protobuf/compiler/csharp/csharp_message.cc b/src/google/protobuf/compiler/csharp/csharp_message.cc index 10cf3585..42fd5065 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message.cc +++ b/src/google/protobuf/compiler/csharp/csharp_message.cc @@ -96,88 +96,11 @@ const std::vector& MessageGenerator::fields_by_number() return fields_by_number_; } -/// Get an identifier that uniquely identifies this type within the file. -/// This is used to declare static variables related to this type at the -/// outermost file scope. -std::string GetUniqueFileScopeIdentifier(const Descriptor* descriptor) { - std::string result = descriptor->full_name(); - std::replace(result.begin(), result.end(), '.', '_'); - return "static_" + result; -} - -void MessageGenerator::GenerateStaticVariables(io::Printer* printer) { - // Because descriptor.proto (Google.Protobuf.DescriptorProtos) is - // used in the construction of descriptors, we have a tricky bootstrapping - // problem. To help control static initialization order, we make sure all - // descriptors and other static data that depends on them are members of - // the proto-descriptor class. This way, they will be initialized in - // a deterministic order. - - std::string identifier = GetUniqueFileScopeIdentifier(descriptor_); - - // The descriptor for this type. - printer->Print( - "internal static pbr::FieldAccessorTable internal__$identifier$__FieldAccessorTable;\n", - "identifier", GetUniqueFileScopeIdentifier(descriptor_), - "full_class_name", full_class_name()); - - for (int i = 0; i < descriptor_->nested_type_count(); i++) { - // Don't generate accessor table fields for maps... - if (!IsMapEntryMessage(descriptor_->nested_type(i))) { - MessageGenerator messageGenerator(descriptor_->nested_type(i)); - messageGenerator.GenerateStaticVariables(printer); - } - } -} - -void MessageGenerator::GenerateStaticVariableInitializers(io::Printer* printer) { - map vars; - vars["identifier"] = GetUniqueFileScopeIdentifier(descriptor_); - vars["full_class_name"] = full_class_name(); - - // Work out how to get to the message descriptor (which may be multiply nested) from the file - // descriptor. - string descriptor_chain; - const Descriptor* current_descriptor = descriptor_; - while (current_descriptor->containing_type()) { - descriptor_chain = ".NestedTypes[" + SimpleItoa(current_descriptor->index()) + "]" + descriptor_chain; - current_descriptor = current_descriptor->containing_type(); - } - descriptor_chain = "descriptor.MessageTypes[" + SimpleItoa(current_descriptor->index()) + "]" + descriptor_chain; - vars["descriptor_chain"] = descriptor_chain; - - printer->Print( - vars, - "internal__$identifier$__FieldAccessorTable = \n" - " new pbr::FieldAccessorTable(typeof($full_class_name$), $descriptor_chain$,\n"); - printer->Print(" new string[] { "); - for (int i = 0; i < descriptor_->field_count(); i++) { - printer->Print("\"$property_name$\", ", - "property_name", GetPropertyName(descriptor_->field(i))); - } - printer->Print("}, new string[] { "); - for (int i = 0; i < descriptor_->oneof_decl_count(); i++) { - printer->Print("\"$oneof_name$\", ", - "oneof_name", - UnderscoresToCamelCase(descriptor_->oneof_decl(i)->name(), true)); - } - printer->Print("});\n"); - - // Generate static member initializers for all non-map-entry nested types. - for (int i = 0; i < descriptor_->nested_type_count(); i++) { - if (!IsMapEntryMessage(descriptor_->nested_type(i))) { - MessageGenerator messageGenerator(descriptor_->nested_type(i)); - messageGenerator.GenerateStaticVariableInitializers(printer); - } - } -} - void MessageGenerator::Generate(io::Printer* printer) { map vars; vars["class_name"] = class_name(); vars["access_level"] = class_access_level(); vars["umbrella_class_name"] = GetFullUmbrellaClassName(descriptor_->file()); - vars["identifier"] = GetUniqueFileScopeIdentifier(descriptor_); printer->Print( "[global::System.Diagnostics.DebuggerNonUserCodeAttribute()]\n"); @@ -221,8 +144,8 @@ void MessageGenerator::Generate(io::Printer* printer) { " get { return $descriptor_accessor$; }\n" "}\n" "\n" - "pbr::FieldAccessorTable pb::IReflectedMessage.Fields {\n" - " get { return $umbrella_class_name$.internal__$identifier$__FieldAccessorTable; }\n" + "pbr::MessageDescriptor pb::IMessage.Descriptor {\n" + " get { return Descriptor; }\n" "}\n" "\n" "private bool _frozen = false;\n" @@ -258,6 +181,7 @@ void MessageGenerator::Generate(io::Printer* printer) { for (int i = 0; i < descriptor_->oneof_decl_count(); i++) { vars["name"] = UnderscoresToCamelCase(descriptor_->oneof_decl(i)->name(), false); vars["property_name"] = UnderscoresToCamelCase(descriptor_->oneof_decl(i)->name(), true); + vars["original_name"] = descriptor_->oneof_decl(i)->name(); printer->Print( vars, "private object $name$_;\n" @@ -275,9 +199,11 @@ void MessageGenerator::Generate(io::Printer* printer) { printer->Print( vars, "private $property_name$OneofCase $name$Case_ = $property_name$OneofCase.None;\n" + "[pbr::ProtobufOneof(\"$original_name$\")]\n" "public $property_name$OneofCase $property_name$Case {\n" " get { return $name$Case_; }\n" "}\n\n" + "[pbr::ProtobufOneof(\"$original_name$\")]\n" "public void Clear$property_name$() {\n" " pb::Freezable.CheckMutable(this);\n" " $name$Case_ = $property_name$OneofCase.None;\n" diff --git a/src/google/protobuf/compiler/csharp/csharp_message.h b/src/google/protobuf/compiler/csharp/csharp_message.h index fbe8a3be..f0c49ac9 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message.h +++ b/src/google/protobuf/compiler/csharp/csharp_message.h @@ -53,8 +53,6 @@ class MessageGenerator : public SourceGeneratorBase { void GenerateCloningCode(io::Printer* printer); void GenerateFreezingCode(io::Printer* printer); void GenerateFrameworkMethods(io::Printer* printer); - void GenerateStaticVariables(io::Printer* printer); - void GenerateStaticVariableInitializers(io::Printer* printer); void Generate(io::Printer* printer); private: diff --git a/src/google/protobuf/compiler/csharp/csharp_message_field.cc b/src/google/protobuf/compiler/csharp/csharp_message_field.cc index d2c3a88b..c2b6ff76 100644 --- a/src/google/protobuf/compiler/csharp/csharp_message_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_message_field.cc @@ -64,6 +64,7 @@ void MessageFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $name$_; }\n" " set {\n" @@ -158,6 +159,7 @@ void MessageOneofFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $has_property_check$ ? ($type_name$) $oneof_name$_ : null; }\n" " set {\n" diff --git a/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc b/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc index 4454ef02..2c9338bd 100644 --- a/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_primitive_field.cc @@ -71,6 +71,7 @@ void PrimitiveFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $name$_; }\n" " set {\n" @@ -174,6 +175,7 @@ void PrimitiveOneofFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $has_property_check$ ? ($type_name$) $oneof_name$_ : $default_value$; }\n" " set {\n" diff --git a/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc b/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc index d5fc6d98..60d06154 100644 --- a/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_repeated_enum_field.cc @@ -59,12 +59,13 @@ void RepeatedEnumFieldGenerator::GenerateMembers(io::Printer* printer) { printer->Print( variables_, "private static readonly pb::FieldCodec<$type_name$> _repeated_$name$_codec\n" - " = pb::FieldCodec.ForEnum($tag$, x => (int) x, x => ($type_name$) x);"); + " = pb::FieldCodec.ForEnum($tag$, x => (int) x, x => ($type_name$) x);\n"); printer->Print(variables_, "private readonly pbc::RepeatedField<$type_name$> $name$_ = new pbc::RepeatedField<$type_name$>();\n"); AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ pbc::RepeatedField<$type_name$> $property_name$ {\n" " get { return $name$_; }\n" "}\n"); diff --git a/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc b/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc index d939fc79..921798b0 100644 --- a/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_repeated_message_field.cc @@ -78,6 +78,7 @@ void RepeatedMessageFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ pbc::RepeatedField<$type_name$> $property_name$ {\n" " get { return $name$_; }\n" "}\n"); diff --git a/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc b/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc index 5b5d9b3d..bcfb9936 100644 --- a/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_repeated_primitive_field.cc @@ -65,6 +65,7 @@ void RepeatedPrimitiveFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ pbc::RepeatedField<$type_name$> $property_name$ {\n" " get { return $name$_; }\n" "}\n"); diff --git a/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc b/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc index 57271d17..3f39250e 100644 --- a/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc +++ b/src/google/protobuf/compiler/csharp/csharp_umbrella_class.cc @@ -63,12 +63,6 @@ UmbrellaClassGenerator::~UmbrellaClassGenerator() { void UmbrellaClassGenerator::Generate(io::Printer* printer) { WriteIntroduction(printer); - printer->Print("#region Static variables\n"); - for (int i = 0; i < file_->message_type_count(); i++) { - MessageGenerator messageGenerator(file_->message_type(i)); - messageGenerator.GenerateStaticVariables(printer); - } - printer->Print("#endregion\n"); WriteDescriptor(printer); // Close the class declaration. printer->Outdent(); @@ -183,24 +177,43 @@ void UmbrellaClassGenerator::WriteDescriptor(io::Printer* printer) { // Invoke InternalBuildGeneratedFileFrom() to build the file. printer->Print( "descriptor = pbr::FileDescriptor.InternalBuildGeneratedFileFrom(descriptorData,\n"); - printer->Print(" new pbr::FileDescriptor[] {\n"); + printer->Print(" new pbr::FileDescriptor[] { "); for (int i = 0; i < file_->dependency_count(); i++) { printer->Print( - " $full_umbrella_class_name$.Descriptor, \n", + "$full_umbrella_class_name$.Descriptor, ", "full_umbrella_class_name", GetFullUmbrellaClassName(file_->dependency(i))); } - printer->Print(" });\n"); - // Then invoke any other static variable initializers, e.g. field accessors. + // Specify all the generated types (messages and enums), recursively, as an array. + printer->Print("},\n" + " new global::System.Type[] { "); for (int i = 0; i < file_->message_type_count(); i++) { - MessageGenerator messageGenerator(file_->message_type(i)); - messageGenerator.GenerateStaticVariableInitializers(printer); + WriteTypeLiterals(file_->message_type(i), printer); } + for (int i = 0; i < file_->enum_type_count(); i++) { + printer->Print("typeof($type_name$), ", "type_name", GetClassName(file_->enum_type(i))); + } + printer->Print("});\n"); + printer->Outdent(); printer->Print("}\n"); printer->Print("#endregion\n\n"); } +void UmbrellaClassGenerator::WriteTypeLiterals(const Descriptor* descriptor, io::Printer* printer) { + if (IsMapEntryMessage(descriptor)) { + printer->Print("null, "); + return; + } + printer->Print("typeof($type_name$), ", "type_name", GetClassName(descriptor)); + for (int i = 0; i < descriptor->nested_type_count(); i++) { + WriteTypeLiterals(descriptor->nested_type(i), printer); + } + for (int i = 0; i < descriptor->enum_type_count(); i++) { + printer->Print("typeof($type_name$), ", "type_name", GetClassName(descriptor->enum_type(i))); + } +} + } // namespace csharp } // namespace compiler } // namespace protobuf diff --git a/src/google/protobuf/compiler/csharp/csharp_umbrella_class.h b/src/google/protobuf/compiler/csharp/csharp_umbrella_class.h index f7533d2d..db1092a9 100644 --- a/src/google/protobuf/compiler/csharp/csharp_umbrella_class.h +++ b/src/google/protobuf/compiler/csharp/csharp_umbrella_class.h @@ -57,6 +57,7 @@ class UmbrellaClassGenerator : public SourceGeneratorBase { void WriteIntroduction(io::Printer* printer); void WriteDescriptor(io::Printer* printer); + void WriteTypeLiterals(const Descriptor* descriptor, io::Printer* printer); GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(UmbrellaClassGenerator); }; diff --git a/src/google/protobuf/compiler/csharp/csharp_wrapper_field.cc b/src/google/protobuf/compiler/csharp/csharp_wrapper_field.cc index 75ef5e50..d6cd0a10 100644 --- a/src/google/protobuf/compiler/csharp/csharp_wrapper_field.cc +++ b/src/google/protobuf/compiler/csharp/csharp_wrapper_field.cc @@ -73,6 +73,7 @@ void WrapperFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $name$_; }\n" " set {\n" @@ -169,6 +170,7 @@ void WrapperOneofFieldGenerator::GenerateMembers(io::Printer* printer) { AddDeprecatedFlag(printer); printer->Print( variables_, + "[pbr::ProtobufField($number$, \"$original_name$\")]\n" "$access_level$ $type_name$ $property_name$ {\n" " get { return $has_property_check$ ? ($type_name$) $oneof_name$_ : ($type_name$) null; }\n" " set {\n" -- cgit v1.2.3