Detect re-entrant substitute calls

[dtchepak]

Not sure how feasible this is but...

Call a substitute from within the .Returns(...) block of another substitute can cause issues:

nsubstitute/NSubstitute#26
nsubstitute/NSubstitute#357

Note that sub.Blah().Returns(x => CreateFoo()) can be ok, whereas sub.Blah().Returns(CreateFoo()) can cause the problem (when CreateFoo also sets Returns() values).

[tpodolak]

I will take a look at this one. However, this might have a negative impact on the performance as I will have to recursively analyze method bodies used as an argument of Returns method

[dtchepak]

I'm just noting down potential cases for detection without thinking about feasibility. 😄 Feel free to ignore/close any suggestions!

[tpodolak]

I did an initial look into and I have some working PoC. Can you guys advise about what kind of message do you expect to see? How should I show to the user that given Returns setup, calls (possible multiple) other Returns methods inside? Should we show warning on the root method or maybe in every re-entrant call, or maybe do you have some other ideas

[dtchepak]

I think something like:

Returns() is set with a method that itself calls Returns. This can cause problems with NSubstitute. Consider replacing with a lambda: Returns(x => TheMethod()).

I think the root method should be sufficient. What do you think?

[dtchepak]

It might just be sufficient to warn on .Returns(SomeMethodCall()) and recommend .Returns(x => SomeMethodCall()) instead? Might be easier with less performance impact?

[tpodolak]

@dtchepak The warning/diagnostic on the root call seems to be ok. I need one clarification though, putting aside potential performance problems, are all reentrant calls problematic from the perspective of NSubstitute or only the ones which are evaluated eagerly? Please take a look at the examples below and comment on them

substitute.Bar().Returns(ReturnThis); // no warning as invoking with delegate ?
substitute.Bar().Returns(MyMethod()); // no warning as MyMethod() returns a delegate?
substitute.Bar().Returns(ReturnThis()); // warning as invoking with result of function which calls Returns ?
substitute.Bar().Returns(ReturnForAnyArgsThis()); // warning as invoking with result of function which calls ReturnWithAnyArg ?
 substitute.Bar().Returns(WhenDoThis()); // warning as invoking with result of function which calls When..Do?

private int ReturnThis(CallInfo arg)
{
  Substitute.For<IFoo>().Bar().Returns(1);
  return 1;
}
private int ReturnThis()
{
  Substitute.For<IFoo>().Bar().Returns(1);
  return 1;
}

private int ReturnForAnyArgsThis()
{
  Substitute.For<IFoo>().Bar().ReturnForAnyArg(1);
  return 1;
}

private int WhenDoThis()
        {
            var @for = Substitute.For<IFoo>();
            @for.When(x => x.Bar()).Do(yy => { });
            return 1;
        }

 Func<CallInfo, int> MyMethod()
        {
            return ReturnThis;
        }

[dtchepak]

Hi @tpodolak,

substitute.Bar().Returns(ReturnThis); // no warning as invoking with delegate ?
substitute.Bar().Returns(MyMethod()); // no warning as MyMethod() returns a delegate?
substitute.Bar().Returns(ReturnThis()); // warning as invoking with result of function which calls Returns ?
substitute.Bar().Returns(ReturnForAnyArgsThis()); // warning as invoking with result of function which calls ReturnWithAnyArg ?
 substitute.Bar().Returns(WhenDoThis()); // warning as invoking with result of function which calls When..Do?

This seems correct. I did some testing with these examples and all the "no warning" examples passed, all the "warning" examples failed.

For completeness, here is the test setup I used:

  public interface IFoo {
            int Bar();
        }
        [Test]
        public void Samples() {
            var substitute = Substitute.For<IFoo>();

            //substitute.Bar().Returns(ReturnThis); // no warning as invoking with delegate ?
            //substitute.Bar().Returns(MyMethod()); // no warning as MyMethod() returns a delegate?
            substitute.Bar().Returns(ReturnThis()); // warning as invoking with result of function which calls Returns ?
            //substitute.Bar().Returns(ReturnForAnyArgsThis()); // warning as invoking with result of function which calls ReturnWithAnyArg ?
            //substitute.Bar().Returns(WhenDoThis()); // warning as invoking with result of function which calls When..Do?

            Assert.AreEqual(1, substitute.Bar());
        }

        // ... copied and pasted the delegates from the earlier question ...