[GH-134] - detecting async Received.InOrder callbacks

[tpodolak]

Closes #134

[coveralls]

Pull Request Test Coverage Report for Build 381

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.04%) to 95.589%

---

Totals
Change from base Build 378:	0.04%
Covered Lines:	2254
Relevant Lines:	2358

---

💛 - Coveralls

[tpodolak]

@dtchepak @zvirja this will detect async callback in following usages

Received.InOrder(async () => { // rest of the code })
Received.InOrder(async delegate { // rest of the code })

should I also make an attempt to detect async void callbacks like

Received.InOrder(Calls);
  
private async void Calls()
{
    await Task.CompletedTask;
}

?

[dtchepak]

Maybe hold off for now. I haven't done much with async/await so I'm not sure, but maybe people will want to try asserting calls on a substitute that is returned from another async call:

private async void Calls() {
  var anotherSub = await sub.GetThingoe();
  anotherSub.SomeCall();
}

AFAICT the problem for NSubstitute is mainly when using Received.InOrder(async () => ...); it seems ok without the async which I think is what we need to avoid. Keeping the option to extract this to a separate method might be a good workaround if people really need this. WDYT @zvirja ?

But I'm happy to be corrected on this as I only have basic experience with async/await.

[zvirja]

@dtchepak I like you idea of keeping Received.InOrder(Calls); for now in case clients have weird reason for that. This way we could also collect the feedback and see whether it's the case at all.

Also it should help keep code simpler and less chance of false-positive :)

[tpodolak]

Thanks for your feedback guys. I am leaving this as it is

[dtchepak]

I think we should add a description of why this is a problem. Maybe just link to the issue?

A violation of this rule occurs when async callback is used in Received.InOrder method. Received.InOrder is used to specify calls which should be received by one or more substitutes. Running/awaiting these calls is not required for this, and can cause some problems as described in issue #604.

A violation of this rule occurs when async callback is used in `Received.InOrder` method.  `Received.InOrder` is used to specify calls which should be received by one or more substitutes. Running/awaiting these calls is not required for this, and can cause some problems as described in issue [#604](https://github.com/nsubstitute/NSubstitute/issues/604).

[tpodolak]

Changed as suggested

[dtchepak]

Maybe hold off for now. I haven't done much with async/await so I'm not sure, but maybe people will want to try asserting calls on a substitute that is returned from another async call:

private async void Calls() {
  var anotherSub = await sub.GetThingoe();
  anotherSub.SomeCall();
}

AFAICT the problem for NSubstitute is mainly when using Received.InOrder(async () => ...); it seems ok without the async which I think is what we need to avoid. Keeping the option to extract this to a separate method might be a good workaround if people really need this. WDYT @zvirja ?

But I'm happy to be corrected on this as I only have basic experience with async/await.