MSTEST0014: DataRow should be valid

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

@@ -11,3 +11,4 @@ MSTEST0010 | Usage | Warning | ClassInitializeShouldBeValidAnalyzer, [Documentat
 MSTEST0011 | Usage | Warning | ClassCleanupShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0011)
 MSTEST0012 | Usage | Warning | AssemblyInitializeShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0012)
 MSTEST0013 | Usage | Warning | AssemblyCleanupShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0013)
+MSTEST0014 | Usage | Warning | DataRowShouldBeValidAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/core/testing/unit-testing-mstest-analyzers-MSTEST0014)

src/Analyzers/MSTest.Analyzers/DataRowShouldBeValidAnalyzer.cs

@@ -0,0 +1,214 @@
+// 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;
+using MSTest.Analyzers.RoslynAnalyzerHelpers;
+
+namespace MSTest.Analyzers;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+public sealed class DataRowShouldBeValidAnalyzer : DiagnosticAnalyzer
+{
+    private static readonly LocalizableResourceString Title = new(nameof(Resources.DataRowShouldBeValidTitle), Resources.ResourceManager, typeof(Resources));
+    private static readonly LocalizableResourceString Description = new(nameof(Resources.DataRowShouldBeValidDescription), Resources.ResourceManager, typeof(Resources));
+    private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.DataRowShouldBeValidMessageFormat_OnTestMethod), Resources.ResourceManager, typeof(Resources));
+
+    internal static readonly DiagnosticDescriptor DataRowOnTestMethodRule = DiagnosticDescriptorHelper.Create(
+        DiagnosticIds.DataRowShouldBeValidRuleId,
+        Title,
+        MessageFormat,
+        Description,
+        Category.Usage,
+        DiagnosticSeverity.Warning,
+        isEnabledByDefault: true);
+
+    internal static readonly DiagnosticDescriptor ArgumentCountMismatchRule = DataRowOnTestMethodRule
+        .WithMessage(new(nameof(Resources.DataRowShouldBeValidMessageFormat_ArgumentCountMismatch), Resources.ResourceManager, typeof(Resources)));
+
+    internal static readonly DiagnosticDescriptor ArgumentTypeMismatchRule = DataRowOnTestMethodRule
+        .WithMessage(new(nameof(Resources.DataRowShouldBeValidMessageFormat_ArgumentTypeMismatch), Resources.ResourceManager, typeof(Resources)));
+
+    public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; }
+       = ImmutableArray.Create(DataRowOnTestMethodRule);
+
+    public override void Initialize(AnalysisContext context)
+    {
+        context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+        context.EnableConcurrentExecution();
+
+        context.RegisterCompilationStartAction(context =>
+        {
+            // No need to register any action if we don't find the TestMethodAttribute symbol since
+            // the current analyzer checks if the DataRow attribute is applied on test methods. No
+            // test methods, nothing to check.
+            if (!context.Compilation.TryGetOrCreateTypeByMetadataName(
+                WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingTestMethodAttribute,
+                out var testMethodAttributeSymbol))
+            {
+                return;
+            }
+
+            if (!context.Compilation.TryGetOrCreateTypeByMetadataName(
+                WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingDataRowAttribute,
+                out var dataRowAttributeSymbol))
+            {
+                return;
+            }
+
+            context.RegisterSymbolAction(
+                context => AnalyzeSymbol(context, testMethodAttributeSymbol, dataRowAttributeSymbol),
+                SymbolKind.Method);
+        });
+    }
+
+    private static void AnalyzeSymbol(
+        SymbolAnalysisContext context,
+        INamedTypeSymbol testMethodAttributeSymbol,
+        INamedTypeSymbol dataRowAttributeSymbol)
+    {
+        IMethodSymbol methodSymbol = (IMethodSymbol)context.Symbol;
+
+        bool isTestMethod = false;
+        List<AttributeData> dataRowAttributes = new();
+        foreach (var methodAttribute in methodSymbol.GetAttributes())
+        {
+            // Current method should be a test method or should inherit from the TestMethod attribute.
+            // If it is, the current analyzer will trigger no diagnostic so it exits.
+            if (methodAttribute.AttributeClass.Inherits(testMethodAttributeSymbol))
+            {
+                isTestMethod = true;
+            }
+
+            if (SymbolEqualityComparer.Default.Equals(methodAttribute.AttributeClass, dataRowAttributeSymbol))
+            {
+                dataRowAttributes.Add(methodAttribute);
+            }
+        }
+
+        // Check if attribute is set on a test method.
+        if (!isTestMethod)
+        {
+            if (dataRowAttributes.Count > 0)
+            {
+                context.ReportDiagnostic(methodSymbol.CreateDiagnostic(DataRowOnTestMethodRule));
+            }
+
+            return;
+        }
+
+        // Check each data row attribute.
+        foreach (var attribute in dataRowAttributes)
+        {
+            AnalyzeAttribute(context, attribute, methodSymbol);
+        }
+    }
+
+    private static void AnalyzeAttribute(SymbolAnalysisContext context, AttributeData attribute, IMethodSymbol methodSymbol)
+    {
+        if (attribute.ApplicationSyntaxReference?.GetSyntax() is not { } syntax)
+        {
+            return;
+        }
+
+        // No constructor arguments and no method parameters -> nothing to check.
+        if (attribute.ConstructorArguments.Length == 0 && methodSymbol.Parameters.Length == 0)
+        {
+            return;
+        }
+
+        // Count mismatch since there's zero method parameters but there's at least one DataRow
+        // constructor argument.
+        if (methodSymbol.Parameters.Length == 0)
+        {
+            context.ReportDiagnostic(syntax.CreateDiagnostic(
+                ArgumentCountMismatchRule,
+                attribute.ConstructorArguments.Length,
+                methodSymbol.Parameters.Length));
+            return;
+        }
+
+        // Possible count mismatch depending on whether last method parameter is an array or not.
+        IParameterSymbol lastMethodParameter = methodSymbol.Parameters.Last();
+        bool lastMethodParameterIsArray = lastMethodParameter.Type.Kind == SymbolKind.ArrayType;
+        if (attribute.ConstructorArguments.Length == 0)
+        {
+            if (!lastMethodParameterIsArray)
+            {
+                context.ReportDiagnostic(syntax.CreateDiagnostic(
+                    ArgumentCountMismatchRule,
+                    attribute.ConstructorArguments.Length,
+                    methodSymbol.Parameters.Length));
+            }
+
+            return;
+        }
+
+        // DataRow constructors have either zero or one argument(s). If we get here, we are
+        // on the one argument case. Check if we match either of the array argument constructors
+        // and expand the array argument if we do.
+        ImmutableArray<TypedConstant> constructorArguments = attribute.ConstructorArguments;
+        if (constructorArguments[0].Kind is TypedConstantKind.Array && !constructorArguments[0].IsNull)
+        {
+            constructorArguments = constructorArguments[0].Values;
+        }
+
+        if (IsArgumentCountMismatch(constructorArguments.Length, methodSymbol.Parameters.Length, lastMethodParameterIsArray))
+        {
+            context.ReportDiagnostic(syntax.CreateDiagnostic(
+                ArgumentCountMismatchRule,
+                constructorArguments.Length,
+                methodSymbol.Parameters.Length));
+            return;
+        }
+
+        // Check constructor argument types match method parameter types.
+        List<(int ConstructorArgumentIndex, int MethodParameterIndex)> typeMismatchIndices = new();
+        for (int i = 0, j = 0; i < constructorArguments.Length; ++i, ++j)
+        {
+            if (constructorArguments[i].IsNull)
+            {
+                continue;
+            }
+
+            ITypeSymbol? argumentType = constructorArguments[i].Type;
+            ITypeSymbol paramType = (lastMethodParameterIsArray && j >= methodSymbol.Parameters.Length - 1)
+                ? ((IArrayTypeSymbol)lastMethodParameter.Type).ElementType
+                : methodSymbol.Parameters[j].Type;
+
+            if (argumentType is not null && !argumentType.IsAssignableTo(paramType, context.Compilation))
+            {
+                typeMismatchIndices.Add((i, j));
+            }
+        }
+
+        // Report diagnostics if there's any type mismatch.
+        if (typeMismatchIndices.Count > 0)
+        {
+            context.ReportDiagnostic(syntax.CreateDiagnostic(
+                ArgumentTypeMismatchRule,
+                FormatTypeMismatchIndexList(typeMismatchIndices)));
+        }
+    }
+
+    private static bool IsArgumentCountMismatch(int constructorArgumentsLength, int methodParametersLength, bool lastMethodParameterIsArray)
+    {
+        // 1. If last method parameter is not an array the lengths must be the same.
+        // 2. If last method parameter is an array the argument count check is relaxed and we only
+        //    need to make sure we don't have less constructor arguments than actual method paramters.
+        return lastMethodParameterIsArray
+            ? constructorArgumentsLength < methodParametersLength - 1
+            : constructorArgumentsLength != methodParametersLength;
+    }
+
+    private static string FormatTypeMismatchIndexList(List<(int ConstructorArgumentIndex, int MethodParameterIndex)> typeMismatchIndices)
+    {
+        return string.Join(", ", typeMismatchIndices.ToArray());
+    }
+}

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

