SetupSet problems with It.IsAny<> and indexer properties

[misulo]

Hi all

The topic is already discussed on Stack Overflow: http://stackoverflow.com/questions/29697406/moq-an-indexed-property-and-use-the-index-value-in-the-return-callback

I ran into it also and then looking if an issue is posted, couldn't find any. So thought I'd post one.

In a nutshell:

// This compiles, but callback never seems to be called
mock.SetupSet(m => m[It.IsAny<int>()] = It.IsAny<int>()).Callback(DoSet);

As a matter of fact, forget the Callback. Turn MockBehaviour to Strict, and try to mock the indexer Setter with a It.IsAny<>. It will not work.

mock.SetupSet(m => m[It.IsAny<int>()] = It.IsAny<int>());

So right now the only workaround is not using the Strict behavior.

Same works without problems with SetupGet.

regards,
Misulo.

[loedeman]

I am experiencing the exact same issue. It would be nice when MockBehaviour.Strict and It.IsAny would work, fortunately in most of those cases omitting the mock behaviour does the trick. Is there any progress or are there any plans on this issue?

[stakx]

I was looking at this a few days ago. IIRC there was a good reason why this doesn't work ATM, indexers and expression trees aren't too good friends and there's even a disabled unit test. I'll take a closer look in the next few days.

[stakx]

PS: Feel free to do the same. If you find a solution you're welcome to send a PR!

[stakx]

@misulo and @loedeman, here's a brief analysis of why this doesn't work:

C# currently cannot convert a lambda involving an assignment to an expression tree. That is, the following won't compile:

Expression<Action<string>> expr = str => str = null; 
//                                       ^^^^^^^^^^
// CS0832: An expression tree may not contain an assignment operator

How then does Moq make it possible for you to pass such a lambda to Mock<T>.SetupSet()? Simply by declaring that method's parameter as Action<T> instead of Expression<Action<T>>. And that, in turn, is why Mock cannot analyse the lambda passed to SetupSet and recognize matchers properly. All it can do is execute the lambda in a recorder-like mode (inside a so-called FluentMockContext) and then look at which members were invoked on the involved mock(s) and base the setup on that… but by that time It.IsAny<> et al. have been evaluated to some default return values, thus giving unexpected results during verification.

As far as I can see, there are only a few options how to get this to work:

1. Create a new overload of Mock<T>.SetupSet<TProperty>(Expression<Func<T, TProperty>> property, TProperty value), which you'd call like this:

   mock.SetupSet(foo => foo.Bar[It.IsAny<string>()], value: It.Is<string>(str => …));

   Not as pretty as having a =, but that's likely the best available option. Note that the value parameter probably doesn't need to be a lambda, nor an Expression<>, since matchers (IIRC) already record their last invocation.

2. Change the current implementation of Mock<T>.SetupSet(Action<T> setter) to decompile the given lambda's IL into an expression tree at runtime. This would be possible (at least for non-AOT-compiled target platforms), but an insane amount of work.

3. Patch the current implementation some more so that all invocations of matchers (along with where in the stack trace they happen) are recorded, not just the last one. This could improve Moq's current abilities somewhat, but there'd still be gaps.

4. Wait for C# to allow assignments in lambdas that get converted to expression trees. (This might or might not happen; see https://github.com/dotnet/roslyn/issues/2060.)

Or, we simply don't fix this at all.

Any thoughts?

[loedeman]

Hi @stakx , thanks for your research and thorough explanation. Although it is not very comfortable not being able to fully test setters, just providing the actual value works well enough most of the time. Some edge cases (where you are not able to predict the setter value beforehand) would make the first option you are suggesting very useful. I agree that options 2 and 3 are not reasonable. Given the huge amount of work and the relatively small amount of edge cases (in my situation), I could simply live with option 4 as well for now ;). Cheers, Bert

[stakx]

I might have been wrong with my previous assessment of option (2):

Change the current implementation of Mock<T>.SetupSet(Action<T> setter) to decompile the given lambda's IL into an expression tree at runtime. This would be possible (at least for non-AOT-compiled target platforms), but an insane amount of work.

It turns out that decompilation might not be so much work after all: I wrote a small (approx. 150 LoC) prototype earlier today that could decompile the kinds of setup lambdas you'd typically see with Moq. Here's a small proof of concept library that I worked on today (functional, but far from being feature-complete).

If we could base Moq on IL decompilation instead of the present mechanism (FluentMockContext), we (a) might have to live with a few additional constraints (which probably wouldn't matter at all in practice), but we would (b) gain the ability to have It.Is* matchers work everywhere, and (c) possibly speed up Moq a lot. (IL decompilation would essentially take the place of expression tree compilation, which is about one order of magnitude slower and currently the top efficiency bottleneck; see #188.)

/cc @kzu

[stakx]

Reminder: When this issue is fixed, remember to restore these two unit tests in UnitTests/PropertiesFixture.cs, for now they get removed:

[Fact(Skip = "Not supported for now")]
public void ShouldSetIndexerWithIndexMatcher()
{
   var foo = new Mock<IIndexedFoo>(MockBehavior.Strict);
   foo.SetupSet(f => f[It.IsAny<int>()] = "foo");
   foo.Object[18] = "foo";
}

[Fact(Skip = "Not supported for now")]
public void ShouldSetIndexerWithBothMatcher()
{
   var foo = new Mock<IIndexedFoo>(MockBehavior.Strict);
   foo.SetupSet(f => f[It.IsAny<int>()] = It.IsAny<string>());
   foo.Object[18] = "foo";
}

[stakx]

Closing this, this problem will be tracked in #414 (together with a couple other related issues).