From ae588a4db5f6165945a93eea81d2b003662f76f7 Mon Sep 17 00:00:00 2001 From: Tony Hallett Date: Thu, 22 Jul 2021 15:26:59 +0100 Subject: [PATCH 1/6] fix #1188 --- src/Moq/Protected/ProtectedMock.cs | 64 ++++++++++++++++++------- tests/Moq.Tests/ProtectedMockFixture.cs | 25 +++++++++- 2 files changed, 71 insertions(+), 18 deletions(-) diff --git a/src/Moq/Protected/ProtectedMock.cs b/src/Moq/Protected/ProtectedMock.cs index 362e03583..c8d26b5b6 100644 --- a/src/Moq/Protected/ProtectedMock.cs +++ b/src/Moq/Protected/ProtectedMock.cs @@ -467,26 +467,15 @@ private static Type[] ToArgTypes(object[] args) { types[index] = ((MethodCallExpression)expr).Method.ReturnType; } + else if(ItRefAnyField(expr) is var itRefAnyField && itRefAnyField != null) + { + types[index] = itRefAnyField.FieldType.MakeByRefType(); + } else if (expr.NodeType == ExpressionType.MemberAccess) { var member = (MemberExpression)expr; if (member.Member is FieldInfo field) { - // Test for special case: `It.Ref.IsAny` - if (field.Name == nameof(It.Ref.IsAny)) - { - var fieldDeclaringType = field.DeclaringType; - if (fieldDeclaringType.IsGenericType) - { - var fieldDeclaringTypeDefinition = fieldDeclaringType.GetGenericTypeDefinition(); - if (fieldDeclaringTypeDefinition == typeof(It.Ref<>)) - { - types[index] = field.FieldType.MakeByRefType(); - continue; - } - } - } - types[index] = field.FieldType; } else if (member.Member is PropertyInfo property) @@ -510,16 +499,57 @@ private static Type[] ToArgTypes(object[] args) return types; } + private static FieldInfo ItRefAnyField(Expression expr) + { + FieldInfo itRefAnyField = null; + + if (expr.NodeType == ExpressionType.MemberAccess) + { + var member = (MemberExpression)expr; + if (member.Member is FieldInfo field) + { + if (field.Name == nameof(It.Ref.IsAny)) + { + var fieldDeclaringType = field.DeclaringType; + if (fieldDeclaringType.IsGenericType) + { + var fieldDeclaringTypeDefinition = fieldDeclaringType.GetGenericTypeDefinition(); + if (fieldDeclaringTypeDefinition == typeof(It.Ref<>)) + { + itRefAnyField = field; + } + } + } + } + } + + return itRefAnyField; + } + private static Expression ToExpressionArg(Type type, object arg) { - if (arg is LambdaExpression lambda) + if (arg is LambdaExpression lambda && !typeof(LambdaExpression).IsAssignableFrom(type)) { return lambda.Body; } if (arg is Expression expression) { - return expression; + if (!typeof(Expression).IsAssignableFrom(type)) + { + return expression; + } + + if (expression.IsMatch(out _)) + { + return expression; + } + + if (ItRefAnyField(expression) != null) + { + return expression; + } + } return Expression.Constant(arg, type); diff --git a/tests/Moq.Tests/ProtectedMockFixture.cs b/tests/Moq.Tests/ProtectedMockFixture.cs index bc3af2ec5..95fd57399 100644 --- a/tests/Moq.Tests/ProtectedMockFixture.cs +++ b/tests/Moq.Tests/ProtectedMockFixture.cs @@ -2,7 +2,7 @@ // All rights reserved. Licensed under the BSD 3-Clause License; see License.txt. using System; - +using System.Linq.Expressions; using Moq.Protected; using Xunit; @@ -855,6 +855,23 @@ public void DoesNotThrowIfVerifySetPropertyTimesReached() mock.Protected().VerifySet("ProtectedValue", Times.Exactly(2), ItExpr.IsAny()); } + [Fact] + public void SetupShouldWorkWithExpressionTypes() + { + var mock = new Mock(); + + var expression = Expression.Constant(1); + ConstantExpression setExpression = null; + mock.Protected().SetupSet("ExpressionProperty", expression).Callback(expr => setExpression = expr); + mock.Object.SetExpressionProperty(expression); + Assert.Equal(expression, setExpression); + + ConstantExpression setExpression2 = null; + mock.Protected().SetupSet("ExpressionProperty", ItExpr.Is( e => (int)e.Value == 2)).Callback(expr => setExpression2 = expr); + mock.Object.SetExpressionProperty(Expression.Constant(2)); + Assert.NotNull(setExpression2); + } + public class MethodOverloads { public void ExecuteDo(int a, int b) @@ -952,6 +969,12 @@ protected virtual FooBase Overloaded(MyDerived myBase) public class FooBase { + protected virtual ConstantExpression ExpressionProperty { get; set; } + public void SetExpressionProperty(ConstantExpression expression) + { + ExpressionProperty = expression; + } + public virtual string PublicValue { get; set; } protected internal virtual string ProtectedInternalValue { get; set; } From 1059cd7682b1caecdc012221c9ecb85fe281227b Mon Sep 17 00:00:00 2001 From: Tony Hallett Date: Tue, 27 Jul 2021 14:42:57 +0100 Subject: [PATCH 2/6] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 201eb257b..eb43d9a40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1 #### Fixed +* mock.Protected().SetupSet fails when argument is of type Expression (@tonyhallett, #1189) * Parameter is invalid in Protected().SetupSet() ... VerifySet (@tonyhallett, #1186) * Virtual properties and automocking not working for `mock.Protected().As<>()` (@tonyhallett, #1185) From c145b2e6741274f6c2b3178e4806c24a768bad6b Mon Sep 17 00:00:00 2001 From: Tony Hallett Date: Mon, 2 Aug 2021 09:04:43 +0100 Subject: [PATCH 3/6] code style --- src/Moq/Protected/ProtectedMock.cs | 2 +- tests/Moq.Tests/ProtectedMockFixture.cs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Moq/Protected/ProtectedMock.cs b/src/Moq/Protected/ProtectedMock.cs index c8d26b5b6..8b0ef67ad 100644 --- a/src/Moq/Protected/ProtectedMock.cs +++ b/src/Moq/Protected/ProtectedMock.cs @@ -467,7 +467,7 @@ private static Type[] ToArgTypes(object[] args) { types[index] = ((MethodCallExpression)expr).Method.ReturnType; } - else if(ItRefAnyField(expr) is var itRefAnyField && itRefAnyField != null) + else if (ItRefAnyField(expr) is FieldInfo itRefAnyField) { types[index] = itRefAnyField.FieldType.MakeByRefType(); } diff --git a/tests/Moq.Tests/ProtectedMockFixture.cs b/tests/Moq.Tests/ProtectedMockFixture.cs index 95fd57399..d23119f97 100644 --- a/tests/Moq.Tests/ProtectedMockFixture.cs +++ b/tests/Moq.Tests/ProtectedMockFixture.cs @@ -3,6 +3,7 @@ using System; using System.Linq.Expressions; + using Moq.Protected; using Xunit; @@ -970,6 +971,7 @@ protected virtual FooBase Overloaded(MyDerived myBase) public class FooBase { protected virtual ConstantExpression ExpressionProperty { get; set; } + public void SetExpressionProperty(ConstantExpression expression) { ExpressionProperty = expression; From 3f381eac467f41e1c036492a7cb39576a61d8abd Mon Sep 17 00:00:00 2001 From: Tony Hallett Date: Mon, 2 Aug 2021 12:46:46 +0100 Subject: [PATCH 4/6] Correct LambdaExpression logic and reduce unnecessary Matcher checks --- src/Moq/Protected/ProtectedMock.cs | 21 +++--- tests/Moq.Tests/ProtectedMockFixture.cs | 92 ++++++++++++++++++++++--- 2 files changed, 94 insertions(+), 19 deletions(-) diff --git a/src/Moq/Protected/ProtectedMock.cs b/src/Moq/Protected/ProtectedMock.cs index 8b0ef67ad..bb72409b1 100644 --- a/src/Moq/Protected/ProtectedMock.cs +++ b/src/Moq/Protected/ProtectedMock.cs @@ -499,6 +499,11 @@ private static Type[] ToArgTypes(object[] args) return types; } + private static bool IsItRefAny(Expression expression) + { + return ItRefAnyField(expression) != null; + } + private static FieldInfo ItRefAnyField(Expression expr) { FieldInfo itRefAnyField = null; @@ -528,24 +533,24 @@ private static FieldInfo ItRefAnyField(Expression expr) private static Expression ToExpressionArg(Type type, object arg) { - if (arg is LambdaExpression lambda && !typeof(LambdaExpression).IsAssignableFrom(type)) - { - return lambda.Body; - } - if (arg is Expression expression) { - if (!typeof(Expression).IsAssignableFrom(type)) + if (!type.IsAssignableFrom(expression.GetType())) { + if(arg is LambdaExpression lambda) + { + expression = lambda.Body; + } return expression; } - if (expression.IsMatch(out _)) + var requiresExpressionMatch = type.IsAssignableFrom(typeof(MethodCallExpression)); + if (requiresExpressionMatch && expression.IsMatch(out _)) { return expression; } - if (ItRefAnyField(expression) != null) + if (IsItRefAny(expression)) { return expression; } diff --git a/tests/Moq.Tests/ProtectedMockFixture.cs b/tests/Moq.Tests/ProtectedMockFixture.cs index d23119f97..946b13edd 100644 --- a/tests/Moq.Tests/ProtectedMockFixture.cs +++ b/tests/Moq.Tests/ProtectedMockFixture.cs @@ -860,17 +860,64 @@ public void DoesNotThrowIfVerifySetPropertyTimesReached() public void SetupShouldWorkWithExpressionTypes() { var mock = new Mock(); + var mocked = mock.Object; + var protectedMock = mock.Protected(); var expression = Expression.Constant(1); - ConstantExpression setExpression = null; - mock.Protected().SetupSet("ExpressionProperty", expression).Callback(expr => setExpression = expr); - mock.Object.SetExpressionProperty(expression); - Assert.Equal(expression, setExpression); - - ConstantExpression setExpression2 = null; - mock.Protected().SetupSet("ExpressionProperty", ItExpr.Is( e => (int)e.Value == 2)).Callback(expr => setExpression2 = expr); - mock.Object.SetExpressionProperty(Expression.Constant(2)); - Assert.NotNull(setExpression2); + Expression setExpression1 = null; + protectedMock.SetupSet("ExpressionProperty", expression).Callback(expr => setExpression1 = expr); + mocked.SetExpressionProperty(expression); + Assert.Same(expression, setExpression1); + + var expression2 = Expression.Constant(2); + Expression setExpression2 = null; + protectedMock.SetupSet("ExpressionProperty", ItExpr.Is(e => (int)e.Value == 2)).Callback(expr => setExpression2 = expr); + mocked.SetExpressionProperty(expression2); + Assert.Same(expression2, setExpression2); + + var constantExpression = Expression.Constant(1); + ConstantExpression setConstantExpression = null; + protectedMock.SetupSet("NotAMatcherExpressionProperty", constantExpression).Callback(expr => setConstantExpression = expr); + mocked.SetNotAMatcherExpressionProperty(constantExpression); + Assert.Same(constantExpression, setConstantExpression); + + var constantExpression2 = Expression.Constant(2); + ConstantExpression setConstantExpression2 = null; + protectedMock.SetupSet("NotAMatcherExpressionProperty", ItExpr.Is(e => (int)e.Value == 2)).Callback(expr => setConstantExpression2 = expr); + mocked.SetNotAMatcherExpressionProperty(constantExpression2); + Assert.Same(constantExpression2, setConstantExpression2); + + var method = typeof(FooBase).GetMethod(nameof(FooBase.MethodForReflection)); + var methodCallExpression = Expression.Call(method); + MethodCallExpression setMethodCallExpression = null; + protectedMock.SetupSet("MatcherExpressionProperty", methodCallExpression).Callback(expr => setMethodCallExpression = expr); + mocked.SetMatcherExpressionProperty(methodCallExpression); + Assert.Same(methodCallExpression, setMethodCallExpression); + + var methodCallExpression2 = Expression.Call(typeof(FooBase).GetMethod(nameof(FooBase.MethodForReflection2))); + MethodCallExpression setMethodCallExpression2 = null; + protectedMock.SetupSet("MatcherExpressionProperty", ItExpr.Is(e => e.Method != method)).Callback(expr => setMethodCallExpression2 = expr); + mocked.SetMatcherExpressionProperty(methodCallExpression2); + Assert.Same(methodCallExpression2, setMethodCallExpression2); + + Expression> lambdaExpression = i => i < 5; + LambdaExpression setLambdaExpression = null; + protectedMock.SetupSet("LambdaExpressionProperty", lambdaExpression).Callback(expr => setLambdaExpression = expr); + mocked.SetLambdaExpressionProperty(lambdaExpression); + Assert.Same(lambdaExpression, setLambdaExpression); + + Expression> lambdaExpression2 = i => i; + LambdaExpression setLambdaExpression2 = null; + protectedMock.SetupSet("LambdaExpressionProperty", ItExpr.Is(e => e == lambdaExpression2)).Callback(expr => setLambdaExpression2 = expr); + mocked.SetLambdaExpressionProperty(lambdaExpression2); + Assert.Same(lambdaExpression2, setLambdaExpression2); + + } + + public class ExpectedException : Exception + { + private static ExpectedException instance = new ExpectedException(); + public static ExpectedException Instance => instance; } public class MethodOverloads @@ -970,13 +1017,36 @@ protected virtual FooBase Overloaded(MyDerived myBase) public class FooBase { - protected virtual ConstantExpression ExpressionProperty { get; set; } + public static void MethodForReflection() { } + public static void MethodForReflection2() { } + protected virtual Expression ExpressionProperty { get; set; } - public void SetExpressionProperty(ConstantExpression expression) + public void SetExpressionProperty(Expression expression) { ExpressionProperty = expression; } + protected virtual ConstantExpression NotAMatcherExpressionProperty { get; set; } + + public void SetNotAMatcherExpressionProperty(ConstantExpression expression) + { + NotAMatcherExpressionProperty = expression; + } + + protected virtual MethodCallExpression MatcherExpressionProperty { get; set; } + + public void SetMatcherExpressionProperty(MethodCallExpression expression) + { + MatcherExpressionProperty = expression; + } + + protected virtual LambdaExpression LambdaExpressionProperty { get; set; } + + public void SetLambdaExpressionProperty(LambdaExpression expression) + { + LambdaExpressionProperty = expression; + } + public virtual string PublicValue { get; set; } protected internal virtual string ProtectedInternalValue { get; set; } From e05680b2bc3c82eb446dd0e6d62acf7eedd41104 Mon Sep 17 00:00:00 2001 From: Tony Hallett Date: Mon, 2 Aug 2021 14:36:00 +0100 Subject: [PATCH 5/6] remove condition match test and reorder match test order --- src/Moq/Protected/ProtectedMock.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Moq/Protected/ProtectedMock.cs b/src/Moq/Protected/ProtectedMock.cs index bb72409b1..ff1a12273 100644 --- a/src/Moq/Protected/ProtectedMock.cs +++ b/src/Moq/Protected/ProtectedMock.cs @@ -543,18 +543,17 @@ private static Expression ToExpressionArg(Type type, object arg) } return expression; } - - var requiresExpressionMatch = type.IsAssignableFrom(typeof(MethodCallExpression)); - if (requiresExpressionMatch && expression.IsMatch(out _)) + + if (IsItRefAny(expression)) { return expression; } - - if (IsItRefAny(expression)) + + if (expression.IsMatch(out _)) { return expression; } - + } return Expression.Constant(arg, type); From f6a083784a56298435ed782726d9d41e62a813f8 Mon Sep 17 00:00:00 2001 From: Tony Hallett Date: Mon, 2 Aug 2021 14:49:42 +0100 Subject: [PATCH 6/6] extend the scope of the fix in the changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb43d9a40..769b60a10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1 #### Fixed -* mock.Protected().SetupSet fails when argument is of type Expression (@tonyhallett, #1189) +* mock.Protected() setup methods fail when argument is of type Expression (@tonyhallett, #1189) * Parameter is invalid in Protected().SetupSet() ... VerifySet (@tonyhallett, #1186) * Virtual properties and automocking not working for `mock.Protected().As<>()` (@tonyhallett, #1185)