Fix VSTHRD002 completion analysis and extensibility by AArnott · Pull Request #1648 · microsoft/vs-threading

Andrew Arnott (AArnott) · 2026-08-21T16:48:49Z

Summary

recognize completed continuation parameters through nested lambdas while retaining diagnostics after reassignment
make completed-task suppression symbol-aware for direct awaits, Task.WhenAll, completion guards, nested blocks, and intervening writes
support project-defined synchronous blocking methods through vs-threading.SyncBlockingMethods.txt without offering unsafe automatic rewrites
harden the await code fix against compiler errors, parameterized waits, consumed WaitAny results, and non-GetAwaiter().GetResult() call chains
document the new configuration and completion-proof behavior

Issue outcomes

Fixes #1123
Fixes #937
Fixes #454
Fixes #344
Fixes #301

All five issues in this subset are addressed; none remain unresolved in this PR.

Validation

focused VSTHRD002 tests across net8.0, net8.0-windows, and net472
full analyzer test project: 1,479 passed, 33 skipped
full Release build: 0 warnings, 0 errors

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR extends VSTHRD002 completion analysis, configurable blocking-method detection, and safer await code fixes.

Changes:

Adds symbol-aware completion and reassignment analysis.
Supports configured synchronous blocking methods.
Hardens code fixes and updates tests and documentation.

Reviewed changes

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

Show a summary per file

File	Summary
`test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD002UseJtfRunAnalyzerTests.cs`	Adds analyzer and code-fix regression coverage.
`test/Microsoft.VisualStudio.Threading.Analyzers.Tests/MultiAnalyzerTests.cs`	Updates completed-task diagnostic expectations.
`src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs`	Adds completion and configuration analysis.
`src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs`	Moderate (2 votes): Misses completion guards expressed with an `else` branch.
`src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD002UseJtfRunCodeFixWithAwait.cs`	Critical (2 votes): Generic fallback can generate non-compiling fixes for parenthesized awaiters and `Task.WhenAll(...).Result`.
`docfx/analyzers/VSTHRD002.md`	Documents completion behavior.
`docfx/analyzers/configuration.md`	Documents custom blocking-method configuration.

Suppressed comments (12)

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:399