@@ -12,6 +12,7 @@ internal static class WellKnownTypeNames
     public const string MicrosoftVisualStudioTestToolsUnitTestingAssemblyCleanupAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.AssemblyCleanupAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingCssIterationAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.CssIterationAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingCssProjectStructureAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.CssProjectStructureAttribute";
+    public const string MicrosoftVisualStudioTestToolsUnitTestingDataRowAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.DataRowAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingDescriptionAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingDiscoverInternalsAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.DiscoverInternalsAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingDoNotParallelizeAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.DoNotParallelizeAttribute";

src/Analyzers/MSTest.Analyzers/Resources.resx

@@ -262,6 +262,24 @@
   <data name="ClassInitializeShouldBeValidTitle" xml:space="preserve">
     <value>ClassInitialize methods should have valid layout</value>
   </data>
+  <data name="DataRowShouldBeValidDescription" xml:space="preserve">
+    <value>DataRow entry should have the following layout to be valid:
+- should only be set on a test method;
+- argument count should match method argument count;
+- argument type should match method argument type.</value>
+  </data>
+  <data name="DataRowShouldBeValidMessageFormat_ArgumentCountMismatch" xml:space="preserve">
+    <value>DataRow argument count should match method parameter count (constructor arguments: {0}, method parameters: {1})</value>
+  </data>
+  <data name="DataRowShouldBeValidMessageFormat_ArgumentTypeMismatch" xml:space="preserve">
+    <value>DataRow argument type should match method parameter type. Mismatches occur at indices: {0}</value>
+  </data>
+  <data name="DataRowShouldBeValidMessageFormat_OnTestMethod" xml:space="preserve">
+    <value>DataRow should only be set on a test method</value>
+  </data>
+  <data name="DataRowShouldBeValidTitle" xml:space="preserve">
+    <value>DataRow should be valid</value>
+  </data>
   <data name="PublicTypeShouldBeTestClassDescription" xml:space="preserve">
     <value>It's considered a good practice to have only test classes marked public in a test project.</value>
   </data>

