Skip to content
Open
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
// 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())
Comment thread
Evangelink marked this conversation as resolved.
Outdated
{
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)
{
// For lambdas/anonymous functions, we don't provide a fix since changing to Task
// would require changing the delegate type as well.
break;
}
Comment thread
Evangelink marked this conversation as resolved.
Outdated
}
}

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(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[Correctness] compilationUnit.Usings only contains file-level (outermost) using directives. Files that place using directives inside a namespace block (a common C# style, and the default in older Visual Studio templates) will have compilationUnit.Usings empty, causing hasUsing to return false even when using System.Threading.Tasks; is already present inside the namespace. The code then inserts a duplicate using at file scope, which changes file style and may trigger SA0210 / IDE0005 duplicate-usings warnings in the consumer's project.

Impact: Files with namespace-scoped using directives get a spurious extra using System.Threading.Tasks; added at the top.

Suggestion: After the compilationUnit.Usings check, also walk each NamespaceDeclarationSyntax or FileScopedNamespaceDeclarationSyntax descendant and check their Usings collection before concluding the import is absent. If a namespace-scoped import is found, either return the document unchanged or add the import inside the same namespace block for consistency.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 44e8a77. EnsureSystemThreadingTasksImportAsync now walks all NamespaceDeclarationSyntax descendants and checks their Usings collection before deciding to insert. A test UseAssertMethodInAsyncVoidMethod_WithNamespaceScopedUsing_NoExtraUsing validates that no duplicate import is added when the using already exists in a namespace block.

u => string.Equals(u.Name?.ToString(), "System.Threading.Tasks", StringComparison.Ordinal));
Comment thread
Evangelink marked this conversation as resolved.
Outdated
if (hasUsing)
{
return document;
}
Comment thread
Evangelink marked this conversation as resolved.
Outdated

UsingDirectiveSyntax usingDirective = SyntaxFactory
.UsingDirective(SyntaxFactory.ParseName("System.Threading.Tasks").WithLeadingTrivia(SyntaxFactory.Space))
.WithTrailingTrivia(SyntaxFactory.ElasticCarriageReturnLineFeed);

// Insert after all existing System.* usings but before any non-System usings to maintain conventional ordering.
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();
Comment thread
Evangelink marked this conversation as resolved.
Outdated
if (nameText is not null && !nameText.StartsWith("System", StringComparison.Ordinal))
{
insertionIndex = i;
break;
}
}

SyntaxList<UsingDirectiveSyntax> newUsings = compilationUnit.Usings.Insert(insertionIndex, usingDirective);
return document.WithSyntaxRoot(compilationUnit.WithUsings(newUsings));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,7 @@
<data name="AvoidOutRefTestMethodParametersFix" xml:space="preserve">
<value>Remove 'out' and 'ref' modifiers</value>
</data>
<data name="AvoidUsingAssertsInAsyncVoidContextFix" xml:space="preserve">
<value>Change return type to 'Task'</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Odebrat modifikátory out a ref</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">Změnit přístupnost metody na private</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Entfernen der Modifizierer „out“ und „ref“</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">Methodenzugriff auf „privat“ ändern</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Quitar modificadores 'out' y 'ref'</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">Cambiar la accesibilidad del método a "private"</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Supprimer les modificateurs « out » et « ref »</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">Remplacer l’accessibilité de la méthode par « privé »</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Rimuovi i modificatori 'out' e 'ref'</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">Modifica l'accessibilità del metodo in 'privato'</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">'out' 修飾子と 'ref' 修飾子を削除する</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">メソッドのアクセシビリティを 'private' に変更する</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">'out' 및 'ref' 한정자 제거</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">메서드 접근성 '비공개'로 변경하기</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Usuń modyfikatory „out” i „ref”</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">Zmień dostępność metody na „private” (prywatna)</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Remova os modificadores ''out'' e ''ref''</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">Alterar a acessibilidade do método para 'privado'</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Удалите модификаторы "out" и "ref"</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">Изменить доступность метода на "private"</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">'out' ve 'ref' değiştiricilerini kaldırın</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">Yöntem erişilebilirliğini ‘özel’ olarak değiştir</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">移除 "out" 或 "ref" 修饰符</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">将方法可访问性更改为“private”</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">移除 'out' 和 'ref' 修飾元</target>
<note />
</trans-unit>
<trans-unit id="AvoidUsingAssertsInAsyncVoidContextFix">
<source>Change return type to 'Task'</source>
<target state="new">Change return type to 'Task'</target>
<note />
</trans-unit>
<trans-unit id="ChangeMethodAccessibilityToPrivateFix">
<source>Change method accessibility to 'private'</source>
<target state="translated">將方法協助工具變更為 'private'</target>
Expand Down
Loading
Loading