Setup with generic args (nullable and non-nullable) always calls latest setup, not taking into account generic type argument of callee

[dschuermans]

Hey,

I recently upgraded from Moq 4.10 to 4.16 and I started experiencing weird NullReferenceErrors in tests that previously worked just fine.

After some investigation, I managed to reproduce the issue with the following code:

[TestClass]
    public class Test
    {
        public interface MyTestInterface
        {
            T DoSomething<T>(string input);
        }

        public enum MyTestEnum
        {
            One = 1,
            Two = 2
        }

        [TestMethod]
        public void MyTestMethod()
        {
            var mock = new Mock<MyTestInterface>(MockBehavior.Strict);

            mock.Setup(x => x.DoSomething<MyTestEnum>(It.IsAny<string>())).Returns((string input) => ParseEnum<MyTestEnum>(input));
            mock.Setup(x => x.DoSomething<MyTestEnum?>(It.IsAny<string>())).Returns((string input) => ParseEnum<MyTestEnum?>(input));

            var mapper = mock.Object;

            Assert.AreEqual(MyTestEnum.Two, mapper.DoSomething<MyTestEnum>("Two"));

            Assert.ThrowsException<Exception>(() => mapper.DoSomething<MyTestEnum>(null));

            Assert.AreEqual(null, mapper.DoSomething<MyTestEnum?>(null));
        }

        private T ParseEnum<T>(string input)
        {
            try
            {
                Type enumType = Nullable.GetUnderlyingType(typeof(T)) ?? typeof(T);
                return (T)Enum.Parse(enumType, input);
            }
            catch (Exception e)
            {
                // If T is non-nullable, thrown exception
                if (Nullable.GetUnderlyingType(typeof(T)) == null)
                {
                    throw new Exception("Failed to parse");
                }

                return default(T);
            }
        }
    }

My first call to .DoSomething<MyTestEnum>("Two") I would expect the first setup to be called. Instead, it is going for the second one, the one with the MyTestEnum? generic argument. As such, T in the ParseEnum is MyTestEnum? and not MyTestEnum
As my input value is valid, this doesn't pose an issue.

The issue however is when I pass an invalid value, if the argument is non-nullable, I expect an exception to be thrown. (My reasoning in this case is: when parsing a string as an non-nullable enum, the parsing must succeed. When its a nullable enum, NULL is the expected value when parsing failed)
But, because T in ParseEnum is now always MyTestEnum?, irregardless of what has been called.

When I swap the order of Setup calls, it will be the other way around. T will always be MyTestEnum

Any thoughts on how to circumvent this?
I have a bunch of enums (nullable and non-nullable) for which i'm registering setups through reflection, which now no longer work :(

[dschuermans]

Fiddled around with it, seems the following is potentially a step in the right direction (and would severely reduce the amount of reflection code required to setup all my enums)

//mock.Setup(x => x.DoSomething<MyTestEnum>(It.IsAny<string>())).Returns((string input) => ParseEnum<MyTestEnum>(input));
            //mock.Setup(x => x.DoSomething<MyTestEnum?>(It.IsAny<string>())).Returns((string input) => ParseEnum<MyTestEnum?>(input));
            mock.Setup(x => x.DoSomething<It.IsAnyType>(It.IsAny<string>())).Returns(new InvocationFunc((IInvocation invocation) =>
            {
                var type = invocation.Method.GetGenericArguments()[0];
                var input = invocation.Arguments[0]?.ToString();
                Type enumType = Nullable.GetUnderlyingType(type) ?? type;

                try
                {

                    return Enum.Parse(enumType, input);
                }
                catch (Exception e)
                {
                    // If T is non-nullable, thrown exception
                    if (Nullable.GetUnderlyingType(type) == null)
                    {
                        throw new Exception("Failed to parse");
                    }

                    return null;
                }
            }));

The only "issue" with this, is that I'm getting a TargetInvocationException and not my custom thrown Exception("Failed to parse")
But on the plus side, it is available as the InnerException on the TargetInvocationException, so I think I can update my tests accordingly

[stakx]

I'm getting a TargetInvocationException and not my custom thrown Exception("Failed to parse")

I cannot help you with your original question, but this sounds like a problem with how Moq internally invokes the callback. We have the necessary infrastructure in place to avoid this problem, it appears we've missed one or two places where it should get used. I'll look into it.

[stakx]

There you go. In the upcoming Moq 4.16.1, the wrapping TargetInvocationException will be gone. Thanks for mentioning this problem!

[stakx]

P.S. You may want to create a type matcher that only matches enum types... perhaps something along the lines of:

[TypeMatcher]
readonly struct AnyEnum : ITypeMatcher
{
    public bool Matches(Type type)
    {
        return (Nullable.GetUnderlyingType(type) ?? type).IsEnum;
    }
}

[dschuermans]

P.S. You may want to create a type matcher that only matches enum types... perhaps something along the lines of:

[TypeMatcher]
readonly struct AnyEnum : ITypeMatcher
{
    public bool Matches(Type type)
    {
        return (Nullable.GetUnderlyingType(type) ?? type).IsEnum;
    }
}

Ah cheers, didn't know such functionality existed 😃
And thank you for solving the issue with the exception.

Anyhow, while I managed to circumvent the issue I was having by using the mock from my second comment, I sure hope someone will be able to find the issue with the setups not being called correctly

[stakx]

My impression is that this problem is larger than just Moq. Nullable value types can at times be confusing, even more so when generic type parameters enter the picture.

The .NET compilers have certain coercion rules brtween some TValueType and TValueType?, and due to that, Moq likewise needs to perform such coercions (e.g. here). I haven't so far gained an exact complete picture when those coercions are necessary... that code predates my involvement with Moq and was likely added based on experiences with particular scenarios.

Because that code has been around quite long, I suspect we cannot easily change the coercion rules (if that's what would be necessary to fix your use case) without breaking a lot of user code.

Let's leave this open a while longer, perhaps someone will be able to come up with alternative setups that resolve this.

[stakx]

Let's leave this open a while longer, perhaps someone will be able to come up with alternative setups that resolve this.

There has been no activity for over a year, so I am finally closing this issue.