Moq tries to evaluate non-evaluable parameter expressions

[stakx]

Reproduction code:

static object identity(object obj) => obj;

var mock = new Mock<Action<object>>();
mock.Setup(m => m(identity(m)));

Actual behavior:

An InvalidOperationException is thrown with the following message and stack trace:

variable 'm' of type 'System.Action`1[System.Object]' referenced from scope '', but it is not defined
   at System.Linq.Expressions.Compiler.VariableBinder.Reference(ParameterExpression node, VariableStorageKind storage)
   at System.Linq.Expressions.Compiler.VariableBinder.VisitParameter(ParameterExpression node)
   at System.Linq.Expressions.ParameterExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.Compiler.VariableBinder.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.VisitArguments(IArgumentProvider nodes)
   at System.Linq.Expressions.ExpressionVisitor.VisitMethodCall(MethodCallExpression node)
   at System.Linq.Expressions.MethodCallExpression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.Compiler.VariableBinder.Visit(Expression node)
   at System.Linq.Expressions.ExpressionVisitor.Visit(ReadOnlyCollection`1 nodes)
   at System.Linq.Expressions.Compiler.VariableBinder.VisitLambda[T](Expression`1 node)
   at System.Linq.Expressions.Expression`1.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.Compiler.VariableBinder.Visit(Expression node)
   at System.Linq.Expressions.Compiler.LambdaCompiler.Compile(LambdaExpression lambda, DebugInfoGenerator debugInfoGenerator)
   at System.Linq.Expressions.Expression`1.Compile()
   at Moq.DefaultExpressionCompiler.Compile[TDelegate](Expression`1 expression) in src\Moq\DefaultExpressionCompiler.cs:line 24
   at Moq.ExpressionExtensions.CompileUsingExpressionCompiler[TDelegate](Expression`1 expression) in src\Moq\ExpressionExtensions.cs:line 28
   at Moq.MatcherFactory.CreateMatcher(Expression expression) in src\Moq\MatcherFactory.cs:line 95
   at Moq.MatcherFactory.CreateMatcher(Expression argument, ParameterInfo parameter) in src\Moq\MatcherFactory.cs:line 61
   at Moq.InvocationShape.GetArgumentMatchers(IReadOnlyList`1 arguments, ParameterInfo[] parameters) in src\Moq\InvocationShape.cs:line 108
   at …

Expected behavior:

Moq should recognize that the expression parameter m cannot be used in any place other than the expression body's (left-most) root. It should throw a NotSupportedException before attempting to evaluate identity(…). This already works for the following simpler example:

var mock = new Mock<Action<object>>();
mock.Setup(m => m(m));

Further details:

This problem occurs because when Moq looks at the arguments in as setup expression, it must determine for any MethodCallExpression or MemberExpression whether they represent an argument matcher (such as It.IsAny<T>()). This is done by compiling and executing that argument expression, then trying to detect a side effect that's only caused by argument matchers.

What Moq currently doesn't do during this process is to first check whether the expression is compilable and executable at all. This obviously isn't the case when a parameter expression is present.

A similar method to that used by the Evaluator.Nominator class could be used to detect un-compilable expressions.

[stakx]

This has been slightly defused by 233063e1. What's more, this invalid usage pattern is probably an edge case that's unlikely to cause much trouble in real-world applications. On the other hand, guarding against it would require expression-tree-walking each and every argument expression appearing in setup/verify expressions. That cost-benefit ratio suggests that fixing this perhaps isn't warranted.

Closing as "won't fix" and unresolved for the time being. If anyone actually stumbles into this problem please report so this can be re-evaluated.

[airbreather]

If anyone actually stumbles into this problem please report so this can be re-evaluated.

We've been holding Moq back at 4.10.1 for a while (probably too long). I've finally taken some time out to figure out what's going on, and this appears to be the root cause for at least some of it.

I've pasted a .NET 6 command-line application below that's a pretty close representation of our code that works in 4.10.1 (writes "6") and fails when we try to upgrade to 4.18.1 with a similar stack trace to what you posted above.

It's easy enough to rewrite it so that x.CreateInnerFoo<int>() is its own independent mock object, but you asked for reports, so here's my report. ~FWIW, there's something else (possibly related, possibly not) that's blocking our upgrade from 4.10.1 with nondeterministic failures which I still haven't nailed down, which is why I haven't come looking for this over the past few years.~

Because I don't want to spam anyone watching this thread with another notification for this unrelated thing, I'm posting this as an edit. As one might have expected, the "nondeterministic failures" I was referring to in the crossed-out text above are not Moq's fault. Our own code was incorrect in a nondeterministic way, and it only became visible after the major rewrite from 4.11+ caused Moq to actually check our matchers that were nondeterministically incorrect. False alarm!

IFooFactory fooFactory = Moq.Mock.Of<IFooFactory>(x =>
    x.CreateOuterFoo(x.CreateInnerFoo<int>(), 10).Foo("hello") == new int[] { 1, 2, 3 });

new FooFactoryUser(fooFactory).Run("hello", 10);

public interface IFoo<T>
{
    T Foo(string s);
}

public interface IFooFactory
{
    IFoo<T> CreateInnerFoo<T>();
    IFoo<IEnumerable<int>> CreateOuterFoo(IFoo<int> inner, int someParameter);
}

public class FooFactoryUser
{
    private readonly IFooFactory _fooFactory;

    public FooFactoryUser(IFooFactory fooFactory)
    {
        _fooFactory = fooFactory;
    }

    public void Run(string s, int i)
    {
        Console.WriteLine(_fooFactory.CreateOuterFoo(_fooFactory.CreateInnerFoo<int>(), i).Foo(s).Sum());
    }
}

[stakx]

@airbreather, I can't promise that I'll reach a different conclusion than four years ago, but I'll take a look at that code soon.

[github-actions[bot]]

Due to lack of recent activity, this issue has been labeled as 'stale'. It will be closed if no further activity occurs within 30 more days. Any new comment will remove the label.