Merge pull request #382 from stakx/interface-method-overrides

Fix mocking of redeclared interface methods

Source/Extensions.cs

@@ -40,6 +40,7 @@
 
 using System;
 using System.Collections;
+using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Linq;
@@ -323,5 +324,55 @@ private static MethodInfo GetInvokeMethodFromUntypedDelegateCallback(Delegate ca
 				return null;
 			}
 		}
+
+		/// <summary>
+		/// Gets all properties of the specified type in depth-first order.
+		/// That is, properties of the furthest ancestors are returned first,
+		/// and the type's own properties are returned last.
+		/// </summary>
+		/// <param name="type">The type whose properties are to be returned.</param>
+		internal static List<PropertyInfo> GetAllPropertiesInDepthFirstOrder(this Type type)
+		{
+			var properties = new List<PropertyInfo>();
+			var none = new HashSet<Type>();
+
+			type.AddPropertiesInDepthFirstOrderTo(properties, typesAlreadyVisited: none);
+
+			return properties;
+		}
+
+		/// <summary>
+		/// This is a helper method supporting <see cref="GetAllPropertiesInDepthFirstOrder(Type)"/>
+		/// and is not supposed to be called directly.
+		/// </summary>
+		private static void AddPropertiesInDepthFirstOrderTo(this Type type, List<PropertyInfo> properties, HashSet<Type> typesAlreadyVisited)
+		{
+			if (!typesAlreadyVisited.Contains(type))
+			{
+				// make sure we do not process properties of the current type twice:
+				typesAlreadyVisited.Add(type);
+
+				//// follow down axis 1: add properties of base class. note that this is currently
+				//// disabled, since it wasn't done previously and this can only result in changed
+				//// behavior.
+				//if (type.GetTypeInfo().BaseType != null)
+				//{
+				//	type.GetTypeInfo().BaseType.AddPropertiesInDepthFirstOrderTo(properties, typesAlreadyVisited);
+				//}
+
+				// follow down axis 2: add properties of inherited / implemented interfaces:
+				var superInterfaceTypes = type.GetInterfaces();
+				foreach (var superInterfaceType in superInterfaceTypes)
+				{
+					superInterfaceType.AddPropertiesInDepthFirstOrderTo(properties, typesAlreadyVisited);
+				}
+
+				// add own properties:
+				foreach (var property in type.GetProperties())
+				{
+					properties.Add(property);
+				}
+			}
+		}
 	}
 }

Source/Mock.cs

@@ -691,12 +691,21 @@ private static void SetupAllProperties(Mock mock, Stack<Type> mockedTypesStack)
 		{
 			var mockType = mock.MockedType;
 			mockedTypesStack.Push(mockType);
-			var properties = mockType.GetProperties()
-				.Concat(mockType.GetInterfaces().SelectMany(i => i.GetProperties()))
+
+			var properties =
+				mockType
+				.GetAllPropertiesInDepthFirstOrder()
+				// ^ Depth-first traversal is important because properties in derived interfaces
+				//   that shadow properties in base interfaces should be set up last. This
+				//   enables the use case where a getter-only property is redeclared in a derived
+				//   interface as a getter-and-setter property.
 				.Where(p =>
 					   p.CanRead && p.CanOverrideGet() &&
 					   p.GetIndexParameters().Length == 0 &&
-					   !(p.CanWrite ^ (p.CanWrite & p.CanOverrideSet())))
+					   p.CanWrite == p.CanOverrideSet())
+					   // ^ The last condition will be true for two kinds of properties:
+					   //    (a) those that are read-only; and
+					   //    (b) those that are writable and whose setter can be overridden.
 				.Distinct();
 
 			var setupPropertyMethod = mock.GetType().GetMethods()

UnitTests/Regressions/IssueReportsFixture.cs

@@ -299,6 +299,31 @@ public virtual void Close()
 
 		#endregion
 
+		#region 162
+
+		public class Issue162
+		{
+			[Fact]
+			public void GetSetPropertyThatOverridesGetPropertyRetainsValueSetUpWithMockOf()
+			{
+				const decimal expectedValue = .14M;
+				var i = Mock.Of<B>(b => b.Value == expectedValue);
+				Assert.Equal(expectedValue, actual: i.Value);
+			}
+
+			public interface A
+			{
+				decimal? Value { get; }
+			}
+
+			public interface B : A
+			{
+				new decimal? Value { get; set; }
+			}
+		}
+
+		#endregion
+
 		#region 163
 
 #if FEATURE_SERIALIZATION
