diff --git a/CHANGELOG.md b/CHANGELOG.md index eefa6f4bd..52ba8e718 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/Moq/Evaluator.cs b/src/Moq/Evaluator.cs index 68fbbcb9c..3af867e37 100644 --- a/src/Moq/Evaluator.cs +++ b/src/Moq/Evaluator.cs @@ -102,7 +102,7 @@ internal HashSet 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; diff --git a/src/Moq/ExpressionComparer.cs b/src/Moq/ExpressionComparer.cs index 07176d660..304bb4125 100644 --- a/src/Moq/ExpressionComparer.cs +++ b/src/Moq/ExpressionComparer.cs @@ -14,6 +14,9 @@ internal sealed class ExpressionComparer : IEqualityComparer { 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); diff --git a/src/Moq/ExpressionExtensions.cs b/src/Moq/ExpressionExtensions.cs index aad628707..22f13cc10 100644 --- a/src/Moq/ExpressionExtensions.cs +++ b/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) diff --git a/src/Moq/Expressions/Visitors/EvaluateCaptures.cs b/src/Moq/Expressions/Visitors/EvaluateCaptures.cs index 67df48da2..26c6d13e8 100644 --- a/src/Moq/Expressions/Visitors/EvaluateCaptures.cs +++ b/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); + } } } diff --git a/tests/Moq.Tests/Regressions/IssueReportsFixture.cs b/tests/Moq.Tests/Regressions/IssueReportsFixture.cs index 798ca0c48..5efb0f5da 100644 --- a/tests/Moq.Tests/Regressions/IssueReportsFixture.cs +++ b/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(); + mock.Setup(x => x.GetSetting(It.Is(y => y == patternKey))).Returns(() => patternKey); + mock.Setup(x => x.GetSetting(It.Is(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