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
pull/1362/head
Jon Skeet 9 years ago
parent 261fde1707
commit 71e8dca083
  1. 98
      csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs

@ -30,9 +30,8 @@
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
#endregion #endregion
using System;
using System.Linq;
using Google.Protobuf.Compatibility; using Google.Protobuf.Compatibility;
using System;
namespace Google.Protobuf.Reflection namespace Google.Protobuf.Reflection
{ {
@ -41,20 +40,35 @@ namespace Google.Protobuf.Reflection
/// </summary> /// </summary>
public sealed class FieldDescriptor : DescriptorBase, IComparable<FieldDescriptor> public sealed class FieldDescriptor : DescriptorBase, IComparable<FieldDescriptor>
{ {
private readonly FieldDescriptorProto proto;
private EnumDescriptor enumType; private EnumDescriptor enumType;
private MessageDescriptor messageType; private MessageDescriptor messageType;
private readonly MessageDescriptor containingType;
private readonly OneofDescriptor containingOneof;
private FieldType fieldType; private FieldType fieldType;
private readonly string propertyName; // Annoyingly, needed in Crosslink. private readonly string propertyName; // Annoyingly, needed in Crosslink.
private IFieldAccessor accessor; 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, internal FieldDescriptor(FieldDescriptorProto proto, FileDescriptor file,
MessageDescriptor parent, int index, string propertyName) MessageDescriptor parent, int index, string propertyName)
: base(file, file.ComputeFullName(parent, proto.Name), index) : base(file, file.ComputeFullName(parent, proto.Name), index)
{ {
this.proto = proto; Proto = proto;
if (proto.Type != 0) if (proto.Type != 0)
{ {
fieldType = GetFieldTypeFromProtoType(proto.Type); fieldType = GetFieldTypeFromProtoType(proto.Type);
@ -64,7 +78,7 @@ namespace Google.Protobuf.Reflection
{ {
throw new DescriptorValidationException(this, "Field numbers must be positive integers."); 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. // OneofIndex "defaults" to -1 due to a hack in FieldDescriptor.OnConstruction.
if (proto.OneofIndex != -1) if (proto.OneofIndex != -1)
{ {
@ -73,7 +87,7 @@ namespace Google.Protobuf.Reflection
throw new DescriptorValidationException(this, 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]; ContainingOneof = parent.Oneofs[proto.OneofIndex];
} }
file.DescriptorPool.AddSymbol(this); 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 // We could trust the generated code and check whether the type of the property is
// a MapField, but that feels a tad nasty. // a MapField, but that feels a tad nasty.
this.propertyName = propertyName; 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> /// <summary>
/// The json_name option of the descriptor's target. /// The brief name of the descriptor's target.
/// </summary> /// </summary>
public string JsonName { get { return proto.JsonName == "" ? JsonFormatter.ToCamelCase(proto.Name) : proto.JsonName; } } public override string Name => Proto.Name;
internal FieldDescriptorProto Proto { get { return proto; } }
/// <summary> /// <summary>
/// Returns the accessor for this field. /// Returns the accessor for this field.
@ -116,7 +124,7 @@ namespace Google.Protobuf.Reflection
/// and this property will return null. /// and this property will return null.
/// </para> /// </para>
/// </remarks> /// </remarks>
public IFieldAccessor Accessor { get { return accessor; } } public IFieldAccessor Accessor => accessor;
/// <summary> /// <summary>
/// Maps a field type as included in the .proto file to a FieldType. /// Maps a field type as included in the .proto file to a FieldType.
@ -169,62 +177,32 @@ namespace Google.Protobuf.Reflection
/// <summary> /// <summary>
/// Returns <c>true</c> if this field is a repeated field; <c>false</c> otherwise. /// Returns <c>true</c> if this field is a repeated field; <c>false</c> otherwise.
/// </summary> /// </summary>
public bool IsRepeated public bool IsRepeated => Proto.Label == FieldDescriptorProto.Types.Label.LABEL_REPEATED;
{
get { return Proto.Label == FieldDescriptorProto.Types.Label.LABEL_REPEATED; }
}
/// <summary> /// <summary>
/// Returns <c>true</c> if this field is a map field; <c>false</c> otherwise. /// Returns <c>true</c> if this field is a map field; <c>false</c> otherwise.
/// </summary> /// </summary>
public bool IsMap public bool IsMap => fieldType == FieldType.Message && messageType.Proto.Options != null && messageType.Proto.Options.MapEntry;
{
get { return fieldType == FieldType.Message && messageType.Proto.Options != null && messageType.Proto.Options.MapEntry; }
}
/// <summary> /// <summary>
/// Returns <c>true</c> if this field is a packed, repeated field; <c>false</c> otherwise. /// Returns <c>true</c> if this field is a packed, repeated field; <c>false</c> otherwise.
/// </summary> /// </summary>
public bool IsPacked public bool IsPacked =>
{
// Note the || rather than && here - we're effectively defaulting to packed, because that *is* // 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 // 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 descriptor.proto, but that's okay, as they're never exposed and we don't use IsPacked
// within the runtime. // within the runtime.
get { return Proto.Options == null || Proto.Options.Packed; } 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; }
}
/// <summary> /// <summary>
/// Returns the type of the field. /// Returns the type of the field.
/// </summary> /// </summary>
public FieldType FieldType public FieldType FieldType => fieldType;
{
get { return fieldType; }
}
/// <summary> /// <summary>
/// Returns the field number declared in the proto file. /// Returns the field number declared in the proto file.
/// </summary> /// </summary>
public int FieldNumber public int FieldNumber => Proto.Number;
{
get { return Proto.Number; }
}
/// <summary> /// <summary>
/// Compares this descriptor with another one, ordering in "canonical" order /// Compares this descriptor with another one, ordering in "canonical" order
@ -234,7 +212,7 @@ namespace Google.Protobuf.Reflection
/// </summary> /// </summary>
public int CompareTo(FieldDescriptor other) public int CompareTo(FieldDescriptor other)
{ {
if (other.containingType != containingType) if (other.ContainingType != ContainingType)
{ {
throw new ArgumentException("FieldDescriptors can only be compared to other FieldDescriptors " + throw new ArgumentException("FieldDescriptors can only be compared to other FieldDescriptors " +
"for fields of the same message type."); "for fields of the same message type.");
@ -337,14 +315,14 @@ namespace Google.Protobuf.Reflection
File.DescriptorPool.AddFieldByNumber(this); 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."); 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. // 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...) // (At the moment, that means it's a map entry message...)
@ -352,10 +330,10 @@ namespace Google.Protobuf.Reflection
{ {
return null; return null;
} }
var property = containingType.ClrType.GetProperty(propertyName); var property = ContainingType.ClrType.GetProperty(propertyName);
if (property == null) 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) return IsMap ? new MapFieldAccessor(property, this)
: IsRepeated ? new RepeatedFieldAccessor(property, this) : IsRepeated ? new RepeatedFieldAccessor(property, this)

Loading…
Cancel
Save