await Task when inside using block

[jnyrup]

IMPORTANT

• If the PR touches the public API, the changes have been approved in a separate issue with the "api-approved" label.
• The code complies with the Coding Guidelines for C#.
• The changes are covered by unit tests which follow the Arrange-Act-Assert syntax and the naming conventions such as is used in these tests.
• If the PR adds a feature or fixes a bug, please update the release notes with a functional description that explains what the change means to consumers of this library, which are published on the website.
• If the PR changes the public API the changes needs to be included by running AcceptApiChanges.ps1 or AcceptApiChanges.sh.
• If the PR affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • Please also run ./build.sh --target spellcheck or .\build.ps1 --target spellcheck before pushing and check the good outcome

Some analyzer I played around with some time ago pointed out that we weren't awaiting these Tasks.

[coveralls]

Pull Request Test Coverage Report for Build 4535199538

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 96.273%

---

Totals
Change from base Build 4525662800:	0.0%
Covered Lines:	8762
Relevant Lines:	8990

---

💛 - Coveralls

[lg2de]

I think the analyzers you mentioned created false positives.
In this casees returning the task and awaiting it creates same result.

[jnyrup]

In this casees returning the task and awaiting it creates same result.

Not sure what you mean by "same result" here.

I guess we can agree that in general awaiting a task or not within a using statement can make a difference.

async Task CompleteWithinAsync()
{
    Console.WriteLine("starting");
    await Task.Delay(1000);
    Console.WriteLine("done");
}

sealed class AssertionScope : IDisposable
{
    public void Dispose()
    {
        Console.WriteLine("disposing");
    }
}

var returning = async () =>
{
    using var _ = new AssertionScope();
    await CompleteWithinAsync();
};

await returning();

starting
disposing
done

var awaiting = async () =>
{
    using var _ = new AssertionScope();
    await CompleteWithinAsync();
};

await awaiting();

starting
done
disposing

I agree that it doesn't seem necessary here to extend the lifetime of the constructed AssertionScope to include the async part of CompleteWithinAsync, but it shouldn't harm and it brings the code closer to how an end-user would use an AssertionScope.

[dennisdoomen]

I think the analyzers you mentioned created false positives.
In this casees returning the task and awaiting it creates same result.

You're right. It doesn't make a difference. I suspect that technically adding async makes it even slower because of the state machine it generates.

[lg2de]

When looking on the code last night on my phone I did not realize the AssertionScope...
Yes, in this case there may be a difference.

[jnyrup]

I think the analyzers you mentioned created false positives.
In this casees returning the task and awaiting it creates same result.

You're right. It doesn't make a difference. I suspect that technically adding async makes it even slower because of the state machine it generates.

@dennisdoomen
Did you see that there's a using var _ = new AssertionScope() right before returning a Task?

[dennisdoomen]

Did you see that there's a using var _ = new AssertionScope() right before returning a Task?

Damnit, I did, but didn't realize the subtle change in behavior.

Was it https://www.meziantou.net/enforcing-asynchronous-code-good-practices-using-a-roslyn-analyzer.htm#ma0100-await-tasks-b?

[jnyrup]

I just had a note lying around, so not sure, but I think it might have been Microsoft.CodeAnalysis.NetAnalyzers which is the successor to Microsoft.CodeAnalysis.FxCopAnalyzers

[jnyrup]

Found the analyzer: https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1229.md