Merge pull request devlooped#654 from stakx/parameter-count-mismatch

Prevent false 'Different number of parameters' error with `Returns`
callback methods that have been compiled from `Expression`s.

CHANGELOG.md

@@ -14,6 +14,7 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 #### Fixed
 
 * More precise `out` parameter detection for mocking COM interfaces with `[in,out]` parameters (@koutinho, #645)
+* Prevent false 'Different number of parameters' error with `Returns` callback methods that have been compiled from `Expression`s (@stakx, #654)
 
 
 ## 4.9.0 (2018-07-13)

src/Moq/MethodCallReturn.cs

@@ -171,17 +171,15 @@ private void ValidateReturnDelegate(Delegate callback)
 		{
 			var callbackMethod = callback.GetMethodInfo();
 
-			ValidateNumberOfCallbackParameters(callbackMethod);
+			// validate number of parameters:
 
-			ValidateCallbackReturnType(callbackMethod);
-		}
-
-		private void ValidateNumberOfCallbackParameters(MethodInfo callbackMethod)
-		{
 			var numberOfActualParameters = callbackMethod.GetParameters().Length;
-			if (callbackMethod.IsExtensionMethod())
+			if (callbackMethod.IsStatic)
 			{
-				numberOfActualParameters--;
+				if (callbackMethod.IsExtensionMethod() || callback.Target != null)
+				{
+					numberOfActualParameters--;
+				}
 			}
 
 			if (numberOfActualParameters > 0)
@@ -197,10 +195,9 @@ private void ValidateNumberOfCallbackParameters(MethodInfo callbackMethod)
 							numberOfActualParameters));
 				}
 			}
-		}
 
