Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leave quoted (nested) expressions unchanged when evaluating captured variables #1262

Merged
merged 4 commits into from May 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion src/Moq/Evaluator.cs
Expand Up @@ -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;
Expand Down
24 changes: 19 additions & 5 deletions src/Moq/ExpressionComparer.cs
Expand Up @@ -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()
{
}
Expand All @@ -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);
}
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/Moq/ExpressionExtensions.cs
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions src/Moq/Expressions/Visitors/EvaluateCaptures.cs
Expand Up @@ -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);
}
}
}
29 changes: 29 additions & 0 deletions tests/Moq.Tests/Regressions/IssueReportsFixture.cs
Expand Up @@ -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
Expand Down