-
Notifications
You must be signed in to change notification settings - Fork 293
Add code fix for MSTEST0040 — AvoidUsingAssertsInAsyncVoidContext #7892
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 7 commits
0d18f9d
9dfec9e
a04fed9
523a6ba
d780534
e83ec4a
53c7127
44e8a77
f6c0efc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,165 @@ | ||||||||||
| // 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.Composition; | ||||||||||
|
|
||||||||||
| using Analyzer.Utilities; | ||||||||||
|
|
||||||||||
| 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 MSTest.Analyzers.Helpers; | ||||||||||
|
|
||||||||||
| namespace MSTest.Analyzers; | ||||||||||
|
|
||||||||||
| /// <summary> | ||||||||||
| /// Code fixer for <see cref="AvoidUsingAssertsInAsyncVoidContextAnalyzer"/>. | ||||||||||
| /// </summary> | ||||||||||
| [ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(AvoidUsingAssertsInAsyncVoidContextFixer))] | ||||||||||
| [Shared] | ||||||||||
| public sealed class AvoidUsingAssertsInAsyncVoidContextFixer : CodeFixProvider | ||||||||||
| { | ||||||||||
| /// <inheritdoc /> | ||||||||||
| public sealed override ImmutableArray<string> FixableDiagnosticIds { get; } | ||||||||||
| = ImmutableArray.Create(DiagnosticIds.AvoidUsingAssertsInAsyncVoidContextRuleId); | ||||||||||
|
|
||||||||||
| /// <inheritdoc /> | ||||||||||
| public override FixAllProvider GetFixAllProvider() | ||||||||||
| // See https://github.com/dotnet/roslyn/blob/main/docs/analyzers/FixAllProvider.md for more information on Fix All Providers | ||||||||||
| => WellKnownFixAllProviders.BatchFixer; | ||||||||||
|
|
||||||||||
| /// <inheritdoc /> | ||||||||||
| public override async Task RegisterCodeFixesAsync(CodeFixContext context) | ||||||||||
| { | ||||||||||
| SyntaxNode root = await context.Document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); | ||||||||||
|
|
||||||||||
| Diagnostic diagnostic = context.Diagnostics[0]; | ||||||||||
| SyntaxNode diagnosticNode = root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true); | ||||||||||
|
|
||||||||||
| // Walk up the ancestors to find the nearest async void method or local function. | ||||||||||
| foreach (SyntaxNode ancestor in diagnosticNode.AncestorsAndSelf()) | ||||||||||
| { | ||||||||||
| if (ancestor is MethodDeclarationSyntax methodDeclaration) | ||||||||||
| { | ||||||||||
| if (methodDeclaration.Modifiers.Any(SyntaxKind.AsyncKeyword) && | ||||||||||
| methodDeclaration.ReturnType.IsVoid() && | ||||||||||
| !methodDeclaration.Modifiers.Any(SyntaxKind.OverrideKeyword)) | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Correctness] The guard skips Impact: Applying the fix to a Suggestion: Add
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Correctness] Explicit interface implementations are not guarded against. A method like Impact: The fix will be offered and applied incorrectly, producing a compile error. Suggestion: Add
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||||||||||
| { | ||||||||||
| context.RegisterCodeFix( | ||||||||||
| CodeAction.Create( | ||||||||||
| title: CodeFixResources.AvoidUsingAssertsInAsyncVoidContextFix, | ||||||||||
| createChangedDocument: ct => ChangeReturnTypeToTaskAsync(context.Document, methodDeclaration, ct), | ||||||||||
| equivalenceKey: nameof(AvoidUsingAssertsInAsyncVoidContextFixer)), | ||||||||||
| diagnostic); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| break; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (ancestor is LocalFunctionStatementSyntax localFunction) | ||||||||||
| { | ||||||||||
| if (localFunction.Modifiers.Any(SyntaxKind.AsyncKeyword) && | ||||||||||
| localFunction.ReturnType.IsVoid()) | ||||||||||
| { | ||||||||||
| context.RegisterCodeFix( | ||||||||||
| CodeAction.Create( | ||||||||||
| title: CodeFixResources.AvoidUsingAssertsInAsyncVoidContextFix, | ||||||||||
| createChangedDocument: ct => ChangeReturnTypeToTaskAsync(context.Document, localFunction, ct), | ||||||||||
| equivalenceKey: nameof(AvoidUsingAssertsInAsyncVoidContextFixer)), | ||||||||||
| diagnostic); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| break; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (ancestor is AnonymousFunctionExpressionSyntax anonymousFunction) | ||||||||||
| { | ||||||||||
| // Only stop at async lambdas/delegates — they represent the async void context. | ||||||||||
| // For non-async lambdas, keep walking up to find the enclosing async void method/local function. | ||||||||||
| if (anonymousFunction.AsyncKeyword.IsKind(SyntaxKind.AsyncKeyword)) | ||||||||||
| { | ||||||||||
| // For async lambdas/anonymous functions, we don't provide a fix since changing to Task | ||||||||||
| // would require changing the delegate type as well. | ||||||||||
| break; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private static async Task<Document> ChangeReturnTypeToTaskAsync( | ||||||||||
| Document document, | ||||||||||
| MethodDeclarationSyntax methodDeclaration, | ||||||||||
| CancellationToken cancellationToken) | ||||||||||
| { | ||||||||||
| DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); | ||||||||||
| MethodDeclarationSyntax newMethodDeclaration = methodDeclaration.WithReturnType( | ||||||||||
| SyntaxFactory.IdentifierName("Task").WithTriviaFrom(methodDeclaration.ReturnType)); | ||||||||||
| editor.ReplaceNode(methodDeclaration, newMethodDeclaration); | ||||||||||
| Document updatedDocument = editor.GetChangedDocument(); | ||||||||||
|
|
||||||||||
| return await EnsureSystemThreadingTasksImportAsync(updatedDocument, cancellationToken).ConfigureAwait(false); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private static async Task<Document> ChangeReturnTypeToTaskAsync( | ||||||||||
| Document document, | ||||||||||
| LocalFunctionStatementSyntax localFunction, | ||||||||||
| CancellationToken cancellationToken) | ||||||||||
| { | ||||||||||
| DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); | ||||||||||
| LocalFunctionStatementSyntax newLocalFunction = localFunction.WithReturnType( | ||||||||||
| SyntaxFactory.IdentifierName("Task").WithTriviaFrom(localFunction.ReturnType)); | ||||||||||
| editor.ReplaceNode(localFunction, newLocalFunction); | ||||||||||
| Document updatedDocument = editor.GetChangedDocument(); | ||||||||||
|
|
||||||||||
| return await EnsureSystemThreadingTasksImportAsync(updatedDocument, cancellationToken).ConfigureAwait(false); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private static async Task<Document> EnsureSystemThreadingTasksImportAsync(Document document, CancellationToken cancellationToken) | ||||||||||
| { | ||||||||||
| SyntaxNode root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); | ||||||||||
| if (root is not CompilationUnitSyntax compilationUnit) | ||||||||||
| { | ||||||||||
| return document; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| bool hasUsing = compilationUnit.Usings.Any( | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Correctness] Impact: Files with namespace-scoped Suggestion: After the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in |
||||||||||
| u => u.Alias is null && | ||||||||||
| !u.StaticKeyword.IsKind(SyntaxKind.StaticKeyword) && | ||||||||||
| string.Equals(u.Name?.ToString(), "System.Threading.Tasks", StringComparison.Ordinal)); | ||||||||||
| if (hasUsing) | ||||||||||
|
||||||||||
| { | ||||||||||
| return document; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| UsingDirectiveSyntax usingDirective = SyntaxFactory | ||||||||||
| .UsingDirective(SyntaxFactory.ParseName("System.Threading.Tasks").WithLeadingTrivia(SyntaxFactory.Space)) | ||||||||||
| .WithTrailingTrivia(SyntaxFactory.ElasticCarriageReturnLineFeed); | ||||||||||
|
|
||||||||||
| // Insert in correct alphabetical position: after System.* usings that sort before "System.Threading.Tasks" | ||||||||||
| // and before any that sort after it or before any non-System usings. | ||||||||||
| int insertionIndex = compilationUnit.Usings.Count; // default: append after all usings | ||||||||||
| for (int i = 0; i < compilationUnit.Usings.Count; i++) | ||||||||||
| { | ||||||||||
| string? nameText = compilationUnit.Usings[i].Name?.ToString(); | ||||||||||
|
Evangelink marked this conversation as resolved.
Outdated
|
||||||||||
| if (nameText is null) | ||||||||||
| { | ||||||||||
| continue; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (!nameText.StartsWith("System", StringComparison.Ordinal) || | ||||||||||
|
||||||||||
| if (!nameText.StartsWith("System", StringComparison.Ordinal) || | |
| bool isSystemNamespace = string.Equals(nameText, "System", StringComparison.Ordinal) || | |
| nameText.StartsWith("System.", StringComparison.Ordinal); | |
| if (!isSystemNamespace || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixer currently offers a return-type change for any
async voidmethod that isn’toverride. For explicit interface implementations (e.g.async void IFoo.M()), changing the return type toTaskwould break the interface contract and likely produce uncompilable code. Consider skipping methods withExplicitInterfaceSpecifier(and potentially other signature-constrained cases) when registering the code fix.