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

Check ReturnsAsync method for null arguments. #909

Closed
voroninp opened this issue Aug 26, 2019 · 7 comments · Fixed by #915
Closed

Check ReturnsAsync method for null arguments. #909

voroninp opened this issue Aug 26, 2019 · 7 comments · Fixed by #915
Assignees
Milestone

Comments

@voroninp
Copy link

I make same mistake again and again.

If you call ReturnsAsync(null) compiler selects the overload which accepts a function, not a value. As a result test fails with NullReferenceException which is very confusing.

It would be nice if method checked that function is not null and threw ArgumentException with the message giving a hint to the reason of this oddity.

@stakx
Copy link
Contributor

stakx commented Aug 26, 2019

@voroninp, thanks for mentioning this. IIRC, we already have something in place to deal with this problem in the regular Returns, I guess we forgot to do the same in ReturnsAsync. I'll take a look later today.

@stakx stakx added this to the 4.13.0 milestone Aug 26, 2019
@stakx stakx self-assigned this Aug 26, 2019
@stakx
Copy link
Contributor

stakx commented Aug 26, 2019

I should add that the fix will probably be that it won't throw an exception; instead, it will internally redirect the null argument to the ReturnsAsync overload that you probably intended to call.

@voroninp
Copy link
Author

@stakx Yes, even better, thanks.

@stakx
Copy link
Contributor

stakx commented Aug 26, 2019

If you call ReturnsAsync(null) compiler selects the overload which accepts a function, not a value

@voroninp: I cannot actually reproduce this. Here's my attempt, but I get a compile-time error:

public interface IX
{
    Task<string> GetStringAsync();
}

var mock = new Mock<IX>();
mock.Setup(x => x.GetStringAsync()).ReturnsAsync(null);
//                                  ^^^^^^^^^^^^
//                                     CS0121

CS0121: The call is ambiguous between the following methods or properties:

  • ReturnsExtensions.ReturnsAsync<TMock, TResult>(IReturns<TMock, Task<TResult>>, TResult)
  • ReturnsExtensions.ReturnsAsync<TMock, TResult>(IReturns<TMock, Task<TResult>>, Func<TResult>)

Can you post a very short code example like the above that demonstrates the incorrect overload selection?

@voroninp
Copy link
Author

@stakx Make return type to be object, not string.

@voroninp
Copy link
Author

@stakx When prototyping and testing I usually start with object =)

@stakx
Copy link
Contributor

stakx commented Aug 26, 2019

When prototyping and testing I usually start with object =)

Right you are, perhaps I should, too. 😆

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

Successfully merging a pull request may close this issue.

2 participants