Commit 47f2017c authored by Sydney Acksman's avatar Sydney Acksman

Review changes

 * Add Syntax enum and make file syntax checks check the enum
 * Made IsPacked a normal get method without ternary op
 * Moved IFieldAccessor.HasValue to IFieldPresenceAccessor
 * Change naming of GetIsExtensionsInitialized
 * Fixed stray text in summary text
parent 29d51868
...@@ -101,7 +101,7 @@ namespace Google.Protobuf.Reflection ...@@ -101,7 +101,7 @@ namespace Google.Protobuf.Reflection
{ {
IMessage message = SampleMessages.CreateFullTestAllTypes(); IMessage message = SampleMessages.CreateFullTestAllTypes();
var fields = message.Descriptor.Fields; var fields = message.Descriptor.Fields;
Assert.Throws<InvalidOperationException>(() => fields[TestProtos.TestAllTypes.SingleBoolFieldNumber].Accessor.HasValue(message)); Assert.Throws<InvalidOperationException>(() => (fields[TestProtos.TestAllTypes.SingleBoolFieldNumber].Accessor as IFieldPresenceAccessor).HasValue(message));
} }
[Test] [Test]
...@@ -109,7 +109,7 @@ namespace Google.Protobuf.Reflection ...@@ -109,7 +109,7 @@ namespace Google.Protobuf.Reflection
{ {
IMessage message = new Proto2.TestAllTypes(); IMessage message = new Proto2.TestAllTypes();
var fields = message.Descriptor.Fields; var fields = message.Descriptor.Fields;
var accessor = fields[Proto2.TestAllTypes.OptionalBoolFieldNumber].Accessor; var accessor = fields[Proto2.TestAllTypes.OptionalBoolFieldNumber].Accessor as IFieldPresenceAccessor;
Assert.False(accessor.HasValue(message)); Assert.False(accessor.HasValue(message));
......
...@@ -148,12 +148,12 @@ namespace Google.Protobuf ...@@ -148,12 +148,12 @@ namespace Google.Protobuf
/// </summary> /// </summary>
public static bool IsInitialized(this IMessage message) public static bool IsInitialized(this IMessage message)
{ {
if (message.Descriptor.File.Proto.Syntax == "proto3") if (message.Descriptor.File.Syntax == Syntax.Proto3)
{ {
return true; return true;
} }
if (!message.Descriptor.GetIsExtensionsInitialized(message)) if (!message.Descriptor.IsExtensionsInitialized(message))
{ {
return false; return false;
} }
...@@ -183,7 +183,7 @@ namespace Google.Protobuf ...@@ -183,7 +183,7 @@ namespace Google.Protobuf
} }
else if (f.FieldType == FieldType.Message || f.FieldType == FieldType.Group) else if (f.FieldType == FieldType.Message || f.FieldType == FieldType.Group)
{ {
if (f.Accessor.HasValue(message)) if ((f.Accessor as IFieldPresenceAccessor).HasValue(message))
{ {
return ((IMessage)f.Accessor.GetValue(message)).IsInitialized(); return ((IMessage)f.Accessor.GetValue(message)).IsInitialized();
} }
...@@ -194,7 +194,7 @@ namespace Google.Protobuf ...@@ -194,7 +194,7 @@ namespace Google.Protobuf
} }
else if (f.IsRequired) else if (f.IsRequired)
{ {
return f.Accessor.HasValue(message); return (f.Accessor as IFieldPresenceAccessor).HasValue(message);
} }
else else
{ {
......
...@@ -201,7 +201,20 @@ namespace Google.Protobuf.Reflection ...@@ -201,7 +201,20 @@ namespace Google.Protobuf.Reflection
/// <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 => File.Proto.Syntax != "proto3" ? Proto.Options?.Packed ?? false : !Proto.Options.HasPacked || Proto.Options.Packed; public bool IsPacked
{
get
{
if (File.Syntax != Syntax.Proto3)
{
return Proto.Options?.Packed ?? false;
}
else
{
return !Proto.Options.HasPacked || Proto.Options.Packed;
}
}
}
/// <summary> /// <summary>
/// Returns <c>true</c> if this field extends another message type; <c>false</c> otherwise. /// Returns <c>true</c> if this field extends another message type; <c>false</c> otherwise.
......
...@@ -42,6 +42,25 @@ using static Google.Protobuf.Reflection.SourceCodeInfo.Types; ...@@ -42,6 +42,25 @@ using static Google.Protobuf.Reflection.SourceCodeInfo.Types;
namespace Google.Protobuf.Reflection namespace Google.Protobuf.Reflection
{ {
/// <summary>
/// The syntax of a .proto file
/// </summary>
public enum Syntax
{
/// <summary>
/// Proto2 syntax
/// </summary>
Proto2,
/// <summary>
/// Proto3 syntax
/// </summary>
Proto3,
/// <summary>
/// An unknown declared syntax
/// </summary>
Unknown
}
/// <summary> /// <summary>
/// Describes a .proto file, including everything defined within. /// Describes a .proto file, including everything defined within.
/// IDescriptor is implemented such that the File property returns this descriptor, /// IDescriptor is implemented such that the File property returns this descriptor,
...@@ -87,6 +106,19 @@ namespace Google.Protobuf.Reflection ...@@ -87,6 +106,19 @@ namespace Google.Protobuf.Reflection
Extensions = new ExtensionCollection(this, generatedCodeInfo?.Extensions); Extensions = new ExtensionCollection(this, generatedCodeInfo?.Extensions);
declarations = new Lazy<Dictionary<IDescriptor, DescriptorDeclaration>>(CreateDeclarationMap, LazyThreadSafetyMode.ExecutionAndPublication); declarations = new Lazy<Dictionary<IDescriptor, DescriptorDeclaration>>(CreateDeclarationMap, LazyThreadSafetyMode.ExecutionAndPublication);
if (!proto.HasSyntax || proto.Syntax == "proto2")
{
Syntax = Syntax.Proto2;
}
else if (proto.Syntax == "proto3")
{
Syntax = Syntax.Proto3;
}
else
{
Syntax = Syntax.Unknown;
}
} }
private Dictionary<IDescriptor, DescriptorDeclaration> CreateDeclarationMap() private Dictionary<IDescriptor, DescriptorDeclaration> CreateDeclarationMap()
...@@ -217,6 +249,11 @@ namespace Google.Protobuf.Reflection ...@@ -217,6 +249,11 @@ namespace Google.Protobuf.Reflection
/// </value> /// </value>
internal FileDescriptorProto Proto { get; } internal FileDescriptorProto Proto { get; }
/// <summary>
/// The syntax of the file
/// </summary>
public Syntax Syntax { get; }
/// <value> /// <value>
/// The file name. /// The file name.
/// </value> /// </value>
......
...@@ -51,11 +51,6 @@ namespace Google.Protobuf.Reflection ...@@ -51,11 +51,6 @@ namespace Google.Protobuf.Reflection
/// </summary> /// </summary>
void Clear(IMessage message); void Clear(IMessage message);
/// <summary>
/// Indicates whether the field in the specified message is set. For proto3 fields, this throws an <see cref="InvalidOperationException"/>
/// </summary>
bool HasValue(IMessage message);
/// <summary> /// <summary>
/// Fetches the field value. For repeated values, this will be an /// Fetches the field value. For repeated values, this will be an
/// <see cref="IList"/> implementation. For map values, this will be an /// <see cref="IList"/> implementation. For map values, this will be an
...@@ -73,4 +68,15 @@ namespace Google.Protobuf.Reflection ...@@ -73,4 +68,15 @@ namespace Google.Protobuf.Reflection
/// <exception cref="InvalidOperationException">The field is not a "simple" field.</exception> /// <exception cref="InvalidOperationException">The field is not a "simple" field.</exception>
void SetValue(IMessage message, object value); void SetValue(IMessage message, object value);
} }
/// <summary>
/// Allows field presence to be checked reflectively. This is implemented for all single field accessors
/// </summary>
public interface IFieldPresenceAccessor : IFieldAccessor
{
/// <summary>
/// Indicates whether the field in the specified message is set. For proto3 fields, this throws an <see cref="InvalidOperationException"/>
/// </summary>
bool HasValue(IMessage message);
}
} }
\ No newline at end of file
...@@ -136,7 +136,7 @@ namespace Google.Protobuf.Reflection ...@@ -136,7 +136,7 @@ namespace Google.Protobuf.Reflection
internal DescriptorProto Proto { get; } internal DescriptorProto Proto { get; }
internal bool GetIsExtensionsInitialized(IMessage message) internal bool IsExtensionsInitialized(IMessage message)
{ {
if (Proto.ExtensionRange.Count == 0) if (Proto.ExtensionRange.Count == 0)
{ {
......
...@@ -71,7 +71,7 @@ namespace Google.Protobuf.Reflection ...@@ -71,7 +71,7 @@ namespace Google.Protobuf.Reflection
/// <summary> /// <summary>
/// Empty Type[] used when calling GetProperty to force property instead of indexer fetching. /// Empty Type[] used when calling GetProperty to force property instead of indexer fetching.
/// </summary>getFieldFunc /// </summary>
internal static readonly Type[] EmptyTypes = new Type[0]; internal static readonly Type[] EmptyTypes = new Type[0];
/// <summary> /// <summary>
......
...@@ -39,7 +39,7 @@ namespace Google.Protobuf.Reflection ...@@ -39,7 +39,7 @@ namespace Google.Protobuf.Reflection
/// <summary> /// <summary>
/// Accessor for single fields. /// Accessor for single fields.
/// </summary> /// </summary>
internal sealed class SingleFieldAccessor : FieldAccessorBase internal sealed class SingleFieldAccessor : FieldAccessorBase, IFieldPresenceAccessor
{ {
// All the work here is actually done in the constructor - it creates the appropriate delegates. // All the work here is actually done in the constructor - it creates the appropriate delegates.
// There are various cases to consider, based on the property type (message, string/bytes, or "genuine" primitive) // There are various cases to consider, based on the property type (message, string/bytes, or "genuine" primitive)
...@@ -57,7 +57,7 @@ namespace Google.Protobuf.Reflection ...@@ -57,7 +57,7 @@ namespace Google.Protobuf.Reflection
throw new ArgumentException("Not all required properties/methods available"); throw new ArgumentException("Not all required properties/methods available");
} }
setValueDelegate = ReflectionUtil.CreateActionIMessageObject(property.GetSetMethod()); setValueDelegate = ReflectionUtil.CreateActionIMessageObject(property.GetSetMethod());
if (descriptor.File.Proto.Syntax == "proto3") if (descriptor.File.Syntax == Syntax.Proto3)
{ {
hasDelegate = message => { hasDelegate = message => {
throw new InvalidOperationException("HasValue is not implemented for proto3 fields"); throw new InvalidOperationException("HasValue is not implemented for proto3 fields");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment