Merge pull request #952 from stakx/split-properties

Fix `SetupProperty` for partially overridden properties

CHANGELOG.md

@@ -14,6 +14,7 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 * `ArgumentException` ("Interface not found") when setting up `object.ToString` on an interface mock (@vslynko, #942)
 * Cannot "return" to original mocked type after downcasting with `Mock.Get` and then upcasting with `mock.As<>` (@pjquirk, #943)
 * `params` arrays in recursive setup expressions are matched by reference equality instead of by structural equality (@danielcweber, #946)
+* `mock.SetupProperty` throws `NullReferenceException` when called for partially overridden property (@stakx, #951)
 
 ## 4.13.0 (2019-08-31)
 

src/Moq/ExpressionExtensions.cs

@@ -284,8 +284,8 @@ void Split(Expression e, out Expression r /* remainder */, out InvocationShape p
 						r = memberAccessExpression.Expression;
 						var parameter = Expression.Parameter(r.Type, r is ParameterExpression ope ? ope.Name : ParameterName);
 						var property = memberAccessExpression.GetReboundProperty();
-						var method = property.CanRead ? property.GetGetMethod(true) : property.GetSetMethod(true);
-						//                    ^^^^^^^                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+						var method = property.CanRead(out var getter) ? getter : property.GetSetMethod(true);
+						//                    ^^^^^^^                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 						// We're in the switch case block for property read access, therefore we prefer the
 						// getter. When a read-write property is being assigned to, we end up here, too, and
 						// select the wrong accessor. However, that doesn't matter because it will be over-
@@ -324,8 +324,10 @@ internal static PropertyInfo GetReboundProperty(this MemberExpression expression
 					.GetMember(property.Name, MemberTypes.Property, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
 					.Cast<PropertyInfo>()
 					.SingleOrDefault(p => p.PropertyType == property.PropertyType);
-				if (derivedProperty != null && derivedProperty.GetMethod.GetBaseDefinition() == property.GetMethod)
+				if (derivedProperty != null)
 				{
+					if ((derivedProperty.CanRead(out var getter) && getter.GetBaseDefinition() == property.GetGetMethod(true)) ||
+						(derivedProperty.CanWrite(out var setter) && setter.GetBaseDefinition() == property.GetSetMethod(true)))
 					return derivedProperty;
 				}
 			}

src/Moq/Extensions.cs

@@ -250,24 +250,12 @@ public static bool CanOverride(this MethodBase method)
 
 		public static bool CanOverrideGet(this PropertyInfo property)
 		{
-			if (property.CanRead)
-			{
-				var getter = property.GetGetMethod(true);
-				return getter != null && getter.CanOverride();
-			}
-
-			return false;
+			return property.CanRead(out var getter) && getter.CanOverride();
 		}
 
 		public static bool CanOverrideSet(this PropertyInfo property)
 		{
-			if (property.CanWrite)
-			{
-				var setter = property.GetSetMethod(true);
-				return setter != null && setter.CanOverride();
-			}
-
-			return false;
+			return property.CanWrite(out var setter) && setter.CanOverride();
 		}
 
 		public static IEnumerable<MethodInfo> GetMethods(this Type type, string name)

src/Moq/Guard.cs

@@ -229,7 +229,7 @@ public static void Positive(TimeSpan delay)
 
 		public static void CanRead(PropertyInfo property)
 		{
-			if (!property.CanRead)
+			if (!property.CanRead(out _))
 			{
 				throw new ArgumentException(string.Format(
 					CultureInfo.CurrentCulture,
@@ -240,7 +240,7 @@ public static void CanRead(PropertyInfo property)
 
 		public static void CanWrite(PropertyInfo property)
 		{
-			if (!property.CanWrite)
+			if (!property.CanWrite(out _))
 			{
 				throw new ArgumentException(string.Format(
 					CultureInfo.CurrentCulture,

src/Moq/Linq/MockSetupsBuilder.cs

@@ -203,7 +203,7 @@ private static Expression ConvertToSetupProperty(Expression targetObject, Expres
 			// where Mock.Get(foo).SetupProperty(mock => mock.Name, "bar") != null
 
 			// if the property is readonly, we can only do a Setup(...) which is the same as a method setup.
-			if (!propertyInfo.CanWrite || propertyInfo.GetSetMethod(true).IsPrivate)
+			if (!propertyInfo.CanWrite(out var setter) || setter.IsPrivate)
 				return ConvertToSetup(targetObject, left, right);
 
 			// This will get up to and including the Mock.Get(foo).Setup(mock => mock.Name) call.

src/Moq/Mock.cs

@@ -813,7 +813,7 @@ internal void AddInnerMockSetup(MethodInfo method, IReadOnlyList<Expression> arg
 				var property = expression.ToPropertyInfo();
 				Guard.CanRead(property);
 
-				Debug.Assert(method == property.GetGetMethod(true));
+				Debug.Assert(property.CanRead(out var getter) && method == getter);
 			}
 
 			this.Setups.Add(new InnerMockSetup(new InvocationShape(expression, method, arguments, exactGenericTypeArguments: true), returnValue));

src/Moq/Protected/ProtectedAsMock.cs

@@ -357,8 +357,8 @@ private static bool IsCorrespondingProperty(PropertyInfo duckProperty, PropertyI
 			{
 				return candidateTargetProperty.Name == duckProperty.Name
 				    && candidateTargetProperty.PropertyType == duckProperty.PropertyType
-				    && candidateTargetProperty.CanRead == duckProperty.CanRead
-				    && candidateTargetProperty.CanWrite == duckProperty.CanWrite;
+				    && candidateTargetProperty.CanRead(out _) == duckProperty.CanRead(out _)
+				    && candidateTargetProperty.CanWrite(out _) == duckProperty.CanWrite(out _);
 
 				// TODO: parameter lists should be compared, too, to properly support indexers.
 			}

src/Moq/Protected/ProtectedMock.cs

@@ -330,7 +330,7 @@ private static void ThrowIfPublicMethod(MethodInfo method, string reflectedTypeN
 
 		private static void ThrowIfPublicGetter(PropertyInfo property, string reflectedTypeName)
 		{
-			if (property.CanRead && property.GetGetMethod() != null)
+			if (property.CanRead(out var getter) && getter.IsPublic)
 			{
 				throw new ArgumentException(string.Format(
 					CultureInfo.CurrentCulture,
@@ -342,7 +342,7 @@ private static void ThrowIfPublicGetter(PropertyInfo property, string reflectedT
 
 		private static void ThrowIfPublicSetter(PropertyInfo property, string reflectedTypeName)
 		{
-			if (property.CanWrite && property.GetSetMethod() != null)
+			if (property.CanWrite(out var setter) && setter.IsPublic)
 			{
 				throw new ArgumentException(string.Format(
 					CultureInfo.CurrentCulture,

tests/Moq.Tests/StubExtensionsFixture.cs

@@ -267,6 +267,15 @@ public void SetupAllProperties_retains_value_of_derived_read_write_property_that
 			Assert.Equal("value", mock.Object.Property);
 		}
 
+		[Fact]
+		public void SetupProperty_retains_value_of_derived_read_write_property_that_overrides_only_setter()
+		{
+			var mock = new Mock<OverridesOnlySetter>();
+			mock.SetupProperty(m => m.Property);
+			mock.Object.Property = "value";
+			Assert.Equal("value", mock.Object.Property);
+		}
+
 		private object GetValue() { return new object(); }
 
 		public interface IFoo