Roslyn-analyzers: List<T>.ForEach warning if given an async action

[benmccallum]

Describe the problem you are trying to solve

We recently had a case of the old "an async thread hadn't completed before the request had" in an ASP.NET Core app and it turned out to be someone in our team using List<T>'s ForEach function but passing it an Action<T> that was an async lambda.

c# new List<int>() { 1, 2, 3, 4, 5 }.ForEach(async x => x = await SomethingAsync());

There may be other apis/cases too, but this one comes to mind immediately since it's the one we had.

I believe the async-ness is just ignored as the ForEach implementation would never await that, so in my case, if the ASP.NET request completed but these tasks were just still hanging around, it fails.

I'm just wondering if we could warn against that, or even error, but there could be a valid case for this I guess? Not too sure.

Describe suggestions on how to achieve the rule

Something along the lines of:
"If a method that doesn't return a Task/ValueTask, but is passed an async delegate as an Action argument, warn"

Not sure here, just a suggestion, perhaps that wouldn't work and would need to be more specific to cases like the List's ForEach.

Additional context

N/A.

[sharwell]

📝 This issue is a request to port VSTHRD101 from vs-threading to roslyn-analyzers.

[sharwell]

I am heavily in favor of this rule. The benefits offered by using async void never outweigh the long-term maintenance costs it introduces, including cases like event handlers.

See also https://github.com/microsoft/vs-threading/issues/469 and https://github.com/microsoft/vs-threading/issues/447

[benmccallum]

FYI @anthony-keller and @kjayalath

[Evangelink]

At some point we were closing issues here to port them to the vs-threading repo and now it seems we are moving back some rules from the other repo. What's the idea? Shall we move all "generic" purpose rules from vs-threading back to this repo?

[benmccallum]

We've added the vs-threading analyzer package to our mono repo so I personally don't have any immediate need for this anymore and can close this if so desired. That being said, I do think something could be done to make vs-threading more visible as I had no idea it existed and it's pretty much essential for any modern project I would think. How is a bigger discussion that probably doesn't belong here though.