Merge pull request #1079 from stakx/setupproperty

Fix `SetupProperty` for split properties (where mocked type overrides only the getter)

CHANGELOG.md

@@ -12,6 +12,10 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 * New method overloads for `It.Is`, `It.IsIn`, and `It.IsNotIn` that compare values using a custom `IEqualityComparer<T>` (@weitzhandler, #1064)
 * New properties `ReturnValue` and `Exception` on `IInvocation` to query recorded invocations return values or exceptions (@MaStr11, #921, #1077)
 
+#### Fixed
+
+* `SetupProperty` fails if property getter and setter are not both defined in mocked type (@stakx, #1017)
+
 
 ## 4.14.7 (2020-10-14)
 

src/Moq/ExpressionExtensions.cs

@@ -244,9 +244,21 @@ void Split(Expression e, out Expression r /* remainder */, out InvocationShape p
 						var parameter = Expression.Parameter(r.Type, r is ParameterExpression ope ? ope.Name : ParameterName);
 						var indexer = indexExpression.Indexer;
 						var arguments = indexExpression.Arguments;
-						var method = !assignment && indexer.CanRead(out var getter)  ? getter
-						           :                indexer.CanWrite(out var setter) ? setter
-						           :                                                   null;
+						MethodInfo method;
+						if (!assignment && indexer.CanRead(out var getter, out var getterIndexer))
+						{
+							method = getter;
+							indexer = getterIndexer;
+						}
+						else if (indexer.CanWrite(out var setter, out var setterIndexer))
+						{
+							method = setter;
+							indexer = setterIndexer;
+						}
+						else  // This should be unreachable.
+						{
+							method = null;
+						}
 						p = new InvocationShape(
 									expression: Expression.Lambda(
 										Expression.MakeIndex(parameter, indexer, arguments),
@@ -281,9 +293,21 @@ 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 = !assignment && property.CanRead(out var getter)  ? getter
-						           :                property.CanWrite(out var setter) ? setter
-						           :                                                    null;
+						MethodInfo method;
+						if (!assignment && property.CanRead(out var getter, out var getterProperty))
+						{
+							method = getter;
+							property = getterProperty;
+						}
+						else if (property.CanWrite(out var setter, out var setterProperty))
+						{
+							method = setter;
+							property = setterProperty;
+						}
+						else  // This should be unreachable.
+						{
+							method = null;
+						}
 						p = new InvocationShape(
 									expression: Expression.Lambda(
 										Expression.MakeMemberAccess(parameter, property),

src/Moq/Extensions.cs

@@ -20,11 +20,17 @@ public static bool CanCreateInstance(this Type type)
 		}
 
 		public static bool CanRead(this PropertyInfo property, out MethodInfo getter)
+		{
+			return property.CanRead(out getter, out _);
+		}
+
+		public static bool CanRead(this PropertyInfo property, out MethodInfo getter, out PropertyInfo getterProperty)
 		{
 			if (property.CanRead)
 			{
 				// The given `PropertyInfo` should be able to provide a getter:
 				getter = property.GetGetMethod(nonPublic: true);
+				getterProperty = property;
 				Debug.Assert(getter != null);
 				return true;
 			}
@@ -47,20 +53,27 @@ public static bool CanRead(this PropertyInfo property, out MethodInfo getter)
 						.GetMember(property.Name, MemberTypes.Property, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
 						.Cast<PropertyInfo>()
 						.First(p => p.GetSetMethod(nonPublic: true) == baseSetter);
-					return baseProperty.CanRead(out getter);
+					return baseProperty.CanRead(out getter, out getterProperty);
 				}
 			}
 
 			getter = null;
+			getterProperty = null;
 			return false;
 		}
 
 		public static bool CanWrite(this PropertyInfo property, out MethodInfo setter)
+		{
+			return property.CanWrite(out setter, out _);
+		}
+
+		public static bool CanWrite(this PropertyInfo property, out MethodInfo setter, out PropertyInfo setterProperty)
 		{
 			if (property.CanWrite)
 			{
 				// The given `PropertyInfo` should be able to provide a setter:
 				setter = property.GetSetMethod(nonPublic: true);
+				setterProperty = property;
 				Debug.Assert(setter != null);
 				return true;
 			}
@@ -83,11 +96,12 @@ public static bool CanWrite(this PropertyInfo property, out MethodInfo setter)
 						.GetMember(property.Name, MemberTypes.Property, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
 						.Cast<PropertyInfo>()
 						.First(p => p.GetGetMethod(nonPublic: true) == baseGetter);
-					return baseProperty.CanWrite(out setter);
+					return baseProperty.CanWrite(out setter, out setterProperty);
 				}
 			}
 
 			setter = null;
+			setterProperty = null;
 			return false;
 		}
 

tests/Moq.Tests/StubExtensionsFixture.cs

@@ -276,6 +276,15 @@ public void SetupProperty_retains_value_of_derived_read_write_property_that_over
 			Assert.Equal("value", mock.Object.Property);
 		}
 
+		[Fact]
+		public void SetupProperty_retains_value_of_derived_read_write_property_that_overrides_only_getter()
+		{
+			var mock = new Mock<OverridesOnlyGetter>();
+			mock.SetupProperty(m => m.Property);
+			mock.Object.Property = "value";
+			Assert.Equal("value", mock.Object.Property);
+		}
+
 		private object GetValue() { return new object(); }
 
 		public interface IFoo
@@ -326,5 +335,10 @@ public class OverridesOnlySetter : WithAutoProperty
 		{
 			public override object Property { set => base.Property = value; }
 		}
+
+		public class OverridesOnlyGetter : WithAutoProperty
+		{
+			public override object Property { get => base.Property; }
+		}
 	}
 }