diff --git a/docfx/analyzers/VSTHRD202.md b/docfx/analyzers/VSTHRD202.md new file mode 100644 index 000000000..4b7891dbb --- /dev/null +++ b/docfx/analyzers/VSTHRD202.md @@ -0,0 +1,70 @@ +# VSTHRD202 Remove unnecessary async state machine + +This informational rule identifies an `async` method whose only `await` is the final operation and whose awaited expression produces exactly the same `Task` or `Task` type as the method returns. Returning that task directly avoids allocating and running an async state machine. + +## Examples of patterns that are flagged by this analyzer + +```csharp +async Task DoSomethingAsync() +{ + return await SomethingElseAsync(); +} +``` + +The analyzer also recognizes `.ConfigureAwait(bool)` and `.ConfigureAwaitRunInline()` on the final expression: + +```csharp +async Task DoSomethingAsync() +{ + await SomethingElseAsync().ConfigureAwait(false); +} +``` + +It does not report methods with multiple awaits, a non-terminal await, an await inside a `try` statement, a `using` declaration, a different returned task type, or a return type other than `Task` or `Task`. + +## Code fixes + +The minimal code fix removes `async`, `await`, and a supported task configuration call such as `.ConfigureAwait(bool)` or `.ConfigureAwaitRunInline()`: + +```csharp +Task DoSomethingAsync() +{ + return SomethingElseAsync(); +} +``` + +This changes how a synchronous exception from `SomethingElseAsync()` or from earlier code in the method is observed: it is thrown directly instead of being stored in the returned task. A second code fix wraps such exceptions in the returned task by adding a `try`/`catch`: + +```csharp +Task DoSomethingAsync() +{ + try + { + return SomethingElseAsync(); + } + catch (OperationCanceledException ex) + { + CancellationToken cancellationToken = ex.CancellationToken.IsCancellationRequested + ? ex.CancellationToken + : new CancellationToken(canceled: true); + return Task.FromCanceled(cancellationToken); + } + catch (Exception ex) + { + return Task.FromException(ex); + } +} +``` + +## Caveats + +Removing the state machine is a performance optimization, but the two forms are not identical: + +* While debugging a continuation in `SomethingElseAsync`, `DoSomethingAsync` no longer appears as an async frame in the call stack. +* Exception stack traces can change. The minimal fix also changes synchronous exceptions from a faulted or canceled returned task into exceptions thrown directly to the caller. The try/catch fix keeps ordinary exceptions in a faulted task and `OperationCanceledException` in a canceled task. +* The returned task is the original task instead of a task created by the wrapper method, which can make task identity observable. +* Compiler warning CS4014 for unawaited calls is only produced within an `async` method. Removing `async` may therefore stop the compiler from warning about other unawaited calls in the method. Consider enabling [VSTHRD110](VSTHRD110.md) before applying this optimization broadly. + +The try/catch fix is only offered when the target framework provides `Task.FromException` and `Task.FromCanceled`. + +These tradeoffs are why this diagnostic has an **Informational** default severity. Suppress or disable it when preserving the debugging experience or the original method boundary is more important than avoiding the state machine. diff --git a/docfx/analyzers/index.md b/docfx/analyzers/index.md index 9ec9e2cbd..b464365c1 100644 --- a/docfx/analyzers/index.md +++ b/docfx/analyzers/index.md @@ -33,6 +33,7 @@ ID | Title | Severity | Supports | Default diagnostic severity [VSTHRD116](VSTHRD116.md) | Use `ThreadStaticAttribute` only with static fields | Advisory | | Warning [VSTHRD117](VSTHRD117.md) | Avoid initialization of `ThreadStatic` fields in a type initializer | Advisory | | Warning [VSTHRD200](VSTHRD200.md) | Use `Async` naming convention | Guideline | [VSTHRD103](VSTHRD103.md) | Warning +[VSTHRD202](VSTHRD202.md) | Remove unnecessary async state machine | Guideline | | Info ## Severity descriptions diff --git a/docfx/analyzers/toc.yml b/docfx/analyzers/toc.yml index 87b57d2c8..3cf710f61 100644 --- a/docfx/analyzers/toc.yml +++ b/docfx/analyzers/toc.yml @@ -30,3 +30,4 @@ items: - href: VSTHRD116.md - href: VSTHRD117.md - href: VSTHRD200.md +- href: VSTHRD202.md diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD202RemoveUnnecessaryAsyncAnalyzer.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD202RemoveUnnecessaryAsyncAnalyzer.cs new file mode 100644 index 000000000..9950a303c --- /dev/null +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD202RemoveUnnecessaryAsyncAnalyzer.cs @@ -0,0 +1,143 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.VisualStudio.Threading.Analyzers; + +/// +/// Identifies methods where an unnecessary state machine can be removed. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class VSTHRD202RemoveUnnecessaryAsyncAnalyzer : DiagnosticAnalyzer +{ + /// + /// The diagnostic ID. + /// + public const string Id = "VSTHRD202"; + + /// + /// The descriptor for this diagnostic. + /// + internal static readonly DiagnosticDescriptor Descriptor = new( + id: Id, + title: new LocalizableResourceString(nameof(Strings.VSTHRD202_Title), Strings.ResourceManager, typeof(Strings)), + messageFormat: new LocalizableResourceString(nameof(Strings.VSTHRD202_MessageFormat), Strings.ResourceManager, typeof(Strings)), + helpLinkUri: Utils.GetHelpLink(Id), + category: "Style", + defaultSeverity: DiagnosticSeverity.Info, + isEnabledByDefault: true); + + /// + public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Descriptor); + + /// + public override void Initialize(AnalysisContext context) + { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterSyntaxNodeAction(Utils.DebuggableWrapper(AnalyzeMethod), SyntaxKind.MethodDeclaration); + } + + private static bool ShouldDescendInto(SyntaxNode node) + => node is not AnonymousFunctionExpressionSyntax and not LocalFunctionStatementSyntax; + + private static void AnalyzeMethod(SyntaxNodeAnalysisContext context) + { + var method = (MethodDeclarationSyntax)context.Node; + SyntaxToken asyncKeyword = method.Modifiers.FirstOrDefault(modifier => modifier.IsKind(SyntaxKind.AsyncKeyword)); + ImmutableArray descendants = method.DescendantNodes(ShouldDescendInto).ToImmutableArray(); + ImmutableArray awaitExpressions = descendants.OfType().ToImmutableArray(); + if (asyncKeyword.RawKind == 0 + || awaitExpressions is not [AwaitExpressionSyntax awaitExpression] + || context.SemanticModel.GetDeclaredSymbol(method, context.CancellationToken) is not IMethodSymbol { IsAsync: true } methodSymbol + || !Utils.IsTask(methodSymbol.ReturnType) + || !IsTerminalAwait(method, awaitExpression) + || descendants.OfType().Any(local => local.UsingKeyword.RawKind != 0) + || descendants.OfType().Any(usingStatement => usingStatement.AwaitKeyword.RawKind != 0) + || descendants.OfType().Any(forEachStatement => forEachStatement.AwaitKeyword.RawKind != 0)) + { + return; + } + + if (context.SemanticModel.GetOperation(awaitExpression, context.CancellationToken) is not IAwaitOperation awaitOperation) + { + return; + } + + IOperation returnedTask = UnwrapConfigureAwait(awaitOperation, context.SemanticModel, context.CancellationToken); + if (SymbolEqualityComparer.Default.Equals(returnedTask.Type, methodSymbol.ReturnType)) + { + context.ReportDiagnostic(Diagnostic.Create(Descriptor, asyncKeyword.GetLocation())); + } + } + + private static bool IsTerminalAwait(MethodDeclarationSyntax method, AwaitExpressionSyntax awaitExpression) + { + SyntaxNode expression = awaitExpression; + while (expression.Parent is ParenthesizedExpressionSyntax parenthesizedExpression) + { + expression = parenthesizedExpression; + } + + if (method.ExpressionBody?.Expression == expression) + { + return true; + } + + StatementSyntax? statement = expression.Parent switch + { + ReturnStatementSyntax returnStatement when returnStatement.Expression == expression => returnStatement, + ExpressionStatementSyntax expressionStatement when expressionStatement.Expression == expression => expressionStatement, + _ => null, + }; + + return statement is object + && method.Body is BlockSyntax body + && statement.Parent == body + && body.Statements.LastOrDefault() == statement; + } + + private static IOperation UnwrapConfigureAwait(IAwaitOperation awaitOperation, SemanticModel semanticModel, CancellationToken cancellationToken) + { + IOperation operation = awaitOperation.Operation; + while (operation is IParenthesizedOperation parenthesizedOperation) + { + operation = parenthesizedOperation.Operand; + } + + if (operation is IInvocationOperation invocation && IsTaskConfigureAwait(invocation)) + { + if (invocation.Instance is IOperation instance) + { + return instance; + } + + ExpressionSyntax expression = ((AwaitExpressionSyntax)awaitOperation.Syntax).Expression; + while (expression is ParenthesizedExpressionSyntax parenthesizedExpression) + { + expression = parenthesizedExpression.Expression; + } + + if (expression is InvocationExpressionSyntax invocationSyntax + && invocationSyntax.Expression is MemberAccessExpressionSyntax memberAccess + && semanticModel.GetOperation(memberAccess.Expression, cancellationToken) is IOperation receiver) + { + return receiver; + } + } + + return operation; + } + + private static bool IsTaskConfigureAwait(IInvocationOperation invocation) + => CommonInterest.TaskConfigureAwait.Any(configureAwait => configureAwait.IsMatch(invocation.TargetMethod)); +} diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD202RemoveUnnecessaryAsyncCodeFix.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD202RemoveUnnecessaryAsyncCodeFix.cs new file mode 100644 index 000000000..f522a75e6 --- /dev/null +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/VSTHRD202RemoveUnnecessaryAsyncCodeFix.cs @@ -0,0 +1,307 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System; +using System.Collections.Immutable; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.VisualStudio.Threading.Analyzers; + +/// +/// Removes an unnecessary async state machine. +/// +[ExportCodeFixProvider(LanguageNames.CSharp)] +public class VSTHRD202RemoveUnnecessaryAsyncCodeFix : CodeFixProvider +{ + /// + /// The equivalence key for the minimal code fix. + /// + public const string MinimalEquivalenceKey = "RemoveAsync"; + + /// + /// The equivalence key for the code fix that wraps synchronous exceptions in the returned task. + /// + public const string WrapSynchronousExceptionsEquivalenceKey = "RemoveAsyncWrapSynchronousExceptions"; + + private static readonly ImmutableArray ReusableFixableDiagnosticIds = ImmutableArray.Create( + VSTHRD202RemoveUnnecessaryAsyncAnalyzer.Id); + + /// + public override ImmutableArray FixableDiagnosticIds => ReusableFixableDiagnosticIds; + + /// + public override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer; + + /// + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + SyntaxNode root = await context.Document.GetSyntaxRootOrThrowAsync(context.CancellationToken).ConfigureAwait(false); + SemanticModel semanticModel = await context.Document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false) + ?? throw new InvalidOperationException("Unable to get the semantic model."); + + foreach (Diagnostic diagnostic in context.Diagnostics) + { + context.RegisterCodeFix( + CodeAction.Create( + Strings.VSTHRD202_CodeFix_Minimal_Title, + cancellationToken => ApplyFixAsync(context.Document, diagnostic, preserveSynchronousExceptions: false, cancellationToken), + MinimalEquivalenceKey), + diagnostic); + + MethodDeclarationSyntax? method = root.FindToken(diagnostic.Location.SourceSpan.Start).Parent? + .FirstAncestorOrSelf(); + if (method is object + && semanticModel.GetDeclaredSymbol(method, context.CancellationToken) is IMethodSymbol methodSymbol + && HasTaskExceptionFactories(semanticModel.Compilation, methodSymbol.ReturnType)) + { + context.RegisterCodeFix( + CodeAction.Create( + Strings.VSTHRD202_CodeFix_WrapExceptions_Title, + cancellationToken => ApplyFixAsync(context.Document, diagnostic, preserveSynchronousExceptions: true, cancellationToken), + WrapSynchronousExceptionsEquivalenceKey), + diagnostic); + } + } + } + + private static async Task ApplyFixAsync(Document document, Diagnostic diagnostic, bool preserveSynchronousExceptions, CancellationToken cancellationToken) + { + SyntaxNode root = await document.GetSyntaxRootOrThrowAsync(cancellationToken).ConfigureAwait(false); + MethodDeclarationSyntax method = root.FindToken(diagnostic.Location.SourceSpan.Start).Parent? + .FirstAncestorOrSelf() + ?? throw new InvalidOperationException("Unable to find the method declaration."); + AwaitExpressionSyntax awaitExpression = method.DescendantNodes(ShouldDescendInto).OfType().Single(); + SemanticModel semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false) + ?? throw new InvalidOperationException("Unable to get the semantic model."); + + ExpressionSyntax returnedTask = GetReturnedTaskExpression(awaitExpression, semanticModel, cancellationToken); + MethodDeclarationSyntax updatedMethod = RemoveAwait(method, awaitExpression, returnedTask); + updatedMethod = RemoveAsyncModifier(updatedMethod); + + if (preserveSynchronousExceptions) + { + IMethodSymbol methodSymbol = semanticModel.GetDeclaredSymbol(method, cancellationToken) + ?? throw new InvalidOperationException("Unable to find the method symbol."); + updatedMethod = AddExceptionHandling(updatedMethod, methodSymbol, semanticModel.Compilation, document); + } + + updatedMethod = updatedMethod.WithAdditionalAnnotations(Formatter.Annotation); + return document.WithSyntaxRoot(root.ReplaceNode(method, updatedMethod)); + } + + private static bool ShouldDescendInto(SyntaxNode node) + => node is not AnonymousFunctionExpressionSyntax and not LocalFunctionStatementSyntax; + + private static ExpressionSyntax GetReturnedTaskExpression(AwaitExpressionSyntax awaitExpression, SemanticModel semanticModel, CancellationToken cancellationToken) + { + ExpressionSyntax expression = awaitExpression.Expression; + while (expression is ParenthesizedExpressionSyntax parenthesizedExpression) + { + expression = parenthesizedExpression.Expression; + } + + if (expression is InvocationExpressionSyntax invocationExpression + && invocationExpression.Expression is MemberAccessExpressionSyntax memberAccess + && semanticModel.GetOperation(expression, cancellationToken) is IInvocationOperation invocation + && IsTaskConfigureAwait(invocation)) + { + return memberAccess.Expression.WithTriviaFrom(awaitExpression); + } + + return awaitExpression.Expression.WithTriviaFrom(awaitExpression); + } + + private static bool IsTaskConfigureAwait(IInvocationOperation invocation) + => CommonInterest.TaskConfigureAwait.Any(configureAwait => configureAwait.IsMatch(invocation.TargetMethod)); + + private static bool HasTaskExceptionFactories(Compilation compilation, ITypeSymbol returnType) + { + INamedTypeSymbol? taskType = compilation.GetTypeByMetadataName(typeof(Task).FullName); + INamedTypeSymbol? exceptionType = compilation.GetTypeByMetadataName(typeof(Exception).FullName); + INamedTypeSymbol? cancellationTokenType = compilation.GetTypeByMetadataName(typeof(CancellationToken).FullName); + if (taskType is null || exceptionType is null || cancellationTokenType is null || returnType is not INamedTypeSymbol namedReturnType) + { + return false; + } + + int requiredArity = namedReturnType.IsGenericType ? 1 : 0; + return HasFactory(nameof(Task.FromException), exceptionType) + && HasFactory(nameof(Task.FromCanceled), cancellationTokenType); + + bool HasFactory(string methodName, ITypeSymbol parameterType) + => taskType.GetMembers(methodName) + .OfType() + .Any(method => method.IsStatic + && method.Arity == requiredArity + && method.Parameters.Length == 1 + && SymbolEqualityComparer.Default.Equals(method.Parameters[0].Type, parameterType)); + } + + private static MethodDeclarationSyntax RemoveAsyncModifier(MethodDeclarationSyntax method) + { + SyntaxToken asyncKeyword = method.Modifiers.First(modifier => modifier.IsKind(SyntaxKind.AsyncKeyword)); + int asyncKeywordIndex = method.Modifiers.IndexOf(asyncKeyword); + SyntaxTriviaList preservedTrivia = asyncKeyword.LeadingTrivia.AddRange(asyncKeyword.TrailingTrivia); + SyntaxTokenList modifiers = method.Modifiers.RemoveAt(asyncKeywordIndex); + if (asyncKeywordIndex > 0 + && method.Modifiers[asyncKeywordIndex - 1].TrailingTrivia.LastOrDefault().IsKind(SyntaxKind.WhitespaceTrivia) + && preservedTrivia.FirstOrDefault().IsKind(SyntaxKind.WhitespaceTrivia)) + { + preservedTrivia = preservedTrivia.RemoveAt(0); + } + + method = method.WithModifiers(modifiers); + + if (asyncKeywordIndex < modifiers.Count) + { + SyntaxToken nextModifier = modifiers[asyncKeywordIndex]; + method = method.WithModifiers(modifiers.Replace(nextModifier, nextModifier.WithLeadingTrivia(preservedTrivia.AddRange(nextModifier.LeadingTrivia)))); + } + else + { + method = method.WithReturnType(method.ReturnType.WithLeadingTrivia(preservedTrivia.AddRange(method.ReturnType.GetLeadingTrivia()))); + } + + return method; + } + + private static MethodDeclarationSyntax RemoveAwait(MethodDeclarationSyntax method, AwaitExpressionSyntax awaitExpression, ExpressionSyntax returnedTask) + { + if (method.ExpressionBody is ArrowExpressionClauseSyntax expressionBody) + { + return method.WithExpressionBody(expressionBody.WithExpression(expressionBody.Expression.ReplaceNode(awaitExpression, returnedTask))); + } + + SyntaxNode expression = awaitExpression; + while (expression.Parent is ParenthesizedExpressionSyntax parenthesizedExpression) + { + expression = parenthesizedExpression; + } + + StatementSyntax originalStatement; + StatementSyntax replacementStatement; + if (expression.Parent is ReturnStatementSyntax returnStatement) + { + originalStatement = returnStatement; + replacementStatement = returnStatement.WithExpression(returnStatement.Expression!.ReplaceNode(awaitExpression, returnedTask)); + } + else if (expression.Parent is ExpressionStatementSyntax expressionStatement) + { + originalStatement = expressionStatement; + replacementStatement = SyntaxFactory.ReturnStatement(expressionStatement.Expression.ReplaceNode(awaitExpression, returnedTask)) + .WithTriviaFrom(expressionStatement); + } + else + { + throw new InvalidOperationException("The await expression is not the terminal operation in the method."); + } + + return method.WithBody(method.Body!.ReplaceNode(originalStatement, replacementStatement)); + } + + private static MethodDeclarationSyntax AddExceptionHandling( + MethodDeclarationSyntax method, + IMethodSymbol methodSymbol, + Compilation compilation, + Document document) + { + INamedTypeSymbol taskType = compilation.GetTypeByMetadataName(typeof(Task).FullName) + ?? throw new InvalidOperationException("Unable to find System.Threading.Tasks.Task."); + INamedTypeSymbol exceptionType = compilation.GetTypeByMetadataName(typeof(Exception).FullName) + ?? throw new InvalidOperationException("Unable to find System.Exception."); + INamedTypeSymbol operationCanceledExceptionType = compilation.GetTypeByMetadataName(typeof(OperationCanceledException).FullName) + ?? throw new InvalidOperationException("Unable to find System.OperationCanceledException."); + INamedTypeSymbol cancellationTokenType = compilation.GetTypeByMetadataName(typeof(CancellationToken).FullName) + ?? throw new InvalidOperationException("Unable to find System.Threading.CancellationToken."); + var returnType = (INamedTypeSymbol)methodSymbol.ReturnType; + SyntaxGenerator generator = SyntaxGenerator.GetGenerator(document); + string exceptionVariableName = GetUniqueExceptionVariableName(method); + + SyntaxNode fromCanceledName = returnType.IsGenericType + ? generator.GenericName(nameof(Task.FromCanceled), returnType.TypeArguments[0]) + : generator.IdentifierName(nameof(Task.FromCanceled)); + SyntaxNode fromExceptionName = returnType.IsGenericType + ? generator.GenericName(nameof(Task.FromException), returnType.TypeArguments[0]) + : generator.IdentifierName(nameof(Task.FromException)); + var cancellationTokenExpression = (ExpressionSyntax)generator.MemberAccessExpression( + generator.IdentifierName(exceptionVariableName), + nameof(OperationCanceledException.CancellationToken)); + var cancellationTokenIsCanceledExpression = (ExpressionSyntax)generator.MemberAccessExpression( + cancellationTokenExpression, + nameof(CancellationToken.IsCancellationRequested)); + ObjectCreationExpressionSyntax canceledTokenExpression = SyntaxFactory.ObjectCreationExpression((TypeSyntax)generator.TypeExpression(cancellationTokenType)) + .WithArgumentList( + SyntaxFactory.ArgumentList( + SyntaxFactory.SingletonSeparatedList( + SyntaxFactory.Argument(SyntaxFactory.LiteralExpression(SyntaxKind.TrueLiteralExpression)) + .WithNameColon(SyntaxFactory.NameColon(SyntaxFactory.IdentifierName("canceled")))))); + var fromCanceledInvocation = (ExpressionSyntax)generator.InvocationExpression( + generator.MemberAccessExpression(generator.TypeExpressionForStaticMemberAccess(taskType), fromCanceledName), + SyntaxFactory.ConditionalExpression( + cancellationTokenIsCanceledExpression, + cancellationTokenExpression, + canceledTokenExpression)); + var fromExceptionInvocation = (ExpressionSyntax)generator.InvocationExpression( + generator.MemberAccessExpression(generator.TypeExpressionForStaticMemberAccess(taskType), fromExceptionName), + generator.IdentifierName(exceptionVariableName)); + + BlockSyntax originalBody; + if (method.Body is BlockSyntax body) + { + originalBody = body; + } + else + { + originalBody = SyntaxFactory.Block(SyntaxFactory.ReturnStatement(method.ExpressionBody!.Expression)) + .WithCloseBraceToken(SyntaxFactory.Token(SyntaxKind.CloseBraceToken).WithTrailingTrivia(method.SemicolonToken.TrailingTrivia)); + } + + CatchClauseSyntax cancellationCatchClause = SyntaxFactory.CatchClause() + .WithDeclaration( + SyntaxFactory.CatchDeclaration( + (TypeSyntax)generator.TypeExpression(operationCanceledExceptionType), + SyntaxFactory.Identifier(exceptionVariableName))) + .WithBlock(SyntaxFactory.Block(SyntaxFactory.ReturnStatement(fromCanceledInvocation))); + CatchClauseSyntax exceptionCatchClause = SyntaxFactory.CatchClause() + .WithDeclaration( + SyntaxFactory.CatchDeclaration( + (TypeSyntax)generator.TypeExpression(exceptionType), + SyntaxFactory.Identifier(exceptionVariableName))) + .WithBlock(SyntaxFactory.Block(SyntaxFactory.ReturnStatement(fromExceptionInvocation))); + TryStatementSyntax tryStatement = SyntaxFactory.TryStatement( + SyntaxFactory.Block(originalBody.Statements), + SyntaxFactory.List(new[] { cancellationCatchClause, exceptionCatchClause }), + null); + BlockSyntax updatedBody = originalBody.WithStatements(SyntaxFactory.SingletonList(tryStatement)); + + return method.WithBody(updatedBody) + .WithExpressionBody(null) + .WithSemicolonToken(default); + } + + private static string GetUniqueExceptionVariableName(MethodDeclarationSyntax method) + { + var identifiers = method.DescendantTokens() + .Where(token => token.IsKind(SyntaxKind.IdentifierToken)) + .Select(token => token.ValueText) + .ToImmutableHashSet(StringComparer.Ordinal); + const string baseName = "ex"; + string name = baseName; + for (int suffix = 1; identifiers.Contains(name); suffix++) + { + name = baseName + suffix; + } + + return name; + } +} diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx index df7bcb807..016f74c47 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx @@ -373,4 +373,20 @@ Start the work within this context, or use JoinableTaskFactory.RunAsync to start ThreadStatic fields should not be initialized by a field initializer or static constructor because the value is only assigned on the type-initializing thread + + Remove unnecessary async state machine + "async" is a C# keyword and should not be translated. + + + This method can return the Task directly instead of using async and await + "Task", "async", and "await" are C# terms and should not be translated. + + + Remove async and await + "async" and "await" are C# keywords and should not be translated. + + + Remove async and await, wrapping synchronous exceptions in the returned Task + "async", "await", and "Task" are C# terms and should not be translated. + \ No newline at end of file diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD202RemoveUnnecessaryAsyncAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD202RemoveUnnecessaryAsyncAnalyzerTests.cs new file mode 100644 index 000000000..0016eac28 --- /dev/null +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD202RemoveUnnecessaryAsyncAnalyzerTests.cs @@ -0,0 +1,330 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using CSVerify = Microsoft.VisualStudio.Threading.Analyzers.Tests.CSharpCodeFixVerifier; + +public class VSTHRD202RemoveUnnecessaryAsyncAnalyzerTests +{ + [Fact] + public async Task TaskOfTMethod_OffersBothFixes() + { + const string source = /* lang=c#-test */ """ + using System; + using System.Threading.Tasks; + + class Test + { + async Task SomethingElseAsync() => "result"; + + public [|async|] /* keep this comment */ Task DoSomethingAsync() + { + return await SomethingElseAsync(); + } + } + """; + const string minimalFix = /* lang=c#-test */ """ + using System; + using System.Threading.Tasks; + + class Test + { + async Task SomethingElseAsync() => "result"; + + public /* keep this comment */ Task DoSomethingAsync() + { + return SomethingElseAsync(); + } + } + """; + const string exceptionPreservingFix = /* lang=c#-test */ """ + using System; + using System.Threading.Tasks; + + class Test + { + async Task SomethingElseAsync() => "result"; + + public /* keep this comment */ Task DoSomethingAsync() + { + try + { + return SomethingElseAsync(); + } + catch (OperationCanceledException ex) + { + return Task.FromCanceled(ex.CancellationToken.IsCancellationRequested ? ex.CancellationToken : new System.Threading.CancellationToken(canceled: true)); + } + catch (Exception ex) + { + return Task.FromException(ex); + } + } + } + """; + + await new CSVerify.Test + { + TestCode = source, + FixedCode = minimalFix, + CodeActionEquivalenceKey = VSTHRD202RemoveUnnecessaryAsyncCodeFix.MinimalEquivalenceKey, + }.RunAsync(); + await new CSVerify.Test + { + TestCode = source, + FixedCode = exceptionPreservingFix, + CodeActionEquivalenceKey = VSTHRD202RemoveUnnecessaryAsyncCodeFix.WrapSynchronousExceptionsEquivalenceKey, + }.RunAsync(); + } + + [Fact] + public async Task TaskMethod_WithPrecedingStatement_OffersExceptionPreservingFix() + { + const string source = /* lang=c#-test */ """ + using System; + using System.Threading.Tasks; + + class Test + { + void Prepare() { } + + Task SomethingElseAsync() => Task.CompletedTask; + + [|async|] Task DoSomethingAsync() + { + Prepare(); + await (SomethingElseAsync().ConfigureAwait(false)); + } + } + """; + const string fixedSource = /* lang=c#-test */ """ + using System; + using System.Threading.Tasks; + + class Test + { + void Prepare() { } + + Task SomethingElseAsync() => Task.CompletedTask; + + Task DoSomethingAsync() + { + try + { + Prepare(); + return SomethingElseAsync(); + } + catch (OperationCanceledException ex) + { + return Task.FromCanceled(ex.CancellationToken.IsCancellationRequested ? ex.CancellationToken : new System.Threading.CancellationToken(canceled: true)); + } + catch (Exception ex) + { + return Task.FromException(ex); + } + } + } + """; + + await new CSVerify.Test + { + TestCode = source, + FixedCode = fixedSource, + CodeActionEquivalenceKey = VSTHRD202RemoveUnnecessaryAsyncCodeFix.WrapSynchronousExceptionsEquivalenceKey, + }.RunAsync(); + } + + [Fact] + public async Task ExpressionBodiedMethod_OffersBothFixesWithMinimalFirst() + { + const string source = /* lang=c#-test */ """ + using System; + using System.Threading.Tasks; + + class Test + { + Task GetValueAsync() => Task.FromResult(1); + + [|async|] Task GetValueWrapperAsync() => await GetValueAsync(); + } + """; + const string minimalFix = /* lang=c#-test */ """ + using System; + using System.Threading.Tasks; + + class Test + { + Task GetValueAsync() => Task.FromResult(1); + + Task GetValueWrapperAsync() => GetValueAsync(); + } + """; + const string exceptionWrappingFix = /* lang=c#-test */ """ + using System; + using System.Threading.Tasks; + + class Test + { + Task GetValueAsync() => Task.FromResult(1); + + Task GetValueWrapperAsync() + { + try + { + return GetValueAsync(); + } + catch (OperationCanceledException ex) + { + return Task.FromCanceled(ex.CancellationToken.IsCancellationRequested ? ex.CancellationToken : new System.Threading.CancellationToken(canceled: true)); + } + catch (Exception ex) + { + return Task.FromException(ex); + } + } + } + """; + + await new CSVerify.Test + { + TestCode = source, + FixedCode = minimalFix, + CodeActionIndex = 0, + }.RunAsync(); + await new CSVerify.Test + { + TestCode = source, + FixedCode = exceptionWrappingFix, + CodeActionIndex = 1, + }.RunAsync(); + } + + [Fact] + 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()); + } + } + """; + const string fixedSource = /* lang=c#-test */ """ + using System.Threading.Tasks; + using Microsoft.VisualStudio.Threading; + + class Test + { + Task SomethingElseAsync() => Task.CompletedTask; + + Task DoSomethingAsync() + { + return SomethingElseAsync(); + } + } + """; + + await new CSVerify.Test + { + TestCode = source, + FixedCode = fixedSource, + CodeActionEquivalenceKey = VSTHRD202RemoveUnnecessaryAsyncCodeFix.MinimalEquivalenceKey, + }.RunAsync(); + } + + [Fact] + public async Task UnsupportedPatterns_DoNotProduceDiagnostics() + { + const string source = /* lang=c#-test */ """ + using System; + using System.Threading.Tasks; + + class Test + { + Task GetStringAsync() => Task.FromResult(""); + ValueTask GetValueTaskAsync() => new ValueTask(""); + + async Task DifferentTaskTypeAsync() + { + return await GetStringAsync(); + } + + async Task MultipleAwaitsAsync() + { + await Task.Yield(); + await Task.Delay(1); + } + + async Task AwaitIsNotTerminalAsync() + { + await Task.Delay(1); + Console.WriteLine(); + } + + async Task AwaitIsInTryAsync() + { + try + { + await Task.Delay(1); + } + catch + { + } + } + + async Task UsingDeclarationAsync() + { + using var disposable = new Disposable(); + await Task.Delay(1); + } + + async ValueTask ValueTaskAsync() + { + return await GetValueTaskAsync(); + } + + async Task AwaitUsingAsync() + { + await using (new AsyncDisposable()) + { + } + + await Task.Delay(1); + } + + async Task AwaitForeachAsync() + { + await foreach (int value in GetValuesAsync()) + { + Console.WriteLine(value); + } + + await Task.Delay(1); + } + + async System.Collections.Generic.IAsyncEnumerable GetValuesAsync() + { + yield break; + } + + sealed class Disposable : IDisposable + { + public void Dispose() { } + } + + sealed class AsyncDisposable : IAsyncDisposable + { + public ValueTask DisposeAsync() => default; + } + } + """; + + await CSVerify.VerifyAnalyzerAsync(source); + } +}