diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a62b1f88..00e05769b 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/Moq/ExpressionExtensions.cs b/src/Moq/ExpressionExtensions.cs index 981566fdc..ba80f773a 100644 --- a/src/Moq/ExpressionExtensions.cs +++ b/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() .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; } } diff --git a/src/Moq/Extensions.cs b/src/Moq/Extensions.cs index 1252c67a3..103a206cc 100644 --- a/src/Moq/Extensions.cs +++ b/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 GetMethods(this Type type, string name) diff --git a/src/Moq/Guard.cs b/src/Moq/Guard.cs index ab0e46b46..dc66d7776 100644 --- a/src/Moq/Guard.cs +++ b/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, diff --git a/src/Moq/Linq/MockSetupsBuilder.cs b/src/Moq/Linq/MockSetupsBuilder.cs index 83823d47c..1a3c5fdef 100644 --- a/src/Moq/Linq/MockSetupsBuilder.cs +++ b/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. diff --git a/src/Moq/Mock.cs b/src/Moq/Mock.cs index f81f39189..2d8751427 100644 --- a/src/Moq/Mock.cs +++ b/src/Moq/Mock.cs @@ -813,7 +813,7 @@ internal void AddInnerMockSetup(MethodInfo method, IReadOnlyList 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)); diff --git a/src/Moq/Protected/ProtectedAsMock.cs b/src/Moq/Protected/ProtectedAsMock.cs index df5c12153..36f9f5423 100644 --- a/src/Moq/Protected/ProtectedAsMock.cs +++ b/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. } diff --git a/src/Moq/Protected/ProtectedMock.cs b/src/Moq/Protected/ProtectedMock.cs index f6c645bd1..fd4cd1fb3 100644 --- a/src/Moq/Protected/ProtectedMock.cs +++ b/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, diff --git a/tests/Moq.Tests/StubExtensionsFixture.cs b/tests/Moq.Tests/StubExtensionsFixture.cs index 33962064c..cb1515e8a 100644 --- a/tests/Moq.Tests/StubExtensionsFixture.cs +++ b/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(); + mock.SetupProperty(m => m.Property); + mock.Object.Property = "value"; + Assert.Equal("value", mock.Object.Property); + } + private object GetValue() { return new object(); } public interface IFoo