Merge pull request #1262 from stakx/bugfix/lazy-setup-expr-evaluation

Leave quoted (nested) expressions unchanged when evaluating captured variables

Author: stakx

CHANGELOG.md

@@ -9,6 +9,7 @@ The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1
 
 #### Fixed
 
+* Regression with lazy evaluation of `It.Is` predicates in setup expressions after updating from 4.13.1 to 4.16.1 (@b3go, #1217)
 * Regression with `SetupProperty` where Moq fails to match a property accessor implementation against its definition in an interface (@Naxemar, #1248)
 * Difference in behavior when mocking async method using `.Result` vs without (@cyungmann, #1253)
 

src/Moq/Evaluator.cs

@@ -102,7 +102,7 @@ internal HashSet<Expression> Nominate(Expression expression)
 
 			public override Expression Visit(Expression expression)
 			{
-				if (expression != null)
+				if (expression != null && expression.NodeType != ExpressionType.Quote)
 				{
 					bool saveCannotBeEvaluated = this.cannotBeEvaluated;
 					this.cannotBeEvaluated = false;

src/Moq/ExpressionComparer.cs

@@ -14,6 +14,9 @@ internal sealed class ExpressionComparer : IEqualityComparer<Expression>
 	{
 		public static readonly ExpressionComparer Default = new ExpressionComparer();
 
+		[ThreadStatic]
+		private static int quoteDepth = 0;
+
 		private ExpressionComparer()
 		{
 		}
@@ -30,15 +33,16 @@ public bool Equals(Expression x, Expression y)
 				return false;
 			}
 
-			// Before actually comparing two nodes, make sure that captures variables have been
-			// evaluated to their current values (as we don't want to compare their identities):
+			// Before actually comparing two nodes, make sure that captured variables have been
+			// evaluated to their current values (as we don't want to compare their identities).
+			// But do so only for the main expression; leave quoted (nested) expressions unchanged.
 
-			if (x is MemberExpression)
+			if (x is MemberExpression && ExpressionComparer.quoteDepth == 0)
 			{
 				x = x.Apply(EvaluateCaptures.Rewriter);
 			}
 
-			if (y is MemberExpression)
+			if (y is MemberExpression && ExpressionComparer.quoteDepth == 0)
 			{
 				y = y.Apply(EvaluateCaptures.Rewriter);
 			}
@@ -47,13 +51,23 @@ public bool Equals(Expression x, Expression y)
 			{
 				switch (x.NodeType)
 				{
+					case ExpressionType.Quote:
+						ExpressionComparer.quoteDepth++;
+						try
+						{
+							return this.EqualsUnary((UnaryExpression)x, (UnaryExpression)y);
+						}
+						finally
+						{
+							ExpressionComparer.quoteDepth--;
+						}
+
 					case ExpressionType.Negate:
 					case ExpressionType.NegateChecked:
 					case ExpressionType.Not:
 					case ExpressionType.Convert:
 					case ExpressionType.ConvertChecked:
 					case ExpressionType.ArrayLength:
-					case ExpressionType.Quote:
 					case ExpressionType.TypeAs:
 					case ExpressionType.UnaryPlus:
 						return this.EqualsUnary((UnaryExpression)x, (UnaryExpression)y);

src/Moq/ExpressionExtensions.cs

@@ -448,6 +448,7 @@ private static bool PartialMatcherAwareEval_ShouldEvaluate(Expression expression
 #pragma warning disable 618
 			return expression.NodeType switch
 			{
+				ExpressionType.Quote        => false,
 				ExpressionType.Parameter    => false,
 				ExpressionType.Extension    => !(expression is MatchExpression),
 				ExpressionType.Call         => !((MethodCallExpression)expression).Method.IsDefined(typeof(MatcherAttribute), true)

src/Moq/Expressions/Visitors/EvaluateCaptures.cs

@@ -31,5 +31,10 @@ protected override Expression VisitMember(MemberExpression node)
 				return base.VisitMember(node);
 			}
 		}
+
+		protected override Expression VisitUnary(UnaryExpression node)
+		{
+			return node.NodeType == ExpressionType.Quote ? node : base.VisitUnary(node);
+		}
 	}
 }

tests/Moq.Tests/Regressions/IssueReportsFixture.cs

@@ -3688,6 +3688,35 @@ public virtual void SecondCall()
 
 		#endregion
 
+		#region 1217
+
+		public class Issue1217
+		{
+			[Fact]
+			public void It_Is_predicates_are_evaluated_lazily()
+			{
+				var patternKey = "";
+				var exeKey = "";
+
+				var mock = new Mock<ISettingsService>();
+				mock.Setup(x => x.GetSetting(It.Is<string>(y => y == patternKey))).Returns(() => patternKey);
+				mock.Setup(x => x.GetSetting(It.Is<string>(y => y == exeKey))).Returns(() => exeKey);
+
+				patternKey = "foo";
+				exeKey = "bar";
+
+				Assert.Equal("foo", mock.Object.GetSetting(patternKey));
+				Assert.Equal("bar", mock.Object.GetSetting(exeKey));
+			}
+
+			public interface ISettingsService
+			{
+				string GetSetting(string key);
+			}
+		}
+
+		#endregion
+
 		#region 1225
 
 		public class Issue1225