Implement HasPresence for C#

FieldDescriptor.HasPresence returns true if both ClearValue and HasValue (on the accessor) can be expected to work. Some fields have a working ClearValue, but no HasValue; HasPresence returns false for those fields.

Generally:

- Extension fields have presence if and only if they're singular
- Repeated fields do not support presence
- Map fields do not support presence
- Message fields support presence
- Oneof fields support presence (this includes synthetic oneof fields, so that covers proto3 optional singular fields)
- Proto2 singular primitive fields support presence
- Proto3 singular primitive fields do not support presence (unless they're in a oneof, covered above)

csharp/src/Google.Protobuf.Test/Reflection/FieldAccessTest.cs

@@ -98,44 +98,112 @@ public void GetValue_IncorrectType()
         }
 
         [Test]
-        public void HasValue_Proto3()
+        public void HasValue_Proto3_Message()
+        {
+            var message = new TestAllTypes();
+            var accessor = ((IMessage) message).Descriptor.Fields[TestProtos.TestAllTypes.SingleForeignMessageFieldNumber].Accessor;
+            Assert.False(accessor.HasValue(message));
+            message.SingleForeignMessage = new ForeignMessage();
+            Assert.True(accessor.HasValue(message));
+            message.SingleForeignMessage = null;
+            Assert.False(accessor.HasValue(message));
+        }
+
+        [Test]
+        public void HasValue_Proto3_Oneof()
+        {
+            TestAllTypes message = new TestAllTypes();
+            var accessor = ((IMessage) message).Descriptor.Fields[TestProtos.TestAllTypes.OneofStringFieldNumber].Accessor;
+            Assert.False(accessor.HasValue(message));
+            // Even though it's the default value, we still have a value.
+            message.OneofString = "";
+            Assert.True(accessor.HasValue(message));
+            message.OneofString = "hello";
+            Assert.True(accessor.HasValue(message));
+            message.OneofUint32 = 10;
+            Assert.False(accessor.HasValue(message));
+        }
+
+        [Test]
+        public void HasValue_Proto3_Primitive_Optional()
+        {
+            var message = new TestProto3Optional();
+            var accessor = ((IMessage) message).Descriptor.Fields[TestProto3Optional.OptionalInt64FieldNumber].Accessor;
+            Assert.IsFalse(accessor.HasValue(message));
+            message.OptionalInt64 = 5L;
+            Assert.IsTrue(accessor.HasValue(message));
+            message.ClearOptionalInt64();
+            Assert.IsFalse(accessor.HasValue(message));
+            message.OptionalInt64 = 0L;
+            Assert.IsTrue(accessor.HasValue(message));
+        }
+
+        [Test]
+        public void HasValue_Proto3_Primitive_NotOptional()
         {
             IMessage message = SampleMessages.CreateFullTestAllTypes();
             var fields = message.Descriptor.Fields;
             Assert.Throws<InvalidOperationException>(() => fields[TestProtos.TestAllTypes.SingleBoolFieldNumber].Accessor.HasValue(message));
         }
 
         [Test]
-        public void HasValue_Proto3Optional()
+        public void HasValue_Proto3_Repeated()
         {
-            IMessage message = new TestProto3Optional
-            {
-                OptionalInt32 = 0,
-                LazyNestedMessage = new TestProto3Optional.Types.NestedMessage()
-            };
-            var fields = message.Descriptor.Fields;
-            Assert.IsFalse(fields[TestProto3Optional.OptionalInt64FieldNumber].Accessor.HasValue(message));
-            Assert.IsFalse(fields[TestProto3Optional.OptionalNestedMessageFieldNumber].Accessor.HasValue(message));
-            Assert.IsTrue(fields[TestProto3Optional.LazyNestedMessageFieldNumber].Accessor.HasValue(message));
-            Assert.IsTrue(fields[TestProto3Optional.OptionalInt32FieldNumber].Accessor.HasValue(message));
+            var message = new TestAllTypes();
+            var accessor = ((IMessage) message).Descriptor.Fields[TestProtos.TestAllTypes.RepeatedBoolFieldNumber].Accessor;
+            Assert.Throws<InvalidOperationException>(() => accessor.HasValue(message));
         }
 
         [Test]
-        public void HasValue()
+        public void HasValue_Proto2_Primitive()
         {
-            IMessage message = new Proto2.TestAllTypes();
-            var fields = message.Descriptor.Fields;
-            var accessor = fields[Proto2.TestAllTypes.OptionalBoolFieldNumber].Accessor;
+            var message = new Proto2.TestAllTypes();
+            var accessor = ((IMessage) message).Descriptor.Fields[Proto2.TestAllTypes.OptionalInt64FieldNumber].Accessor;
+
+            Assert.IsFalse(accessor.HasValue(message));
+            message.OptionalInt64 = 5L;
+            Assert.IsTrue(accessor.HasValue(message));
+            message.ClearOptionalInt64();
+            Assert.IsFalse(accessor.HasValue(message));
+            message.OptionalInt64 = 0L;
+            Assert.IsTrue(accessor.HasValue(message));
+        }
 
