[Andrew Arnott (AArnott)]

VSTHRD103 currently offers await Task; for static Task.WaitAll calls, producing invalid code. This change recognizes the safely awaitable overload and rewrites it to await Task.WhenAll(...).

Cancellation-token and timeout overloads continue to report the synchronous wait, but no automated fix is offered because replacing them with WhenAll would not preserve their semantics. Regression tests cover both behaviors.

Fixes #951

[Copilot]

Pull request overview

This PR corrects the VSTHRD103 code fix behavior for static Task.WaitAll(...) calls so that the safely awaitable overload is rewritten to await Task.WhenAll(...), while overloads with cancellation/timeout continue to report but do not offer an automated fix (to avoid semantic changes).

Changes:

• Adds a Task.WaitAll(Task[])-specific path to rewrite the call into await Task.WhenAll(...).
• Prevents offering a code fix for non-preservable Task.WaitAll overloads (e.g., cancellation token).
• Adds regression tests for both the awaitable overload fix and the “no fix offered” behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD103UseAsyncOptionCodeFix.cs	Adds logic to detect awaitable Task.WaitAll(Task[]) and rewrite to await Task.WhenAll(...).
test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD103UseAsyncOptionAnalyzerTests.cs	Adds regression tests covering WaitAll(Task[]) code fix and WaitAll(..., CancellationToken) no-fix behavior.

Suppressed comments (2)

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD103UseAsyncOptionCodeFix.cs:85

• blockingMethod is derived from GetSymbolInfo(memberAccessExpression), which often returns a method group (Symbol == null) for overloaded methods like Task.WaitAll. When that happens, this branch won’t run and the code fix can fall back to the GetAwaiter lookup on Task, reintroducing the invalid await Task; fix for non-awaitable WaitAll overloads (e.g., CancellationToken/timeout). Resolve the invoked overload via the containing InvocationExpressionSyntax instead.

This issue also appears on line 158 of the same file.

            var memberAccessExpression = blockingIdentifier?.Parent as MemberAccessExpressionSyntax;
            IMethodSymbol? blockingMethod = memberAccessExpression is object
                ? semanticModel.GetSymbolInfo(memberAccessExpression, context.CancellationToken).Symbol as IMethodSymbol
                : null;

            // Check whether this code was already calling the awaiter (in a synchronous fashion).
            asyncAlternativeExists |= memberAccessExpression?.Expression is InvocationExpressionSyntax invoke && invoke.Expression is MemberAccessExpressionSyntax parentMemberAccess && parentMemberAccess.Name.Identifier.Text == nameof(Task.GetAwaiter);

            if (string.IsNullOrEmpty(diagnostic.Properties[VSTHRD103UseAsyncOptionAnalyzer.AsyncMethodKeyName]) && blockingMethod?.IsStatic is true)
            {
                asyncAlternativeExists = IsAwaitableTaskWaitAll(blockingMethod);
            }
            else if (!asyncAlternativeExists)
            {
                // If we fail to recognize the container, assume it exists since the analyzer thought it would.
                ITypeSymbol? container = memberAccessExpression is object ? semanticModel.GetTypeInfo(memberAccessExpression.Expression, context.CancellationToken).ConvertedType : null;
                asyncAlternativeExists = container is null || semanticModel?.LookupSymbols(diagnostic.Location.SourceSpan.Start, name: asyncMethodName, container: container, includeReducedExtensionMethods: true).Any() is true;
            }

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD103UseAsyncOptionCodeFix.cs:162

• isAwaitableTaskWaitAll is computed using GetSymbolInfo(syncMethodName), which can return null for overloaded method groups. For reliability (and to avoid falling back to the generic “await the receiver” path), bind the symbol from the containing InvocationExpressionSyntax so overload resolution is applied.

            SemanticModel? semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
            AnonymousFunctionExpressionSyntax? originalAnonymousMethodContainerIfApplicable = syncMethodName.FirstAncestorOrSelf<AnonymousFunctionExpressionSyntax>();
            MethodDeclarationSyntax originalMethodDeclaration = syncMethodName.FirstAncestorOrSelf<MethodDeclarationSyntax>() ?? throw new InvalidOperationException("Unable to find containing method.");
            bool isAwaitableTaskWaitAll = semanticModel?.GetSymbolInfo(syncMethodName, cancellationToken).Symbol is IMethodSymbol methodSymbol && IsAwaitableTaskWaitAll(methodSymbol);

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD103UseAsyncOptionAnalyzerTests.cs:265

• The PR description calls out that timeout overloads of Task.WaitAll should continue to report but offer no automated fix. There’s a regression test for the cancellation-token overload, but none covering timeout overloads (e.g. WaitAll(tasks, 0) / WaitAll(tasks, TimeSpan.Zero)), so this behavior isn’t currently locked in.

    [Fact]
    public async Task TaskWaitAllWithCancellationTokenOffersNoFix()
    {

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD103UseAsyncOptionCodeFix.cs:72

• semanticModel is declared nullable, but it’s dereferenced when calling GetSymbolInfo, which can lead to a possible null dereference (and may also introduce nullable warnings if warnings are treated as errors). Guard against semanticModel being null before calling GetSymbolInfo.

            var blockingInvocation = memberAccessExpression?.Parent as InvocationExpressionSyntax;
            IMethodSymbol? blockingMethod = blockingInvocation is object
                ? semanticModel.GetSymbolInfo(blockingInvocation, context.CancellationToken).Symbol as IMethodSymbol
                : null;

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD103UseAsyncOptionCodeFix.cs:104

• IsAwaitableTaskWaitAll matches symbols by simple name/namespace strings and only checks the parameter element type’s name. This can misidentify user-defined types (e.g., a custom System.Threading.Tasks.Task in the compilation) and offer a WhenAll rewrite even when WhenAll doesn’t exist on the resolved type, producing uncompilable fixes. Prefer matching the fully-qualified containing type and parameter element type to the BCL System.Threading.Tasks.Task.

    private static bool IsAwaitableTaskWaitAll(IMethodSymbol method)
    {
        return method.Name == nameof(Task.WaitAll)
            && method.ContainingType.Name == nameof(Task)
            && method.ContainingNamespace.ToDisplayString() == typeof(Task).Namespace

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD103UseAsyncOptionCodeFix.cs:72

• blockingInvocation is only derived from memberAccessExpression?.Parent, so for unqualified calls like using static System.Threading.Tasks.Task; WaitAll(tasks, CancellationToken.None); the method symbol is never resolved. That falls back to container is null => asyncAlternativeExists = true, which can incorrectly offer a code fix for non-awaitable overloads (and may later throw when the fix tries to find a MemberAccessExpressionSyntax). Consider resolving the invocation via FirstAncestorOrSelf<InvocationExpressionSyntax>() so the IsAwaitableTaskWaitAll gate works for both qualified and unqualified calls.

            var memberAccessExpression = blockingIdentifier?.Parent as MemberAccessExpressionSyntax;
            var blockingInvocation = memberAccessExpression?.Parent as InvocationExpressionSyntax;
            IMethodSymbol? blockingMethod = semanticModel is object && blockingInvocation is object
                ? semanticModel.GetSymbolInfo(blockingInvocation, context.CancellationToken).Symbol as IMethodSymbol
                : null;

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.