This only compares the symbol for the whole assignment left-hand side. For a deconstruction such as (task, other) = (Task.Run(() => 2), 0), assignment.Left is a tuple and has no symbol, so the write is missed; a prior await task then incorrectly suppresses the warning for the newly assigned task's Result. Walk the writable elements of deconstruction assignments (and handle member/other LHS forms without treating their receivers as writes).

        foreach (AssignmentExpressionSyntax assignment in node.DescendantNodes(DescendIntoChildren).OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart > afterPosition
                && assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskSymbol))
            {
                return true;

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:190

This traversal stops at any enclosing statement other than an if or a block. Consequently, an unconditional await task; before a try block is not considered when analyzing task.Result inside that block, so the new completion proof still reports a false positive for a nested block. Please continue through safe constructs such as try (while retaining the existing write/control-flow checks).

            StatementSyntax? outerStatement = containingStatement.Ancestors().OfType<StatementSyntax>().FirstOrDefault(statement => statement.Parent is BlockSyntax);
            if (outerStatement is not IfStatementSyntax and not BlockSyntax)
            {
                return false;

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:399

The DescendIntoChildren predicate skips every anonymous function when looking for intervening writes. That makes a definite synchronous write invisible, for example ((Action)(() => task = Task.Run(() => 2)))(); between await task and task.Result; the lambda is invoked immediately, but the analyzer still carries the old completion proof and suppresses VSTHRD002. Nested bodies should not be discarded without accounting for calls that execute them synchronously, or the analysis should conservatively invalidate the proof.

        static bool DescendIntoChildren(SyntaxNode node) => node is not AnonymousFunctionExpressionSyntax and not LocalFunctionStatementSyntax;

        foreach (AssignmentExpressionSyntax assignment in node.DescendantNodes(DescendIntoChildren).OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart > afterPosition
                && assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskSymbol))
            {
                return true;

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:181

For a member access inside a deferred lambda, FirstAncestorOrSelf<StatementSyntax>() is the lambda's containing declaration, so this scan only considers writes before the lambda is created. It therefore suppresses a real violation in await task; Action a = () => task.Result; task = Task.Run(...); a();: the lambda executes after the reassignment and can block on the new task, but the earlier await proof is still used. Captured task completions must be invalidated by writes after capture (or the deferred delegate must be analyzed with an appropriate capture boundary).

        StatementSyntax? containingStatement = memberAccessSyntax.FirstAncestorOrSelf<StatementSyntax>();
        if (containingStatement is null)
        {
            return false;
        }

        while (containingStatement.Parent is BlockSyntax block)
        {
            if (MayReassignTask(context, containingStatement, taskSymbol, containingStatement.SpanStart - 1, memberAccessSyntax.SpanStart))
            {
                return false;
            }

            int statementIndex = block.Statements.IndexOf(containingStatement);
            for (int i = statementIndex - 1; i >= 0; i--)
            {
                StatementSyntax statement = block.Statements[i];
                if (StatementCompletesTask(context, statement, taskSymbol))
                {
                    return true;
                }

                if (MayReassignTask(context, statement, taskSymbol))

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:305

StatementCompletesTask handles direct await statements and the special negative-guard form, but not an unconditional nested block. For { await task; } followed by task.Result, TryGetAwaitExpression rejects the block because its inner expression statement is treated as conditional, and the block is never passed to StatementDefinitelyAwaitsTask, so the result is still reported despite the documented awaited-task proof. Propagate completion through BlockSyntax as well.

        return statement is IfStatementSyntax { Else: null } ifStatement
            && ConditionProvesCompletion(context, ifStatement.Condition, taskSymbol, conditionValue: false)
            && StatementDefinitelyAwaitsTask(context, ifStatement.Statement, taskSymbol);
    }

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:149

This descriptor-independent completion suppression is also used by VSTHRD102, which passes CommonInterest.JTFSyncBlockers (including JoinableTask.Join). Consequently, if (joinableTask.IsCompleted) joinableTask.Join(); now skips VSTHRD102 even though that rule limits synchronous frames in internal members. Scope HasTaskCompleted to VSTHRD002 or make it an opt-in for callers.

    private static bool HasTaskCompleted(SyntaxNodeAnalysisContext context, MemberAccessExpressionSyntax memberAccessSyntax)
    {
        ISymbol? taskSymbol = context.SemanticModel.GetSymbolInfo(GetTaskReceiver(memberAccessSyntax), context.CancellationToken).Symbol;
        if (taskSymbol is not ILocalSymbol and not IParameterSymbol)

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:380

This proof returns true as soon as any Task.WhenAll argument is the task symbol, without checking for ref/out writes inside the same invocation. In await Task.WhenAll(task, Replace(ref task)), Replace can replace task with an incomplete task after the first argument is evaluated; a later task.Result is then still a synchronous wait but is suppressed. Ref/out mutations in the awaited expression must invalidate this proof, or the analyzer should conservatively avoid suppressing it.

            return whenAllInvocation.ArgumentList.Arguments.Any(argument => IsSameTask(context, argument.Expression, taskSymbol));

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:397

The reassignment scan compares assignment symbols directly with taskSymbol, so it misses writes through ref-local aliases. For example, after ref Task<int> alias = ref task, assigning a new task to alias inside a completion guard does not invalidate the proof, and task.Result can be incorrectly suppressed. Track ref aliases in the enclosing executable scope, including aliases declared before the guarded region.

        foreach (AssignmentExpressionSyntax assignment in node.DescendantNodes(DescendIntoChildren).OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart > afterPosition
                && assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskSymbol))

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:176

The completion search only walks statements preceding the containing statement, so it ignores an ordered await in the same statement. Consume(await task, task.Result) therefore still reports VSTHRD002 even though C# evaluates the first argument and completes task before reading Result. Account for awaits that occur earlier in the containing statement.

            int statementIndex = block.Statements.IndexOf(containingStatement);
            for (int i = statementIndex - 1; i >= 0; i--)
            {
                StatementSyntax statement = block.Statements[i];
                if (StatementCompletesTask(context, statement, taskSymbol))

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:344

TryGetAwaitExpression returns only the last unconditionally executed await. In Consume(await task, await otherTask); task.Result, it selects the otherTask await and misses the earlier await of task, producing a false positive. The analysis needs to consider all relevant ordered await candidates instead of stopping at the first match in reverse order.

        foreach (AwaitExpressionSyntax candidate in statement.DescendantNodes(DescendIntoChildren).OfType<AwaitExpressionSyntax>().Reverse())
        {
            IEnumerable<SyntaxNode> ancestorsWithinStatement = candidate.Ancestors().TakeWhile(node => node != statement);
            bool isConditionallyExecuted = ancestorsWithinStatement.Any(
                node => node is StatementSyntax or ConditionalExpressionSyntax or SwitchExpressionSyntax or ConditionalAccessExpressionSyntax

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:251

The continuation reassignment check has the same whole-LHS limitation: a deconstruction assignment to the continuation parameter is not represented by a symbol on assignment.Left. For example, (t, other) = (Task.Run(() => 6), 0); Console.WriteLine(t.Result); is treated as using the completed antecedent and the warning is suppressed, although t now refers to an incomplete task. Inspect deconstruction elements here as well.

        foreach (AssignmentExpressionSyntax assignment in continuation.DescendantNodes().OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskParameter))
            {

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:251

For a direct access in the continuation body, continuation.DescendantNodes() also descends into lambdas that are merely declared before the access. An assignment inside such a lambda is not executed when the lambda is created, but it is counted as a prior reassignment; e.g. Action reassign = () => t = Task.Run(() => 6); Console.WriteLine(t.Result); gets a warning even though t is still the completed antecedent. Skip nested function bodies for non-nested accesses (while keeping the conservative handling for accesses inside nested functions).

        foreach (AssignmentExpressionSyntax assignment in continuation.DescendantNodes().OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskParameter))
            {

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

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Suppressed comments (15)

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:421

MayReassignTask only inspects assignment expressions whose left side resolves directly to task (and ref/out invocation arguments). A ref local alias is missed: after ref Task<int> alias = ref task; alias = new TaskCompletionSource<int>().Task;, task.Result can still block even though this guard is treated as proof of completion. Track ref-local aliases (and writes through them), or conservatively invalidate the proof when such an alias is introduced.

        foreach (ArgumentSyntax argument in node.DescendantNodes(DescendIntoChildren).OfType<ArgumentSyntax>())
        {
            if (argument.SpanStart > afterPosition
                && argument.SpanStart < beforePosition
                && (argument.RefKindKeyword.IsKind(SyntaxKind.RefKeyword) || argument.RefKindKeyword.IsKind(SyntaxKind.OutKeyword))

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:317

When the completion proof comes from a compound condition, writes performed while evaluating that condition are not checked. For example, if (task.IsCompletedSuccessfully && Replace(ref task)) { } else { await task; } can reassign task to an incomplete task in Replace; the positive path then skips the await, but this code still treats the whole if as completing the task and suppresses a later task.Result. Invalidate the proof when the condition itself may reassign the task.

        return (ConditionProvesCompletion(context, ifStatement.Condition, taskSymbol, conditionValue: true)
                && !MayReassignTask(context, ifStatement.Statement, taskSymbol)
                && StatementDefinitelyAwaitsTask(context, ifStatement.Else.Statement, taskSymbol))
            || (ConditionProvesCompletion(context, ifStatement.Condition, taskSymbol, conditionValue: false)
                && StatementDefinitelyAwaitsTask(context, ifStatement.Statement, taskSymbol)

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:414

Tuple/deconstruction assignments are not recognized as writes here because GetSymbolInfo on a TupleExpressionSyntax is null. For example, after await task; (task, other) = (Task.Run(...), ...);, the scan misses the reassignment and suppresses the later task.Result diagnostic even though the new task is incomplete. Inspect the symbols in the assignment target (or use the operation write) rather than only the whole left expression.

            if (assignment.SpanStart > afterPosition
                && assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskSymbol))
            {
                return true;
            }

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:143

Parentheses are unwrapped only after the GetAwaiter/ConfigureAwait patterns are checked. Consequently (task.GetAwaiter()).GetResult() is mapped to the GetAwaiter method symbol instead of task, so even after await task this access still gets a VSTHRD002 warning. Normalize the receiver before each chain match, as the code fix already does for this syntax.

    private static ExpressionSyntax GetTaskReceiver(MemberAccessExpressionSyntax memberAccessSyntax)
    {
        ExpressionSyntax receiver = memberAccessSyntax.Expression;
        if (receiver is InvocationExpressionSyntax getAwaiterInvocation
            && getAwaiterInvocation.Expression is MemberAccessExpressionSyntax { Name.Identifier.ValueText: "GetAwaiter" } getAwaiterAccess)
        {
            receiver = getAwaiterAccess.Expression;
        }

        if (receiver is InvocationExpressionSyntax configureAwaitInvocation
            && configureAwaitInvocation.Expression is MemberAccessExpressionSyntax { Name.Identifier.ValueText: nameof(Task.ConfigureAwait) } configureAwaitAccess)
        {
            receiver = configureAwaitAccess.Expression;
        }

        return UnwrapParentheses(receiver);

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:299

Writes inside the awaited expression are excluded from the invalidation check because only positions after awaitExpression.Span.End are examined. For example, await task.ConfigureAwait(Replace(ref task)); can await the original task while Replace assigns a new, incomplete task to the variable; the following task.Result is then incorrectly suppressed. Include ref/out writes and assignments within the await expression (or conservatively invalidate on any such write before proving completion).

    private static bool StatementCompletesTask(SyntaxNodeAnalysisContext context, StatementSyntax statement, ISymbol taskSymbol)
    {
        if (TryGetAwaitExpression(statement, out AwaitExpressionSyntax? awaitExpression)
            && AwaitCompletesTask(context, awaitExpression, taskSymbol)
            && !MayReassignTask(context, statement, taskSymbol, awaitExpression.Span.End, statement.Span.End + 1))
        {
            return true;

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:358

TryGetAwaitExpression returns only the last unconditionally executed await in a statement. For example, Consume(await task, await other); task.Result; selects await other, AwaitCompletesTask returns false, and the completed task is still reported. Enumerate/select all unconditional awaits (or pass the tracked symbol into this search) so an earlier await/WhenAll proof is considered.

        foreach (AwaitExpressionSyntax candidate in statement.DescendantNodes(DescendIntoChildren).OfType<AwaitExpressionSyntax>().Reverse())
        {
            IEnumerable<SyntaxNode> ancestorsWithinStatement = candidate.Ancestors().TakeWhile(node => node != statement);
            bool isConditionallyExecuted = ancestorsWithinStatement.Any(
                node => node is StatementSyntax or ConditionalExpressionSyntax or SwitchExpressionSyntax or ConditionalAccessExpressionSyntax

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:305

StatementCompletesTask falls through for a preceding standalone BlockSyntax, even though TryGetAwaitExpression intentionally rejects awaits nested under the block's child statement. Thus a valid sequence { await task; } task.Result; is not recognized as completed. Handle a block with the existing StatementDefinitelyAwaitsTask logic before the IfStatementSyntax check.

        if (statement is not IfStatementSyntax ifStatement)
        {
            return false;
        }

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:251

The continuation-specific reassignment scan has the same alias gap: ref Task<int> alias = ref t; alias = ...; return t.Result; is not detected because the initializer is not an ArgumentSyntax and the assignment targets alias. This causes the new nested-continuation suppression to hide a real blocking wait after the continuation parameter was changed.

        foreach (AssignmentExpressionSyntax assignment in continuation.DescendantNodes().OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskParameter))
            {

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:180

The configured-method path bypasses the existing nameof guard used by CSharpCommonInterest. Consequently, nameof(waiter.Join()) can produce a VSTHRD002 diagnostic for a configured Join, even though the call is not executed. Skip configured-method reporting when the invocation is inside nameof, just as the built-in path does.

        if (configuredSyncBlockingMethods.IsEmpty
            || context.SemanticModel.GetSymbolInfo(invocationExpressionSyntax, context.CancellationToken).Symbol is not IMethodSymbol invokedMethod)
        {

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:200

Diagnostics produced for configured methods use the same message as built-in task/awaiter waits, which says “Synchronously waiting on tasks or awaiters...”. The documented configuration and the added test support arbitrary blockers such as CustomWaiter.Join, so users receive a misleading message for those diagnostics. Use a message/descriptor that also describes configured synchronous blockers, or provide a distinct configured-method message.

                ImmutableDictionary<string, string?> properties = ImmutableDictionary<string, string?>.Empty.Add("SuppressAwaitCodeFix", null);
                context.ReportDiagnostic(Diagnostic.Create(Descriptor, methodName.GetLocation(), properties));

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:253

Continuation reassignment detection also misses deconstruction writes because GetSymbolInfo on the tuple left-hand side is null. A continuation such as (t, other) = (Task.Run(...), ...); useResultLater(); can therefore suppress t.Result even though t now refers to an incomplete task. Check the symbols within the assignment target, not just the target expression itself.

        foreach (AssignmentExpressionSyntax assignment in continuation.DescendantNodes().OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskParameter))
            {
                return true;
            }

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:195

Conditional-access calls use MemberBindingExpressionSyntax, so waiter?.Join() resolves to the configured method but this switch produces methodName = null and skips the diagnostic. This leaves a valid instance-method call unreported despite the documented extensibility support. Include member-binding expressions when selecting the diagnostic name.

            SimpleNameSyntax? methodName = invocationExpressionSyntax.Expression switch
            {
                MemberAccessExpressionSyntax memberAccess => memberAccess.Name,
                SimpleNameSyntax simpleName => simpleName,
                _ => null,
            };

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD002UseJtfRunCodeFixWithAwait.cs:185

The new invocation matcher correctly narrows the wait cases, but the following member-access fallback still offers a rewrite when the blocking access is part of a larger chain. For task.Result[0] or task.Result.ToString(), replacing only task.Result with await task yields an invalid or differently bound expression (such as await task[0]) rather than (await task)[0]. Do not offer this fix for chained receivers, or rewrite the complete expression with the required parentheses.

        else if (FindInstanceWaitReceiver(parentInvocation) is object)
        {
            // This method will not return null for the provided 'target' argument
            transform = NullableHelpers.AsNonNullReturnUnchecked<ExpressionSyntax, CancellationToken, ExpressionSyntax>(FindInstanceWaitReceiver);
            target = parentInvocation!;

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD002UseJtfRunCodeFixWithAwait.cs:212

CanUseAwaitCodeFix only validates Task.Wait when the resolved containing type is Task; a custom extension such as static void Wait(this Task task) skips this branch and falls through to return true. The syntax matcher then transforms it to await task, changing the call's semantics. Return false for any Wait whose resolved method is not the framework Task.Wait.

        if (method.Name == nameof(Task.Wait) && Utils.IsTask(method.ContainingType))
        {
            return method.Parameters.IsEmpty;
        }

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD002UseJtfRunCodeFixWithAwait.cs:128

FindGetAwaiterReceiver validates only member names and call shape, not the resolved method. A valid extension such as GetResult(this TaskAwaiter awaiter, int value) can therefore be rewritten from task.GetAwaiter().GetResult(42) to await task, silently dropping the argument. Require the symbol to be the parameterless framework awaiter's instance GetResult (and the expected GetAwaiter) before offering this fix.

        ExpressionSyntax? FindGetAwaiterReceiver(ExpressionSyntax? from, CancellationToken cancellationToken = default(CancellationToken))
        {
            if (from is not InvocationExpressionSyntax
                {
                    Expression: MemberAccessExpressionSyntax
                    {
                        Name.Identifier.ValueText: nameof(TaskAwaiter.GetResult),
                    } getResultAccess
                })

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Suppressed comments (14)

Previously missed (4) — in code that hasn't changed since the last review.

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:395

Any short-circuits as soon as it sees the original task, so later arguments are not checked for writes. With await Task.WhenAll(task, Replace(ref task)); task.Result;, the first argument makes this return true while Replace can replace task with a new, incomplete task; the result access can then block without a diagnostic. Inspect all WhenAll arguments for intervening writes before accepting the completion proof.

        if (awaitedExpression is InvocationExpressionSyntax whenAllInvocation
            && context.SemanticModel.GetSymbolInfo(whenAllInvocation, context.CancellationToken).Symbol is IMethodSymbol whenAllMethod
            && whenAllMethod.Name == nameof(Task.WhenAll)
            && whenAllMethod.ContainingType.Name == nameof(Task)
            && whenAllMethod.ContainingType.BelongsToNamespace(Namespaces.SystemThreadingTasks))
        {
            return whenAllInvocation.ArgumentList.Arguments.Any(argument => IsSameTask(context, argument.Expression, taskSymbol));
        }

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:406

Skipping anonymous and local-function bodies here allows an executed closure to invalidate the proof without being detected. For example, after await task, a closure can assign task = Task.Run(...), be invoked, and then task.Result is analyzed; the declaration and call contain no visible assignment to this scan, so the earlier await incorrectly suppresses VSTHRD002. Track writes from closures that execute before the access, or conservatively invalidate the proof when that control flow cannot be established.

    private static bool MayReassignTask(SyntaxNodeAnalysisContext context, SyntaxNode node, ISymbol taskSymbol)
        => MayReassignTask(context, node, taskSymbol, node.SpanStart - 1, node.Span.End + 1);

    private static bool MayReassignTask(SyntaxNodeAnalysisContext context, SyntaxNode node, ISymbol taskSymbol, int afterPosition, int beforePosition)
    {
        static bool DescendIntoChildren(SyntaxNode node) => node is not AnonymousFunctionExpressionSyntax and not LocalFunctionStatementSyntax;

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:130

The receiver is unwrapped only after the GetAwaiter/ConfigureAwait pattern checks, so (task.GetAwaiter()).GetResult() is resolved as the GetAwaiter invocation rather than as task. Consequently, even after await task, this new completion proof still reports VSTHRD002 for the parenthesized form. Normalize parentheses before matching the receiver chain.

        ExpressionSyntax receiver = UnwrapParentheses(memberAccessSyntax.Expression);
        if (receiver is InvocationExpressionSyntax getAwaiterInvocation
            && getAwaiterInvocation.Expression is MemberAccessExpressionSyntax { Name.Identifier.ValueText: "GetAwaiter" } getAwaiterAccess)

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:224

The continuation-specific receiver extraction has the same parenthesis gap: for task.ContinueWith(t => (t.GetAwaiter()).GetResult()), the receiver is a ParenthesizedExpressionSyntax, so this code never reaches t and misses the fact that the continuation parameter is complete. The new nested-continuation suppression therefore still emits VSTHRD002 for this valid equivalent spelling; share a receiver-normalization path with the common completion analysis.

    private static ISymbol? GetTaskReceiverSymbol(SyntaxNodeAnalysisContext context, MemberAccessExpressionSyntax memberAccessSyntax)
    {
        ExpressionSyntax receiver = UnwrapParentheses(memberAccessSyntax.Expression);
        if (receiver is InvocationExpressionSyntax getAwaiterInvocation
            && getAwaiterInvocation.Expression is MemberAccessExpressionSyntax { Name.Identifier.ValueText: "GetAwaiter" } getAwaiterAccess)

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:310

These guard proofs do not reject writes in the condition itself. For example, if (!task.IsCompleted || (task = Replace()) == null) await task; task.Result; can take the false path after replacing an already-completed task, so the current task was never awaited, but ConditionProvesCompletion(..., false) returns true from the left operand and suppresses VSTHRD002. Check the condition for assignments and ref/out writes before using either the single-branch or two-branch proof below.

        if (ifStatement.Else is null)
        {
            return ConditionProvesCompletion(context, ifStatement.Condition, taskSymbol, conditionValue: false)
                && StatementDefinitelyAwaitsTask(context, ifStatement.Statement, taskSymbol);

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:385

The direct-await path also accepts a task receiver without checking writes in the await expression's arguments. For example, await task.ConfigureAwait(Replace(ref task)); task.Result; awaits the old task while Replace replaces the local during argument evaluation, but AwaitCompletesTask strips ConfigureAwait, sees task, and returns true. Check the complete awaited expression for assignments and ref/out writes before using it as a completion proof.

    private static bool AwaitCompletesTask(SyntaxNodeAnalysisContext context, AwaitExpressionSyntax awaitExpression, ISymbol taskSymbol)
    {
        ExpressionSyntax awaitedExpression = UnwrapParentheses(awaitExpression.Expression);
        if (awaitedExpression is InvocationExpressionSyntax configureAwaitInvocation
            && configureAwaitInvocation.Expression is MemberAccessExpressionSyntax { Name.Identifier.ValueText: nameof(Task.ConfigureAwait) } configureAwaitAccess)
        {
            awaitedExpression = UnwrapParentheses(configureAwaitAccess.Expression);
        }

        if (IsSameTask(context, awaitedExpression, taskSymbol))
        {
            return true;

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:411

This reassignment scan requires the entire left-hand expression to have the task symbol. In a valid deconstruction such as (task, _) = (Task.Run(...), 0), the tuple left has no symbol, so the write is ignored; after an earlier await task, a later task.Result is incorrectly treated as completed even though task now refers to a new task. Inspect the assignable elements/operations of deconstruction assignments.

        foreach (AssignmentExpressionSyntax assignment in node.DescendantNodes(DescendIntoChildren).OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart > afterPosition
                && assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskSymbol))

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:176

Completion is only searched in statements preceding the statement containing the member access. This misses C#'s left-to-right evaluation within one statement: in Consume(await task, task.Result), the await task argument completes task before the Result argument is evaluated, but this code still reports VSTHRD002. The completion proof should also inspect earlier await expressions in the current statement and honor intervening writes.

            int statementIndex = block.Statements.IndexOf(containingStatement);
            for (int i = statementIndex - 1; i >= 0; i--)
            {
                StatementSyntax statement = block.Statements[i];
                if (StatementCompletesTask(context, statement, taskSymbol))

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:358

TryGetAwaitExpression treats an await directly in any statement's condition as unconditional. For example, do { break; } while (await task); has an await that is skipped by the break, but the preceding DoStatement is accepted by StatementCompletesTask; a following task.Result is therefore incorrectly suppressed. Account for loop control flow (or conservatively reject await expressions in loop conditions/increments) before using them as completion proofs.

        foreach (AwaitExpressionSyntax candidate in statement.DescendantNodes(DescendIntoChildren).OfType<AwaitExpressionSyntax>().Reverse())
        {
            IEnumerable<SyntaxNode> ancestorsWithinStatement = candidate.Ancestors().TakeWhile(node => node != statement);
            bool isConditionallyExecuted = ancestorsWithinStatement.Any(
                node => node is StatementSyntax or ConditionalExpressionSyntax or SwitchExpressionSyntax or ConditionalAccessExpressionSyntax

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:305

A standalone nested block is treated as neither a completion statement nor an if here. Consequently, in await task; followed by a separate { ... } block and then task.Result, the scan skips the awaited statement inside that block and reports VSTHRD002 despite the task having completed. Handle BlockSyntax with the existing definite-await analysis.

        if (statement is not IfStatementSyntax ifStatement)
        {
            return false;
        }

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:251

The continuation reassignment scan also misses deconstruction writes such as (t, _) = (Task.Run(...), 0), because GetSymbolInfo(assignment.Left) is null for the tuple. A nested delegate that later reads t.Result is then suppressed even though the continuation parameter was replaced with a new, incomplete task. Handle deconstruction targets when checking for parameter reassignment.


    private static bool IsTaskReassignedInContinuation(
        SyntaxNodeAnalysisContext context,
        AnonymousFunctionExpressionSyntax continuation,
        MemberAccessExpressionSyntax memberAccess,

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:194

Conditional-access calls use MemberBindingExpressionSyntax, not MemberAccessExpressionSyntax or SimpleNameSyntax. Thus a configured invocation such as waiter?.Join() can match the configured symbol but leaves methodName null, so no VSTHRD002 diagnostic is reported for this valid call form. Include MemberBindingExpressionSyntax when selecting the diagnostic name (or use CSharpUtils.IsolateMethodName).

            SimpleNameSyntax? methodName = invocationExpressionSyntax.Expression switch
            {
                MemberAccessExpressionSyntax memberAccess => memberAccess.Name,
                SimpleNameSyntax simpleName => simpleName,
                _ => null,

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:200

The configured-method path reports directly here and bypasses CSharpCommonInterest.ShouldIgnoreContext (CSharpCommonInterest.cs:43-59), the existing VSTHRD002 filter for Xaml2CS <auto-generated> code. Consequently, configuring a custom blocker causes warnings in generated files even though built-in blockers remain suppressed there. Apply the same generated-code check before reporting the configured diagnostic.

            if (methodName is object)
            {
                ImmutableDictionary<string, string?> properties = ImmutableDictionary<string, string?>.Empty.Add("SuppressAwaitCodeFix", null);
                context.ReportDiagnostic(Diagnostic.Create(Descriptor, methodName.GetLocation(), properties));

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD002UseJtfRunCodeFixWithAwait.cs:167

The final FindParentMemberAccess fallback still accepts every member-access diagnostic after these branches. For a known awaiter obtained through a non-GetAwaiter chain such as GetCustomAwaiter().GetResult(), FindGetAwaiterReceiver fails and the fallback offers await GetCustomAwaiter(). The returned awaiter is not necessarily awaitable, so the generated fix can fail to compile; only use this fallback for recognized result properties or validate the transformed expression.

        else if (FindInstanceWaitReceiver(parentInvocation) is object)
        {
            // This method will not return null for the provided 'target' argument
            transform = NullableHelpers.AsNonNullReturnUnchecked<ExpressionSyntax, CancellationToken, ExpressionSyntax>(FindInstanceWaitReceiver);
            target = parentInvocation!;

Copilot

Pull request overview

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

Suppressed comments (13)

Previously missed (4) — in code that hasn't changed since the last review.

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:395

A Task.WhenAll argument can reassign the tracked variable through a ref/out call. Because this returns as soon as any direct argument matches task, and the caller only checks writes after the whole await expression, await Task.WhenAll(task, Replace(ref task)) can suppress VSTHRD002 even when Replace stores an incomplete task into task. Reject or analyze ref/out and assignment writes inside the WhenAll invocation before using it as a completion proof.

        if (awaitedExpression is InvocationExpressionSyntax whenAllInvocation
            && context.SemanticModel.GetSymbolInfo(whenAllInvocation, context.CancellationToken).Symbol is IMethodSymbol whenAllMethod
            && whenAllMethod.Name == nameof(Task.WhenAll)
            && whenAllMethod.ContainingType.Name == nameof(Task)
            && whenAllMethod.ContainingType.BelongsToNamespace(Namespaces.SystemThreadingTasks))
        {
            return whenAllInvocation.ArgumentList.Arguments.Any(argument => IsSameTask(context, argument.Expression, taskSymbol));
        }

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:310

This proof path does not account for writes in the guard condition itself. For example, if (!task.IsCompleted || Replace(ref task)) { await task; } can skip the body after Replace replaces task, but ConditionProvesCompletion(..., false) succeeds from !task.IsCompleted and the result after the if is then incorrectly treated as completed. Reject a MayReassignTask in the condition before accepting this one-branch proof.

            return ConditionProvesCompletion(context, ifStatement.Condition, taskSymbol, conditionValue: false)
                && StatementDefinitelyAwaitsTask(context, ifStatement.Statement, taskSymbol);

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:218

The reassignment scan compares only the symbol written by the assignment/ref argument with taskSymbol; it does not follow ref-local aliases. For example, after ref Task<int> alias = ref task, alias = Task.Run(...); inside this guard leaves task.Result incorrectly suppressed because the guard's MayReassignTask check misses the write. Track ref aliases (or conservatively treat a ref-local write as invalidating the guarded task).

            if (ifStatement.Statement.FullSpan.Contains(memberAccessSyntax.Span)
                && ConditionProvesCompletion(context, ifStatement.Condition, taskSymbol, conditionValue: true)
                && !MayReassignTask(context, ifStatement, taskSymbol, ifStatement.Condition.SpanStart - 1, memberAccessSyntax.SpanStart))
            {
                return true;

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:200

Configured-method diagnostics bypass ShouldIgnoreContext, whereas the built-in path goes through CSharpCommonInterest.InspectMemberAccess, which suppresses Xaml2CS auto-generated code. With vs-threading.SyncBlockingMethods.txt present, a configured wait in the auto-generated namespace is therefore reported despite the existing DoNotReportWarningOnCodeGeneratedByXaml2CS behavior. Apply the same generated-code check before reporting here.

            if (methodName is object)
            {
                ImmutableDictionary<string, string?> properties = ImmutableDictionary<string, string?>.Empty.Add("SuppressAwaitCodeFix", null);
                context.ReportDiagnostic(Diagnostic.Create(Descriptor, methodName.GetLocation(), properties));

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:413

Deconstruction assignments are not detected here because GetSymbolInfo(assignment.Left, ...) is null when the left side is a tuple. Thus await task; (task, other) = (Task.Run(() => 2), 0); task.Result; is treated as using the awaited task even though the variable was replaced. Assignment-target analysis needs to include deconstruction elements, not just a directly symbol-bearing left expression.

        foreach (AssignmentExpressionSyntax assignment in node.DescendantNodes(DescendIntoChildren).OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart > afterPosition
                && assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskSymbol))
            {
                return true;

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:179

Completion is only searched in statements before containingStatement, so an await that precedes the result access in the same statement is ignored. For example, Consume(await task, task.Result); still gets a VSTHRD002 warning even though C# evaluates arguments left-to-right and the await completes task before Result is read. Please account for an unconditional await within the current statement when it occurs before the member access, while still checking for writes between them.

            int statementIndex = block.Statements.IndexOf(containingStatement);
            for (int i = statementIndex - 1; i >= 0; i--)
            {
                StatementSyntax statement = block.Statements[i];
                if (StatementCompletesTask(context, statement, taskSymbol))
                {
                    return true;
                }

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:358

TryGetAwaitExpression returns the last unconditional await in a statement without considering which task it awaits. Thus Consume(await task, await otherTask); task.Result; selects await otherTask, AwaitCompletesTask returns false, and the earlier await of task is missed even though it proves completion. The helper/callers need to examine all unconditional await candidates until one completes the tracked task.

        foreach (AwaitExpressionSyntax candidate in statement.DescendantNodes(DescendIntoChildren).OfType<AwaitExpressionSyntax>().Reverse())
        {
            IEnumerable<SyntaxNode> ancestorsWithinStatement = candidate.Ancestors().TakeWhile(node => node != statement);
            bool isConditionallyExecuted = ancestorsWithinStatement.Any(
                node => node is StatementSyntax or ConditionalExpressionSyntax or SwitchExpressionSyntax or ConditionalAccessExpressionSyntax

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:245

The condition logic handles && when true and || when false, but not the safe converse where both operands independently prove completion. if (task.IsCompleted || task.IsCanceled) { task.Result; } guarantees that the task is complete on either true path, yet this code falls through and reports VSTHRD002. For a true ||, accept the proof only when both operands prove completion.

            if (conditionValue && binary.IsKind(SyntaxKind.LogicalAndExpression))
            {
                return ConditionProvesCompletion(context, binary.Left, taskSymbol, conditionValue: true)
                    || ConditionProvesCompletion(context, binary.Right, taskSymbol, conditionValue: true);

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:360

The unconditional-await filter treats awaits in loop-control expressions as unconditional because it stops before the enclosing loop statement. A for iterator (for example, for (int i = 0; i < 0; i++, await task) { }) may never execute, so the subsequent task.Result is not proven safe even though this helper returns that await and suppresses the diagnostic. Account for loop control flow (and similar do conditions that can be skipped) before treating the candidate as completion proof.

            IEnumerable<SyntaxNode> ancestorsWithinStatement = candidate.Ancestors().TakeWhile(node => node != statement);
            bool isConditionallyExecuted = ancestorsWithinStatement.Any(
                node => node is StatementSyntax or ConditionalExpressionSyntax or SwitchExpressionSyntax or ConditionalAccessExpressionSyntax
                    || (node is BinaryExpressionSyntax binary
                        && (binary.IsKind(SyntaxKind.LogicalAndExpression)

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:409

The DescendIntoChildren predicate skips anonymous and local-function bodies while looking for intervening writes. That hides writes from delegates that execute synchronously before the result, for example await task; ((Action)(() => task = new TaskCompletionSource<int>().Task))(); task.Result;. The await is accepted as the prior completion and the invoked lambda's assignment is ignored, so VSTHRD002 is incorrectly suppressed even though the new task can block. Track writes from delegates known to execute here, or conservatively invalidate the proof when a captured task may be written.

        static bool DescendIntoChildren(SyntaxNode node) => node is not AnonymousFunctionExpressionSyntax and not LocalFunctionStatementSyntax;

        foreach (AssignmentExpressionSyntax assignment in node.DescendantNodes(DescendIntoChildren).OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart > afterPosition

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:263

The continuation reassignment check has the same gap for tuple assignments: GetSymbolInfo(assignment.Left, ...) does not identify the captured parameter in (t, other) = (Task.Run(() => 2), 0). A continuation can therefore reassign t and then read t.Result without a diagnostic, contrary to the new reassignment-proof behavior. Inspect the individual assignment targets so deconstruction writes invalidate the completion proof.

        foreach (AssignmentExpressionSyntax assignment in continuation.DescendantNodes().OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskParameter))
            {
                return true;
            }

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:261

The continuation-specific reassignment scan has the same ref-local alias gap. A continuation such as t => { ref Task<int> alias = ref t; alias = Task.Run(...); return t.Result; } still matches the completed continuation parameter and suppresses the diagnostic, because neither scan recognizes the alias write as a write to t.

        foreach (AssignmentExpressionSyntax assignment in continuation.DescendantNodes().OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart < beforePosition
                && SymbolEqualityComparer.Default.Equals(context.SemanticModel.GetSymbolInfo(assignment.Left, context.CancellationToken).Symbol, taskParameter))
            {

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD002UseJtfRunCodeFixWithAwait.cs:167

The generic parent-member fallback below this changed Wait matcher still offers a fix for a direct awaiter variable call such as TaskAwaiter<int> awaiter = task.GetAwaiter(); awaiter.GetResult();. FindGetAwaiterReceiver correctly returns null because there is no GetAwaiter() invocation in the call, but the fallback selects awaiter and rewrites it to await awaiter; TaskAwaiter is an awaiter, not an awaitable, so the resulting code does not compile. Restrict that fallback to the known Result properties or otherwise validate that the selected receiver is awaitable.

        else if (FindInstanceWaitReceiver(parentInvocation) is object)
        {
            // This method will not return null for the provided 'target' argument
            transform = NullableHelpers.AsNonNullReturnUnchecked<ExpressionSyntax, CancellationToken, ExpressionSyntax>(FindInstanceWaitReceiver);
            target = parentInvocation!;

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:453

This write analysis only compares assignment targets with taskSymbol itself (plus ref/out arguments), so it misses ref-local aliases. For example, ref Task<int> alias = ref task; await task; alias = Task.Run(() => 2); _ = task.Result; writes the original task storage, but the assignment resolves to alias, so the prior await proof is incorrectly retained and VSTHRD002 is suppressed on an incomplete task. Track ref-local aliases or conservatively invalidate completion proofs when a ref alias is written, and add a regression test.

        foreach (AssignmentExpressionSyntax assignment in node.DescendantNodesAndSelf(DescendIntoChildren).OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart > afterPosition
                && assignment.SpanStart < beforePosition
                && IsAssignmentToTask(context, assignment.Left, taskSymbol))

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:398

The conditional-node check treats every await nested in a logical &&/|| expression as conditionally executed. That misses a definite completion such as if (await task && condition) { }: the left operand is always evaluated, so task.Result afterward is safe, but this path is discarded and the analyzer still reports VSTHRD002. Make the short-circuit analysis position-aware so only awaits whose evaluation can be skipped are rejected.

            IEnumerable<SyntaxNode> ancestorsWithinStatement = candidate.Ancestors().TakeWhile(node => node != statement);
            bool isConditionallyExecuted = ancestorsWithinStatement.Any(
                node => node is StatementSyntax or ConditionalExpressionSyntax or SwitchExpressionSyntax or ConditionalAccessExpressionSyntax
                    || (node is BinaryExpressionSyntax binary
                        && (binary.IsKind(SyntaxKind.LogicalAndExpression)
                            || binary.IsKind(SyntaxKind.LogicalOrExpression)
                            || binary.IsKind(SyntaxKind.CoalesceExpression))));

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:257

The completion prover handles && when the condition is true and || when it is false, but not the safe dual case. For example, if (task.IsCompleted || task.IsCanceled) { _ = task.Result; } guarantees that the task is terminal on the true branch, yet this returns false and still reports VSTHRD002. Since both operands are recognized completion properties, extend the boolean proof (and cover it with a regression test).

            if (conditionValue && binary.IsKind(SyntaxKind.LogicalAndExpression))
            {
                return ConditionProvesCompletion(context, binary.Left, taskSymbol, conditionValue: true)
                    || ConditionProvesCompletion(context, binary.Right, taskSymbol, conditionValue: true);

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:396

The conditional-execution check does not recognize a WhenClauseSyntax. An await task in a switch when guard is therefore treated as a definite await for the entire switch statement, even though the default or another case can skip that guard; a later task.Result is then incorrectly suppressed. Include pattern/when guards and other conditional clauses in this control-flow test.

            IEnumerable<SyntaxNode> ancestorsWithinStatement = candidate.Ancestors().TakeWhile(node => node != statement);
            bool isConditionallyExecuted = ancestorsWithinStatement.Any(
                node => node is StatementSyntax or ConditionalExpressionSyntax or SwitchExpressionSyntax or ConditionalAccessExpressionSyntax
                    || (node is BinaryExpressionSyntax binary
                        && (binary.IsKind(SyntaxKind.LogicalAndExpression)

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:479

NestedFunctionMayReassignTask only searches functions nested inside the function containing the access. Thus writes in an enclosing method are ignored when a captured local is read inside a local/anonymous function: an async void local can await task, while its caller reassigns task after invoking it, and the subsequent task.Result uses the new incomplete task even though this proof suppresses the warning. Include enclosing-scope writes or avoid this suppression for captured locals.

    private static bool NestedFunctionMayReassignTask(SyntaxNodeAnalysisContext context, SyntaxNode node, ISymbol taskSymbol)
    {
        SyntaxNode? containingFunction = node.Ancestors().FirstOrDefault(
            ancestor => ancestor is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax or BaseMethodDeclarationSyntax);
        return containingFunction?.DescendantNodes()
            .Where(descendant => descendant is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax)
            .Any(nestedFunction => MayReassignTask(context, nestedFunction, taskSymbol)) is true;

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:201

This configured-method path does not exclude nameof expressions. In nameof(waiter.Join()), Roslyn still visits the inner waiter.Join() invocation, so its symbol matches the configured method and VSTHRD002 is reported even though the call is not executed. Skip invocations with a nameof invocation ancestor before reporting (the normal member-access path already applies a nameof check).

            if (methodName is object && !CSharpCommonInterest.ShouldIgnoreContext(context))
            {
                ImmutableDictionary<string, string?> properties = ImmutableDictionary<string, string?>.Empty.Add("SuppressAwaitCodeFix", null);
                context.ReportDiagnostic(Diagnostic.Create(Descriptor, methodName.GetLocation(), properties));

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:262

The continuation reassignment scan has the same direct-symbol limitation: IsAssignmentToParameter does not recognize a ref-local alias of the continuation parameter. A continuation can declare ref Task<int> alias = ref t, assign through alias, and then read t.Result; the antecedent proof is stale, but this path returns without reporting. Track or conservatively reject ref aliases here as well.

        foreach (AssignmentExpressionSyntax assignment in continuation.DescendantNodes().OfType<AssignmentExpressionSyntax>())
        {
            if (assignment.SpanStart < beforePosition
                && IsAssignmentToParameter(context, assignment.Left, taskParameter))
            {

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Suppressed comments (7)

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:44

The completion proof is unsound for a captured task used inside a nested async void lambda/local function. GetSymbolAndRefAliases chooses the nearest function scope, and the later block walk stops at that boundary, so code such as async void L() { await task; _ = task.Result; } can suppress VSTHRD002 even when the caller invokes L() and then reassigns task before the awaited task completes; the await captured the old task but Result reads the new one. Account for writes in enclosing scopes for captured symbols, or disable this proof when the task is captured.

        SyntaxNode searchRoot = node.AncestorsAndSelf().FirstOrDefault(
            ancestor => ancestor is AnonymousFunctionExpressionSyntax
                or LocalFunctionStatementSyntax
                or BaseMethodDeclarationSyntax
                or AccessorDeclarationSyntax) ?? node;

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:65

The alias set is built only in the initializedFrom -> local direction. When the receiver is a ref local, this misses the original storage symbol: ref Task alias = ref task; await alias; task = Task.Run(...); _ = alias.Result; starts with alias, so the later assignment to task is not seen and the await proof incorrectly suppresses the warning even though alias now reads the reassigned task. Build the ref-alias closure bidirectionally (for both declarations and = ref assignments), while conservatively handling rebinding.

                ISymbol? initializedFrom = context.SemanticModel.GetSymbolInfo(UnwrapParentheses(initializer), context.CancellationToken).Symbol;
                if (initializedFrom is object && symbols.Contains(initializedFrom) && symbols.Add(local))
                {
                    addedAlias = true;
                }

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:596

IsSameTask compares only the single taskSymbol, so completion proofs do not work through the ref aliases that GetSymbolAndRefAliases now collects. For example, ref Task<int> alias = ref task; if (alias.IsCompleted) _ = task.Result; still reports VSTHRD002 (and await alias; task.Result is likewise missed), although both names refer to the same task storage. Use the alias set when matching guarded/awaited receivers.

    private static bool IsAssignmentToTask(SyntaxNodeAnalysisContext context, ExpressionSyntax expression, IImmutableSet<ISymbol> taskSymbols)
    {
        expression = UnwrapParentheses(expression);
        if (expression is TupleExpressionSyntax tuple)

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:207

HasTaskCompleted is used by every CSharpCommonInterest.InspectMemberAccess caller, including VSTHRD102 for JoinableTask.Join, but this guard only checks that the receiver is a local/parameter and never verifies it is a Task/ValueTask. Consequently if (jt.IsCompleted) { jt.Join(); } (where jt is a JoinableTask) is treated as completed and the VSTHRD102 diagnostic is suppressed, even though JoinableTask.IsCompleted is not the Task completion proof this analyzer is implementing. Restrict this logic to Task receivers or to VSTHRD002.

    private static bool HasTaskCompleted(SyntaxNodeAnalysisContext context, MemberAccessExpressionSyntax memberAccessSyntax)
    {
        ISymbol? taskSymbol = context.SemanticModel.GetSymbolInfo(GetTaskReceiver(memberAccessSyntax), context.CancellationToken).Symbol;
        if (taskSymbol is not ILocalSymbol and not IParameterSymbol)
        {
            return false;

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:262

For a direct access in the continuation, beforePosition is the access position, so writes inside a local function declared later are ignored. Local functions are in scope before their declaration, so Replace(); _ = t.Result; void Replace() => t = Task.Run(...); can reassign t before the result is read. This early return bypasses the common nested-function check and suppresses a real VSTHRD002 diagnostic; account for writes reachable through nested/local functions before suppressing.

        IParameterSymbol taskParameter)
    {
        bool accessIsNested = memberAccess.Ancestors().TakeWhile(node => node != continuation).Any(node => node is AnonymousFunctionExpressionSyntax or LocalFunctionStatementSyntax);
        int beforePosition = accessIsNested ? continuation.Span.End + 1 : memberAccess.SpanStart;
        ImmutableHashSet<ISymbol> taskSymbols = CSharpCommonInterest.GetSymbolAndRefAliases(context, continuation, taskParameter);

        foreach (AssignmentExpressionSyntax assignment in continuation.DescendantNodes().OfType<AssignmentExpressionSyntax>())
        {

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:153

The continuation fast path still requires the receiver symbol to equal the parameter symbol, so a ref-local alias inside the continuation is treated as unrelated and falls through to a warning. For example, task.ContinueWith(t => { ref Task<int> alias = ref t; _ = alias.Result; }) reports even though the alias refers to the completed continuation parameter. Match the receiver against the parameter's ref-alias set before invoking the general analyzer.

            ParameterSyntax? firstParameter = GetFirstParameter(anonymousFunctionSyntax);
            if (firstParameter is object
                && context.SemanticModel.GetDeclaredSymbol(firstParameter, context.CancellationToken) is IParameterSymbol completedTask
                && SymbolEqualityComparer.Default.Equals(GetTaskReceiverSymbol(context, memberAccessSyntax), completedTask)
                && !IsTaskReassignedInContinuation(context, anonymousFunctionSyntax, memberAccessSyntax, completedTask))

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD002UseJtfRunCodeFixWithAwait.cs:135

This matcher checks only the GetAwaiter member name, not its argument list. A valid custom GetAwaiter(int) can return a TaskAwaiter, causing VSTHRD002 to report value.GetAwaiter(1).GetResult(); the code fix then rewrites it to await value, which does not compile because the await pattern requires a parameterless GetAwaiter(). Require the matched GetAwaiter invocation to have zero arguments (and otherwise do not offer this fix).

            var getAwaiterAccess = (getAwaiterInvocationExpression as InvocationExpressionSyntax)?.Expression as MemberAccessExpressionSyntax;
            return getAwaiterAccess?.Name.Identifier.ValueText == "GetAwaiter" ? getAwaiterAccess.Expression : null;

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Suppressed comments (5)

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:63

The alias set is used for write invalidation, but completion proofs still compare conditions to the single taskSymbol (and the set itself only grows from a known source to ref locals). Thus ref Task<int> alias = ref task; if (alias.IsCompleted) { _ = task.Result; } is still reported even though both names refer to the same storage; the reverse direction (if (task.IsCompleted) { _ = alias.Result; }) has the same gap. Make the ref-alias set an equivalence closure and use it in the completion/await comparisons as well.

                ISymbol? initializedFrom = context.SemanticModel.GetSymbolInfo(UnwrapParentheses(initializer), context.CancellationToken).Symbol;
                if (initializedFrom is object
                    && (symbols.Contains(initializedFrom) || symbols.Contains(local))

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:433

This helper does not recurse into an if statement when it is asked whether a guard body definitely awaits the task; it only handles a direct await or a block's direct statements. Therefore a valid proof such as if (!task.IsCompleted) { if (condition) await task; else await task; } still reports task.Result afterward, even though every path through the negative branch awaits the task. Recurse through nested conditional statements (and their branches) when determining definite completion.

            return ConditionProvesCompletion(context, ifStatement.Condition, taskSymbols, conditionValue: false)
                && StatementDefinitelyAwaitsTask(context, ifStatement.Statement, taskSymbols);
        }

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/CSharpCommonInterest.cs:205

The completion proof treats any local/parameter receiver as a completed task after peeling a GetAwaiter call, without verifying that the receiver is Task/ValueTask. A custom awaitable can return TaskAwaiter from GetAwaiter; after await wrapper, a later wrapper.GetAwaiter().GetResult() may obtain a fresh, incomplete operation, but this path is suppressed as completed. Restrict this proof to the supported task types (and validate the corresponding awaiter/configuration methods semantically).

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD002UseJtfRunAnalyzer.cs:258

For a direct access, beforePosition stops at the Result/Wait expression, so writes inside a local function declared later are ignored. A continuation can invoke that local function before reading the result, e.g. task.ContinueWith(t => { Reassign(); _ = t.Result; void Reassign() { t = Task.Run(() => 6); } });; the parameter is then no longer the completed antecedent, but this path suppresses the diagnostic. Account for callable nested-function writes with control-flow (or conservatively treat them as reassignment) instead of relying only on source order.

        SyntaxNodeAnalysisContext context,
        AnonymousFunctionExpressionSyntax continuation,

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD002UseJtfRunCodeFixWithAwait.cs:135

FindGetAwaiterReceiver assumes every X.GetAwaiter(...).GetResult() chain can be rewritten by awaiting X. For a qualified static or unrelated instance factory such as AwaiterFactory.GetAwaiter(task).GetResult(), this returns AwaiterFactory and produces await AwaiterFactory, which does not compile. The matcher needs semantic validation that the call is the direct/reduced-extension awaiter form, or it should decline the fix for other call shapes.

            var getAwaiterInvocation = getAwaiterInvocationExpression as InvocationExpressionSyntax;
            var getAwaiterAccess = getAwaiterInvocation?.Expression as MemberAccessExpressionSyntax;

Fix VSTHRD002 completion analysis and extensibility

b5d41ba

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI lite review requested due to automatic review settings August 21, 2026 16:48

Copilot started reviewing on behalf of Andrew Arnott (AArnott) August 21, 2026 16:50

View session