-            Assert.False(accessor.HasValue(message));
+        [Test]
+        public void HasValue_Proto2_Message()
+        {
+            var message = new Proto2.TestAllTypes();
+            var field = ((IMessage) message).Descriptor.Fields[Proto2.TestAllTypes.OptionalForeignMessageFieldNumber];
+            Assert.False(field.Accessor.HasValue(message));
+            message.OptionalForeignMessage = new Proto2.ForeignMessage();
+            Assert.True(field.Accessor.HasValue(message));
+            message.OptionalForeignMessage = null;
+            Assert.False(field.Accessor.HasValue(message));
+        }
 
-            accessor.SetValue(message, true);
+        [Test]
+        public void HasValue_Proto2_Oneof()
+        {
+            var message = new Proto2.TestAllTypes();
+            var accessor = ((IMessage) message).Descriptor.Fields[Proto2.TestAllTypes.OneofStringFieldNumber].Accessor;
+            Assert.False(accessor.HasValue(message));
+            // Even though it's the default value, we still have a value.
+            message.OneofString = "";
             Assert.True(accessor.HasValue(message));
-
-            accessor.Clear(message);
+            message.OneofString = "hello";
+            Assert.True(accessor.HasValue(message));
+            message.OneofUint32 = 10;
             Assert.False(accessor.HasValue(message));
         }
 
+        [Test]
+        public void HasValue_Proto2_Repeated()
+        {
+            var message = new Proto2.TestAllTypes();
+            var accessor = ((IMessage) message).Descriptor.Fields[Proto2.TestAllTypes.RepeatedBoolFieldNumber].Accessor;
+            Assert.Throws<InvalidOperationException>(() => accessor.HasValue(message));
+        }
+
         [Test]
         public void SetValue_SingleFields()
         {
@@ -262,6 +330,42 @@ public void Clear_Proto3Optional()
             Assert.Null(message.OptionalNestedMessage);
         }
 
+        [Test]
+        public void Clear_Proto3_Oneof()
+        {
+            var message = new TestAllTypes();
+            var accessor = ((IMessage) message).Descriptor.Fields[TestProtos.TestAllTypes.OneofUint32FieldNumber].Accessor;
+
+            // The field accessor Clear method only affects a oneof if the current case is the one being cleared.
+            message.OneofString = "hello";
+            Assert.AreEqual(TestProtos.TestAllTypes.OneofFieldOneofCase.OneofString, message.OneofFieldCase);
+            accessor.Clear(message);
+            Assert.AreEqual(TestProtos.TestAllTypes.OneofFieldOneofCase.OneofString, message.OneofFieldCase);
+
+            message.OneofUint32 = 100;
+            Assert.AreEqual(TestProtos.TestAllTypes.OneofFieldOneofCase.OneofUint32, message.OneofFieldCase);
+            accessor.Clear(message);
+            Assert.AreEqual(TestProtos.TestAllTypes.OneofFieldOneofCase.None, message.OneofFieldCase);
+        }
+
+        [Test]
+        public void Clear_Proto2_Oneof()
+        {
+            var message = new Proto2.TestAllTypes();
+            var accessor = ((IMessage) message).Descriptor.Fields[Proto2.TestAllTypes.OneofUint32FieldNumber].Accessor;
+
+            // The field accessor Clear method only affects a oneof if the current case is the one being cleared.
+            message.OneofString = "hello";
+            Assert.AreEqual(Proto2.TestAllTypes.OneofFieldOneofCase.OneofString, message.OneofFieldCase);
+            accessor.Clear(message);
+            Assert.AreEqual(Proto2.TestAllTypes.OneofFieldOneofCase.OneofString, message.OneofFieldCase);
+
+            message.OneofUint32 = 100;
+            Assert.AreEqual(Proto2.TestAllTypes.OneofFieldOneofCase.OneofUint32, message.OneofFieldCase);
+            accessor.Clear(message);
+            Assert.AreEqual(Proto2.TestAllTypes.OneofFieldOneofCase.None, message.OneofFieldCase);
+        }
+
         [Test]
         public void FieldDescriptor_ByName()
         {
@@ -301,5 +405,32 @@ public void GetRepeatedExtensionValue()
             message.ClearExtension(RepeatedBoolExtension);
             Assert.IsNull(message.GetExtension(RepeatedBoolExtension));
         }
