Skip to content

Add analyzer for redundant async state machines - #1666

Open
Andrew Arnott (AArnott) wants to merge 6 commits into
mainfrom
aarnott-resolve-analyzer-518
Open

Add analyzer for redundant async state machines#1666
Andrew Arnott (AArnott) wants to merge 6 commits into
mainfrom
aarnott-resolve-analyzer-518

Conversation

@AArnott

Copy link
Copy Markdown
Member

Async methods that only return one terminal awaited Task incur a state-machine cost that can be avoided. This adds informational guideline VSTHRD202 to identify conservative cases where the exact task can be returned directly.

The analyzer excludes transformations involving multiple or non-terminal awaits, asynchronous disposal/enumeration, different task types, and other cases that would change required lifetime semantics. It offers two ordered fixes: a minimal removal of async/await, and a try/catch variant that wraps synchronous exceptions when Task.FromException is available.

The diagnostic documentation describes the observable trade-offs, including changed debugging call stacks, exception and cancellation behavior, task identity, and the loss of compiler warning CS4014 after removing async.

Closes #518

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 27, 2026 23:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new C# analyzer + code fix pair (VSTHRD202) that flags conservative cases of redundant async state machines where a method’s sole terminal await can be replaced by directly returning the underlying Task/Task<T>, along with documentation and tests to validate supported and excluded patterns.

Changes:

  • Introduces VSTHRD202RemoveUnnecessaryAsyncAnalyzer to detect single terminal-await methods returning the exact same Task/Task<T> (with optional .ConfigureAwait(bool) unwrapping).
  • Adds VSTHRD202RemoveUnnecessaryAsyncCodeFix offering a minimal fix and an exception-wrapping fix when Task.FromException is available.
  • Adds docfx documentation/TOC/index entries, localized strings, and analyzer/code-fix tests.

Reviewed changes

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

Show a summary per file
File Description
test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD202RemoveUnnecessaryAsyncAnalyzerTests.cs Verifies diagnostics/fixes and ensures unsupported patterns are excluded.
src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx Adds localized title/message and code-fix titles for VSTHRD202.
src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD202RemoveUnnecessaryAsyncAnalyzer.cs Implements the VSTHRD202 analyzer logic and conservative exclusions.
src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD202RemoveUnnecessaryAsyncCodeFix.cs Implements ordered code fixes (minimal + exception-wrapping).
docfx/analyzers/VSTHRD202.md Documents what the analyzer flags, available fixes, and behavioral caveats.
docfx/analyzers/toc.yml Adds VSTHRD202 to the analyzers TOC.
docfx/analyzers/index.md Adds VSTHRD202 to the analyzer index table.