src/Analyzers/MSTest.Analyzers/RoslynAnalyzerHelpers/TypeAssignabilityChecker.cs

@@ -0,0 +1,20 @@
+// 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;
+using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
+using System.Text;
+
+using Microsoft.CodeAnalysis;
+
+namespace MSTest.Analyzers.RoslynAnalyzerHelpers;
+
+internal static class TypeAssignabilityChecker
+{
+    public static bool IsAssignableTo(
+        [NotNullWhen(returnValue: true)] this ITypeSymbol? fromSymbol,
+        [NotNullWhen(returnValue: true)] ITypeSymbol? toSymbol,
+        Compilation compilation)
+        => fromSymbol != null && toSymbol != null && compilation.ClassifyCommonConversion(fromSymbol, toSymbol).IsImplicit;
+}

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

@@ -253,6 +253,37 @@
         <target state="new">ClassInitialize methods should have valid layout</target>
         <note />
       </trans-unit>
+      <trans-unit id="DataRowShouldBeValidDescription">
+        <source>DataRow entry should have the following layout to be valid:
+- should only be set on a test method;
+- argument count should match method argument count;
+- argument type should match method argument type.</source>
+        <target state="new">DataRow entry should have the following layout to be valid:
+- should only be set on a test method;
+- argument count should match method argument count;
+- argument type should match method argument type.</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="DataRowShouldBeValidMessageFormat_ArgumentCountMismatch">
+        <source>DataRow argument count should match method parameter count (constructor arguments: {0}, method parameters: {1})</source>
+        <target state="new">DataRow argument count should match method parameter count (constructor arguments: {0}, method parameters: {1})</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="DataRowShouldBeValidMessageFormat_ArgumentTypeMismatch">
+        <source>DataRow argument type should match method parameter type. Mismatches occur at indices: {0}</source>
+        <target state="new">DataRow argument type should match method parameter type. Mismatches occur at indices: {0}</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="DataRowShouldBeValidMessageFormat_OnTestMethod">
+        <source>DataRow should only be set on a test method</source>
+        <target state="new">DataRow should only be set on a test method</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="DataRowShouldBeValidTitle">
+        <source>DataRow should be valid</source>
+        <target state="new">DataRow should be valid</target>
+        <note />
+      </trans-unit>
       <trans-unit id="PublicTypeShouldBeTestClassDescription">
         <source>It's considered a good practice to have only test classes marked public in a test project.</source>
         <target state="translated">Osvědčeným postupem je označit jako veřejné v testovacím projektu jen testovací třídy.</target>