Repository navigation
VSTHRD100 (avoid async void) doesn't work for local functions #1074
Description
Bug description
VSTHRD100 fires for async void methods, but not for local functions.
Repro steps
class C
{
async void M() // VSTHRD100
{
async void L() {} // no diagnostic
}
}Expected behavior
VSTHRD100 should fire on local function L in the above example.
Actual behavior
VSTHRD100 does not fire on local function L in the above example.
- Version used: 17.3.22-alpha
- Application (if applicable): dotnet/project-system
Activity
drewnoakes commentedon Jul 25, 2022
This library is using a relatively old version of
Microsoft.CodeAnalysis.Common(2.10.0) which doesn't appear to support local functions.Bumping to the latest version (4.2.0) requires increasing the target framework from
netstandard1.3tonetstandard2.0.Any concerns with that?
drewnoakes commentedon Jul 25, 2022
This diagnostic should also fire for
async voidlambdas.AArnott commentedon Jul 25, 2022
I have no concern about bumping the target framework. But jumping to 4.2.0 in the package references means that the vs-threading analyzers won't support older VS versions, correct? Do we have a table mapping compiler versions to VS versions so we know what we're leaving behind?
drewnoakes commentedon Jul 25, 2022
I ended up not needing to bump those levels in the PR, but forgot to update this issue.
AArnott commentedon Jul 26, 2022
#1075 regressed VSTHRD100 because now it fires on lambdas, which is the purview of VSTHRD101. As a result, we had an insertion blockage due to new warnings that were all redundant with VSTHRD101 anyway.
This issue was about identifying local functions, but the PR to address that added lambda detection as well, which I should have noticed since that was in the PR title and in fact was the only thing the new tests covered rather than covering the local function scenario.
AArnott commentedon Jul 26, 2022
Drew Noakes (@drewnoakes) would you like to take another try at this issue, including rolling back the lambda detection? This is blocking vs-threading insertions so we need to address this right away. If you're not available I'll give it a quick try but just for expediency, we might roll back the original PR till we can fix this issue in a better way.
drewnoakes commentedon Jul 28, 2022
Seems like #1076 should cover it.
AArnott commentedon Jul 28, 2022
Thank you!