From 3d9ddb6599b108437877d04b32b4a76fdde2d934 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Wed, 20 Mar 2024 10:24:52 +0100 Subject: [PATCH] Fix MSTEST0014 problems with arrays (#2607) --- .../DataRowShouldBeValidAnalyzer.cs | 48 +++++++++++----- .../DataRowShouldBeValidAnalyzerTests.cs | 55 +++++++++++++++++++ .../testsbaseline.txt | 2 + 3 files changed, 90 insertions(+), 15 deletions(-) diff --git a/src/Analyzers/MSTest.Analyzers/DataRowShouldBeValidAnalyzer.cs b/src/Analyzers/MSTest.Analyzers/DataRowShouldBeValidAnalyzer.cs index ba0782b17..1ab52cd7e 100644 --- a/src/Analyzers/MSTest.Analyzers/DataRowShouldBeValidAnalyzer.cs +++ b/src/Analyzers/MSTest.Analyzers/DataRowShouldBeValidAnalyzer.cs @@ -111,7 +111,7 @@ public sealed class DataRowShouldBeValidAnalyzer : DiagnosticAnalyzer private static void AnalyzeAttribute(SymbolAnalysisContext context, AttributeData attribute, IMethodSymbol methodSymbol) { - if (attribute.ApplicationSyntaxReference?.GetSyntax() is not { } syntax) + if (attribute.ApplicationSyntaxReference?.GetSyntax() is not { } dataRowSyntax) { return; } @@ -126,7 +126,7 @@ public sealed class DataRowShouldBeValidAnalyzer : DiagnosticAnalyzer // constructor argument. if (methodSymbol.Parameters.Length == 0) { - context.ReportDiagnostic(syntax.CreateDiagnostic( + context.ReportDiagnostic(dataRowSyntax.CreateDiagnostic( ArgumentCountMismatchRule, attribute.ConstructorArguments.Length, methodSymbol.Parameters.Length)); @@ -134,13 +134,11 @@ public sealed class DataRowShouldBeValidAnalyzer : DiagnosticAnalyzer } // 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) + if (methodSymbol.Parameters[^1].Type.Kind != SymbolKind.ArrayType) { - context.ReportDiagnostic(syntax.CreateDiagnostic( + context.ReportDiagnostic(dataRowSyntax.CreateDiagnostic( ArgumentCountMismatchRule, attribute.ConstructorArguments.Length, methodSymbol.Parameters.Length)); @@ -160,7 +158,7 @@ public sealed class DataRowShouldBeValidAnalyzer : DiagnosticAnalyzer if (IsArgumentCountMismatch(constructorArguments.Length, methodSymbol.Parameters)) { - context.ReportDiagnostic(syntax.CreateDiagnostic( + context.ReportDiagnostic(dataRowSyntax.CreateDiagnostic( ArgumentCountMismatchRule, constructorArguments.Length, methodSymbol.Parameters.Length)); @@ -169,34 +167,54 @@ public sealed class DataRowShouldBeValidAnalyzer : DiagnosticAnalyzer // Check constructor argument types match method parameter types. List<(int ConstructorArgumentIndex, int MethodParameterIndex)> typeMismatchIndices = new(); - for (int i = 0; i < constructorArguments.Length; ++i) + for (int currentArgumentIndex = 0; currentArgumentIndex < constructorArguments.Length; currentArgumentIndex++) { // Null is considered as default for non-nullable types. - if (constructorArguments[i].IsNull) + if (constructorArguments[currentArgumentIndex].IsNull) { continue; } - ITypeSymbol? argumentType = constructorArguments[i].Type; - ITypeSymbol paramType = (lastMethodParameterIsArray && i >= methodSymbol.Parameters.Length - 1) - ? ((IArrayTypeSymbol)lastMethodParameter.Type).ElementType - : methodSymbol.Parameters[i].Type; + ITypeSymbol? argumentType = constructorArguments[currentArgumentIndex].Type; + ITypeSymbol paramType = GetParameterType(methodSymbol, currentArgumentIndex, constructorArguments.Length); if (argumentType is not null && !argumentType.IsAssignableTo(paramType, context.Compilation)) { - typeMismatchIndices.Add((i, Math.Min(i, methodSymbol.Parameters.Length - 1))); + typeMismatchIndices.Add((currentArgumentIndex, Math.Min(currentArgumentIndex, methodSymbol.Parameters.Length - 1))); } } // Report diagnostics if there's any type mismatch. if (typeMismatchIndices.Count > 0) { - context.ReportDiagnostic(syntax.CreateDiagnostic( + context.ReportDiagnostic(dataRowSyntax.CreateDiagnostic( ArgumentTypeMismatchRule, string.Join(", ", typeMismatchIndices))); } } + private static ITypeSymbol GetParameterType(IMethodSymbol methodSymbol, int currentArgumentIndex, int argumentsCount) + { + if (currentArgumentIndex >= methodSymbol.Parameters.Length - 1) + { + IParameterSymbol lastParameter = methodSymbol.Parameters[^1]; + + // When last parameter is params, we want to check that the extra arguments match the type of the array + if (lastParameter.IsParams) + { + return ((IArrayTypeSymbol)lastParameter.Type).ElementType; + } + + // When only parameter is array and we have more than one argument, we want to check the array type + if (argumentsCount > 1 && methodSymbol.Parameters.Length == 1 && lastParameter.Type.Kind == SymbolKind.ArrayType) + { + return ((IArrayTypeSymbol)lastParameter.Type).ElementType; + } + } + + return methodSymbol.Parameters[currentArgumentIndex].Type; + } + private static bool IsArgumentCountMismatch(int constructorArgumentsLength, ImmutableArray methodParameters) { int optionalParametersCount = methodParameters.Count(x => x.HasExplicitDefaultValue); diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/DataRowShouldBeValidAnalyzerTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/DataRowShouldBeValidAnalyzerTests.cs index 4b63fd566..db439daa8 100644 --- a/test/UnitTests/MSTest.Analyzers.UnitTests/DataRowShouldBeValidAnalyzerTests.cs +++ b/test/UnitTests/MSTest.Analyzers.UnitTests/DataRowShouldBeValidAnalyzerTests.cs @@ -131,6 +131,29 @@ public sealed class DataRowShouldBeValidAnalyzerTests(ITestExecutionContext test await VerifyCS.VerifyAnalyzerAsync(code); } + public async Task WhenDataRowPassesOneItemAndParameterExpectsArray_Diagnostic() + { + var code = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [{|#0:DataRow(1)|}] + [TestMethod] + public void TestMethod1(object[] o) + { + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync( + code, + VerifyCS.Diagnostic(DataRowShouldBeValidAnalyzer.ArgumentTypeMismatchRule) + .WithLocation(0) + .WithArguments((0, 0))); + } + public async Task WhenDataRowHasThreeArgumentsAndMethodHasAnIntegerAndAnArrayArgument_Diagnostic() { var code = """ @@ -487,4 +510,36 @@ public sealed class DataRowShouldBeValidAnalyzerTests(ITestExecutionContext test VerifyCS.Diagnostic(DataRowShouldBeValidAnalyzer.ArgumentCountMismatchRule).WithLocation(1).WithArguments(1, 3), VerifyCS.Diagnostic(DataRowShouldBeValidAnalyzer.ArgumentCountMismatchRule).WithLocation(2).WithArguments(1, 5)); } + + public async Task Testfx_2606_NullArgumentForArray() + { + string code = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [DataTestMethod] + [DataRow( + "123", + new string[] { "something" }, + null)] + [DataRow( + "123", + null, + new string[] { "something" })] + public void TestSomething( + string x, + string[] y, + string[] z) + { + Assert.AreEqual("123", x); + Assert.IsNotNull(y); + Assert.IsNull(z); + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(code); + } } diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/testsbaseline.txt b/test/UnitTests/MSTest.Analyzers.UnitTests/testsbaseline.txt index ef9f6ecbf..622eb4d36 100644 --- a/test/UnitTests/MSTest.Analyzers.UnitTests/testsbaseline.txt +++ b/test/UnitTests/MSTest.Analyzers.UnitTests/testsbaseline.txt @@ -64,6 +64,7 @@ MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.ClassInitializeShouldBeValidAna MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.ClassInitializeShouldBeValidAnalyzerTests.WhenClassInitializeReturnTypeIsNotValid_Diagnostic() MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.ClassInitializeShouldBeValidAnalyzerTests.WhenClassInitializeReturnTypeIsValid_NoDiagnostic() MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DataRowShouldBeValidAnalyzerTests.DefaultArguments() +MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DataRowShouldBeValidAnalyzerTests.Testfx_2606_NullArgumentForArray() MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DataRowShouldBeValidAnalyzerTests.WhenDataRowHasArgumentMismatchWithTestMethod_Diagnostic() MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DataRowShouldBeValidAnalyzerTests.WhenDataRowHasArgumentMismatchWithTestMethod2_Diagnostic() MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DataRowShouldBeValidAnalyzerTests.WhenDataRowHasArgumentMismatchWithTestMethod3_Diagnostic() @@ -85,6 +86,7 @@ MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DataRowShouldBeValidAnalyzerTes MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DataRowShouldBeValidAnalyzerTests.WhenDataRowIsCorrectlyDefinedWithThreeArgumentsAndMethodHasArrayArgument_NoDiagnostic() MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DataRowShouldBeValidAnalyzerTests.WhenDataRowIsCorrectlyDefinedWithThreeArgumentsAndMethodHasParamsArgument_NoDiagnostic() MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DataRowShouldBeValidAnalyzerTests.WhenDataRowIsNotSetOnATestMethod_Diagnostic() +MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DataRowShouldBeValidAnalyzerTests.WhenDataRowPassesOneItemAndParameterExpectsArray_Diagnostic() MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DoNotNegateBooleanAssertionAnalyzerTests.WhenAssertionIsNegated_Diagnostic() MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DoNotNegateBooleanAssertionAnalyzerTests.WhenAssertionIsNotNegated_NoDiagnostic() MSTest.Analyzers.UnitTests.MSTest.Analyzers.Test.DoNotStoreStaticTestContextAnalyzerTests.WhenAssemblyInitializeOrClassInitialize_Diagnostic()