New algorithm for matching invoked methods against expected methods

CHANGELOG.md

@@ -19,12 +19,16 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 
 * Moq will throw when it detects that an argument matcher will never match anything due to the presence of an implicit conversion operator. (@michelcedric, #897, #898)
 
+* New algorithm for matching invoked methods against methods specified in setup/verification expressions. (@stakx, #904)
+
 #### Added
 
 * Added support for setup and verification of the event handlers through `Setup[Add|Remove]` and `Verify[Add|Remove|All]` (@lepijohnny, #825) 
 
 #### Fixed
 
+* Moq does not mock explicit interface implementation and `protected virtual` correctly. (@oddbear, #657)
+
 * `Invocations.Clear()` does not cause `Verify` to fail (@jchessir, #733)
 
 * Regression: `SetupAllProperties` can no longer set up properties whose names start with `Item`. (@mattzink, #870; @kaan-kaya, #869)
@@ -35,6 +39,8 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 
 * Regression in 4.12.0: `SetupAllProperties` removes indexer setups. (@stakx, #901)
 
+* Parameter types are ignored when matching an invoked generic method against setups. (@stakx, #903)
+
 
 ## 4.12.0 (2019-06-20)
 

src/Moq/Extensions.cs

@@ -26,6 +26,44 @@ public static object GetDefaultValue(this Type type)
 			return type.IsValueType ? Activator.CreateInstance(type) : null;
 		}
 
+		/// <summary>
+		///   Gets the least-derived <see cref="MethodInfo"/> in the given type that provides
+		///   the implementation for the given <paramref name="method"/>.
+		/// </summary>
+		public static MethodInfo GetImplementingMethod(this MethodInfo method, Type proxyType)
+		{
+			Debug.Assert(method != null);
+			Debug.Assert(proxyType != null);
+			Debug.Assert(proxyType.IsClass);
+
+			if (method.IsGenericMethod)
+			{
+				method = method.GetGenericMethodDefinition();
+			}
+
+			var declaringType = method.DeclaringType;
+
+			if (declaringType.IsInterface)
+			{
+				Debug.Assert(declaringType.IsAssignableFrom(proxyType));
+
+				var map = proxyType.GetInterfaceMap(method.DeclaringType);
+				var index = Array.IndexOf(map.InterfaceMethods, method);
+				Debug.Assert(index >= 0);
+				return map.TargetMethods[index].GetBaseDefinition();
+			}
+			else if (declaringType.IsDelegateType())
+			{
+				return proxyType.GetMethod("Invoke");
+			}
+			else
+			{
+				Debug.Assert(declaringType.IsAssignableFrom(proxyType));
+
+				return method.GetBaseDefinition();
+			}
+		}
+
 		public static object InvokePreserveStack(this Delegate del, params object[] args)
 		{
 			try

src/Moq/Invocation.cs

@@ -1,6 +1,7 @@
 // Copyright (c) 2007, Clarius Consulting, Manas Technology Solutions, InSTEDD.
 // All rights reserved. Licensed under the BSD 3-Clause License; see License.txt.
 
+using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Reflection;
@@ -12,28 +13,46 @@ internal abstract class Invocation : IInvocation
 	{
 		private object[] arguments;
 		private MethodInfo method;
+		private MethodInfo methodImplementation;
+		private readonly Type proxyType;
 		private object returnValue;
 		private VerificationState verificationState;
 
 		/// <summary>
 		/// Initializes a new instance of the <see cref="Invocation"/> class.
 		/// </summary>
+		/// <param name="proxyType">The <see cref="Type"/> of the concrete proxy object on which a method is being invoked.</param>
 		/// <param name="method">The method being invoked.</param>
 		/// <param name="arguments">The arguments with which the specified <paramref name="method"/> is being invoked.</param>
-		protected Invocation(MethodInfo method, params object[] arguments)
+		protected Invocation(Type proxyType, MethodInfo method, params object[] arguments)
 		{
+			Debug.Assert(proxyType != null);
 			Debug.Assert(arguments != null);
 			Debug.Assert(method != null);
 
 			this.arguments = arguments;
 			this.method = method;
+			this.proxyType = proxyType;
 		}
 
 		/// <summary>
 		/// Gets the method of the invocation.
 		/// </summary>
 		public MethodInfo Method => this.method;
 
+		public MethodInfo MethodImplementation
+		{
+			get
+			{
+				if (this.methodImplementation == null)
+				{
+					this.methodImplementation = this.method.GetImplementingMethod(this.proxyType);
+				}
+
+				return this.methodImplementation;
+			}
+		}
+
 		/// <summary>
 		/// Gets the arguments of the invocation.
 		/// </summary>
@@ -45,6 +64,8 @@ protected Invocation(MethodInfo method, params object[] arguments)
 
 		IReadOnlyList<object> IInvocation.Arguments => this.arguments;
 
+		public Type ProxyType => this.proxyType;
+
 		public object ReturnValue => this.returnValue;
 
 		internal bool Verified => this.verificationState == VerificationState.Verified;

src/Moq/InvocationShape.cs

@@ -28,7 +28,11 @@ internal sealed class InvocationShape : IEquatable<InvocationShape>
 		public readonly IReadOnlyList<Expression> Arguments;
 
 		private readonly IMatcher[] argumentMatchers;
+		private MethodInfo methodImplementation;
 		private Expression[] partiallyEvaluatedArguments;
+#if DEBUG
+		private Type proxyType;
+#endif
 
 		public InvocationShape(LambdaExpression expression, MethodInfo method, IReadOnlyList<Expression> arguments = null)
 		{
@@ -60,17 +64,12 @@ public void Deconstruct(out LambdaExpression expression, out MethodInfo method,
 
 		public bool IsMatch(Invocation invocation)
 		{
-			var arguments = invocation.Arguments;
-			if (this.argumentMatchers.Length != arguments.Length)
-			{
-				return false;
-			}
-
-			if (invocation.Method != this.Method && !this.IsOverride(invocation.Method))
+			if (invocation.Method != this.Method && !this.IsOverride(invocation))
 			{
 				return false;
 			}
 
+			var arguments = invocation.Arguments;
 			for (int i = 0, n = this.argumentMatchers.Length; i < n; ++i)
 			{
 				if (this.argumentMatchers[i].Matches(arguments[i]) == false)
@@ -91,21 +90,37 @@ public void SetupEvaluatedSuccessfully(Invocation invocation)
 			}
 		}
 
-		private bool IsOverride(MethodInfo invocationMethod)
+		private bool IsOverride(Invocation invocation)
 		{
+			Debug.Assert(invocation.Method != this.Method);
+
 			var method = this.Method;
+			var invocationMethod = invocation.Method;
 
-			if (!method.DeclaringType.IsAssignableFrom(invocationMethod.DeclaringType))
+			var proxyType = invocation.ProxyType;
+#if DEBUG
+			// The following `if` block is a sanity check to ensure this `InvocationShape` always
+			// runs against the same proxy type. This is important because we're caching the result
+			// of mapping methods into that particular proxy type. We have no cache invalidation
+			// logic in place; instead, we simply assume that the cached results will stay valid.
+			// If the below assertion fails, that assumption was wrong.
+			if (this.proxyType == null)
 			{
-				return false;
+				this.proxyType = proxyType;
+			}
+			else
+			{
+				Debug.Assert(this.proxyType == proxyType);
 			}
+#endif
 
-			if (!method.Name.Equals(invocationMethod.Name, StringComparison.Ordinal))
+			// If not already in the cache, map this `InvocationShape`'s method into the proxy type:
+			if (this.methodImplementation == null)
 			{
-				return false;
+				this.methodImplementation = method.GetImplementingMethod(proxyType);
 			}
 
-			if (method.ReturnType != invocationMethod.ReturnType)
+			if (invocation.MethodImplementation != this.methodImplementation)
 			{
 				return false;
 			}
@@ -117,13 +132,6 @@ private bool IsOverride(MethodInfo invocationMethod)
 					return false;
 				}
 			}
-			else
-			{
-				if (!invocationMethod.GetParameterTypes().CompareTo(method.GetParameterTypes(), exact: true))
-				{
-					return false;
-				}
-			}
 
 			return true;
 		}

src/Moq/ProxyFactories/CastleProxyFactory.cs

@@ -129,7 +129,7 @@ private sealed class Invocation : Moq.Invocation
 		{
 			private Castle.DynamicProxy.IInvocation underlying;
 
-			internal Invocation(Castle.DynamicProxy.IInvocation underlying) : base(underlying.Method, underlying.Arguments)
+			internal Invocation(Castle.DynamicProxy.IInvocation underlying) : base(underlying.Proxy.GetType(), underlying.Method, underlying.Arguments)
 			{
 				this.underlying = underlying;
 			}

src/Moq/ProxyFactories/InterfaceProxy.cs

@@ -1,6 +1,7 @@
 // Copyright (c) 2007, Clarius Consulting, Manas Technology Solutions, InSTEDD.
 // All rights reserved. Licensed under the BSD 3-Clause License; see License.txt.
 
+using System;
 using System.ComponentModel;
 using System.Diagnostics;
 using System.Reflection;
@@ -25,8 +26,9 @@ public abstract class InterfaceProxy
 		public sealed override bool Equals(object obj)
 		{
 			// Forward this call to the interceptor, so that `object.Equals` can be set up.
-			var invocation = new Invocation(equalsMethod, obj);
-			((IInterceptor)((IProxy)this).Interceptor).Intercept(invocation);
+			var interceptor = (IInterceptor)((IProxy)this).Interceptor;
+			var invocation = new Invocation(interceptor.GetType(), equalsMethod, obj);
+			interceptor.Intercept(invocation);
 			return (bool)invocation.ReturnValue;
 		}
 
@@ -35,8 +37,9 @@ public sealed override bool Equals(object obj)
 		public sealed override int GetHashCode()
 		{
 			// Forward this call to the interceptor, so that `object.GetHashCode` can be set up.
-			var invocation = new Invocation(getHashCodeMethod);
-			((IInterceptor)((IProxy)this).Interceptor).Intercept(invocation);
+			var interceptor = (IInterceptor)((IProxy)this).Interceptor;
+			var invocation = new Invocation(interceptor.GetType(), getHashCodeMethod);
+			interceptor.Intercept(invocation);
 			return (int)invocation.ReturnValue;
 		}
 
@@ -45,22 +48,23 @@ public sealed override int GetHashCode()
 		public sealed override string ToString()
 		{
 			// Forward this call to the interceptor, so that `object.ToString` can be set up.
-			var invocation = new Invocation(toStringMethod);
-			((IInterceptor)((IProxy)this).Interceptor).Intercept(invocation);
+			var interceptor = (IInterceptor)((IProxy)this).Interceptor;
+			var invocation = new Invocation(interceptor.GetType(), toStringMethod);
+			interceptor.Intercept(invocation);
 			return (string)invocation.ReturnValue;
 		}
 
 		private sealed class Invocation : Moq.Invocation
 		{
 			private static object[] noArguments = new object[0];
 
-			public Invocation(MethodInfo method, params object[] arguments)
-				: base(method, arguments)
+			public Invocation(Type proxyType, MethodInfo method, params object[] arguments)
+				: base(proxyType, method, arguments)
 			{
 			}
 
-			public Invocation(MethodInfo method)
-				: base(method, noArguments)
+			public Invocation(Type proxyType, MethodInfo method)
+				: base(proxyType, method, noArguments)
 			{
 			}
 

tests/Moq.Tests/AsInterfaceFixture.cs

@@ -171,6 +171,33 @@ public void ShouldNotThrowIfCallExplicitlyImplementedInterfacesMethodWhenCallBas
 			bag.Get("test");
 		}
 
+		[Fact]
+		public void Setup_targets_method_implementing_interface_not_other_method_with_same_signature()
+		{
+			var mock = new Mock<Service>() { CallBase = true };
+			mock.As<IService>().Setup(m => m.GetValue()).Returns(3);
+
+			// The public method should have been left alone since it's not the one implementing `IService`:
+			var valueOfOtherMethod = mock.Object.GetValue();
+			Assert.Equal(1, valueOfOtherMethod);
+
+			// The method implementing the interface method should have be mocked:
+			var valueOfSetupMethod = ((IService)mock.Object).GetValue();
+			Assert.Equal(3, valueOfSetupMethod);
+		}
+
+		public class Service : IService
+		{
+			public virtual int GetValue() => 1;
+
+			int IService.GetValue() => 2;
+		}
+
+		public interface IService
+		{
+			int GetValue();
+		}
+
 		// see also test fixture `Issue458` in `IssueReportsFixture`
 
 		public interface IFoo