Fix #6. Collect only observable fields.

ChebanovDD · ChebanovDD · commit a808c0c74e1c · 2023-05-04T14:25:10.000+08:00
diff --git a/src/UnityMvvmToolkit.Core/Internal/BindingContextMemberProvider.cs b/src/UnityMvvmToolkit.Core/Internal/BindingContextMemberProvider.cs
@@ -63,40 +63,49 @@ private static bool TryGetFieldHashCode(Type contextType, FieldInfo fieldInfo, o
                 return TryGetHashCode(contextType, fieldInfo.Name, fieldInfo.FieldType, out hashCode);
             }
 
-            if (TryGetPropertyNameFromAttribute(fieldInfo, out var fieldName))
+            if (HasObservableAttribute(fieldInfo, out var propertyName) == false)
             {
-                return TryGetHashCode(contextType, fieldName, fieldInfo.FieldType, out hashCode);
+                hashCode = default;
+                return false;
             }
 
-            fieldName = fieldInfo.Name;
+            return string.IsNullOrWhiteSpace(propertyName)
+                ? TryGetHashCode(contextType, GetFieldName(fieldInfo.Name), fieldInfo.FieldType, out hashCode)
+                : TryGetHashCode(contextType, propertyName, fieldInfo.FieldType, out hashCode);
+        }
 
