MSTEST0009: TestCleanup should have valid layout

src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md

@@ -5,4 +5,5 @@
 Rule ID | Category | Severity | Notes
 --------|----------|----------|-------
 MSTEST0007 | Usage | Info | UseAttributeOnTestMethodAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/unit-testing-mstest-analyzers-MSTEST0007)
-MSTEST0008 | Usage | Warning | TestInitializeShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/unit-testing-mstest-analyzers-MSTEST0008)
+MSTEST0008 | Usage | Warning | TestInitializeShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/unit-testing-mstest-analyzers-MSTEST0008)
+MSTEST0009 | Usage | Warning | TestCleanupShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/unit-testing-mstest-analyzers-MSTEST0009)

src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs

@@ -13,4 +13,5 @@ internal static class DiagnosticIds
     public const string AvoidExpectedExceptionAttributeRuleId = "MSTEST0006";
     public const string UseAttributeOnTestMethodRuleId = "MSTEST0007";
     public const string TestInitializeShouldBeValidRuleId = "MSTEST0008";
+    public const string TestCleanupShouldBeValidRuleId = "MSTEST0009";
 }

src/Analyzers/MSTest.Analyzers/Resources.resx

@@ -156,6 +156,44 @@
   <data name="TestClassShouldBeValidTitle" xml:space="preserve">
     <value>Test classes should have valid layout</value>
   </data>
+  <data name="TestCleanupShouldBeValidDescription" xml:space="preserve">
+    <value>TestCleanup attribute should follow the following layout to be valid:
+- it should be 'public' 
+- it should not be 'static'
+- it should not be generic
+- it should not be 'abstract'
+- it should not take any parameter
+- return type should be 'void', 'Task' or 'ValueTask'
+- it should not be 'async void'
+- it should not be a special method (finalizer, operator...).</value>
+  </data>
+  <data name="TestCleanupShouldBeValidMessageFormat_NoParameters" xml:space="preserve">
+    <value>TestCleanup '{0}' should not take any parameter</value>
+  </data>
+  <data name="TestCleanupShouldBeValidMessageFormat_NotAbstract" xml:space="preserve">
+    <value>TestCleanup '{0}' should not be 'abstract'</value>
+  </data>
+  <data name="TestCleanupShouldBeValidMessageFormat_NotAsyncVoid" xml:space="preserve">
+    <value>TestCleanup '{0}' should not be 'async void'</value>
+  </data>
+  <data name="TestCleanupShouldBeValidMessageFormat_NotGeneric" xml:space="preserve">
+    <value>TestCleanup '{0}' should not be generic</value>
+  </data>
+  <data name="TestCleanupShouldBeValidMessageFormat_NotStatic" xml:space="preserve">
+    <value>TestCleanup '{0}' should not be 'static'</value>
+  </data>
+  <data name="TestCleanupShouldBeValidMessageFormat_Ordinary" xml:space="preserve">
+    <value>TestCleanup '{0}' should be an 'ordinary' method</value>
+  </data>
+  <data name="TestCleanupShouldBeValidMessageFormat_Public" xml:space="preserve">
+    <value>TestCleanup '{0}' should be 'public'</value>
+  </data>
+  <data name="TestCleanupShouldBeValidMessageFormat_ReturnType" xml:space="preserve">
+    <value>TestCleanup '{0}' should return 'void', 'Task' or 'ValueTask'</value>
+  </data>
+  <data name="TestCleanupShouldBeValidTitle" xml:space="preserve">
+    <value>TestCleanup method should have valid layout</value>
+  </data>
   <data name="TestContextShouldBeValidDescription" xml:space="preserve">
     <value>TestContext property should follow the following layout to be valid:
 - it should be a property
@@ -196,19 +234,19 @@
     <value>Test Initialize '{0}' should not take any parameter</value>
   </data>
   <data name="TestInitializeShouldBeValidMessageFormat_NotAbstract" xml:space="preserve">
-    <value>Test initialize '{0}' should not be 'abstract'</value>
+    <value>Test Initialize '{0}' should not be 'abstract'</value>
   </data>
   <data name="TestInitializeShouldBeValidMessageFormat_NotAsyncVoid" xml:space="preserve">
     <value>Test Initialize '{0}' should not be 'async void'</value>
   </data>
   <data name="TestInitializeShouldBeValidMessageFormat_NotGeneric" xml:space="preserve">
-    <value>Test initialize '{0}' should not be generic</value>
+    <value>Test Initialize '{0}' should not be generic</value>
   </data>
   <data name="TestInitializeShouldBeValidMessageFormat_NotStatic" xml:space="preserve">
     <value>Test Initialize '{0}' should not be 'static'</value>
   </data>
   <data name="TestInitializeShouldBeValidMessageFormat_Ordinary" xml:space="preserve">
-    <value>Test initialize '{0}' should be an 'ordinary' method</value>
+    <value>Test Initialize '{0}' should be an 'ordinary' method</value>
   </data>
   <data name="TestInitializeShouldBeValidMessageFormat_Public" xml:space="preserve">
     <value>Test Initialize '{0}' should be 'public'</value>