@@ -696,6 +721,50 @@ public void strict_mock_accepts_null_as_nullable_guid_value()
 
 		#endregion // #184
 
+		#region 239
+
+		public class Issue239
+		{
+			[Fact]
+			public void PropertyInBaseInterfaceRetainsValueSetUpWitMockOf()
+			{
+				var i1 = Mock.Of<Interface1>(i => i.ABoolean == true);
+				Assert.True(i1.ABoolean);
+			}
+			[Fact]
+			public void RedeclaredPropertyInDerivedInterfaceRetainsValueSetUpWithNewMockAndSetupReturns()
+			{
+				var i2 = new Mock<Interface2>();
+				i2.Setup(i => i.ABoolean).Returns(true);
+				Assert.True(i2.Object.ABoolean);
+			}
+			[Fact]
+			public void RedeclaredPropertyInDerivedInterfaceRetainsValueSetUpWithMockOf()
+			{
+				var i2 = Mock.Of<Interface2>(i => i.ABoolean == true);
+				Assert.True(i2.ABoolean);
+			}
+			[Fact]
+			public void RedeclaredPropertyInDerivedInterfaceRetainsValueSetUpWitSetupAllPropertiesAndSetter()
+			{
+				var i2 = new Mock<Interface2>();
+				i2.SetupAllProperties();
+				i2.Object.ABoolean = true;
+				Assert.True(i2.Object.ABoolean);
+			}
+
+			public interface Interface1
+			{
+				bool ABoolean { get; }
+			}
+			public interface Interface2 : Interface1
+			{
+				new bool ABoolean { get; set; }
+			}
+		}
+
+		#endregion
+
 		#region #252
 
 		public class Issue252
@@ -750,6 +819,87 @@ public interface IMyInterface
 
 		#endregion // #252
 
+		#region 275
+
+		public class Issue275
+		{
+			private const int EXPECTED = int.MaxValue;
+
+			[Fact]
+			public void Root1Test()
+			{
+				var mock = Mock.Of<IRoot1>(c => c.Value == EXPECTED);
+				Assert.Equal(EXPECTED, mock.Value);
+			}
+
+			[Fact]
+			public void Derived1Test()
+			{
+				var mock = Mock.Of<IDerived1>(c => c.Value == EXPECTED);
+				Assert.Equal(EXPECTED, mock.Value);
+			}
+
+			[Fact]
+			public void Implementation1Test()
+			{
+				var mock = Mock.Of<Implementation1>(c => c.Value == EXPECTED);
+				Assert.Equal(EXPECTED, mock.Value);
+			}
+
+			[Fact]
+			public void Root2Test()
+			{
+				var mock = Mock.Of<IRoot2>(c => c.Value == EXPECTED);
+				Assert.Equal(EXPECTED, mock.Value);
+			}
+
+			[Fact]
+			public void Derived2Test()
+			{
+				var mock = Mock.Of<IDerived2>(c => c.Value == EXPECTED);
+				Assert.Equal(EXPECTED, mock.Value);
+			}
+
+			[Fact]
+			public void Implementation2Test()
+			{
+				var mock = Mock.Of<Implementation2>(c => c.Value == EXPECTED);
+				Assert.Equal(EXPECTED, mock.Value);
+			}
+
+			public interface IRoot1
+			{
+				int Value { get; }
+			}
+
+			public interface IRoot2
+			{
+				int Value { get; set; }
+			}
+
+			public interface IDerived1 : IRoot1
+			{
+				new int Value { get; set; }
+			}
+
+			public interface IDerived2 : IRoot2
+			{
+				new int Value { get; set; }
+			}
+
+			public class Implementation1 : IDerived1
+			{
+				public int Value { get; set; }
+			}
+
+			public class Implementation2 : IDerived1
+			{
+				public int Value { get; set; }
+			}
+		}
+
+		#endregion
+
 		#region 311
 
 		public sealed class Issue311