From 72ec33676fd40ccfe719ace162fcf859ae9251bc Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Thu, 19 Nov 2015 17:13:38 +0000 Subject: Tidy up reflection in advance of attempting to implement DynamicMessage. There are corner cases where MessageDescriptor.{ClrType,Parser} will return null, and these are now documented. However, normally they *should* be implemented, even for descriptors of for dynamic messages. Ditto FieldDescriptor.Accessor. We'll still need a fair amount of work to implement dynamic messages, but this change means that the public API will be remain intact. Additionally, this change starts making use of C# 6 features in the files that it touches. This is far from exhaustive, and later PRs will have more. Generated code changes coming in the next commit. --- .../Reflection/DescriptorsTest.cs | 18 +-- csharp/src/Google.Protobuf/JsonFormatter.cs | 3 +- csharp/src/Google.Protobuf/JsonParser.cs | 3 +- .../Google.Protobuf/Reflection/EnumDescriptor.cs | 8 +- .../Google.Protobuf/Reflection/FieldDescriptor.cs | 33 +++-- .../Google.Protobuf/Reflection/FileDescriptor.cs | 128 ++++++---------- .../Reflection/GeneratedCodeInfo.cs | 41 +++++- .../Reflection/MessageDescriptor.cs | 162 +++++++++------------ .../Google.Protobuf/Reflection/OneofDescriptor.cs | 15 +- 9 files changed, 187 insertions(+), 224 deletions(-) (limited to 'csharp/src') diff --git a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs index 51eff053..086a4e98 100644 --- a/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs +++ b/csharp/src/Google.Protobuf.Test/Reflection/DescriptorsTest.cs @@ -63,7 +63,7 @@ namespace Google.Protobuf.Reflection Assert.AreEqual(UnittestImportProto3Reflection.Descriptor, file.Dependencies[0]); MessageDescriptor messageType = TestAllTypes.Descriptor; - Assert.AreSame(typeof(TestAllTypes), messageType.GeneratedType); + Assert.AreSame(typeof(TestAllTypes), messageType.ClrType); Assert.AreSame(TestAllTypes.Parser, messageType.Parser); Assert.AreEqual(messageType, file.MessageTypes[0]); Assert.AreEqual(messageType, file.FindTypeByName("TestAllTypes")); @@ -227,18 +227,12 @@ namespace Google.Protobuf.Reflection } [Test] - public void ConstructionWithoutGeneratedCodeInfo() + public void MapEntryMessageDescriptor() { - var data = UnittestIssuesReflection.Descriptor.Proto.ToByteArray(); - var newDescriptor = Google.Protobuf.Reflection.FileDescriptor.InternalBuildGeneratedFileFrom(data, new Reflection.FileDescriptor[] { }, null); - - // We should still be able to get at a field... - var messageDescriptor = newDescriptor.FindTypeByName("ItemField"); - var fieldDescriptor = messageDescriptor.FindFieldByName("item"); - // But there shouldn't be an accessor (or a generated type for the message, or parser) - Assert.IsNull(fieldDescriptor.Accessor); - Assert.IsNull(messageDescriptor.GeneratedType); - Assert.IsNull(messageDescriptor.Parser); + var descriptor = MapWellKnownTypes.Descriptor.NestedTypes[0]; + Assert.IsNull(descriptor.Parser); + Assert.IsNull(descriptor.ClrType); + Assert.IsNull(descriptor.Fields[1].Accessor); } // From TestFieldOrdering: diff --git a/csharp/src/Google.Protobuf/JsonFormatter.cs b/csharp/src/Google.Protobuf/JsonFormatter.cs index 08e3142e..51bb4bf3 100644 --- a/csharp/src/Google.Protobuf/JsonFormatter.cs +++ b/csharp/src/Google.Protobuf/JsonFormatter.cs @@ -388,8 +388,7 @@ namespace Google.Protobuf // If it's the message form, we can extract the value first, which *will* be the (possibly boxed) native value, // and then proceed, writing it as if we were definitely in a field. (We never need to wrap it in an extra string... // WriteValue will do the right thing.) - // TODO: Detect this differently when we have dynamic messages. - if (descriptor.File == Int32Value.Descriptor.File) + if (descriptor.IsWrapperType) { if (value is IMessage) { diff --git a/csharp/src/Google.Protobuf/JsonParser.cs b/csharp/src/Google.Protobuf/JsonParser.cs index b4a35d7c..2019029b 100644 --- a/csharp/src/Google.Protobuf/JsonParser.cs +++ b/csharp/src/Google.Protobuf/JsonParser.cs @@ -291,8 +291,7 @@ namespace Google.Protobuf { // Parse wrapper types as their constituent types. // TODO: What does this mean for null? - // TODO: Detect this differently when we have dynamic messages, and put it in one place... - if (field.MessageType.IsWellKnownType && field.MessageType.File == Int32Value.Descriptor.File) + if (field.MessageType.IsWrapperType) { field = field.MessageType.Fields[WrappersReflection.WrapperValueFieldNumber]; fieldType = field.FieldType; diff --git a/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs index 285f26f3..57abbbff 100644 --- a/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/EnumDescriptor.cs @@ -43,13 +43,13 @@ namespace Google.Protobuf.Reflection private readonly EnumDescriptorProto proto; private readonly MessageDescriptor containingType; private readonly IList values; - private readonly Type generatedType; + private readonly Type clrType; 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; + this.clrType = generatedType; containingType = parent; if (proto.Value.Count == 0) @@ -73,9 +73,9 @@ 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. + /// The CLR type for this enum. For generated code, this will be a CLR enum type. /// - public Type GeneratedType { get { return generatedType; } } + public Type ClrType { get { return clrType; } } /// /// If this is a nested type, get the outer descriptor, otherwise null. diff --git a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs index 3d6cc59f..c6caaec6 100644 --- a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs @@ -62,8 +62,7 @@ namespace Google.Protobuf.Reflection if (FieldNumber <= 0) { - throw new DescriptorValidationException(this, - "Field numbers must be positive integers."); + throw new DescriptorValidationException(this, "Field numbers must be positive integers."); } containingType = parent; // OneofIndex "defaults" to -1 due to a hack in FieldDescriptor.OnConstruction. @@ -72,7 +71,7 @@ namespace Google.Protobuf.Reflection if (proto.OneofIndex < 0 || proto.OneofIndex >= parent.Proto.OneofDecl.Count) { throw new DescriptorValidationException(this, - "FieldDescriptorProto.oneof_index is out of range for type " + parent.Name); + $"FieldDescriptorProto.oneof_index is out of range for type {parent.Name}"); } containingOneof = parent.Oneofs[proto.OneofIndex]; } @@ -94,13 +93,22 @@ namespace Google.Protobuf.Reflection internal FieldDescriptorProto Proto { get { return proto; } } /// - /// Returns the accessor for this field, or null if this descriptor does - /// not support reflective access. + /// Returns the accessor for this field. /// /// + /// /// While a describes the field, it does not provide /// any way of obtaining or changing the value of the field within a specific message; /// that is the responsibility of the accessor. + /// + /// + /// The value returned by this property will be non-null for all regular fields. However, + /// if a message containing a map field is introspected, the list of nested messages will include + /// an auto-generated nested key/value pair message for the field. This is not represented in any + /// generated type, and the value of the map field itself is represented by a dictionary in the + /// reflection API. There are never instances of those "hidden" messages, so no accessor is provided + /// and this property will return null. + /// /// public IFieldAccessor Accessor { get { return accessor; } } @@ -281,7 +289,7 @@ namespace Google.Protobuf.Reflection } else { - throw new DescriptorValidationException(this, "\"" + Proto.TypeName + "\" is not a type."); + throw new DescriptorValidationException(this, $"\"{Proto.TypeName}\" is not a type."); } } @@ -289,8 +297,7 @@ namespace Google.Protobuf.Reflection { if (!(typeDescriptor is MessageDescriptor)) { - throw new DescriptorValidationException(this, - "\"" + Proto.TypeName + "\" is not a message type."); + throw new DescriptorValidationException(this, $"\"{Proto.TypeName}\" is not a message type."); } messageType = (MessageDescriptor) typeDescriptor; @@ -303,7 +310,7 @@ namespace Google.Protobuf.Reflection { if (!(typeDescriptor is EnumDescriptor)) { - throw new DescriptorValidationException(this, "\"" + Proto.TypeName + "\" is not an enum type."); + throw new DescriptorValidationException(this, $"\"{Proto.TypeName}\" is not an enum type."); } enumType = (EnumDescriptor) typeDescriptor; } @@ -333,14 +340,16 @@ namespace Google.Protobuf.Reflection private IFieldAccessor CreateAccessor(string propertyName) { - if (containingType.GeneratedType == null || propertyName == null) + // If we're given no property name, that's because we really don't want an accessor. + // (At the moment, that means it's a map entry message...) + if (propertyName == null) { return null; } - var property = containingType.GeneratedType.GetProperty(propertyName); + var property = containingType.ClrType.GetProperty(propertyName); if (property == null) { - throw new DescriptorValidationException(this, "Property " + propertyName + " not found in " + containingType.GeneratedType); + throw new DescriptorValidationException(this, $"Property {propertyName} not found in {containingType.ClrType}"); } return IsMap ? new MapFieldAccessor(property, this) : IsRepeated ? new RepeatedFieldAccessor(property, this) diff --git a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs index 549dfe28..48ee5596 100644 --- a/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FileDescriptor.cs @@ -43,35 +43,26 @@ namespace Google.Protobuf.Reflection /// public sealed class FileDescriptor : IDescriptor { - private readonly ByteString descriptorData; - private readonly FileDescriptorProto proto; - private readonly IList messageTypes; - private readonly IList enumTypes; - private readonly IList services; - private readonly IList dependencies; - private readonly IList publicDependencies; - private readonly DescriptorPool pool; - private FileDescriptor(ByteString descriptorData, FileDescriptorProto proto, FileDescriptor[] dependencies, DescriptorPool pool, bool allowUnknownDependencies, GeneratedCodeInfo generatedCodeInfo) { - this.descriptorData = descriptorData; - this.pool = pool; - this.proto = proto; - this.dependencies = new ReadOnlyCollection((FileDescriptor[]) dependencies.Clone()); + SerializedData = descriptorData; + DescriptorPool = pool; + Proto = proto; + Dependencies = new ReadOnlyCollection((FileDescriptor[]) dependencies.Clone()); - publicDependencies = DeterminePublicDependencies(this, proto, dependencies, allowUnknownDependencies); + PublicDependencies = DeterminePublicDependencies(this, proto, dependencies, allowUnknownDependencies); pool.AddPackage(Package, this); - messageTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.MessageType, + MessageTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.MessageType, (message, index) => - new MessageDescriptor(message, this, null, index, generatedCodeInfo == null ? null : generatedCodeInfo.NestedTypes[index])); + new MessageDescriptor(message, this, null, index, generatedCodeInfo.NestedTypes[index])); - enumTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.EnumType, + EnumTypes = DescriptorUtil.ConvertAndMakeReadOnly(proto.EnumType, (enumType, index) => - new EnumDescriptor(enumType, this, null, index, generatedCodeInfo == null ? null : generatedCodeInfo.NestedEnums[index])); + new EnumDescriptor(enumType, this, null, index, generatedCodeInfo.NestedEnums[index])); - services = DescriptorUtil.ConvertAndMakeReadOnly(proto.Service, + Services = DescriptorUtil.ConvertAndMakeReadOnly(proto.Service, (service, index) => new ServiceDescriptor(service, this, index)); } @@ -132,99 +123,63 @@ namespace Google.Protobuf.Reflection /// /// The descriptor in its protocol message representation. /// - internal FileDescriptorProto Proto - { - get { return proto; } - } + internal FileDescriptorProto Proto { get; } /// /// The file name. /// - public string Name - { - get { return proto.Name; } - } + public string Name => Proto.Name; /// /// The package as declared in the .proto file. This may or may not /// be equivalent to the .NET namespace of the generated classes. /// - public string Package - { - get { return proto.Package; } - } + public string Package => Proto.Package; /// /// Unmodifiable list of top-level message types declared in this file. /// - public IList MessageTypes - { - get { return messageTypes; } - } + public IList MessageTypes { get; } /// /// Unmodifiable list of top-level enum types declared in this file. /// - public IList EnumTypes - { - get { return enumTypes; } - } + public IList EnumTypes { get; } /// /// Unmodifiable list of top-level services declared in this file. /// - public IList Services - { - get { return services; } - } + public IList Services { get; } /// /// Unmodifiable list of this file's dependencies (imports). /// - public IList Dependencies - { - get { return dependencies; } - } + public IList Dependencies { get; } /// /// Unmodifiable list of this file's public dependencies (public imports). /// - public IList PublicDependencies - { - get { return publicDependencies; } - } + public IList PublicDependencies { get; } /// /// The original serialized binary form of this descriptor. /// - public ByteString SerializedData - { - get { return descriptorData; } - } + public ByteString SerializedData { get; } /// /// Implementation of IDescriptor.FullName - just returns the same as Name. /// - string IDescriptor.FullName - { - get { return Name; } - } + string IDescriptor.FullName => Name; /// /// Implementation of IDescriptor.File - just returns this descriptor. /// - FileDescriptor IDescriptor.File - { - get { return this; } - } + FileDescriptor IDescriptor.File => this; /// /// Pool containing symbol descriptors. /// - internal DescriptorPool DescriptorPool - { - get { return pool; } - } + internal DescriptorPool DescriptorPool { get; } /// /// Finds a type (message, enum, service or extension) in the file by name. Does not find nested types. @@ -245,7 +200,7 @@ namespace Google.Protobuf.Reflection { name = Package + "." + name; } - T result = pool.FindSymbol(name); + T result = DescriptorPool.FindSymbol(name); if (result != null && result.File == this) { return result; @@ -264,7 +219,7 @@ namespace Google.Protobuf.Reflection /// file's dependencies, in the exact order listed in the .proto file. May be null, /// in which case it is treated as an empty array. /// Whether unknown dependencies are ignored (true) or cause an exception to be thrown (false). - /// Reflection information, if any. May be null, specifically for non-generated code. + /// Details about generated code, for the purposes of 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. @@ -291,15 +246,17 @@ namespace Google.Protobuf.Reflection // need. if (dependencies.Length != proto.Dependency.Count) { - throw new DescriptorValidationException(result, - "Dependencies passed to FileDescriptor.BuildFrom() don't match " + - "those listed in the FileDescriptorProto."); + throw new DescriptorValidationException( + result, + "Dependencies passed to FileDescriptor.BuildFrom() don't match " + + "those listed in the FileDescriptorProto."); } for (int i = 0; i < proto.Dependency.Count; i++) { if (dependencies[i].Name != proto.Dependency[i]) { - throw new DescriptorValidationException(result, + throw new DescriptorValidationException( + result, "Dependencies passed to FileDescriptor.BuildFrom() don't match " + "those listed in the FileDescriptorProto. Expected: " + proto.Dependency[i] + " but was: " + dependencies[i].Name); @@ -312,28 +269,29 @@ namespace Google.Protobuf.Reflection private void CrossLink() { - foreach (MessageDescriptor message in messageTypes) + foreach (MessageDescriptor message in MessageTypes) { message.CrossLink(); } - foreach (ServiceDescriptor service in services) + foreach (ServiceDescriptor service in Services) { service.CrossLink(); } } /// - /// Creates an instance for generated code. + /// Creates a descriptor for generated code. /// /// - /// The parameter should be null for descriptors which don't correspond to - /// generated types. Otherwise, it should be a with nested types and nested - /// enums corresponding to the types and enums contained within the file descriptor. + /// This method is only designed to be used by the results of generating code with protoc, + /// which creates the appropriate dependencies etc. It has to be public because the generated + /// code is "external", but should not be called directly by end users. /// - public static FileDescriptor InternalBuildGeneratedFileFrom(byte[] descriptorData, - FileDescriptor[] dependencies, - GeneratedCodeInfo generatedCodeInfo) + public static FileDescriptor FromGeneratedCode( + byte[] descriptorData, + FileDescriptor[] dependencies, + GeneratedCodeInfo generatedCodeInfo) { FileDescriptorProto proto; try @@ -345,8 +303,6 @@ namespace Google.Protobuf.Reflection throw new ArgumentException("Failed to parse protocol buffer descriptor for generated code.", e); } - - try { // When building descriptors for generated code, we allow unknown @@ -355,7 +311,7 @@ namespace Google.Protobuf.Reflection } catch (DescriptorValidationException e) { - throw new ArgumentException("Invalid embedded descriptor for \"" + proto.Name + "\".", e); + throw new ArgumentException($"Invalid embedded descriptor for \"{proto.Name}\".", e); } } @@ -367,7 +323,7 @@ namespace Google.Protobuf.Reflection /// public override string ToString() { - return "FileDescriptor for " + proto.Name; + return $"FileDescriptor for {Name}"; } /// diff --git a/csharp/src/Google.Protobuf/Reflection/GeneratedCodeInfo.cs b/csharp/src/Google.Protobuf/Reflection/GeneratedCodeInfo.cs index ff4ad0aa..e5fe9fb7 100644 --- a/csharp/src/Google.Protobuf/Reflection/GeneratedCodeInfo.cs +++ b/csharp/src/Google.Protobuf/Reflection/GeneratedCodeInfo.cs @@ -1,3 +1,34 @@ +#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 @@ -20,31 +51,31 @@ namespace Google.Protobuf.Reflection /// /// Irrelevant for file descriptors; the parser for message descriptors. /// - public MessageParser Parser { get; private set; } + public MessageParser Parser { get; } /// /// Irrelevant for file descriptors; the CLR property names (in message descriptor field order) /// for fields in the message for message descriptors. /// - public string[] PropertyNames { get; private set; } + public string[] PropertyNames { get; } /// /// Irrelevant for file descriptors; the CLR property "base" names (in message descriptor oneof order) /// for oneofs in the message for message descriptors. It is expected that for a oneof name of "Foo", /// there will be a "FooCase" property and a "ClearFoo" method. /// - public string[] OneofNames { get; private set; } + public string[] OneofNames { get; } /// /// The reflection information for types within this file/message descriptor. Elements may be null /// if there is no corresponding generated type, e.g. for map entry types. /// - public GeneratedCodeInfo[] NestedTypes { get; private set; } + public GeneratedCodeInfo[] NestedTypes { get; } /// /// The CLR types for enums within this file/message descriptor. /// - public Type[] NestedEnums { get; private set; } + public Type[] NestedEnums { get; } /// /// Creates a GeneratedCodeInfo for a message descriptor, with nested types, nested enums, the CLR type, property names and oneof names. diff --git a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs index 65040e42..769ac3d5 100644 --- a/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/MessageDescriptor.cs @@ -56,153 +56,145 @@ namespace Google.Protobuf.Reflection "google/protobuf/type.proto", }; - private readonly DescriptorProto proto; - private readonly MessageDescriptor containingType; - private readonly IList nestedTypes; - private readonly IList enumTypes; private readonly IList fieldsInDeclarationOrder; private readonly IList fieldsInNumberOrder; private readonly IDictionary jsonFieldMap; - private readonly FieldCollection fields; - private readonly IList oneofs; - // CLR representation of the type described by this descriptor, if any. - private readonly Type generatedType; - private readonly MessageParser parser; internal MessageDescriptor(DescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int typeIndex, GeneratedCodeInfo generatedCodeInfo) : base(file, file.ComputeFullName(parent, proto.Name), typeIndex) { - this.proto = proto; - parser = generatedCodeInfo == null ? null : generatedCodeInfo.Parser; - generatedType = generatedCodeInfo == null ? null : generatedCodeInfo.ClrType; - - containingType = parent; - - oneofs = DescriptorUtil.ConvertAndMakeReadOnly( + Proto = proto; + Parser = generatedCodeInfo?.Parser; + ClrType = generatedCodeInfo?.ClrType; + ContainingType = parent; + + // Note use of generatedCodeInfo. rather than generatedCodeInfo?. here... we don't expect + // to see any nested oneofs, types or enums in "not actually generated" code... we do + // expect fields though (for map entry messages). + Oneofs = DescriptorUtil.ConvertAndMakeReadOnly( proto.OneofDecl, (oneof, index) => - new OneofDescriptor(oneof, file, this, index, generatedCodeInfo == null ? null : generatedCodeInfo.OneofNames[index])); + new OneofDescriptor(oneof, file, this, index, generatedCodeInfo.OneofNames[index])); - nestedTypes = DescriptorUtil.ConvertAndMakeReadOnly( + NestedTypes = DescriptorUtil.ConvertAndMakeReadOnly( proto.NestedType, (type, index) => - new MessageDescriptor(type, file, this, index, generatedCodeInfo == null ? null : generatedCodeInfo.NestedTypes[index])); + new MessageDescriptor(type, file, this, index, generatedCodeInfo.NestedTypes[index])); - enumTypes = DescriptorUtil.ConvertAndMakeReadOnly( + EnumTypes = DescriptorUtil.ConvertAndMakeReadOnly( proto.EnumType, (type, index) => - new EnumDescriptor(type, file, this, index, generatedCodeInfo == null ? null : generatedCodeInfo.NestedEnums[index])); + new EnumDescriptor(type, file, this, index, generatedCodeInfo.NestedEnums[index])); fieldsInDeclarationOrder = DescriptorUtil.ConvertAndMakeReadOnly( proto.Field, (field, index) => - new FieldDescriptor(field, file, this, index, generatedCodeInfo == null ? null : generatedCodeInfo.PropertyNames[index])); + new FieldDescriptor(field, file, this, index, generatedCodeInfo?.PropertyNames[index])); fieldsInNumberOrder = new ReadOnlyCollection(fieldsInDeclarationOrder.OrderBy(field => field.FieldNumber).ToArray()); // TODO: Use field => field.Proto.JsonName when we're confident it's appropriate. (And then use it in the formatter, too.) jsonFieldMap = new ReadOnlyDictionary(fieldsInNumberOrder.ToDictionary(field => JsonFormatter.ToCamelCase(field.Name))); file.DescriptorPool.AddSymbol(this); - fields = new FieldCollection(this); - } - - /// - /// Returns the total number of nested types and enums, recursively. - /// - private int CountTotalGeneratedTypes() - { - return nestedTypes.Sum(nested => nested.CountTotalGeneratedTypes()) + enumTypes.Count; + Fields = new FieldCollection(this); } /// /// The brief name of the descriptor's target. /// - public override string Name { get { return proto.Name; } } + public override string Name => Proto.Name; - internal DescriptorProto Proto { get { return proto; } } + internal DescriptorProto Proto { get; } /// - /// The generated type for this message, or null if the descriptor does not represent a generated type. + /// The CLR type used to represent message instances from this descriptor. /// - public Type GeneratedType { get { return generatedType; } } + /// + /// + /// The value returned by this property will be non-null for all regular fields. However, + /// if a message containing a map field is introspected, the list of nested messages will include + /// an auto-generated nested key/value pair message for the field. This is not represented in any + /// generated type, so this property will return null in such cases. + /// + /// + /// For wrapper types ( and the like), the type returned here + /// will be the generated message type, not the native type used by reflection for fields of those types. Code + /// using reflection should call to determine whether a message descriptor represents + /// a wrapper type, and handle the result appropriately. + /// + /// + public Type ClrType { get; } /// /// A parser for this message type. /// /// + /// /// As is not generic, this cannot be statically - /// typed to the relevant type, but if returns a non-null value, the parser returned + /// typed to the relevant type, but it should produce objects of a type compatible with . + /// + /// + /// The value returned by this property will be non-null for all regular fields. However, + /// if a message containing a map field is introspected, the list of nested messages will include + /// an auto-generated nested key/value pair message for the field. No message parser object is created for + /// such messages, so this property will return null in such cases. + /// + /// + /// For wrapper types ( and the like), the parser returned here + /// will be the generated message type, not the native type used by reflection for fields of those types. Code + /// using reflection should call to determine whether a message descriptor represents + /// a wrapper type, and handle the result appropriately. + /// /// - public MessageParser Parser { get { return parser; } } + public MessageParser Parser { get; } /// /// Returns whether this message is one of the "well known types" which may have runtime/protoc support. /// - internal bool IsWellKnownType - { - get - { - return File.Package == "google.protobuf" && WellKnownTypeNames.Contains(File.Name); - } - } + internal bool IsWellKnownType => File.Package == "google.protobuf" && WellKnownTypeNames.Contains(File.Name); + + /// + /// Returns whether this message is one of the "wrapper types" used for fields which represent primitive values + /// with the addition of presence. + /// + internal bool IsWrapperType => File.Package == "google.protobuf" && File.Name == "google/protobuf/wrappers.proto"; /// /// If this is a nested type, get the outer descriptor, otherwise null. /// - public MessageDescriptor ContainingType - { - get { return containingType; } - } + public MessageDescriptor ContainingType { get; } /// /// A collection of fields, which can be retrieved by name or field number. /// - public FieldCollection Fields - { - get { return fields; } - } + public FieldCollection Fields { get; } /// /// An unmodifiable list of this message type's nested types. /// - public IList NestedTypes - { - get { return nestedTypes; } - } + public IList NestedTypes { get; } /// /// An unmodifiable list of this message type's enum types. /// - public IList EnumTypes - { - get { return enumTypes; } - } + public IList EnumTypes { get; } /// /// An unmodifiable list of the "oneof" field collections in this message type. /// - public IList Oneofs - { - get { return oneofs; } - } + public IList Oneofs { get; } /// /// Finds a field by field name. /// /// The unqualified name of the field (e.g. "foo"). /// The field's descriptor, or null if not found. - public FieldDescriptor FindFieldByName(String name) - { - return File.DescriptorPool.FindSymbol(FullName + "." + name); - } + public FieldDescriptor FindFieldByName(String name) =>File.DescriptorPool.FindSymbol(FullName + "." + name); /// /// Finds a field by field number. /// /// The field number within this message type. /// The field's descriptor, or null if not found. - public FieldDescriptor FindFieldByNumber(int number) - { - return File.DescriptorPool.FindFieldByNumber(this, number); - } + public FieldDescriptor FindFieldByNumber(int number) => File.DescriptorPool.FindFieldByNumber(this, number); /// /// Finds a nested descriptor by name. The is valid for fields, nested @@ -210,18 +202,15 @@ namespace Google.Protobuf.Reflection /// /// The unqualified name of the descriptor, e.g. "Foo" /// The descriptor, or null if not found. - public T FindDescriptor(string name) - where T : class, IDescriptor - { - return File.DescriptorPool.FindSymbol(FullName + "." + name); - } + public T FindDescriptor(string name) where T : class, IDescriptor => + File.DescriptorPool.FindSymbol(FullName + "." + name); /// /// Looks up and cross-links all fields and nested types. /// internal void CrossLink() { - foreach (MessageDescriptor message in nestedTypes) + foreach (MessageDescriptor message in NestedTypes) { message.CrossLink(); } @@ -231,7 +220,7 @@ namespace Google.Protobuf.Reflection field.CrossLink(); } - foreach (OneofDescriptor oneof in oneofs) + foreach (OneofDescriptor oneof in Oneofs) { oneof.CrossLink(); } @@ -253,10 +242,7 @@ namespace Google.Protobuf.Reflection /// Returns the fields in the message as an immutable list, in the order in which they /// are declared in the source .proto file. /// - public IList InDeclarationOrder() - { - return messageDescriptor.fieldsInDeclarationOrder; - } + public IList InDeclarationOrder() => messageDescriptor.fieldsInDeclarationOrder; /// /// Returns the fields in the message as an immutable list, in ascending field number @@ -264,10 +250,7 @@ namespace Google.Protobuf.Reflection /// index in the list to the field number; to retrieve a field by field number, it is better /// to use the indexer. /// - public IList InFieldNumberOrder() - { - return messageDescriptor.fieldsInNumberOrder; - } + public IList InFieldNumberOrder() => messageDescriptor.fieldsInNumberOrder; // TODO: consider making this public in the future. (Being conservative for now...) @@ -276,10 +259,7 @@ namespace Google.Protobuf.Reflection /// in the JSON representation to the field descriptors. For example, a field foo_bar /// in the message would result in an entry with a key fooBar. /// - internal IDictionary ByJsonName() - { - return messageDescriptor.jsonFieldMap; - } + internal IDictionary ByJsonName() => messageDescriptor.jsonFieldMap; /// /// Retrieves the descriptor for the field with the given number. diff --git a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs index d51ee526..22020acf 100644 --- a/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/OneofDescriptor.cs @@ -86,8 +86,7 @@ namespace Google.Protobuf.Reflection /// in a particular message. /// /// - /// The accessor used for reflective access, or null if reflection is not - /// supported by this descriptor. + /// The accessor used for reflective access. /// public OneofAccessor Accessor { get { return accessor; } } @@ -106,19 +105,15 @@ namespace Google.Protobuf.Reflection private OneofAccessor CreateAccessor(string clrName) { - if (containingType.GeneratedType == null || clrName == null) - { - return null; - } - var caseProperty = containingType.GeneratedType.GetProperty(clrName + "Case"); + var caseProperty = containingType.ClrType.GetProperty(clrName + "Case"); if (caseProperty == null) { - throw new DescriptorValidationException(this, "Property " + clrName + "Case not found in " + containingType.GeneratedType); + throw new DescriptorValidationException(this, $"Property {clrName}Case not found in {containingType.ClrType}"); } - var clearMethod = containingType.GeneratedType.GetMethod("Clear" + clrName); + var clearMethod = containingType.ClrType.GetMethod("Clear" + clrName); if (clearMethod == null) { - throw new DescriptorValidationException(this, "Method Clear" + clrName + " not found in " + containingType.GeneratedType); + throw new DescriptorValidationException(this, $"Method Clear{clrName} not found in {containingType.ClrType}"); } return new OneofAccessor(caseProperty, clearMethod, this); -- cgit v1.2.3