From 71e8dca08312d9f9d76db6c2f762f8b395d05226 Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Wed, 30 Mar 2016 09:42:37 +0100 Subject: Refactoring of FieldDescriptor This makes no externally visible behavioral changes. Internally and non-behaviorally: - We use a field (compiler-generated) to store the JsonName to avoid recomputing it repeatedly - The documentation for JsonName is updated to reflect the meaning better - Readonly autoprops and expression-bodied properties used where possible --- .../Google.Protobuf/Reflection/FieldDescriptor.cs | 100 ++++++++------------- 1 file changed, 39 insertions(+), 61 deletions(-) diff --git a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs index 6083f171..de6e5717 100644 --- a/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs +++ b/csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs @@ -30,9 +30,8 @@ // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #endregion -using System; -using System.Linq; using Google.Protobuf.Compatibility; +using System; namespace Google.Protobuf.Reflection { @@ -41,20 +40,35 @@ namespace Google.Protobuf.Reflection /// public sealed class FieldDescriptor : DescriptorBase, IComparable { - private readonly FieldDescriptorProto proto; private EnumDescriptor enumType; private MessageDescriptor messageType; - private readonly MessageDescriptor containingType; - private readonly OneofDescriptor containingOneof; private FieldType fieldType; private readonly string propertyName; // Annoyingly, needed in Crosslink. private IFieldAccessor accessor; + /// + /// Get the field's containing message type. + /// + public MessageDescriptor ContainingType { get; } + + /// + /// Returns the oneof containing this field, or null if it is not part of a oneof. + /// + public OneofDescriptor ContainingOneof { get; } + + /// + /// The effective JSON name for this field. This is usually the lower-camel-cased form of the field name, + /// but can be overridden using the json_name option in the .proto file. + /// + public string JsonName { get; } + + internal FieldDescriptorProto Proto { get; } + internal FieldDescriptor(FieldDescriptorProto proto, FileDescriptor file, MessageDescriptor parent, int index, string propertyName) : base(file, file.ComputeFullName(parent, proto.Name), index) { - this.proto = proto; + Proto = proto; if (proto.Type != 0) { fieldType = GetFieldTypeFromProtoType(proto.Type); @@ -64,7 +78,7 @@ namespace Google.Protobuf.Reflection { throw new DescriptorValidationException(this, "Field numbers must be positive integers."); } - containingType = parent; + ContainingType = parent; // OneofIndex "defaults" to -1 due to a hack in FieldDescriptor.OnConstruction. if (proto.OneofIndex != -1) { @@ -73,7 +87,7 @@ namespace Google.Protobuf.Reflection throw new DescriptorValidationException(this, $"FieldDescriptorProto.oneof_index is out of range for type {parent.Name}"); } - containingOneof = parent.Oneofs[proto.OneofIndex]; + ContainingOneof = parent.Oneofs[proto.OneofIndex]; } file.DescriptorPool.AddSymbol(this); @@ -83,20 +97,14 @@ namespace Google.Protobuf.Reflection // We could trust the generated code and check whether the type of the property is // a MapField, but that feels a tad nasty. this.propertyName = propertyName; + JsonName = Proto.JsonName == "" ? JsonFormatter.ToCamelCase(Proto.Name) : Proto.JsonName; } + /// /// The brief name of the descriptor's target. /// - public override string Name { get { return proto.Name; } } - - - /// - /// The json_name option of the descriptor's target. - /// - public string JsonName { get { return proto.JsonName == "" ? JsonFormatter.ToCamelCase(proto.Name) : proto.JsonName; } } - - internal FieldDescriptorProto Proto { get { return proto; } } + public override string Name => Proto.Name; /// /// Returns the accessor for this field. @@ -116,7 +124,7 @@ namespace Google.Protobuf.Reflection /// and this property will return null. /// /// - public IFieldAccessor Accessor { get { return accessor; } } + public IFieldAccessor Accessor => accessor; /// /// Maps a field type as included in the .proto file to a FieldType. @@ -169,62 +177,32 @@ namespace Google.Protobuf.Reflection /// /// Returns true if this field is a repeated field; false otherwise. /// - public bool IsRepeated - { - get { return Proto.Label == FieldDescriptorProto.Types.Label.LABEL_REPEATED; } - } + public bool IsRepeated => Proto.Label == FieldDescriptorProto.Types.Label.LABEL_REPEATED; /// /// Returns true if this field is a map field; false otherwise. /// - public bool IsMap - { - get { return fieldType == FieldType.Message && messageType.Proto.Options != null && messageType.Proto.Options.MapEntry; } - } + public bool IsMap => fieldType == FieldType.Message && messageType.Proto.Options != null && messageType.Proto.Options.MapEntry; /// /// Returns true if this field is a packed, repeated field; false otherwise. /// - public bool IsPacked - { + public bool IsPacked => // Note the || rather than && here - we're effectively defaulting to packed, because that *is* // the default in proto3, which is all we support. We may give the wrong result for the protos // within descriptor.proto, but that's okay, as they're never exposed and we don't use IsPacked // within the runtime. - get { return Proto.Options == null || Proto.Options.Packed; } - } - - /// - /// Get the field's containing message type. - /// - public MessageDescriptor ContainingType - { - get { return containingType; } - } - - /// - /// Returns the oneof containing this field, or null if it is not part of a oneof. - /// - public OneofDescriptor ContainingOneof - { - get { return containingOneof; } - } - + Proto.Options == null || Proto.Options.Packed; + /// /// Returns the type of the field. /// - public FieldType FieldType - { - get { return fieldType; } - } + public FieldType FieldType => fieldType; /// /// Returns the field number declared in the proto file. /// - public int FieldNumber - { - get { return Proto.Number; } - } + public int FieldNumber => Proto.Number; /// /// Compares this descriptor with another one, ordering in "canonical" order @@ -234,7 +212,7 @@ namespace Google.Protobuf.Reflection /// public int CompareTo(FieldDescriptor other) { - if (other.containingType != containingType) + if (other.ContainingType != ContainingType) { throw new ArgumentException("FieldDescriptors can only be compared to other FieldDescriptors " + "for fields of the same message type."); @@ -337,14 +315,14 @@ namespace Google.Protobuf.Reflection File.DescriptorPool.AddFieldByNumber(this); - if (containingType != null && containingType.Proto.Options != null && containingType.Proto.Options.MessageSetWireFormat) + if (ContainingType != null && ContainingType.Proto.Options != null && ContainingType.Proto.Options.MessageSetWireFormat) { throw new DescriptorValidationException(this, "MessageSet format is not supported."); } - accessor = CreateAccessor(propertyName); + accessor = CreateAccessor(); } - private IFieldAccessor CreateAccessor(string propertyName) + private IFieldAccessor CreateAccessor() { // 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...) @@ -352,10 +330,10 @@ namespace Google.Protobuf.Reflection { return null; } - var property = containingType.ClrType.GetProperty(propertyName); + var property = ContainingType.ClrType.GetProperty(propertyName); if (property == null) { - throw new DescriptorValidationException(this, $"Property {propertyName} not found in {containingType.ClrType}"); + throw new DescriptorValidationException(this, $"Property {propertyName} not found in {ContainingType.ClrType}"); } return IsMap ? new MapFieldAccessor(property, this) : IsRepeated ? new RepeatedFieldAccessor(property, this) -- cgit v1.2.3