+
+        [Test]
+        public void HasPresence()
+        {
+            // Proto3
+            var fields = TestProtos.TestAllTypes.Descriptor.Fields;
+            Assert.IsFalse(fields[TestProtos.TestAllTypes.SingleBoolFieldNumber].HasPresence);
+            Assert.IsTrue(fields[TestProtos.TestAllTypes.OneofBytesFieldNumber].HasPresence);
+            Assert.IsTrue(fields[TestProtos.TestAllTypes.SingleForeignMessageFieldNumber].HasPresence);
+            Assert.IsFalse(fields[TestProtos.TestAllTypes.RepeatedBoolFieldNumber].HasPresence);
+
+            fields = TestMap.Descriptor.Fields;
+            Assert.IsFalse(fields[TestMap.MapBoolBoolFieldNumber].HasPresence);
+
+            fields = TestProto3Optional.Descriptor.Fields;
+            Assert.IsTrue(fields[TestProto3Optional.OptionalBoolFieldNumber].HasPresence);
+
+            // Proto2
+            fields = Proto2.TestAllTypes.Descriptor.Fields;
+            Assert.IsTrue(fields[Proto2.TestAllTypes.OptionalBoolFieldNumber].HasPresence);
+            Assert.IsTrue(fields[Proto2.TestAllTypes.OneofBytesFieldNumber].HasPresence);
+            Assert.IsTrue(fields[Proto2.TestAllTypes.OptionalForeignMessageFieldNumber].HasPresence);
+            Assert.IsFalse(fields[Proto2.TestAllTypes.RepeatedBoolFieldNumber].HasPresence);
+
+            fields = Proto2.TestRequired.Descriptor.Fields;
+            Assert.IsTrue(fields[Proto2.TestRequired.AFieldNumber].HasPresence);
+        }
     }
 }

csharp/src/Google.Protobuf/Extension.cs

@@ -55,6 +55,8 @@ protected Extension(int fieldNumber)
         /// Gets the field number of this extension
         /// </summary>
         public int FieldNumber { get; }
+
+        internal abstract bool IsRepeated { get; }
     }
 
     /// <summary>
@@ -79,6 +81,8 @@ public Extension(int fieldNumber, FieldCodec<TValue> codec) : base(fieldNumber)
 
         internal override Type TargetType => typeof(TTarget);
 
