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

1 files 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
     /// </summary>
     public sealed class FieldDescriptor : DescriptorBase, IComparable<FieldDescriptor>
     {
-        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;
 
+        /// <summary>
+        /// Get the field's containing message type.
+        /// </summary>
+        public MessageDescriptor ContainingType { get; }
+
+        /// <summary>
+        /// Returns the oneof containing this field, or <c>null</c> if it is not part of a oneof.
+        /// </summary>
+        public OneofDescriptor ContainingOneof { get; }
+
+        /// <summary>
+        /// 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 <c>json_name</c> option in the .proto file.
+        /// </summary>
+        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;
         }
+    
 
         /// <summary>
         /// The brief name of the descriptor's target.
         /// </summary>
-        public override string Name { get { return proto.Name; } }
-
-
-        /// <summary>
-        /// The json_name option of the descriptor's target.
-        /// </summary>
-        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;
 
         /// <summary>
         /// Returns the accessor for this field.
@@ -116,7 +124,7 @@ namespace Google.Protobuf.Reflection
         /// and this property will return null.
         /// </para>
         /// </remarks>
-        public IFieldAccessor Accessor { get { return accessor; } }
+        public IFieldAccessor Accessor => accessor;
         
         /// <summary>
         /// Maps a field type as included in the .proto file to a FieldType.
@@ -169,62 +177,32 @@ namespace Google.Protobuf.Reflection
         /// <summary>
         /// Returns <c>true</c> if this field is a repeated field; <c>false</c> otherwise.
         /// </summary>
-        public bool IsRepeated
-        {
-            get { return Proto.Label == FieldDescriptorProto.Types.Label.LABEL_REPEATED; }
-        }
+        public bool IsRepeated => Proto.Label == FieldDescriptorProto.Types.Label.LABEL_REPEATED;
 
         /// <summary>
         /// Returns <c>true</c> if this field is a map field; <c>false</c> otherwise.
         /// </summary>
-        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;
 
         /// <summary>
         /// Returns <c>true</c> if this field is a packed, repeated field; <c>false</c> otherwise.
         /// </summary>
-        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; }
-        }        
-
-        /// <summary>
-        /// Get the field's containing message type.
-        /// </summary>
-        public MessageDescriptor ContainingType
-        {
-            get { return containingType; }
-        }
-
-        /// <summary>
-        /// Returns the oneof containing this field, or <c>null</c> if it is not part of a oneof.
-        /// </summary>
-        public OneofDescriptor ContainingOneof
-        {
-            get { return containingOneof; }
-        }
-
+            Proto.Options == null || Proto.Options.Packed;
+        
         /// <summary>
         /// Returns the type of the field.
         /// </summary>
-        public FieldType FieldType
-        {
-            get { return fieldType; }
-        }
+        public FieldType FieldType => fieldType;
 
         /// <summary>
         /// Returns the field number declared in the proto file.
         /// </summary>
-        public int FieldNumber
-        {
-            get { return Proto.Number; }
-        }
+        public int FieldNumber => Proto.Number;
 
         /// <summary>
         /// Compares this descriptor with another one, ordering in "canonical" order
@@ -234,7 +212,7 @@ namespace Google.Protobuf.Reflection
         /// </summary>
         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)