Skip to content

Commit 122c52e

Browse files
authored
MSTEST0009: TestCleanup should have valid layout (#2312)
1 parent af38d49 commit 122c52e

20 files changed

+1480
-85
lines changed

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,5 @@
55
Rule ID | Category | Severity | Notes
66
--------|----------|----------|-------
77
MSTEST0007 | Usage | Info | UseAttributeOnTestMethodAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/unit-testing-mstest-analyzers-MSTEST0007)
8-
MSTEST0008 | Usage | Warning | TestInitializeShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/unit-testing-mstest-analyzers-MSTEST0008)
8+
MSTEST0008 | Usage | Warning | TestInitializeShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/unit-testing-mstest-analyzers-MSTEST0008)
9+
MSTEST0009 | Usage | Warning | TestCleanupShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/unit-testing-mstest-analyzers-MSTEST0009)

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

+1
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,5 @@ internal static class DiagnosticIds
1313
public const string AvoidExpectedExceptionAttributeRuleId = "MSTEST0006";
1414
public const string UseAttributeOnTestMethodRuleId = "MSTEST0007";
1515
public const string TestInitializeShouldBeValidRuleId = "MSTEST0008";
16+
public const string TestCleanupShouldBeValidRuleId = "MSTEST0009";
1617
}

src/Analyzers/MSTest.Analyzers/Resources.Designer.cs

+101-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Analyzers/MSTest.Analyzers/Resources.resx

