Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid Callback error when Returns argument has return type object #622

Closed
jimnoble opened this issue Jun 1, 2018 · 4 comments
Closed

Comments

@jimnoble
Copy link

jimnoble commented Jun 1, 2018

There appears to be a behavior change or regression with Moq 4.8.2 that did not occur with 4.7.145.

I have a reusable library for creating mocks at runtime based on an interface that the end developer provides. This means that some reflection must be used, and that generic type arguments cannot be used. Here is a simplified demonstration of the behavior:

using Moq;
using System;
using System.Linq.Expressions;
using System.Reflection;

class Program
{
    static void Main(string[] args)
    {
        var mock = new Mock<IMyInterface>();

        SetupMockProperty(
            mock, 
            propertyInfo: typeof(IMyInterface).GetProperty(nameof(IMyInterface.MyProperty)),
            lazyValueProvider: new Lazy<object>(() => "hello"));

        var result = mock.Object.MyProperty; // result should be "hello".
    }

    static void SetupMockProperty<TInterface>(
        Mock<TInterface> mock,
        PropertyInfo propertyInfo, 
        Lazy<object> lazyValueProvider) where TInterface : class
    {
        // 4.8.2 errors at runtime with:
        // 'Invalid callback. Setup on method with return type (System.String) 
        // cannot invoke callback with return type (System.Object).'

        mock
            .Setup(BuildPropertyLambda<TInterface>(propertyInfo))
            .Returns(() => lazyValueProvider.Value);
    }

    static Expression<Func<TInterface, object>> BuildPropertyLambda<TInterface>(
        PropertyInfo propertyInfo)
    {
        var parameter = Expression.Parameter(typeof(TInterface));

        var body = Expression.PropertyOrField(parameter, propertyInfo.Name);

        return Expression.Lambda<Func<TInterface, object>>(body, parameter);
    }
}

public interface IMyInterface
{
    string MyProperty { get; }
}

The SetupMockProperty represents the isolation from generic type arguments that is required for this use case. This code works great with 4.7.145, but gets the specified error with 4.8.2.

Is this new behavior intentional? And if so is there a different way that this use case can be approached?

@stakx
Copy link
Contributor

stakx commented Jun 2, 2018

Is this new behavior intentional?

Yes, see the changelog for version 4.8.0-rc1. This was requested in #445 and implemented in #520.

What stops you from wrapping your custom Expression with Expression.TypeAs or Expression.Convert to ensure the final expression has a static type that is assignable-to the set up member's return type?

@stakx
Copy link
Contributor

stakx commented Jun 9, 2018

Sorry for the somewhat naïve suggestion above.

The easiest is if you could add a TResult generic parameter in your two methods:

static void SetupMockProperty<TInterface, TResult>(
    Mock<TInterface> mock,
    PropertyInfo propertyInfo,
    Lazy<TResult> lazyValueProvider) where TInterface : class
{
    mock
        .Setup(BuildPropertyLambda<TInterface, TResult>(propertyInfo))
        .Returns(() => lazyValueProvider.Value);
}

static Expression<Func<TInterface, TResult>> BuildPropertyLambda<TInterface, TResult>(
    PropertyInfo propertyInfo)
{
    var parameter = Expression.Parameter(typeof(TInterface));
    var body = Expression.PropertyOrField(parameter, propertyInfo.Name);
    return Expression.Lambda<Func<TInterface, TResult>>(body, parameter);
}

If that is not an option, then it's going to get a little more complicated. You have a PropertyInfo, so you know the property type. You can build an Expression<Func<TInterface, TPropertyType>> using reflection (looking for the right Expression.Lambda method, then closing it over the generic arguments using MethodInfo.MakeGenericMethod, then invoking it using .Invoke). You'd also have to convert the code in SetupMockProperty to late-bound calls (again, using reflection to find the right Mock.Setup method, then .Invoke-ing it).

(I admit that this could (and should) be easier. You'll find that Moq has to pull that kind of trick in its own code base. This would be a lot easier if Moq exposed its internal, non-generic API that the generic API is built upon. But it's what we've got.)

@stakx
Copy link
Contributor

stakx commented Jun 9, 2018

Please let me know how you'd like to proceed with this issue. If I don't hear back from you, I'll close this in about 2-3 weeks' time.

@jimnoble
Copy link
Author

jimnoble commented Jun 9, 2018

I think since this is intentional and permanent behavior, and that there appears to be a work around, we can close this issue. Thanks for the help.

@jimnoble jimnoble closed this as completed Jun 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants