Skip to content

Commit 10256a0

Browse files
authored
MSTEST0014: DataRow should be valid (#2352)
1 parent 57eabd6 commit 10256a0

21 files changed

+1196
-0
lines changed

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

+1
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@ MSTEST0010 | Usage | Warning | ClassInitializeShouldBeValidAnalyzer, [Documentat
1111
MSTEST0011 | Usage | Warning | ClassCleanupShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0011)
1212
MSTEST0012 | Usage | Warning | AssemblyInitializeShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0012)
1313
MSTEST0013 | Usage | Warning | AssemblyCleanupShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0013)
14+
MSTEST0014 | Usage | Warning | DataRowShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/unit-testing-mstest-analyzers-MSTEST0014)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
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+
using MSTest.Analyzers.RoslynAnalyzerHelpers;
13+
14+
namespace MSTest.Analyzers;
15+
16+
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
17+
public sealed class DataRowShouldBeValidAnalyzer : DiagnosticAnalyzer
18+
{
19+
private static readonly LocalizableResourceString Title = new(nameof(Resources.DataRowShouldBeValidTitle), Resources.ResourceManager, typeof(Resources));
20+
private static readonly LocalizableResourceString Description = new(nameof(Resources.DataRowShouldBeValidDescription), Resources.ResourceManager, typeof(Resources));
21+
private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.DataRowShouldBeValidMessageFormat_OnTestMethod), Resources.ResourceManager, typeof(Resources));
22+
23+
internal static readonly DiagnosticDescriptor DataRowOnTestMethodRule = DiagnosticDescriptorHelper.Create(
24+
DiagnosticIds.DataRowShouldBeValidRuleId,
25+
Title,
26+
MessageFormat,
27+
Description,
28+
Category.Usage,
29+
DiagnosticSeverity.Warning,
30+
isEnabledByDefault: true);
31+
32+
internal static readonly DiagnosticDescriptor ArgumentCountMismatchRule = DataRowOnTestMethodRule
33+
.WithMessage(new(nameof(Resources.DataRowShouldBeValidMessageFormat_ArgumentCountMismatch), Resources.ResourceManager, typeof(Resources)));
34+
35+
internal static readonly DiagnosticDescriptor ArgumentTypeMismatchRule = DataRowOnTestMethodRule
36+
.WithMessage(new(nameof(Resources.DataRowShouldBeValidMessageFormat_ArgumentTypeMismatch), Resources.ResourceManager, typeof(Resources)));
37+
38+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
39+
= ImmutableArray.Create(DataRowOnTestMethodRule);
40+
41+
public override void Initialize(AnalysisContext context)
42+
{
43+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
44+
context.EnableConcurrentExecution();
45+
46+
context.RegisterCompilationStartAction(context =>
47+
{
48+
// No need to register any action if we don't find the TestMethodAttribute symbol since
49+
// the current analyzer checks if the DataRow attribute is applied on test methods. No
50+
// test methods, nothing to check.
51+
if (!context.Compilation.TryGetOrCreateTypeByMetadataName(
52+
WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestMethodAttribute,
53+
out var testMethodAttributeSymbol))
54+
{
55+
return;
56+
}
57+
58+
if (!context.Compilation.TryGetOrCreateTypeByMetadataName(
59+
WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingDataRowAttribute,
60+
out var dataRowAttributeSymbol))
61+
{
62+
return;
63+
}
64+
65+
context.RegisterSymbolAction(
66+
context => AnalyzeSymbol(context, testMethodAttributeSymbol, dataRowAttributeSymbol),
67+
SymbolKind.Method);
68+
});
69+
}
70+
71+
private static void AnalyzeSymbol(
72+
SymbolAnalysisContext context,
73+
INamedTypeSymbol testMethodAttributeSymbol,
74+
INamedTypeSymbol dataRowAttributeSymbol)
75+
{
76+
IMethodSymbol methodSymbol = (IMethodSymbol)context.Symbol;
77+
78+
bool isTestMethod = false;
79+
List<AttributeData> dataRowAttributes = new();
80+
foreach (var methodAttribute in methodSymbol.GetAttributes())
81+
{
82+
// Current method should be a test method or should inherit from the TestMethod attribute.
83+
// If it is, the current analyzer will trigger no diagnostic so it exits.
84+
if (methodAttribute.AttributeClass.Inherits(testMethodAttributeSymbol))
85+
{
86+
isTestMethod = true;
87+
}
88+
89+
if (SymbolEqualityComparer.Default.Equals(methodAttribute.AttributeClass, dataRowAttributeSymbol))
90+
{
91+
dataRowAttributes.Add(methodAttribute);
92+
}
93+
}
94+
95+
// Check if attribute is set on a test method.
96+
if (!isTestMethod)
97+
{
98+
if (dataRowAttributes.Count > 0)
99+
{
100+
context.ReportDiagnostic(methodSymbol.CreateDiagnostic(DataRowOnTestMethodRule));
101+
}
102+
103+
return;
104+
}
105+
106+
// Check each data row attribute.
107+
foreach (var attribute in dataRowAttributes)
108+
{
109+
AnalyzeAttribute(context, attribute, methodSymbol);
110+
}
111+
}
112+
113+
private static void AnalyzeAttribute(SymbolAnalysisContext context, AttributeData attribute, IMethodSymbol methodSymbol)
114+
{
115+
if (attribute.ApplicationSyntaxReference?.GetSyntax() is not { } syntax)
116+
{
117+
return;
118+
}
119+
120+
// No constructor arguments and no method parameters -> nothing to check.
121+
if (attribute.ConstructorArguments.Length == 0 && methodSymbol.Parameters.Length == 0)
122+
{
123+
return;
124+
}
125+
126+
// Count mismatch since there's zero method parameters but there's at least one DataRow
127+
// constructor argument.
128+
if (methodSymbol.Parameters.Length == 0)
129+
{
130+
context.ReportDiagnostic(syntax.CreateDiagnostic(
131+
ArgumentCountMismatchRule,
132+
attribute.ConstructorArguments.Length,
133+
methodSymbol.Parameters.Length));
134+
return;
135+
}
136+
137+
// Possible count mismatch depending on whether last method parameter is an array or not.
138+
IParameterSymbol lastMethodParameter = methodSymbol.Parameters.Last();
139+
bool lastMethodParameterIsArray = lastMethodParameter.Type.Kind == SymbolKind.ArrayType;
140+
if (attribute.ConstructorArguments.Length == 0)
141+
{
142+
if (!lastMethodParameterIsArray)
143+
{
144+
context.ReportDiagnostic(syntax.CreateDiagnostic(
145+
ArgumentCountMismatchRule,
146+
attribute.ConstructorArguments.Length,
147+
methodSymbol.Parameters.Length));
148+
}
149+
150+
return;
151+
}
152+
153+
// DataRow constructors have either zero or one argument(s). If we get here, we are
154+
// on the one argument case. Check if we match either of the array argument constructors
155+
// and expand the array argument if we do.
156+
ImmutableArray<TypedConstant> constructorArguments = attribute.ConstructorArguments;
157+
if (constructorArguments[0].Kind is TypedConstantKind.Array && !constructorArguments[0].IsNull)
158+
{
159+
constructorArguments = constructorArguments[0].Values;
160+
}
161+
162+
if (IsArgumentCountMismatch(constructorArguments.Length, methodSymbol.Parameters))
163+
{
164+
context.ReportDiagnostic(syntax.CreateDiagnostic(
165+
ArgumentCountMismatchRule,
166+
constructorArguments.Length,
167+
methodSymbol.Parameters.Length));
168+
return;
169+
}
170+
171+
// Check constructor argument types match method parameter types.
172+
List<(int ConstructorArgumentIndex, int MethodParameterIndex)> typeMismatchIndices = new();
173+
for (int i = 0; i < constructorArguments.Length; ++i)
174+
{
175+
// Null is considered as default for non-nullable types.
176+
if (constructorArguments[i].IsNull)
177+
{
178+
continue;
179+
}
180+
181+
ITypeSymbol? argumentType = constructorArguments[i].Type;
182+
ITypeSymbol paramType = (lastMethodParameterIsArray && i >= methodSymbol.Parameters.Length - 1)
183+
? ((IArrayTypeSymbol)lastMethodParameter.Type).ElementType
184+
: methodSymbol.Parameters[i].Type;
185+
186+
if (argumentType is not null && !argumentType.IsAssignableTo(paramType, context.Compilation))
187+
{
188+
typeMismatchIndices.Add((i, Math.Min(i, methodSymbol.Parameters.Length - 1)));
189+
}
190+
}
191+
192+
// Report diagnostics if there's any type mismatch.
193+
if (typeMismatchIndices.Count > 0)
194+
{
195+
context.ReportDiagnostic(syntax.CreateDiagnostic(
196+
ArgumentTypeMismatchRule,
197+
string.Join(", ", typeMismatchIndices)));
198+
}
199+
}
200+
201+
private static bool IsArgumentCountMismatch(int constructorArgumentsLength, ImmutableArray<IParameterSymbol> methodParameters)
202+
{
203+
IParameterSymbol lastMethodParameter = methodParameters.Last();
204+
205+
bool lastMethodParameterIsArray = lastMethodParameter.Type.Kind == SymbolKind.ArrayType;
206+
bool lastMethodParameterIsParams = lastMethodParameter.IsParams;
207+
bool uniqueMethodParameter = methodParameters.Length == 1;
208+
bool hasDefaultValue = lastMethodParameter.HasExplicitDefaultValue;
209+
210+
// Strict length matching should be done in the following cases:
211+
// 1. Last method parameter is not an array (so it's not params either) so it doesn't
212+
// matter if it's the only parameter or if it has any default value.
213+
// 2. Last method parameter is an array, but is not params, is not the only method
214+
// parameter and has no default value.
215+
bool strictMatch =
216+
!lastMethodParameterIsArray
217+
|| (lastMethodParameterIsArray
218+
&& !lastMethodParameterIsParams
219+
&& !uniqueMethodParameter
220+
&& !hasDefaultValue);
221+
222+
// 1. If strict matching is required then the constructor arguments count and method
223+
// parameter count must be the same.
224+
// 2. If strict matching is not required then the argument count check is relaxed and we
225+
// only need to make sure we don't have less constructor arguments than actual method
226+
// parameters.
227+
return strictMatch
228+
? constructorArgumentsLength != methodParameters.Length
229+
: constructorArgumentsLength < methodParameters.Length - 1;
230+
}
231+
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ internal static class WellKnownTypeNames
1212
public const string MicrosoftVisualStudioTestToolsUnitTestingAssemblyCleanupAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.AssemblyCleanupAttribute";
1313
public const string MicrosoftVisualStudioTestToolsUnitTestingCssIterationAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.CssIterationAttribute";
1414
public const string MicrosoftVisualStudioTestToolsUnitTestingCssProjectStructureAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.CssProjectStructureAttribute";
15+
public const string MicrosoftVisualStudioTestToolsUnitTestingDataRowAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.DataRowAttribute";
1516
public const string MicrosoftVisualStudioTestToolsUnitTestingDescriptionAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute";
1617
public const string MicrosoftVisualStudioTestToolsUnitTestingDiscoverInternalsAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.DiscoverInternalsAttribute";
1718
public const string MicrosoftVisualStudioTestToolsUnitTestingDoNotParallelizeAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.DoNotParallelizeAttribute";

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

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