💡 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 AI review requested due to automatic review settings August 27, 2026 23:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

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

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD202RemoveUnnecessaryAsyncAnalyzer.cs:64

  • The analyzer repeatedly re-traverses the method body via method.DescendantNodes(ShouldDescendInto) (for awaits, using declarations, await-using, await-foreach). In Roslyn analyzers this can add noticeable overhead; cache the descendant nodes once and query that collection instead.

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

        SyntaxToken asyncKeyword = method.Modifiers.FirstOrDefault(modifier => modifier.IsKind(SyntaxKind.AsyncKeyword));
        ImmutableArray<AwaitExpressionSyntax> awaitExpressions = method.DescendantNodes(ShouldDescendInto).OfType<AwaitExpressionSyntax>().ToImmutableArray();
        if (asyncKeyword.RawKind == 0
            || awaitExpressions is not [AwaitExpressionSyntax awaitExpression]
            || context.SemanticModel.GetDeclaredSymbol(method, context.CancellationToken) is not IMethodSymbol { IsAsync: true } methodSymbol

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD202RemoveUnnecessaryAsyncAnalyzer.cs:123

  • IsTaskConfigureAwait matches by method name/signature and Utils.IsTask(...). Elsewhere in this codebase, ConfigureAwait-like methods are matched via CommonInterest.TaskConfigureAwait + QualifiedMember.IsMatch (e.g. CommonInterest.cs:69-72; VSTHRD003UseJtfRunAsyncAnalyzer.cs:245). Using that shared symbol matcher here reduces the risk of false positives/negatives if additional ConfigureAwait-like members are added or type names collide.
    private static bool IsTaskConfigureAwait(IInvocationOperation invocation)
        => invocation.TargetMethod.Name == nameof(Task.ConfigureAwait)
            && invocation.TargetMethod.Parameters.Length == 1
            && invocation.TargetMethod.Parameters[0].Type.SpecialType == SpecialType.System_Boolean
            && Utils.IsTask(invocation.TargetMethod.ContainingType);

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 27, 2026 23:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (1)

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

src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx:390

  • The resource comment for this code-fix title mentions only "async"/"await", but the title text also includes the term "Task". For localization guidance consistency with VSTHRD202_MessageFormat, the comment should also call out that "Task" should not be translated.
    <comment>"async" and "await" are C# keywords and should not be translated.</comment>

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 28, 2026 00:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (2)

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

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD202RemoveUnnecessaryAsyncCodeFix.cs:165

  • RemoveAsyncModifier removes the async modifier after calling asyncKeyword.WithoutTrivia(), which drops any trailing trivia (e.g., comments/whitespace) attached to the async keyword. This can silently delete comments such as async /* comment */ Task ... when applying the code fix. Preserve the removed token’s trailing trivia by transferring it to the next token (next modifier or return type) instead of discarding it.
        SyntaxToken asyncKeyword = method.Modifiers.First(modifier => modifier.IsKind(SyntaxKind.AsyncKeyword));
        int asyncKeywordIndex = method.Modifiers.IndexOf(asyncKeyword);
        SyntaxTokenList modifiers = method.Modifiers.Replace(asyncKeyword, asyncKeyword.WithoutTrivia());
        modifiers = modifiers.RemoveAt(asyncKeywordIndex);
        method = method.WithModifiers(modifiers);

docfx/analyzers/VSTHRD202.md:49

  • The documentation’s try/catch example uses new CancellationToken(canceled: true), but the code fix currently emits new CancellationToken(true) (see tests in VSTHRD202RemoveUnnecessaryAsyncAnalyzerTests). Align the doc example with the actual code fix output to avoid confusion.
            : new CancellationToken(canceled: true);

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 28, 2026 00:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Suppressed comments (5)

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

src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD202RemoveUnnecessaryAsyncCodeFix.cs:116

  • GetReturnedTaskExpression only strips ConfigureAwait when awaitExpression.Expression is an InvocationExpressionSyntax. If the awaited expression is wrapped in parentheses (e.g. await (SomethingElseAsync().ConfigureAwait(false))), this method will return the parenthesized invocation and the minimal fix will produce return (SomethingElseAsync().ConfigureAwait(false));, which won’t compile because it returns a ConfiguredTaskAwaitable instead of Task. Unwrap ParenthesizedExpressionSyntax before checking for ConfigureAwait (but keep returning the original expression when no ConfigureAwait is present).
    private static ExpressionSyntax GetReturnedTaskExpression(AwaitExpressionSyntax awaitExpression, SemanticModel semanticModel, CancellationToken cancellationToken)
    {
        if (awaitExpression.Expression is InvocationExpressionSyntax invocationExpression
            && invocationExpression.Expression is MemberAccessExpressionSyntax memberAccess
            && semanticModel.GetOperation(awaitExpression.Expression, cancellationToken) is IInvocationOperation invocation

src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD202RemoveUnnecessaryAsyncAnalyzer.cs:127

  • UnwrapConfigureAwait only handles awaitOperation.Operation when it is directly an IInvocationOperation. If the awaited expression is parenthesized, Roslyn can surface it as an IParenthesizedOperation, which would prevent ConfigureAwait unwrapping and can lead to missed diagnostics and/or incorrect fix application for patterns like await (task.ConfigureAwait(false)). Consider unwrapping parenthesized operations (and the corresponding syntax) before testing for ConfigureAwait.
    private static IOperation UnwrapConfigureAwait(IAwaitOperation awaitOperation, SemanticModel semanticModel, CancellationToken cancellationToken)
    {
        if (awaitOperation.Operation is IInvocationOperation invocation && IsTaskConfigureAwait(invocation))
        {
            if (invocation.Instance is IOperation instance)

docfx/analyzers/VSTHRD202.md:14

  • This rule also recognizes ConfigureAwaitRunInline (via CommonInterest.TaskConfigureAwait), not just .ConfigureAwait(bool). Updating the wording here would keep the documentation aligned with the implementation.

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

The analyzer also recognizes `.ConfigureAwait(bool)` on the final expression:

test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD202RemoveUnnecessaryAsyncAnalyzerTests.cs:216

  • There’s currently no test covering a parenthesized awaited ConfigureAwait expression (e.g. await (SomethingElseAsync().ConfigureAwait(false))). Adding one would help prevent regressions in ConfigureAwait unwrapping and ensure both analyzer and fixes behave correctly with extra parentheses.
    public async Task ConfigureAwaitRunInline_OffersMinimalFix()
    {
        const string source = /* lang=c#-test */ """
            using System.Threading.Tasks;
            using Microsoft.VisualStudio.Threading;

            class Test
            {
                Task SomethingElseAsync() => Task.CompletedTask;

                [|async|] Task DoSomethingAsync()
                {
                    await SomethingElseAsync().ConfigureAwaitRunInline();
                }
            }

docfx/analyzers/VSTHRD202.md:27

  • The minimal fix removes any ConfigureAwait call matched by CommonInterest.TaskConfigureAwait (including ConfigureAwaitRunInline), not only .ConfigureAwait(bool). Updating this sentence would avoid under-documenting what the fixer removes.
The minimal code fix removes `async`, `await`, and any `.ConfigureAwait(bool)` call:

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 28, 2026 00:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyzer proposal: remove redundant state machine

2 participants