Add code fix for MSTEST0021 (#3827)

Co-authored-by: Youssef Victor <[email protected]>

Directory.Packages.props

@@ -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 -->

src/Analyzers/MSTest.Analyzers.CodeFixes/CodeFixResources.resx

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/PreferDisposeOverTestCleanupFixer.cs

@@ -0,0 +1,133 @@
+// 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) ||
+            testCleanupMethod.Parent is not TypeDeclarationSyntax containingType)
+        {
+            return;
+        }
+
+        context.RegisterCodeFix(
+            CodeAction.Create(
+                CodeFixResources.ReplaceWithDisposeFix,
+                c => AddDisposeAndBaseClassAsync(context.Document, testCleanupMethod, containingType, 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,
+        TypeDeclarationSyntax containingType,
+        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.");
+
+        SyntaxGenerator generator = editor.Generator;
+        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;
+}

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.cs.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.de.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.es.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.fr.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.it.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.ja.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.ko.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.pl.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.pt-BR.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.ru.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.tr.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.zh-Hans.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.zh-Hant.xlf

@@ -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>

src/Analyzers/MSTest.Analyzers/Helpers/WellKnownTypeNames.cs

@@ -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";

test/UnitTests/MSTest.Analyzers.UnitTests/MSTest.Analyzers.UnitTests.csproj

@@ -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" />