CouldNotSetReturnDueToTypeMismatchException when using Substitute and acessing it in Arg.Is

[rklec]

Describe the bug If you use Arg.Is when setting the return value and do something elaborate as accessing an outer saved Substitute, you get the CouldNotSetReturnDueToTypeMismatchException when (and only when!) you configure the second time.

To Reproduce Analyzer was used.

Here is an example adapted from here:

using NSubstitute;
using NSubstitute.ClearExtensions;
using NUnit.Framework;
using System;
using System.Collections.Generic;
using System.Threading.Tasks;

namespace SomeProject
{
    public class NSubtituteTest
    {
        public interface ISomeModal
        {
            public string MyString { get; set; }
        }

        public class SomeModal : ISomeModal
        {
            public string MyString { get; set; } = "";
        }

        public interface ICommand
        {
            Task<int> Execute(IList<string> list);

            event EventHandler Executed;
        }

        public class SomethingThatNeedsACommand
        {
            private readonly ICommand command;

            public SomethingThatNeedsACommand(ICommand command)
            {
                this.command = command;
            }

            public void DoSomething(ISomeModal anlage) =>
                this.command.Execute(new List<string> { anlage.MyString });
        }

        [Test]
        public void NSubstitute_Test()
        {
            var modal = Substitute.For<ISomeModal>();
            modal.MyString = "example";

            //Arrange
            var command = Substitute.For<ICommand>();
            var something = new SomethingThatNeedsACommand(command);
            command.Execute(Arg.Is<IList<string>>(x => x.Contains(modal.MyString))).Returns(25);
            command.ClearSubstitute(); // does not help

            // if you comment the line below por run it in a new "Isloated" test or so, it helps
            command.Execute(Arg.Is<IList<string>>(x => x.Contains(modal.MyString))).Returns(25); // here it throws!

            //Act
            something.DoSomething(modal);

            //Assert
            command.ReceivedWithAnyArgs().Execute(default!);
        }
    }
}

Actual behaviour

NSubstitute.Exceptions.CouldNotSetReturnDueToTypeMismatchException : Can not return value of type Task`1 for ISomeModal.get_MyString (expected type String).

Make sure you called Returns() after calling your substitute (for example: mySub.SomeMethod().Returns(value)),
and that you are not configuring other substitutes within Returns() (for example, avoid this: mySub.SomeMethod().Returns(ConfigOtherSub())).

If you substituted for a class rather than an interface, check that the call to your substitute was on a virtual/abstract member.
Return values cannot be configured for non-virtual/non-abstract members.

Correct use:
	mySub.SomeMethod().Returns(returnValue);

Potentially problematic use:
	mySub.SomeMethod().Returns(ConfigOtherSub());
Instead try:
	var returnValue = ConfigOtherSub();
	mySub.SomeMethod().Returns(returnValue);

   at NSubstitute.Core.ConfigureCall.CheckResultIsCompatibleWithCall(IReturn valueToReturn, ICallSpecification spec)
   at NSubstitute.Core.ConfigureCall.SetResultForLastCall(IReturn valueToReturn, MatchArgs matchArgs, PendingSpecificationInfo pendingSpecInfo)
   at NSubstitute.Core.CallRouter.LastCallShouldReturn(IReturn returnValue, MatchArgs matchArgs, PendingSpecificationInfo pendingSpecInfo)
   at NSubstitute.Core.ThreadLocalContext.LastCallShouldReturn(IReturn value, MatchArgs matchArgs)
   at NSubstitute.SubstituteExtensions.ConfigureReturn[T](MatchArgs matchArgs, T returnThis, T[] returnThese)
   at NSubstitute.SubstituteExtensions.Returns[T](Task`1 value, T returnThis, T[] returnThese)
   at SomeProject.NSubtituteTest.NSubstitute_Test() in [...]\NSubstituteTest.cs:line 55

It works when:

