ReturnsForAnyArgs and WhenForAnyArgs and generic arguments

[alexandrnikitin]

This is more a question rather than a bug report. Inspired by a question on SO Suppose we have an interface with a generic method:

public interface IInterfaceWithGenericMethod
{
    int GenericMethod<T>(T arg);
}

Then when we specify the sub to return a value for any argument then it fails on the call with another generic type.

var sub = Substitute.For<IInterfaceWithGenericMethod>();

const int expectedInt = 1;
sub.GenericMethod(Arg.Any<int>()).ReturnsForAnyArgs(expectedInt); // Specifying for int

Assert.AreEqual(expectedInt, sub.GenericMethod(0)); // Works for int
Assert.AreEqual(expectedInt, sub.GenericMethod("")); // Fails for string

The same is right for WhenForAnyArgs(). Imo this isn't obvious behavior. Thoughts?

[dtchepak]

I think whether it is obvious or not depends on how we think about generic methods. AFAICT when we use a generic method it will be compiled as a specific, non-generic instance of that generic method. I think we tend to approximate this to "calling a generic method" with a varying type parameter, in which case we'd conclude that we should be able to setup calls to sub.GenericMethod<T>(). Dropping the approximation leads us to the opposite conclusion: sub.GenericMethod<int>() must be configured separately to sub.GenericMethod<string>() because they are different methods. So I think the current behaviour makes sense.

It could still be useful to come up with a way off configuring all instances of GenericMethod<T>, but I think it will need a different syntax.

[alexandrnikitin]

I think it's more a question of how we treat ReturnsForAnyArgs() method and "ForAnyArgs" suffix there. Is it more "Any" or more "Returns"? Does it really mean "Any"?

Your position is fine for me but for Returns() method with argument specifications. E.g. sub.GenericMethod(Arg.Any<int>()).Returns(expectedInt); Is fine to return expectedInt only for args of int type. There generic specification could have a different syntax e.g. sub.GenericMethod(Arg.Any<int>().OrAnyOtherType()).Returns(expectedInt);

But it's not so obvious for ReturnsForAnyArgs() which imho a special case. And meant to cover any args regardless of their specification, types and so on. I'm trying to think from end-user perspective here.

[dtchepak]

I take your point. I've had this cause confusion for people before. I think a good example could be WhenForAnyArgs because it doesn't set a return type to constrain the generic.

sub.WhenForAnyArgs(x => x.GenericMethod(0)).Do(...) sets up GenericMethod<int>, but if we think of type arguments as regular arguments then it gets confusing. We could be trying to set up GenericMethod<T> for all T.

To me the former still makes sense provided we're thinking of generics accurately (i.e. "ForAnyArgs" means any arguments passed to the closed generic GenericMethod<int>). The latter could still be a useful thing to do, but NSub doesn't currently support it.

[alexfoxgill]

Came here via #185, this would be useful for a generic repository pattern. I think it would need its own syntax though; .ReturnsForAnyArgs() is strongly typed.

The syntax could look something like

var x = Substitute.For<IInterfaceWithGenericMethod>();
x.GenericMethod<object>().ReturnsForAnyGeneric(call => Activator.CreateInstance(call.GenericArgs[0]))

[dtchepak]

Closed #185 so we can continue discussion here.

How frequently do you need something like this? In my tests I haven't run in to a case where I've had to stub numerous generic instances of calls with the same logic.

I think we could come up with a way of stubbing/getting callbacks for multiple generic instances, but I'm not sure it is worth the complexity (compared to, say, hand-rolling a fake IRepository).

[smirnfil]

I think this is a nice functionality to have.

I want this behaviour for my repo stubs - if someone somewhere asks for some IQueryable<T> just give an empty one to him. I'll probably be happy with just #67(should look into it as it seems much easier to implement).

However, I understands the concerns about complexity. Faking doesn't looks as nice as mocking (one of repos that we use have 13 methods and I really care about 1 or 2 generic ones) but I could live with it.

[dtchepak]

Would be great to have #67 sorted! :)

[julealgon]

Stumbled upon this while attempting to validate a call into Microsoft.Extensions.Logging.ILogger.

The logger interface takes a T state, but the "public" APIs (i.e. common extension methods) do not expose this directly.

For example, consumer code calls it like this:

logger.Critical("A critical problem has occurred!");

This is translated into a call to the interface's method, which has this signature:

public void Log<TState>(
            LogLevel logLevel,
            EventId eventId,
            TState state,
            Exception exception,
            Func<TState, Exception, string> formatter)

The string parameter that represents the message in the original call is not passed directly into this method, but instead is wrapped in an internal type and passed as state. Thus, it is actually impossible to perform an assertion on ILogger, since you cannot specify the internal type without getting a compilation error.

I actually solved this before by creating an abstract TestableLogger that exposes a non-generic version of the base interface method and then redirecting the implementation to it, and then using .Received on top of that new method. Obviously, this is extremely convoluted.

I'd really appreciate if NSubstitute provided a way to specify Any<AnyType>() that would check for any generic argument on a given parameter.

[dtchepak]

@julealgon Thanks for the good example. I've created https://github.com/nsubstitute/NSubstitute/issues/634 for this feature request.