-		private void ValidateCallbackReturnType(MethodInfo callbackMethod)
-		{
+			// validate return type:
+
 			var expectedReturnType = this.Method.ReturnType;
 			var actualReturnType = callbackMethod.ReturnType;
 

tests/Moq.Tests/CallbackDelegateValidationFixture.cs

@@ -0,0 +1,106 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Linq.Expressions;
+
+using Moq.Language.Flow;
+
+using Xunit;
+
+namespace Moq.Tests
+{
+	/// <summary>
+	///   This fixture targets the `Callback` delegate validation logic.
+	/// </summary>
+	/// <seealso cref="ReturnsDelegateValidationFixture"/>
+	public class CallbackDelegateValidationFixture
+	{
+		private ISetup<IFoo> setup;
+
+		public CallbackDelegateValidationFixture()
+		{
+			var mock = new Mock<IFoo>();
+			this.setup = mock.Setup(m => m.Action(It.IsAny<int>()));
+		}
+
+		// Nothing surprising here.
+		[Fact]
+		public void Callback_accepts_instance_method_as_callback()
+		{
+			var instance = new Instance();
+			Action<int> callback = instance.Action;
+			Assert.Single(callback.Method.GetParameters());
+			Assert.Same(instance, callback.Target);
+
+			this.setup.Callback(callback);
+		}
+
+		// Nothing surprising here.
+		[Fact]
+		public void Callback_accepts_static_method_as_callback()
+		{
+			Action<int> callback = Static.Action;
+			Assert.Single(callback.Method.GetParameters());
+			Assert.Null(callback.Target);
+
+			this.setup.Callback(callback);
+		}
+
+		// This may seem surprising because the extension method has a different number
+		// of declared parameters (2) than the method being set up (1). Since C# supports
+		// this syntax just fine (proof below), Moq should accept this too. The reason why
+		// this works is because the first (`this`) parameter is bound to an object.
+		[Fact]
+		public void Callback_accepts_bound_extension_method_as_callback_despite_additional_parameter()
+		{
+			var instance = Enumerable.Range(1, 10);
+			Action<int> callback = instance.Action;
+			Assert.Equal(2, callback.Method.GetParameters().Length);
+			Assert.Same(instance, callback.Target);
+
+			this.setup.Callback(callback);
+		}
+
+		// This doesn't look suspicious at all, but it is very similar to the above test.
+		// Methods that result from compiling a LINQ expression tree have an additional
+		// (bound) first parameter of type `System.Runtime.CompilerServices.Closure`.
+		// That is, they have a different parameter count than the method being set up,
+		// but Moq should still accept it.
+		[Fact]
+		public void Callback_accepts_compiled_method_as_callback_despite_additional_parameter()
+		{
+			Expression<Action<int>> callbackExpr = x => Console.WriteLine();
+			Action<int> callback = callbackExpr.Compile();
+			Assert.Equal(2, callback.Method.GetParameters().Length);
+			Assert.NotNull(callback.Target);
+
+			this.setup.Callback(callback);
+		}
+
+		public interface IFoo
+		{
+			void Action(int x);
+		}
+	}
+
+	public partial class Instance
+	{
+		public void Action(int x)
+		{
+		}
+	}
+
+	public static partial class Static
+	{
+		public static void Action(int x)
+		{
+		}
+	}
+
+	public static partial class Extension
+	{
+		public static void Action(this IEnumerable<int> self, int x)
+		{
+		}
+	}
+}

tests/Moq.Tests/Regressions/IssueReportsFixture.cs

@@ -1974,6 +1974,39 @@ public void Struct_ByRef_Delegate()
 
 		#endregion
 
+		#region 652
+
+		public class Issue652
+		{
+			[Fact]
+			public void Callback_delegates_compiled_from_Expression_are_usable_despite_additional_hidden_Closure_parameter()
+			{
+				Mock<IMyInterface> mock = new Moq.Mock<IMyInterface>();
+				var setupAction = mock.Setup(my => my.MyAction(It.IsAny<int>()));
+				var setupFunc = mock.Setup(my => my.MyFunc(It.IsAny<int>()));
+
+				int a = 5;
+				Expression<Action<int>> callback = x => Console.WriteLine(a);
+				Action<int> callbackCompiled = callback.Compile();
+
+				int b = 4;
+				Expression<Func<int, int>> funcResult = x => b;
+				Func<int, int> funcResultCompiled = funcResult.Compile();
+
+				setupAction.Callback(callbackCompiled); // OK
+				setupFunc.Callback(callbackCompiled); // OK
+				setupFunc.Returns(funcResultCompiled); // Fail
+			}
+
+			public interface IMyInterface
+			{
+				void MyAction(int x);
+				int MyFunc(int x);
+			}
+		}
+
+		#endregion
+
 		// Old @ Google Code
 
 		#region #47

tests/Moq.Tests/ReturnsDelegateValidationFixture.cs

@@ -0,0 +1,109 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Linq.Expressions;
+
+using Moq.Language.Flow;
+
+using Xunit;
+
+namespace Moq.Tests
+{
+	/// <summary>
+	///   This fixture targets the `Returns` delegate validation logic.
+	/// </summary>
+	/// <seealso cref="CallbackDelegateValidationFixture"/>
+	public class ReturnsDelegateValidationFixture
+	{
+		private ISetup<IFoo, bool> setup;
+
+		public ReturnsDelegateValidationFixture()
+		{
+			var mock = new Mock<IFoo>();
+			this.setup = mock.Setup(m => m.Func(It.IsAny<int>()));
+		}
+
+		// Nothing surprising here.
+		[Fact]
+		public void Returns_accepts_instance_method_as_callback()
+		{
+			var instance = new Instance();
+			Func<int, bool> callback = instance.Func;
+			Assert.Single(callback.Method.GetParameters());
+			Assert.Same(instance, callback.Target);
+
+			this.setup.Returns(callback);
+		}
+
+		// Nothing surprising here.
+		[Fact]
+		public void Returns_accepts_static_method_as_callback()
+		{
+			Func<int, bool> callback = Static.Func;
+			Assert.Single(callback.Method.GetParameters());
+			Assert.Null(callback.Target);
+
+			this.setup.Returns(callback);
+		}
+
+		// This may seem surprising because the extension method has a different number
+		// of declared parameters (2) than the method being set up (1). Since C# supports
+		// this syntax just fine (proof below), Moq should accept this too. The reason why
+		// this works is because the first (`this`) parameter is bound to an object.
+		[Fact]
+		public void Returns_accepts_bound_extension_method_as_callback_despite_additional_parameter()
+		{
+			var instance = Enumerable.Range(1, 10);
+			Func<int, bool> callback = instance.Func;
+			Assert.Equal(2, callback.Method.GetParameters().Length);
+			Assert.Same(instance, callback.Target);
+
+			this.setup.Returns(callback);
+		}
+
+		// This doesn't look suspicious at all, but it is very similar to the above test.
+		// Methods that result from compiling a LINQ expression tree have an additional
+		// (bound) first parameter of type `System.Runtime.CompilerServices.Closure`.
+		// That is, they have a different parameter count than the method being set up,
+		// but Moq should still accept it.
+		[Fact]
+		public void Returns_accepts_compiled_method_as_callback_despite_additional_parameter()
+		{
+			Expression<Func<int, bool>> callbackExpr = x => x == 0;
+			Func<int, bool> callback = callbackExpr.Compile();
+			Assert.Equal(2, callback.Method.GetParameters().Length);
+			Assert.NotNull(callback.Target);
+
+			this.setup.Returns(callback);
+		}
+
+		public interface IFoo
+		{
+			bool Func(int x);
+		}
+	}
+
+	public partial class Instance
+	{
+		public bool Func(int x)
+		{
+			return x == 0;
+		}
+	}
+
+	public static partial class Static
+	{
+		public static bool Func(int x)
+		{
+			return x == 0;
+		}
+	}
+
+	public static partial class Extension
+	{
+		public static bool Func(this IEnumerable<int> self, int x)
+		{
+			return x == 0;
+		}
+	}
+}