+41-3
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,44 @@
156156
<data name="TestClassShouldBeValidTitle" xml:space="preserve">
157157
<value>Test classes should have valid layout</value>
158158
</data>
159+
<data name="TestCleanupShouldBeValidDescription" xml:space="preserve">
160+
<value>TestCleanup attribute should follow the following layout to be valid:
161+
- it should be 'public'
162+
- it should not be 'static'
163+
- it should not be generic
164+
- it should not be 'abstract'
165+
- it should not take any parameter
166+
- return type should be 'void', 'Task' or 'ValueTask'
167+
- it should not be 'async void'
168+
- it should not be a special method (finalizer, operator...).</value>
169+
</data>
170+
<data name="TestCleanupShouldBeValidMessageFormat_NoParameters" xml:space="preserve">
171+
<value>TestCleanup '{0}' should not take any parameter</value>
172+
</data>
173+
<data name="TestCleanupShouldBeValidMessageFormat_NotAbstract" xml:space="preserve">
174+
<value>TestCleanup '{0}' should not be 'abstract'</value>
175+
</data>
176+
<data name="TestCleanupShouldBeValidMessageFormat_NotAsyncVoid" xml:space="preserve">
177+
<value>TestCleanup '{0}' should not be 'async void'</value>
178+
</data>
179+
<data name="TestCleanupShouldBeValidMessageFormat_NotGeneric" xml:space="preserve">
180+
<value>TestCleanup '{0}' should not be generic</value>
181+
</data>
182+
<data name="TestCleanupShouldBeValidMessageFormat_NotStatic" xml:space="preserve">
183+
<value>TestCleanup '{0}' should not be 'static'</value>
184+
</data>
185+
<data name="TestCleanupShouldBeValidMessageFormat_Ordinary" xml:space="preserve">
186+
<value>TestCleanup '{0}' should be an 'ordinary' method</value>
187+
</data>
188+
<data name="TestCleanupShouldBeValidMessageFormat_Public" xml:space="preserve">
189+
<value>TestCleanup '{0}' should be 'public'</value>
190+
</data>
191+
<data name="TestCleanupShouldBeValidMessageFormat_ReturnType" xml:space="preserve">
192+
<value>TestCleanup '{0}' should return 'void', 'Task' or 'ValueTask'</value>
193+
</data>
194+
<data name="TestCleanupShouldBeValidTitle" xml:space="preserve">
195+
<value>TestCleanup method should have valid layout</value>
196+
</data>
159197
<data name="TestContextShouldBeValidDescription" xml:space="preserve">
160198
<value>TestContext property should follow the following layout to be valid:
161199
- it should be a property
@@ -196,19 +234,19 @@
196234
<value>Test Initialize '{0}' should not take any parameter</value>
197235
</data>
198236
<data name="TestInitializeShouldBeValidMessageFormat_NotAbstract" xml:space="preserve">
199-
<value>Test initialize '{0}' should not be 'abstract'</value>
237+
<value>Test Initialize '{0}' should not be 'abstract'</value>
200238
</data>
201239
<data name="TestInitializeShouldBeValidMessageFormat_NotAsyncVoid" xml:space="preserve">
202240
<value>Test Initialize '{0}' should not be 'async void'</value>
203241
</data>
204242
<data name="TestInitializeShouldBeValidMessageFormat_NotGeneric" xml:space="preserve">
205-
<value>Test initialize '{0}' should not be generic</value>
243+
<value>Test Initialize '{0}' should not be generic</value>
206244
</data>
207245
<data name="TestInitializeShouldBeValidMessageFormat_NotStatic" xml:space="preserve">
208246
<value>Test Initialize '{0}' should not be 'static'</value>
209247
</data>
210248
<data name="TestInitializeShouldBeValidMessageFormat_Ordinary" xml:space="preserve">
211-
<value>Test initialize '{0}' should be an 'ordinary' method</value>
249+
<value>Test Initialize '{0}' should be an 'ordinary' method</value>
212250
</data>
213251
<data name="TestInitializeShouldBeValidMessageFormat_Public" xml:space="preserve">
214252
<value>Test Initialize '{0}' should be 'public'</value>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System.Collections.Immutable;
5+
6+
using Analyzer.Utilities.Extensions;
7+
8+
using Microsoft.CodeAnalysis;
9+
using Microsoft.CodeAnalysis.Diagnostics;
10+
11+
using MSTest.Analyzers.Helpers;
12+
13+
namespace MSTest.Analyzers;
14+
15+
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
16+
public sealed class TestCleanupShouldBeValidAnalyzer : DiagnosticAnalyzer
17+
{
18+
private static readonly LocalizableResourceString Title = new(nameof(Resources.TestCleanupShouldBeValidTitle), Resources.ResourceManager, typeof(Resources));
19+
private static readonly LocalizableResourceString Description = new(nameof(Resources.TestCleanupShouldBeValidDescription), Resources.ResourceManager, typeof(Resources));
20+
private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_Public), Resources.ResourceManager, typeof(Resources));
21+
22+
internal static readonly DiagnosticDescriptor PublicRule = DiagnosticDescriptorHelper.Create(
23+
DiagnosticIds.TestCleanupShouldBeValidRuleId,
24+
Title,
25+
MessageFormat,
26+
Description,
27+
Category.Usage,
28+
DiagnosticSeverity.Warning,
29+
isEnabledByDefault: true);
30+
31+
internal static readonly DiagnosticDescriptor NotStaticRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_NotStatic), Resources.ResourceManager, typeof(Resources)));
32+
internal static readonly DiagnosticDescriptor NoParametersRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_NoParameters), Resources.ResourceManager, typeof(Resources)));
33+
internal static readonly DiagnosticDescriptor ReturnTypeRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_ReturnType), Resources.ResourceManager, typeof(Resources)));
34+
internal static readonly DiagnosticDescriptor NotAsyncVoidRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_NotAsyncVoid), Resources.ResourceManager, typeof(Resources)));
35+
internal static readonly DiagnosticDescriptor NotAbstractRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_NotAbstract), Resources.ResourceManager, typeof(Resources)));
36+
internal static readonly DiagnosticDescriptor NotGenericRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_NotGeneric), Resources.ResourceManager, typeof(Resources)));
37+
internal static readonly DiagnosticDescriptor OrdinaryRule = PublicRule.WithMessage(new(nameof(Resources.TestCleanupShouldBeValidMessageFormat_Ordinary), Resources.ResourceManager, typeof(Resources)));
38+
39+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(PublicRule);
40+
41+
public override void Initialize(AnalysisContext context)
42+
{
43+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
44+
context.EnableConcurrentExecution();
45+
46+
context.RegisterCompilationStartAction(context =>
47+
{
48+
if (context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestCleanupAttribute, out var testCleanupAttributeSymbol))
49+
{
50+
var taskSymbol = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksTask);
51+
var valueTaskSymbol = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingTasksValueTask);
52+
context.RegisterSymbolAction(
53+
context => AnalyzeSymbol(context, testCleanupAttributeSymbol, taskSymbol, valueTaskSymbol),
54+
SymbolKind.Method);
55+
}
56+
});
57+
}
58+
59+
private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbol testCleanupAttributeSymbol, INamedTypeSymbol? taskSymbol,
60+
INamedTypeSymbol? valueTaskSymbol)
61+
{
62+
var methodSymbol = (IMethodSymbol)context.Symbol;
63+
if (!methodSymbol.GetAttributes().Any(attr => SymbolEqualityComparer.Default.Equals(attr.AttributeClass, testCleanupAttributeSymbol)))
64+
{
65+
return;
66+
}
67+
68+
if (methodSymbol.MethodKind != MethodKind.Ordinary)
69+
{
70+
context.ReportDiagnostic(methodSymbol.CreateDiagnostic(OrdinaryRule, methodSymbol.Name));
71+
72+
// Do not check the other criteria, users should fix the method kind first.
73+
return;
74+
}
75+
76+
if (methodSymbol.IsAbstract)
77+
{
78+
context.ReportDiagnostic(methodSymbol.CreateDiagnostic(NotAbstractRule, methodSymbol.Name));
79+
}
80+
81+
if (methodSymbol.IsGenericMethod)
82+
{
83+
context.ReportDiagnostic(methodSymbol.CreateDiagnostic(NotGenericRule, methodSymbol.Name));
84+
}
85+
86+
if (methodSymbol.Parameters.Length > 0)
87+
{
88+
context.ReportDiagnostic(methodSymbol.CreateDiagnostic(NoParametersRule, methodSymbol.Name));
89+
}
90+
91+
if (methodSymbol.IsStatic)
92+
{
93+
context.ReportDiagnostic(methodSymbol.CreateDiagnostic(NotStaticRule, methodSymbol.Name));
94+
}
95+
96+
if (methodSymbol.ReturnsVoid && methodSymbol.IsAsync)
97+
{
98+
context.ReportDiagnostic(methodSymbol.CreateDiagnostic(NotAsyncVoidRule, methodSymbol.Name));
99+
}
100+
101+
if (methodSymbol.DeclaredAccessibility != Accessibility.Public || methodSymbol.GetResultantVisibility() != SymbolVisibility.Public)
102+
{
103+
context.ReportDiagnostic(methodSymbol.CreateDiagnostic(PublicRule, methodSymbol.Name));
104+
}
105+
106+
if (!methodSymbol.ReturnsVoid
107+
&& (taskSymbol is null || !SymbolEqualityComparer.Default.Equals(methodSymbol.ReturnType, taskSymbol))
108+
&& (valueTaskSymbol is null || !SymbolEqualityComparer.Default.Equals(methodSymbol.ReturnType, valueTaskSymbol)))
109+
{
110+
context.ReportDiagnostic(methodSymbol.CreateDiagnostic(ReturnTypeRule, methodSymbol.Name));
111+
}
112+
}
113+
}

0 commit comments

Comments
 (0)