src/Analyzers/MSTest.Analyzers/TestCleanupShouldBeValidAnalyzer.cs

@@ -0,0 +1,113 @@
+// 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 Analyzer.Utilities.Extensions;
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+
+using MSTest.Analyzers.Helpers;
+
+namespace MSTest.Analyzers;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+public sealed class TestCleanupShouldBeValidAnalyzer : DiagnosticAnalyzer
+{
+    private static readonly LocalizableResourceString Title = new(nameof(Resources.TestCleanupShouldBeValidTitle), Resources.ResourceManager, typeof(Resources));
+    private static readonly LocalizableResourceString Description = new(nameof(Resources.TestCleanupShouldBeValidDescription), Resources.ResourceManager, typeof(Resources));
+    private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_Public), Resources.ResourceManager, typeof(Resources));
+
+    internal static readonly DiagnosticDescriptor PublicRule = DiagnosticDescriptorHelper.Create(
+        DiagnosticIds.TestCleanupShouldBeValidRuleId,
+        Title,
+        MessageFormat,
+        Description,
+        Category.Usage,
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true);
+
+    internal static readonly DiagnosticDescriptor NotStaticRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_NotStatic), Resources.ResourceManager, typeof(Resources)));
+    internal static readonly DiagnosticDescriptor NoParametersRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_NoParameters), Resources.ResourceManager, typeof(Resources)));
+    internal static readonly DiagnosticDescriptor ReturnTypeRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_ReturnType), Resources.ResourceManager, typeof(Resources)));
+    internal static readonly DiagnosticDescriptor NotAsyncVoidRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_NotAsyncVoid), Resources.ResourceManager, typeof(Resources)));
+    internal static readonly DiagnosticDescriptor NotAbstractRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_NotAbstract), Resources.ResourceManager, typeof(Resources)));
+    internal static readonly DiagnosticDescriptor NotGenericRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_NotGeneric), Resources.ResourceManager, typeof(Resources)));
+    internal static readonly DiagnosticDescriptor OrdinaryRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_Ordinary), Resources.ResourceManager, typeof(Resources)));
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(PublicRule);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+
+        context.RegisterCompilationStartAction(context =>
+        {
+            if (context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestCleanupAttribute, out var testCleanupAttributeSymbol))
+            {
+                var taskSymbol = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksTask);
+                var valueTaskSymbol = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksValueTask);
+                context.RegisterSymbolAction(
+                    context => AnalyzeSymbol(context, testCleanupAttributeSymbol, taskSymbol, valueTaskSymbol),
+                    SymbolKind.Method);
+            }
+        });
+    }
+
+    private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbol testCleanupAttributeSymbol, INamedTypeSymbol? taskSymbol,
+        INamedTypeSymbol? valueTaskSymbol)
+    {
+        var methodSymbol = (IMethodSymbol)context.Symbol;
+        if (!methodSymbol.GetAttributes().Any(attr => SymbolEqualityComparer.Default.Equals(attr.AttributeClass, testCleanupAttributeSymbol)))
+        {
+            return;
+        }
+
+        if (methodSymbol.MethodKind != MethodKind.Ordinary)
+        {
+            context.ReportDiagnostic(methodSymbol.CreateDiagnostic(OrdinaryRule, methodSymbol.Name));
+
+            // Do not check the other criteria, users should fix the method kind first.
+            return;
+        }
+
+        if (methodSymbol.IsAbstract)
+        {
+            context.ReportDiagnostic(methodSymbol.CreateDiagnostic(NotAbstractRule, methodSymbol.Name));
+        }
+
+        if (methodSymbol.IsGenericMethod)
+        {
+            context.ReportDiagnostic(methodSymbol.CreateDiagnostic(NotGenericRule, methodSymbol.Name));
+        }
+
+        if (methodSymbol.Parameters.Length > 0)
+        {
+            context.ReportDiagnostic(methodSymbol.CreateDiagnostic(NoParametersRule, methodSymbol.Name));
+        }
+
+        if (methodSymbol.IsStatic)
+        {
+            context.ReportDiagnostic(methodSymbol.CreateDiagnostic(NotStaticRule, methodSymbol.Name));
+        }
+
+        if (methodSymbol.ReturnsVoid && methodSymbol.IsAsync)
+        {
+            context.ReportDiagnostic(methodSymbol.CreateDiagnostic(NotAsyncVoidRule, methodSymbol.Name));
+        }
+
+        if (methodSymbol.DeclaredAccessibility != Accessibility.Public || methodSymbol.GetResultantVisibility() != SymbolVisibility.Public)
+        {
+            context.ReportDiagnostic(methodSymbol.CreateDiagnostic(PublicRule, methodSymbol.Name));
+        }
+
+        if (!methodSymbol.ReturnsVoid
+            && (taskSymbol is null || !SymbolEqualityComparer.Default.Equals(methodSymbol.ReturnType, taskSymbol))
+            && (valueTaskSymbol is null || !SymbolEqualityComparer.Default.Equals(methodSymbol.ReturnType, valueTaskSymbol)))
+        {
+            context.ReportDiagnostic(methodSymbol.CreateDiagnostic(ReturnTypeRule, methodSymbol.Name));
+        }
+    }
+}