src/Analyzers/MSTest.Analyzers/Resources.resx

+18
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,24 @@
262262
<data name="ClassInitializeShouldBeValidTitle" xml:space="preserve">
263263
<value>ClassInitialize methods should have valid layout</value>
264264
</data>
265+
<data name="DataRowShouldBeValidDescription" xml:space="preserve">
266+
<value>DataRow entry should have the following layout to be valid:
267+
- should only be set on a test method;
268+
- argument count should match method argument count;
269+
- argument type should match method argument type.</value>
270+
</data>
271+
<data name="DataRowShouldBeValidMessageFormat_ArgumentCountMismatch" xml:space="preserve">
272+
<value>DataRow argument count should match method parameter count (constructor arguments: {0}, method parameters: {1})</value>
273+
</data>
274+
<data name="DataRowShouldBeValidMessageFormat_ArgumentTypeMismatch" xml:space="preserve">
275+
<value>DataRow argument type should match method parameter type. Mismatches occur at indices: {0}</value>
276+
</data>
277+
<data name="DataRowShouldBeValidMessageFormat_OnTestMethod" xml:space="preserve">
278+
<value>DataRow should only be set on a test method</value>
279+
</data>
280+
<data name="DataRowShouldBeValidTitle" xml:space="preserve">
281+
<value>DataRow should be valid</value>
282+
</data>
265283
<data name="PublicTypeShouldBeTestClassDescription" xml:space="preserve">
266284
<value>It's considered a good practice to have only test classes marked public in a test project.</value>
267285
</data>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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;
5+
using System.Collections.Generic;
6+
using System.Diagnostics.CodeAnalysis;
7+
using System.Text;
8+
9+
using Microsoft.CodeAnalysis;
10+
11+
namespace MSTest.Analyzers.RoslynAnalyzerHelpers;
12+
13+
internal static class TypeAssignabilityChecker
14+
{
15+
public static bool IsAssignableTo(
16+
[NotNullWhen(returnValue: true)] this ITypeSymbol? fromSymbol,
17+
[NotNullWhen(returnValue: true)] ITypeSymbol? toSymbol,
18+
Compilation compilation)
19+
=> fromSymbol != null && toSymbol != null && compilation.ClassifyCommonConversion(fromSymbol, toSymbol).IsImplicit;
20+
}

