Ensure SetupAllProperties stubs all accessors of partially overridden properties

CHANGELOG.md

@@ -8,6 +8,7 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 
 #### Fixed
 
+* `SetupAllProperties` does not recognize property as read-write if only setter is overridden (@stakx, #886)
 * Regression: `InvalidCastException` caused by Moq erroneously reusing a cached auto-mocked (`DefaultValue.Mock`) return value for a different generic method instantiation (@BrunoJuchli, #932)
 * AmbiguousMatchException when setting up the property, that hides another one (@ishatalkin, #939)
 * `ArgumentException` ("Interface not found") when setting up `object.ToString` on an interface mock (@vslynko, #942)

src/Moq/Extensions.cs

@@ -19,6 +19,78 @@ public static bool CanCreateInstance(this Type type)
  return type.IsValueType || type.GetConstructor(Type.EmptyTypes) != null;
  }
 
+ public static bool CanRead(this PropertyInfo property, out MethodInfo getter)
+ {
+ if (property.CanRead)
+ {
+ // The given `PropertyInfo` should be able to provide a getter:
+ getter = property.GetGetMethod(nonPublic: true);
+ Debug.Assert(getter != null);
+ return true;
+ }
+ else
+ {
+ // The given `PropertyInfo` cannot provide a getter... but there may still be one in a base class'
+ // corresponding `PropertyInfo`! We need to find that base `PropertyInfo`, and because `PropertyInfo`
+ // does not have `.GetBaseDefinition()`, we'll find it via the setter's `.GetBaseDefinition()`.
+ // (We may assume that there's a setter because properties/indexers must have at least one accessor.)
+ Debug.Assert(property.CanWrite);
+ var setter = property.GetSetMethod(nonPublic: true);
+ Debug.Assert(setter != null);
+
+ var baseSetter = setter.GetBaseDefinition();
+ if (baseSetter != setter)
+ {
+ var baseProperty =
+ baseSetter
+ .DeclaringType
+ .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);
+ }
+ }
+
+ getter = null;
+ return false;
+ }
+
+ public static bool CanWrite(this PropertyInfo property, out MethodInfo setter)
+ {
+ if (property.CanWrite)
+ {
+ // The given `PropertyInfo` should be able to provide a setter:
+ setter = property.GetSetMethod(nonPublic: true);
+ Debug.Assert(setter != null);
+ return true;
+ }
+ else
+ {
+ // The given `PropertyInfo` cannot provide a setter... but there may still be one in a base class'
+ // corresponding `PropertyInfo`! We need to find that base `PropertyInfo`, and because `PropertyInfo`
+ // does not have `.GetBaseDefinition()`, we'll find it via the getter's `.GetBaseDefinition()`.
+ // (We may assume that there's a getter because properties/indexers must have at least one accessor.)
+ Debug.Assert(property.CanRead);
+ var getter = property.GetGetMethod(nonPublic: true);
+ Debug.Assert(getter != null);
+
+ var baseGetter = getter.GetBaseDefinition();
+ if (baseGetter != getter)
+ {
+ var baseProperty =
+ baseGetter
+ .DeclaringType
+ .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);
+ }
+ }
+
+ setter = null;
+ return false;
+ }
+
  /// <summary>
  /// Gets the default value for the specified type. This is the Reflection counterpart of C#'s <see langword="default"/> operator.
  /// </summary>

src/Moq/Interception/InterceptionAspects.cs

@@ -314,9 +314,8 @@ public static bool Handle(Invocation invocation, Mock mock)
  object propertyValue;
 
  Setup getterSetup = null;
- if (property.CanRead)
+ if (property.CanRead(out var getter))
  {
- var getter = property.GetGetMethod(true);
  if (ProxyFactory.Instance.IsMethodVisible(getter, out _))
  {
  propertyValue = CreateInitialPropertyValue(mock, getter);
@@ -336,9 +335,8 @@ public static bool Handle(Invocation invocation, Mock mock)
  }
 
  Setup setterSetup = null;
- if (property.CanWrite)
+ if (property.CanWrite(out var setter))
  {
- MethodInfo setter = property.GetSetMethod(nonPublic: true);
  if (ProxyFactory.Instance.IsMethodVisible(setter, out _))
  {
  setterSetup = new AutoImplementedPropertySetterSetup(expression, setter, (newValue) =>

tests/Moq.Tests/ExtensionsFixture.cs

@@ -118,6 +118,56 @@ private string GetParameterTypeList(string methodName)
  return method.GetParameterTypeList();
  }
 
+ [Fact]
+ public void CanRead_returns_false_for_true_write_only_property()
+ {
+ var property = typeof(WithWriteOnlyProperty).GetProperty("Property");
+ Assert.False(property.CanRead(out var getter));
+ Assert.Null(getter);
+ }
+
+ [Fact]
+ public void CanRead_identifies_getter_in_true_read_only_property()
+ {
+ var property = typeof(WithReadOnlyProperty).GetProperty("Property");
+ Assert.True(property.CanRead(out var getter));
+ Assert.Equal(typeof(WithReadOnlyProperty), getter.DeclaringType);
+ }
+
+ [Fact]
+ public void CanRead_identifies_getter_when_declared_in_base_class()
+ {
+ var property = typeof(OverridesOnlySetter).GetProperty("Property");
+ Assert.False(property.CanRead);
+ Assert.True(property.CanRead(out var getter));
+ Assert.Equal(typeof(WithAutoProperty), getter.DeclaringType);
+ }
+
+ [Fact]
+ public void CanWrite_returns_false_for_true_read_only_property()
+ {
+ var property = typeof(WithReadOnlyProperty).GetProperty("Property");
+ Assert.False(property.CanWrite(out var setter));
+ Assert.Null(setter);
+ }
+
+ [Fact]
+ public void CanWrite_identifies_setter_in_true_write_only_property()
+ {
+ var property = typeof(WithWriteOnlyProperty).GetProperty("Property");
+ Assert.True(property.CanWrite(out var setter));
+ Assert.Equal(typeof(WithWriteOnlyProperty), setter.DeclaringType);
+ }
+
+ [Fact]
+ public void CanWrite_identifies_setter_when_declared_in_base_class()
+ {
+ var property = typeof(OverridesOnlyGetter).GetProperty("Property");
+ Assert.False(property.CanWrite);
+ Assert.True(property.CanWrite(out var setter));
+ Assert.Equal(typeof(WithAutoProperty), setter.DeclaringType);
+ }
+
  public interface IMethods
  {
  void Empty();
@@ -128,6 +178,31 @@ public interface IMethods
  void OutInt(out int arg1);
  void BoolAndParamsString(bool arg1, params string[] arg2);
  }
+
+ public class WithReadOnlyProperty
+ {
+ public virtual object Property => null;
+ }
+
+ public class WithWriteOnlyProperty
+ {
+ public virtual object Property { set { } }
+ }
+
+ public class WithAutoProperty
+ {
+ public virtual object Property { get; set; }
+ }
+
+ public class OverridesOnlyGetter : WithAutoProperty
+ {
+ public override object Property { get => base.Property; }
+ }
+
+ public class OverridesOnlySetter : WithAutoProperty
+ {
+ public override object Property { set => base.Property = value; }
+ }
  }
 
  public interface IFooReset

tests/Moq.Tests/StubExtensionsFixture.cs

@@ -258,6 +258,15 @@ public void SetupAllProperties_should_override_regular_setups()
  Assert.Equal(0, mock.Object.Value);
  }
 
+ [Fact]
+ public void SetupAllProperties_retains_value_of_derived_read_write_property_that_overrides_only_setter()
+ {
+ var mock = new Mock<OverridesOnlySetter>();
+ mock.SetupAllProperties();
+ mock.Object.Property = "value";
+ Assert.Equal("value", mock.Object.Property);
+ }
+
  private object GetValue() { return new object(); }
 
  public interface IFoo
@@ -298,5 +307,15 @@ public interface IHierarchy
  {
  IHierarchy Hierarchy { get; set; }
  }
+
+ public class WithAutoProperty
+ {
+ public virtual object Property { get; set; }
+ }
+
+ public class OverridesOnlySetter : WithAutoProperty
+ {
+ public override object Property { set => base.Property = value; }
+ }
  }
 }