• If you do only substitute the same substitute once.
• I run the second call in a test that is Isolated, i.e. it is a random test failure, which makes it ugly to debug.
• If you do not access the modal object, but directly assert a string: command.Execute(Arg.Is<IList<string>>(x => x.Contains("modal.MyString"))).Returns(25);
• if you do not use NSubstitute for the modal, but create it as it is, i.e. var modal = new SomeModal();
• if you do not use Arg.Is, but e.g. ReturnsForAnyArguments with default.
• Edit: When you use the Isolated attribute from NCrunch.

It also happens:

• when you use a simple string comparison e.g. instead (just as an example): command.Execute(Arg.Is<IList<string>>(x => modal.MyString.Equals("example"))).Returns(25);

Expected behaviour If the behaviour is unexpected, it should not happen.

If you expect it like that:

• Could you improve the error message? Currently, the error message cares about the return value, but the return value is an int, so everything l see, looks okay.
• Maybe add an Analyser check?

Environment:

• NSubstitute version: 4.2.2
• NSubstitute.Analyzers version: 1.0.14
• Platform: netcoreapp3.1 on Windows 10, Visual Studio

Additional context N/A (internal ID here: !6324)

[rklec]

Does anyone have any clue here?

[dtchepak]

Hi @rklec ,

Thanks for the good repro.

This happens because x => x.Contains(modal.MyString) invokes the MyString property on the modal substitute, which confuses NSubstitute as to which call is being stubbed (it tries to make modal.MyString return a Task<int> instead of a string).

If possible try to avoid calling one substitute while configuring another. In this case, I'd pull out a constant for configuration:

var exampleString = "example";
var modal = Substitute.For<ISomeModal>();
modal.MyString = exampleString;
//...
command.Execute(Arg.Is<IList<string>>(x => x.Contains(exampleString))).Returns(25);

[rklec]

Okay, thanks a lot for your reply. I thought modal.MyString was already constant enough. Still, I wonder:

• why does not it always fail then and depends on the execution of other tests? E..g in Isolated it works.
• Why does not it show the correct error message for this case here? It assures you "that you are not configuring other substitutes within Returns" and - which I don't, yet alone, because it is not about any return, but Arg.Is- and the "non-virtual/non-abstract members" is also misleading.

So can NSubstitute or NSubstitiute.Analyzers maybe (bettter) detect and prevent this misconfiguration error? Or at least, show a more useful/accurate error message?

[dtchepak]

I thought modal.MyString was already constant enough.

It's not the value that is the issue, it is the call to another substitute. If it was doing command.Execute(modal.MyString).Returns(...) it would like be ok, because modal.MyString would be evaluated before the command.Execute(...) call.

NSubstitute does some dodgy things to get its syntax to work. On a substituted call it works a bit like this:

For a call like command.Execute(...).Returns(...), first it will run Execute():

override Task<int> Execute(string s) {
    // (1) Tell NSubstitute (~global state) that the last call received was Execute with argument s.
    // If s is default, check any queued up argument matchers and use those instead of argument s.

    // (2) Record call in this substitute's state.

    // (3) Find return value
    // If no argument matchers queued:
    // - record call in this substitute's state
    // - look up stubbed calls in this substitute's state, and return value from there if it has been stubbed. Otherwise return default.
    // If arg matchers queued:
    // - we are configuring call, don't record call as received
    // - return default for call
}

Then when Returns is called, NSubstitute:

• looks up what the last call received
• remove the call recorded in (2) (as the sub is not meant to receive this call, it is instead doing configuration)
• stub the last call to return the value specified.

In this case, I think step (3) is running the argument matcher on the second attempt to stub the call, which means when Returns runs NSubstitute sees the last call as modal.MyString instead of Execute.

We might be able to disable setting last call from arg matcher callbacks, but sometimes those changes can have very unintended side-effects so would require a bit of care.

@tpodolak Do you think it is worth adding a rule similar to NS4000 for re-entrant sub calls in arg matchers? One issue is that I think it will only be a trouble when stubbing a call for a second time. 🤔