-            if (fieldName.Length > 1)
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static bool TryGetPropertyHashCode(Type contextType, PropertyInfo propertyInfo, out int hashCode)
+        {
+            if (propertyInfo.GetMethod.IsPrivate)
             {
-                if (fieldName[0] == '_')
-                {
-                    fieldName = fieldName[1..]; // TODO: Get rid of allocation.
-                }
-
-                if (fieldName[0] == 'm' && fieldName[1] == '_')
-                {
-                    fieldName = fieldName[2..]; // TODO: Get rid of allocation.
-                }
+                hashCode = default;
+                return false;
             }
 
-            if (string.IsNullOrEmpty(fieldName))
+            return TryGetHashCode(contextType, propertyInfo.Name, propertyInfo.PropertyType, out hashCode);
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static bool TryGetHashCode(Type contextType, string memberName, Type memberType, out int hashCode)
+        {
+            if (typeof(IBaseCommand).IsAssignableFrom(memberType) ||
+                typeof(IBaseProperty).IsAssignableFrom(memberType))
             {
-                throw new InvalidOperationException($"Field name '{fieldName}' is not supported.");
+                hashCode = HashCodeHelper.GetMemberHashCode(contextType, memberName);
+                return true;
             }
 
-            return TryGetHashCode(contextType, fieldName, fieldInfo.FieldType, out hashCode);
+            hashCode = default;
+            return false;
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private static bool TryGetPropertyNameFromAttribute(MemberInfo fieldInfo, out string propertyName)
+        private static bool HasObservableAttribute(MemberInfo fieldInfo, out string propertyName)
         {
             var observableAttribute = fieldInfo.GetCustomAttribute<ObservableAttribute>();
 
-            if (observableAttribute == null || string.IsNullOrWhiteSpace(observableAttribute.PropertyName))
+            if (observableAttribute == null)
             {
                 propertyName = default;
                 return false;
@@ -107,29 +116,29 @@ private static bool TryGetPropertyNameFromAttribute(MemberInfo fieldInfo, out st
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private static bool TryGetPropertyHashCode(Type contextType, PropertyInfo propertyInfo, out int hashCode)
+        private static string GetFieldName(string fieldName)
         {
-            if (propertyInfo.GetMethod.IsPrivate)
+            var resultName = fieldName;
+
+            if (resultName.Length > 1)
             {
-                hashCode = default;
-                return false;
-            }
+                if (resultName[0] == '_')
+                {
+                    resultName = resultName[1..]; // TODO: Get rid of allocation.
+                }
 
-            return TryGetHashCode(contextType, propertyInfo.Name, propertyInfo.PropertyType, out hashCode);
-        }
+                if (resultName[0] == 'm' && resultName[1] == '_')
+                {
+                    resultName = resultName[2..]; // TODO: Get rid of allocation.
+                }
+            }
 
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private static bool TryGetHashCode(Type contextType, string memberName, Type memberType, out int hashCode)
-        {
-            if (typeof(IBaseCommand).IsAssignableFrom(memberType) ||
-                typeof(IBaseProperty).IsAssignableFrom(memberType))
+            if (string.IsNullOrEmpty(resultName))
             {
-                hashCode = HashCodeHelper.GetMemberHashCode(contextType, memberName);
-                return true;
+                throw new InvalidOperationException($"Field name '{resultName}' is not supported.");
             }
 
-            hashCode = default;
-            return false;
+            return resultName;
         }
     }
 }
diff --git a/src/UnityMvvmToolkit.UnityPackage/Assets/Plugins/UnityMvvmToolkit/Runtime/Core/Internal/BindingContextMemberProvider.cs b/src/UnityMvvmToolkit.UnityPackage/Assets/Plugins/UnityMvvmToolkit/Runtime/Core/Internal/BindingContextMemberProvider.cs
@@ -63,40 +63,49 @@ private static bool TryGetFieldHashCode(Type contextType, FieldInfo fieldInfo, o
                 return TryGetHashCode(contextType, fieldInfo.Name, fieldInfo.FieldType, out hashCode);
             }
 
-            if (TryGetPropertyNameFromAttribute(fieldInfo, out var fieldName))
+            if (HasObservableAttribute(fieldInfo, out var propertyName) == false)
             {
-                return TryGetHashCode(contextType, fieldName, fieldInfo.FieldType, out hashCode);
+                hashCode = default;
+                return false;
             }
 
-            fieldName = fieldInfo.Name;
+            return string.IsNullOrWhiteSpace(propertyName)
+                ? TryGetHashCode(contextType, GetFieldName(fieldInfo.Name), fieldInfo.FieldType, out hashCode)
+                : TryGetHashCode(contextType, propertyName, fieldInfo.FieldType, out hashCode);
+        }
 
-            if (fieldName.Length > 1)
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static bool TryGetPropertyHashCode(Type contextType, PropertyInfo propertyInfo, out int hashCode)
+        {
+            if (propertyInfo.GetMethod.IsPrivate)
             {
-                if (fieldName[0] == '_')
-                {
-                    fieldName = fieldName[1..]; // TODO: Get rid of allocation.
-                }
-
-                if (fieldName[0] == 'm' && fieldName[1] == '_')
-                {
-                    fieldName = fieldName[2..]; // TODO: Get rid of allocation.
-                }
+                hashCode = default;
+                return false;
             }
 
-            if (string.IsNullOrEmpty(fieldName))
+            return TryGetHashCode(contextType, propertyInfo.Name, propertyInfo.PropertyType, out hashCode);
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static bool TryGetHashCode(Type contextType, string memberName, Type memberType, out int hashCode)
+        {
+            if (typeof(IBaseCommand).IsAssignableFrom(memberType) ||
+                typeof(IBaseProperty).IsAssignableFrom(memberType))
             {
-                throw new InvalidOperationException($"Field name '{fieldName}' is not supported.");
+                hashCode = HashCodeHelper.GetMemberHashCode(contextType, memberName);
+                return true;
             }
 
-            return TryGetHashCode(contextType, fieldName, fieldInfo.FieldType, out hashCode);
+            hashCode = default;
+            return false;
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private static bool TryGetPropertyNameFromAttribute(MemberInfo fieldInfo, out string propertyName)
+        private static bool HasObservableAttribute(MemberInfo fieldInfo, out string propertyName)
         {
             var observableAttribute = fieldInfo.GetCustomAttribute<ObservableAttribute>();
 
-            if (observableAttribute == null || string.IsNullOrWhiteSpace(observableAttribute.PropertyName))
+            if (observableAttribute == null)
             {
                 propertyName = default;
                 return false;
@@ -107,29 +116,29 @@ private static bool TryGetPropertyNameFromAttribute(MemberInfo fieldInfo, out st
         }
 
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private static bool TryGetPropertyHashCode(Type contextType, PropertyInfo propertyInfo, out int hashCode)
+        private static string GetFieldName(string fieldName)
         {
-            if (propertyInfo.GetMethod.IsPrivate)
+            var resultName = fieldName;
+
+            if (resultName.Length > 1)
             {
-                hashCode = default;
-                return false;
-            }
+                if (resultName[0] == '_')
+                {
+                    resultName = resultName[1..]; // TODO: Get rid of allocation.
+                }
 
-            return TryGetHashCode(contextType, propertyInfo.Name, propertyInfo.PropertyType, out hashCode);
-        }
+                if (resultName[0] == 'm' && resultName[1] == '_')
+                {
+                    resultName = resultName[2..]; // TODO: Get rid of allocation.
+                }
+            }
 
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        private static bool TryGetHashCode(Type contextType, string memberName, Type memberType, out int hashCode)
-        {
-            if (typeof(IBaseCommand).IsAssignableFrom(memberType) ||
-                typeof(IBaseProperty).IsAssignableFrom(memberType))
+            if (string.IsNullOrEmpty(resultName))
             {
-                hashCode = HashCodeHelper.GetMemberHashCode(contextType, memberName);
-                return true;
+                throw new InvalidOperationException($"Field name '{resultName}' is not supported.");
             }
 
-            hashCode = default;
-            return false;
+            return resultName;
         }
     }
 }
diff --git a/tests/UnityMvvmToolkit.Test.Integration/TestBindingContext/MyBindingContext.cs b/tests/UnityMvvmToolkit.Test.Integration/TestBindingContext/MyBindingContext.cs
@@ -10,6 +10,7 @@ namespace UnityMvvmToolkit.Test.Integration.TestBindingContext;
 
 public class MyBindingContext : IBindingContext
 {
+    [Observable]
     private readonly IProperty<int> _count = new Property<int>();
     
     [Observable(nameof(IntReadOnlyValue))]
diff --git a/tests/UnityMvvmToolkit.Test.Unit/BindingContextMemberProviderTests.cs b/tests/UnityMvvmToolkit.Test.Unit/BindingContextMemberProviderTests.cs
@@ -111,7 +111,7 @@ public void GetBindingContextMembers_ShouldThrow_WhenTypeIsNotAssignableFromIBin
     public void GetBindingContextMembers_ShouldThrow_WhenResultsDictionaryIsNull()
     {
         // Arrange
-        var bindingContextType = typeof(PrivateFieldBindingContext);
+        var bindingContextType = typeof(ObservableFieldBindingContext);
 
         // Assert
         _memberProvider
@@ -126,6 +126,7 @@ private static IEnumerable<object[]> BindingContextDataSets()
         yield return GetObservableFieldBindingContextTestData();
         yield return GetPublicFieldBindingContextTestData();
         yield return GetPrivatePropertyBindingContextTestData();
+        yield return GetSameFieldAndPropertyNamesBindingContextTestData();
         yield return GetPublicPropertyBindingContextTestData();
         yield return GetCommandBindingContextTestData();
         yield return GetNoObservableFieldsBindingContextTestData();
@@ -134,17 +135,7 @@ private static IEnumerable<object[]> BindingContextDataSets()
     private static object[] GetPrivateFieldBindingContextTestData()
     {
         var bindingContextType = typeof(PrivateFieldBindingContext);
-        return new object[]
-        {
-            bindingContextType,
-            new (int, MemberTypes)[]
-            {
-                (HashCodeHelper.GetMemberHashCode(bindingContextType, "BoolField"), MemberTypes.Field),
-                (HashCodeHelper.GetMemberHashCode(bindingContextType, "IntField"), MemberTypes.Field),
-                (HashCodeHelper.GetMemberHashCode(bindingContextType, "FloatField"), MemberTypes.Field),
-                (HashCodeHelper.GetMemberHashCode(bindingContextType, "StrField"), MemberTypes.Field)
-            }
-        };
+        return new object[] { bindingContextType, Array.Empty<(int, MemberTypes)>() };
     }
 
     private static object[] GetObservableFieldBindingContextTestData()
@@ -186,6 +177,20 @@ private static object[] GetPrivatePropertyBindingContextTestData()
         return new object[] { typeof(PrivatePropertyBindingContext), Array.Empty<(int, MemberTypes)>() };
     }
 
+    private static object[] GetSameFieldAndPropertyNamesBindingContextTestData()
+    {
+        var bindingContextType = typeof(SameFieldAndPropertyNamesBindingContext);
+        return new object[]
+        {
+            bindingContextType,
+            new (int, MemberTypes)[]
+            {
+                (HashCodeHelper.GetMemberHashCode(bindingContextType, nameof(SameFieldAndPropertyNamesBindingContext.Count)),
+                    MemberTypes.Property)
+            }
+        };
+    }
+
     private static object[] GetPublicPropertyBindingContextTestData()
     {
         var bindingContextType = typeof(PublicPropertyBindingContext);
diff --git a/tests/UnityMvvmToolkit.Test.Unit/TestBindingContext/InvalidFieldNameBindingContext.cs b/tests/UnityMvvmToolkit.Test.Unit/TestBindingContext/InvalidFieldNameBindingContext.cs
@@ -1,4 +1,5 @@
 ﻿using UnityMvvmToolkit.Core;
+using UnityMvvmToolkit.Core.Attributes;
 using UnityMvvmToolkit.Core.Interfaces;
 
 // ReSharper disable UnusedMember.Local
@@ -8,6 +9,9 @@ namespace UnityMvvmToolkit.Test.Unit.TestBindingContext;
 
 public class InvalidFieldNameBindingContext : IBindingContext
 {
+    [Observable]
     private IProperty<int> _ = new Property<int>();
+
+    [Observable]
     private IProperty<int> m_ = new Property<int>();
 }
diff --git a/tests/UnityMvvmToolkit.Test.Unit/TestBindingContext/SameFieldAndPropertyNamesBindingContext.cs b/tests/UnityMvvmToolkit.Test.Unit/TestBindingContext/SameFieldAndPropertyNamesBindingContext.cs
@@ -0,0 +1,11 @@
+﻿using UnityMvvmToolkit.Core;
+using UnityMvvmToolkit.Core.Interfaces;
+
+namespace UnityMvvmToolkit.Test.Unit.TestBindingContext;
+
+public class SameFieldAndPropertyNamesBindingContext : IBindingContext
+{
+    private readonly IProperty<int> _count = new Property<int>();
+
+    public IReadOnlyProperty<int> Count => _count;
+}

Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,7 @@ namespace UnityMvvmToolkit.Test.Integration.TestBindingContext;`
`10`	`10`
`11`	`11`	`public class MyBindingContext : IBindingContext`
`12`	`12`	`{`
	`13`	`+ [Observable]`
`13`	`14`	`private readonly IProperty<int> _count = new Property<int>();`
`14`	`15`
`15`	`16`	`[Observable(nameof(IntReadOnlyValue))]`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`using UnityMvvmToolkit.Core;`
	`2`	`+using UnityMvvmToolkit.Core.Attributes;`
`2`	`3`	`using UnityMvvmToolkit.Core.Interfaces;`
`3`	`4`
`4`	`5`	`// ReSharper disable UnusedMember.Local`
`@@ -8,6 +9,9 @@ namespace UnityMvvmToolkit.Test.Unit.TestBindingContext;`
`8`	`9`
`9`	`10`	`public class InvalidFieldNameBindingContext : IBindingContext`
`10`	`11`	`{`
	`12`	`+ [Observable]`
`11`	`13`	`private IProperty<int> _ = new Property<int>();`
	`14`	`+`
	`15`	`+ [Observable]`
`12`	`16`	`private IProperty<int> m_ = new Property<int>();`
`13`	`17`	`}`