Skip to content
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

Add code fix for MSTEST0021 #3827

Merged
merged 20 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<MicrosoftBuildVersion>17.11.4</MicrosoftBuildVersion>
<MicrosoftCodeAnalysisAnalyzersVersion>3.11.0-beta1.24508.2</MicrosoftCodeAnalysisAnalyzersVersion>
<MicrosoftCodeAnalysisVersion>3.11.0</MicrosoftCodeAnalysisVersion>
<MicrosoftCodeAnalysisVersionForTests>4.8.0</MicrosoftCodeAnalysisVersionForTests>
<MicrosoftCodeAnalysisPublicApiAnalyzersVersion>$(MicrosoftCodeAnalysisAnalyzersVersion)</MicrosoftCodeAnalysisPublicApiAnalyzersVersion>
<MicrosoftCodeAnalysisBannedApiAnalyzersVersion>$(MicrosoftCodeAnalysisPublicApiAnalyzersVersion)</MicrosoftCodeAnalysisBannedApiAnalyzersVersion>
<!-- UWP and WinUI dependencies -->
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@
<data name="ReplaceWithConstructorFix" xml:space="preserve">
<value>Replace TestInitialize method with constructor</value>
</data>
<data name="ReplaceWithDisposeFix" xml:space="preserve">
<value>Replace TestCleanup with Dispose method</value>
</data>
<data name="ReplaceWithTestInitializeFix" xml:space="preserve">
<value>Replace constructor with TestInitialize method</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
// 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 Analyzer.Utilities.Extensions;

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.Simplification;
using Microsoft.CodeAnalysis.Text;

using MSTest.Analyzers.Helpers;

namespace MSTest.Analyzers;

[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(PreferDisposeOverTestCleanupFixer))]
[Shared]
public sealed class PreferDisposeOverTestCleanupFixer : CodeFixProvider
{
public override ImmutableArray<string> FixableDiagnosticIds { get; }
= ImmutableArray.Create(DiagnosticIds.PreferDisposeOverTestCleanupRuleId);

public override FixAllProvider GetFixAllProvider()
=> WellKnownFixAllProviders.BatchFixer;

public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
SyntaxNode root = await context.Document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);

Diagnostic diagnostic = context.Diagnostics[0];
TextSpan diagnosticSpan = diagnostic.Location.SourceSpan;

SyntaxToken syntaxToken = root.FindToken(diagnosticSpan.Start);
if (syntaxToken.Parent is null)
{
return;
}

// Find the TestCleanup method declaration identified by the diagnostic.
MethodDeclarationSyntax testCleanupMethod = syntaxToken.Parent.AncestorsAndSelf().OfType<MethodDeclarationSyntax>().FirstOrDefault();
if (testCleanupMethod == null || !IsTestCleanupMethodValid(testCleanupMethod))
{
return;
}

context.RegisterCodeFix(
CodeAction.Create(
CodeFixResources.ReplaceWithDisposeFix,
c => AddDisposeAndBaseClassAsync(context.Document, testCleanupMethod, c),
nameof(PreferDisposeOverTestCleanupFixer)),
diagnostic);
}

// The fix will be only for void TestCleanup.
// We can use DisposeAsync with other types but in that case we would also need to detect if the test is using multi-tfm as DisposeAsync is not available in netfx so we could only fix for netcore.
private static bool IsTestCleanupMethodValid(MethodDeclarationSyntax methodDeclaration) =>
// Check if the return type is void
methodDeclaration.ReturnType is PredefinedTypeSyntax predefinedType &&
predefinedType.Keyword.IsKind(SyntaxKind.VoidKeyword);

private static async Task<Document> AddDisposeAndBaseClassAsync(Document document, MethodDeclarationSyntax testCleanupMethod, CancellationToken cancellationToken)
{
DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
SemanticModel semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false)
?? throw new InvalidOperationException("SemanticModel cannot be null.");
Evangelink marked this conversation as resolved.
Show resolved Hide resolved

SyntaxGenerator generator = editor.Generator;

// Find the class containing the TestCleanup method
if (testCleanupMethod.Parent is TypeDeclarationSyntax containingType)
Evangelink marked this conversation as resolved.
Show resolved Hide resolved
{
TypeDeclarationSyntax? newParent = containingType;
INamedTypeSymbol? iDisposableSymbol = semanticModel.Compilation.GetTypeByMetadataName(WellKnownTypeNames.SystemIDisposable);
INamedTypeSymbol? testCleanupAttributeSymbol = semanticModel.Compilation.GetTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestCleanupAttribute);

// Move the code from TestCleanup to Dispose method
MethodDeclarationSyntax? existingDisposeMethod = containingType.Members
.OfType<MethodDeclarationSyntax>()
.FirstOrDefault(m => semanticModel.GetDeclaredSymbol(m) is IMethodSymbol methodSymbol && methodSymbol.IsDisposeImplementation(iDisposableSymbol));

BlockSyntax? cleanupBody = testCleanupMethod.Body;

