Fix VSTHRD003 to allow for awaiting more Task properties #1252
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When integrating into the VS repo, recent enhancements to VSTHRD003 made insertion of the new analyzers problematic. In particular, two patterns were quite common and arguably should be allowed:
JoinableTask.Task
property on the result of aJoinableTaskFactory.RunAsync
method call. In general, if a method returns an object, awaiting one of that object's properties might not be fair game, but it seems more likely than not. So in this change, that is now allowed.TaskCompletionSource<T>.Task
property on a TCS that was created in that same method. Test methods often do this. Awaiting a TCS.Task property is never guaranteed to be safe as far as analyzers can tell (though the 3rd threading rule can make it safe), and when common patterns appear that are typically safe, we should allow it.As a part of this change, I updated a couple utility functions, including in at least one way that wasn't directly relevant to these changes but seemed like a good change considering the nature of the analyzer bugs that keep coming in (e.g. recognizing LocalFunctionDeclarationSyntax in more places).
I also made the diagnostic's location focus on the awaited object itself rather than including the
.ConfigurueAwait
that might follow it.