src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf

+31
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,37 @@
253253
<target state="new">ClassInitialize methods should have valid layout</target>
254254
<note />
255255
</trans-unit>
256+
<trans-unit id="DataRowShouldBeValidDescription">
257+
<source>DataRow entry should have the following layout to be valid:
258+
- should only be set on a test method;
259+
- argument count should match method argument count;
260+
- argument type should match method argument type.</source>
261+
<target state="new">DataRow entry should have the following layout to be valid:
262+
- should only be set on a test method;
263+
- argument count should match method argument count;
264+
- argument type should match method argument type.</target>
265+
<note />
266+
</trans-unit>
267+
<trans-unit id="DataRowShouldBeValidMessageFormat_ArgumentCountMismatch">
268+
<source>DataRow argument count should match method parameter count (constructor arguments: {0}, method parameters: {1})</source>
269+
<target state="new">DataRow argument count should match method parameter count (constructor arguments: {0}, method parameters: {1})</target>
270+
<note />
271+
</trans-unit>
272+
<trans-unit id="DataRowShouldBeValidMessageFormat_ArgumentTypeMismatch">
273+
<source>DataRow argument type should match method parameter type. Mismatches occur at indices: {0}</source>
274+
<target state="new">DataRow argument type should match method parameter type. Mismatches occur at indices: {0}</target>
275+
<note />
276+
</trans-unit>
277+
<trans-unit id="DataRowShouldBeValidMessageFormat_OnTestMethod">
278+
<source>DataRow should only be set on a test method</source>
279+
<target state="new">DataRow should only be set on a test method</target>
280+
<note />
281+
</trans-unit>
282+
<trans-unit id="DataRowShouldBeValidTitle">
283+
<source>DataRow should be valid</source>
284+
<target state="new">DataRow should be valid</target>
285+
<note />
286+
</trans-unit>
256287
<trans-unit id="PublicTypeShouldBeTestClassDescription">
257288
<source>It's considered a good practice to have only test classes marked public in a test project.</source>
258289
<target state="translated">Osvědčeným postupem je označit jako veřejné v testovacím projektu jen testovací třídy.</target>

0 commit comments

Comments
 (0)