if (existingDisposeMethod != null)
{
// Append the TestCleanup body to the existing Dispose method
StatementSyntax[]? cleanupStatements = cleanupBody?.Statements.ToArray();
MethodDeclarationSyntax newDisposeMethod;
if (existingDisposeMethod.Body != null)
{
BlockSyntax newDisposeBody = existingDisposeMethod.Body.AddStatements(cleanupStatements ?? Array.Empty<StatementSyntax>());
newDisposeMethod = existingDisposeMethod.WithBody(newDisposeBody);
}
else
{
newDisposeMethod = existingDisposeMethod.WithBody(cleanupBody);
}

editor.ReplaceNode(existingDisposeMethod, newDisposeMethod);
editor.RemoveNode(testCleanupMethod);
}
else
{
// Create a new Dispose method with the TestCleanup body
var disposeMethod = (MethodDeclarationSyntax)generator.MethodDeclaration("Dispose", accessibility: Accessibility.Public);
disposeMethod = disposeMethod.WithBody(cleanupBody);
newParent = newParent.ReplaceNode(testCleanupMethod, disposeMethod);

// Ensure the class implements IDisposable
if (iDisposableSymbol != null && !ImplementsIDisposable(containingType, iDisposableSymbol, semanticModel))
{
newParent = (TypeDeclarationSyntax)generator.AddInterfaceType(newParent, generator.TypeExpression(iDisposableSymbol, addImport: true).WithAdditionalAnnotations(Simplifier.AddImportsAnnotation));
}

editor.ReplaceNode(containingType, newParent);
}
}

return editor.GetChangedDocument();
}

private static bool ImplementsIDisposable(TypeDeclarationSyntax typeDeclaration, INamedTypeSymbol iDisposableSymbol, SemanticModel semanticModel)
=> typeDeclaration.BaseList?.Types
.Any(t => semanticModel.GetSymbolInfo(t.Type).Symbol is INamedTypeSymbol typeSymbol &&
SymbolEqualityComparer.Default.Equals(typeSymbol, iDisposableSymbol)) == true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Nahradit metodu TestInitialize konstruktorem</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">Nahradit kontrolní výraz za Assert.Fail()</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">TestInitialize-Methode durch Konstruktor ersetzen</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">Ersetzen Sie die Assertion durch „Assert.Fail()“.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Reemplazar el método TestInitialize por el constructor</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">Reemplazar la aserción por "Assert.Fail()"</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Remplacer la méthode TestInitialize par un constructeur</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">Remplacer l’assertion avec « Assert.Fail() »</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Sostituisci il metodo TestInitialize con il costruttore</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">Sostituire l'asserzione con 'Assert.Fail()'</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">TestInitialize メソッドをコンストラクターに置き換える</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">アサーションを 'Assert.Fail()' に置き換えます</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">TestInitialize 메서드를 생성자로 바꾸기</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">어설션을 'Assert.Fail()'으로 바꾸기</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Zastąp metodę TestInitialize konstruktorem</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">Zastąp asercję elementem „Assert.Fail()”</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Substituir o método TestInitialize pelo construtor.</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">Substituir a asserção por "Assert.Fail()"</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">Заменить метод TestInitialize конструктором</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">Заменить утверждение на "Assert.Fail()"</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">TestInitialize yöntemini oluşturucuyla değiştir</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">Onaylamayı 'Assert.Fail()' ile 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">将 TestInitialize 方法替换为构造函数</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">将断言替换为 "Assert.Fail()"</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
<target state="translated">以建構函式取代 TestInitialize 方法</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithDisposeFix">
<source>Replace TestCleanup with Dispose method</source>
<target state="new">Replace TestCleanup with Dispose method</target>
<note />
</trans-unit>
<trans-unit id="ReplaceWithFailAssertionFix">
<source>Replace the assertion with 'Assert.Fail()'</source>
<target state="translated">使用 'Assert.Fail()' 取代判斷提示</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ internal static class WellKnownTypeNames
public const string MicrosoftVisualStudioTestToolsUnitTestingTestPropertyAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.TestPropertyAttribute";
public const string MicrosoftVisualStudioTestToolsUnitTestingWorkItemAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.WorkItemAttribute";

public const string System = "System";
public const string SystemCollectionsGenericIEnumerable1 = "System.Collections.Generic.IEnumerable`1";
public const string SystemDescriptionAttribute = "System.ComponentModel.DescriptionAttribute";
public const string SystemIAsyncDisposable = "System.IAsyncDisposable";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis" />
<!-- We use a more recent version for tests so that we have bug fixes like https://github.com/dotnet/roslyn/pull/69309 -->
<PackageReference Include="Microsoft.CodeAnalysis" VersionOverride="$(MicrosoftCodeAnalysisVersionForTests)" />
<PackageReference Include="Microsoft.CodeAnalysis.Common" VersionOverride="$(MicrosoftCodeAnalysisVersionForTests)" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Workspaces" VersionOverride="$(MicrosoftCodeAnalysisVersionForTests)" />

<PackageReference Include="Microsoft.CodeAnalysis.CSharp.Analyzer.Testing" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeFix.Testing" />
<PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeRefactoring.Testing" />
Expand Down
Loading
Loading