+        internal override bool IsRepeated => false;
+
         internal override IExtensionValue CreateValue()
         {
             return new ExtensionValue<TValue>(codec);
@@ -105,6 +109,8 @@ public RepeatedExtension(int fieldNumber, FieldCodec<TValue> codec) : base(field
 
         internal override Type TargetType => typeof(TTarget);
 
+        internal override bool IsRepeated => true;
+
         internal override IExtensionValue CreateValue()
         {
             return new RepeatedExtensionValue<TValue>(codec);

csharp/src/Google.Protobuf/Reflection/FieldDescriptor.cs

@@ -70,6 +70,21 @@ public sealed class FieldDescriptor : DescriptorBase, IComparable<FieldDescripto
         /// </summary>
         public string JsonName { get; }
 
+        /// <summary>
+        /// Indicates whether this field supports presence, either implicitly (e.g. due to it being a message
+        /// type field) or explicitly via Has/Clear members. If this returns true, it is safe to call
+        /// <see cref="IFieldAccessor.Clear(IMessage)"/> and <see cref="IFieldAccessor.HasValue(IMessage)"/>
+        /// on this field's accessor with a suitable message.
+        /// </summary>
+        public bool HasPresence =>
+            Extension != null ? !Extension.IsRepeated
+            : IsRepeated ? false
+            : IsMap ? false
+            : FieldType == FieldType.Message ? true
+            // This covers "real oneof members" and "proto3 optional fields"
+            : ContainingOneof != null ? true
+            : File.Syntax == Syntax.Proto2;
+
         internal FieldDescriptorProto Proto { get; }
 
         /// <summary>

csharp/src/Google.Protobuf/Reflection/SingleFieldAccessor.cs

@@ -57,63 +57,68 @@ internal SingleFieldAccessor(PropertyInfo property, FieldDescriptor descriptor)
                 throw new ArgumentException("Not all required properties/methods available");
             }
             setValueDelegate = ReflectionUtil.CreateActionIMessageObject(property.GetSetMethod());
-            if (descriptor.File.Syntax == Syntax.Proto3 && !descriptor.Proto.Proto3Optional)
+
+            // Note: this looks worrying in that we access the containing oneof, which isn't valid until cross-linking
+            // is complete... but field accessors aren't created until after cross-linking.
+            // The oneof itself won't be cross-linked yet, but that's okay: the oneof accessor is created
+            // earlier.
+
+            // Message fields always support presence, via null checks.
+            if (descriptor.FieldType == FieldType.Message)
+            {
+                hasDelegate = message => GetValue(message) != null;
+                clearDelegate = message => SetValue(message, null);
+            }
+            // Oneof fields always support presence, via case checks.
+            // Note that clearing the field is a no-op unless that specific field is the current "case".
+            else if (descriptor.RealContainingOneof != null)
             {
-                hasDelegate = message =>
+                var oneofAccessor = descriptor.RealContainingOneof.Accessor;
+                hasDelegate = message => oneofAccessor.GetCaseFieldDescriptor(message) == descriptor;
+                clearDelegate = message =>
                 {
-                    throw new InvalidOperationException("HasValue is not implemented for non-optional proto3 fields");
+                    // Clear on a field only affects the oneof itself if the current case is the field we're accessing.
+                    if (oneofAccessor.GetCaseFieldDescriptor(message) == descriptor)
+                    {
+                        oneofAccessor.Clear(message);
+                    }
                 };
-                var clrType = property.PropertyType;
-
-                // TODO: Validate that this is a reasonable single field? (Should be a value type, a message type, or string/ByteString.)
-                object defaultValue =
-                    descriptor.FieldType == FieldType.Message ? null
-                    : clrType == typeof(string) ? ""
-                    : clrType == typeof(ByteString) ? ByteString.Empty
-                    : Activator.CreateInstance(clrType);
-                clearDelegate = message => SetValue(message, defaultValue);
             }
-            else
+            // Primitive fields always support presence in proto2, and support presence in proto3 for optional fields.
+            else if (descriptor.File.Syntax == Syntax.Proto2 || descriptor.Proto.Proto3Optional)
             {
-                // For message fields, just compare with null and set to null.
-                // For primitive fields, use the Has/Clear methods.
-
-                if (descriptor.FieldType == FieldType.Message)
+                MethodInfo hasMethod = property.DeclaringType.GetRuntimeProperty("Has" + property.Name).GetMethod;
+                if (hasMethod == null)
                 {
-                    hasDelegate = message => GetValue(message) != null;
-                    clearDelegate = message => SetValue(message, null);
+                    throw new ArgumentException("Not all required properties/methods are available");
                 }
-                else
+                hasDelegate = ReflectionUtil.CreateFuncIMessageBool(hasMethod);
+                MethodInfo clearMethod = property.DeclaringType.GetRuntimeMethod("Clear" + property.Name, ReflectionUtil.EmptyTypes);
+                if (clearMethod == null)
                 {
-                    MethodInfo hasMethod = property.DeclaringType.GetRuntimeProperty("Has" + property.Name).GetMethod;
-                    if (hasMethod == null)
-                    {
-                        throw new ArgumentException("Not all required properties/methods are available");
-                    }
-                    hasDelegate = ReflectionUtil.CreateFuncIMessageBool(hasMethod);
-                    MethodInfo clearMethod = property.DeclaringType.GetRuntimeMethod("Clear" + property.Name, ReflectionUtil.EmptyTypes);
-                    if (clearMethod == null)
-                    {
-                        throw new ArgumentException("Not all required properties/methods are available");
-                    }
-                    clearDelegate = ReflectionUtil.CreateActionIMessage(clearMethod);
+                    throw new ArgumentException("Not all required properties/methods are available");
                 }
+                clearDelegate = ReflectionUtil.CreateActionIMessage(clearMethod);
             }
-        }
+            // What's left?
+            // Primitive proto3 fields without the optional keyword, which aren't in oneofs.
+            else
+            {
+                hasDelegate = message => { throw new InvalidOperationException("Presence is not implemented for this field"); };
 
-        public override void Clear(IMessage message)
-        {
-            clearDelegate(message);
-        }
+                // While presence isn't supported, clearing still is; it's just setting to a default value.
+                var clrType = property.PropertyType;
 
-        public override bool HasValue(IMessage message)
-        {
-            return hasDelegate(message);
+                object defaultValue =
+                    clrType == typeof(string) ? ""
+                    : clrType == typeof(ByteString) ? ByteString.Empty
+                    : Activator.CreateInstance(clrType);
+                clearDelegate = message => SetValue(message, defaultValue);
+            }
         }
 
-        public override void SetValue(IMessage message, object value)
-        {
-            setValueDelegate(message, value);
-        }
+        public override void Clear(IMessage message) => clearDelegate(message);
+        public override bool HasValue(IMessage message) => hasDelegate(message);
+        public override void SetValue(IMessage message, object value) => setValueDelegate(message, value);
     }
 }