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

Can't setup protected methods with nullable parameters #92

Closed
RobSiklos opened this issue Feb 11, 2014 · 7 comments
Closed

Can't setup protected methods with nullable parameters #92

RobSiklos opened this issue Feb 11, 2014 · 7 comments

Comments

@RobSiklos
Copy link
Contributor

It seems that it's not possible to set up a protected method if it has nullable parameters.

Example:

public abstract class MyService
{
    protected abstract void MyMethod(int? val);
}

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

        mock.Protected().Setup("MyMethod", (int?)1);
    }
}

The output of running this program is an ArgumentException with the following message:

Expression of type 'System.Int32' cannot be used for parameter of type 'System.Nullable1[System.Int32]' of method 'Void MyMethod(System.Nullable1[System.Int32])'

@RobSiklos
Copy link
Contributor Author

I took a quick peek in the code and I believe the solution can be implemented easily. The reason for the problem is that when nullable values are cast to object, it's the underlying value that gets boxed, rather than the nullable instance being referenced. So we've completely lost the fact that an arg is nullable just by looking at its type.

The solution is that in ProtectedMock.ToExpressionArg(), the call to Expression.Constant() should also specify the type (using the other overload for that method).

Here are my modified versions of ToExpressionArg() and ToExpressionArgs():

private static Expression ToExpressionArg(ParameterInfo paramInfo, object arg)
{
    var lambda = arg as LambdaExpression;
    if (lambda != null)
    {
        return lambda.Body;
    }

    var expression = arg as Expression;
    if (expression != null)
    {
        return expression;
    }

    return Expression.Constant(arg, paramInfo.ParameterType);
}

private static IEnumerable<Expression> ToExpressionArgs(MethodInfo method, object[] args)
{
    ParameterInfo[] methodParams = method.GetParameters();
    for (int i = 0; i < args.Length; i++)
    {
        yield return ToExpressionArg(methodParams[i], args[i]);
    }
}

@kzu
Copy link
Contributor

kzu commented Mar 7, 2014

Looks good!

Want to send a PR so you get credited appropriately and this fix makes it
into the next release right away?

/kzu from mobile
On Mar 7, 2014 1:42 PM, "Rob Siklos" notifications@github.com wrote:

I took a quick peek in the code and I believe the solution can be
implemented easily. The reason for the problem is that when nullable values
are cast to object, it's the underlying value that gets boxed, rather
than the nullable instance being referenced. So we've completely lost the
fact that an arg is nullable just by looking at its type.

The solution is that in ProtectedMock.ToExpressionArg(), the call to
Expression.Constant() should also specify the type (using the other
overload for that method).

Here are my modified versions of ToExpressionArg() and ToExpressionArgs():

private static Expression ToExpressionArg(ParameterInfo paramInfo, object arg){
var lambda = arg as LambdaExpression;
if (lambda != null)
{
return lambda.Body;
}

var expression = arg as Expression;
if (expression != null)
{
    return expression;
}

return Expression.Constant(arg, paramInfo.ParameterType);}

private static IEnumerable ToExpressionArgs(MethodInfo method, object[] args){
ParameterInfo[] methodParams = method.GetParameters();
for (int i = 0; i < args.Length; i++)
{
yield return ToExpressionArg(methodParams[i], args[i]);
}}

Reply to this email directly or view it on GitHubhttps://github.com//issues/92#issuecomment-37041881
.

@RobSiklos
Copy link
Contributor Author

@kzu: Unfortunately, I don't have any git integration set up (we're a TFS-based shop) and I don't have time to get it all set up and create a PR. I don't really care about being credited, so if you or someone else would be able to integrate the change, that would be great.

Thanks!

@RobSiklos
Copy link
Contributor Author

@kzu Ok - I finally got around to doing this :) Pull request #199.

RobSiklos pushed a commit to RobSiklos/moq4 that referenced this issue Sep 15, 2015
RobSiklos pushed a commit to RobSiklos/moq4 that referenced this issue Sep 15, 2015
@RobSiklos
Copy link
Contributor Author

Pull request #200 (single commit).

RobSiklos added a commit to RobSiklos/moq4 that referenced this issue Feb 16, 2016
RobSiklos added a commit to RobSiklos/moq4 that referenced this issue Feb 16, 2016
…d methods with nullable parameters"

This reverts commit f413644.
RobSiklos added a commit to RobSiklos/moq4 that referenced this issue Feb 16, 2016
@RobSiklos
Copy link
Contributor Author

@kzu Any reason why the pull request hasn't been merged yet?

kzu pushed a commit that referenced this issue Mar 24, 2017
kzu pushed a commit that referenced this issue Mar 24, 2017
… with nullable parameters"

This reverts commit f413644.
kzu pushed a commit that referenced this issue Mar 24, 2017
@stakx
Copy link
Contributor

stakx commented Jun 3, 2017

I guess we can close this issue, now that #200 has been merged.

@stakx stakx closed this as completed Jun 3, 2017
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

3 participants