Add type & name the MemberData points to tothe messages for xUnit1039…

… and xUnit1040 since the location it highlights is the parameter, not the MemberData attribute
xunit · Nov 7, 2023 · 98e842b · 98e842b
1 parent 93e31af
commit 98e842b
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 42 deletions.
diff --git a/src/xunit.analyzers.tests/Analyzers/X1000/MemberDataShouldReferenceValidMemberTests.cs b/src/xunit.analyzers.tests/Analyzers/X1000/MemberDataShouldReferenceValidMemberTests.cs
@@ -861,7 +861,7 @@ public void TestMethod(string f) {{ }}
 				.Diagnostic("xUnit1039")
 				.WithSpan(6, 28, 6, 34)
 				.WithSeverity(DiagnosticSeverity.Error)
-				.WithArguments(type, "f")
+				.WithArguments(type, "TestClass", "TestData", "f")
 		};
 
 		await Verify.VerifyAnalyzer(LanguageVersion.CSharp10, source, expected);
@@ -889,12 +889,12 @@ public void TestMethod(int n, string f) {{ }}
 				.Diagnostic("xUnit1039")
 				.WithSpan(7, 28, 7, 31)
 				.WithSeverity(DiagnosticSeverity.Error)
-				.WithArguments("int?", "n"),
+				.WithArguments("int?", "TestClass", "TestData", "n"),
 			Verify
 				.Diagnostic("xUnit1040")
 				.WithSpan(7, 35, 7, 41)
 				.WithSeverity(DiagnosticSeverity.Warning)
-				.WithArguments("string?", "f")
+				.WithArguments("string?", "TestClass", "TestData", "f")
 		};
 
 		await Verify.VerifyAnalyzer(LanguageVersion.CSharp10, source, expected);

diff --git a/src/xunit.analyzers/Utility/Descriptors.cs b/src/xunit.analyzers/Utility/Descriptors.cs
@@ -392,7 +392,7 @@ static DiagnosticDescriptor Rule(
 			"The type argument to TheoryData is not compatible with the type of the corresponding test method parameter",
 			Usage,
 			Error,
-			"The type argument {0} to TheoryData is not compatible with the type of the corresponding test method parameter {1}."
+			"The type argument {0} from {1}.{2} is not compatible with the type of the corresponding test method parameter {3}."
 		);
 
 	public static DiagnosticDescriptor X1040_MemberDataTheoryDataTypeArgumentsMustMatchTestMethodParameters_IncompatibleNullability { get; } =
@@ -401,7 +401,7 @@ static DiagnosticDescriptor Rule(
 			"The type argument to TheoryData is nullable, while the type of the corresponding test method parameter is not",
 			Usage,
 			Warning,
-			"The type argument {0} to TheoryData is nullable, while the type of the corresponding test method parameter {1} is not. Make the TheoryData type non-nullable, or make the test method parameter nullable."
+			"The type argument {0} from {1}.{2} is nullable, while the type of the corresponding test method parameter {3} is not. Make the TheoryData type non-nullable, or make the test method parameter nullable."
 		);
 
 	// Placeholder for rule X1041

diff --git a/src/xunit.analyzers/X1000/MemberDataShouldReferenceValidMember.cs b/src/xunit.analyzers/X1000/MemberDataShouldReferenceValidMember.cs
@@ -103,14 +103,14 @@ public override void AnalyzeCompilation(
 				}
 
 				// Ensure we pointing to a field, method, or property
-				var memberType = memberSymbol switch
+				var memberReturnType = memberSymbol switch
 				{
 					IFieldSymbol field => field.Type,
 					IMethodSymbol method => method.ReturnType,
 					IPropertySymbol prop => prop.Type,
 					_ => null,
 				};
-				if (memberType is null)
+				if (memberReturnType is null)
 				{
 					ReportIncorrectMemberType(context, attributeSyntax);
 					return;
@@ -138,7 +138,7 @@ public override void AnalyzeCompilation(
 					ReportNonStatic(context, attributeSyntax, memberProperties);
 
 				// Make sure the member returns a compatible type
-				VerifyDataSourceReturnType(context, compilation, xunitContext, memberType, memberProperties, attributeSyntax);
+				VerifyDataSourceReturnType(context, compilation, xunitContext, memberReturnType, memberProperties, attributeSyntax);
 
 				// Make sure public properties have a public getter
 				if (memberSymbol.Kind == SymbolKind.Property && memberSymbol.DeclaredAccessibility == Accessibility.Public)
@@ -150,7 +150,7 @@ public override void AnalyzeCompilation(
 					}
 
 				// If the member returns TheoryData, ensure that the types are compatible
-				VerifyTheoryDataUsage(semanticModel, context, testMethod, theoryDataTypes, memberType, memberName, attributeSyntax);
+				VerifyTheoryDataUsage(semanticModel, context, testMethod, theoryDataTypes, memberReturnType, memberName, declaredMemberTypeSymbol, attributeSyntax);
 
 				// Get the arguments that are to be passed to the method
 				var extraArguments = attributeSyntax.ArgumentList.Arguments.Skip(1).TakeWhile(a => a.NameEquals is null).ToList();
@@ -356,14 +356,16 @@ static void ReportMemberMethodTheoryDataIncompatibleType(
 		SyntaxNodeAnalysisContext context,
 		Location location,
 		ITypeSymbol theoryDataTypeParameter,
-		IParameterSymbol parameter,
-		ImmutableDictionary<string, string?>.Builder builder) =>
+		INamedTypeSymbol memberType,
+		string memberName,
+		IParameterSymbol parameter) =>
 			context.ReportDiagnostic(
 				Diagnostic.Create(
 					Descriptors.X1039_MemberDataTheoryDataTypeArgumentsMustMatchTestMethodParameters_IncompatibleTypes,
 					location,
-					builder.ToImmutable(),
 					SymbolDisplay.ToDisplayString(theoryDataTypeParameter),
+					memberType.Name,
+					memberName,
 					parameter.Name
 				)
 			);
@@ -372,14 +374,16 @@ static void ReportMemberMethodTheoryDataNullability(
 		SyntaxNodeAnalysisContext context,
 		Location location,
 		ITypeSymbol theoryDataTypeParameter,
-		IParameterSymbol parameter,
-		ImmutableDictionary<string, string?>.Builder builder) =>
+		INamedTypeSymbol memberType,
+		string memberName,
+		IParameterSymbol parameter) =>
 			context.ReportDiagnostic(
 				Diagnostic.Create(
 					Descriptors.X1040_MemberDataTheoryDataTypeArgumentsMustMatchTestMethodParameters_IncompatibleNullability,
 					location,
-					builder.ToImmutable(),
 					SymbolDisplay.ToDisplayString(theoryDataTypeParameter),
+					memberType.Name,
+					memberName,
 					parameter.Name
 				)
 			);
@@ -615,15 +619,21 @@ static void VerifyTheoryDataUsage(
 		SyntaxNodeAnalysisContext context,
 		MethodDeclarationSyntax testMethod,
 		Dictionary<int, INamedTypeSymbol> theoryDataTypes,
-		ITypeSymbol? memberType,
+		ITypeSymbol? memberReturnType,
 		string memberName,
+		ITypeSymbol memberType,
 		AttributeSyntax attributeSyntax)
 	{
-		if (memberType is not INamedTypeSymbol methodType || !methodType.IsGenericType)
+		if (memberType is not INamedTypeSymbol namedMemberType)
+			return;
+
+		if (memberReturnType is not INamedTypeSymbol namedReturnType || !namedReturnType.IsGenericType)
 			return;
 
-		var methodTypeArguments = methodType.TypeArguments;
-		if (!SymbolEqualityComparer.Default.Equals(theoryDataTypes[methodTypeArguments.Length], methodType.OriginalDefinition))
+		var returnTypeArguments = namedReturnType.TypeArguments;
+		if (!theoryDataTypes.TryGetValue(returnTypeArguments.Length, out var theoryDataType))
+			return;
+		if (!SymbolEqualityComparer.Default.Equals(theoryDataType, namedReturnType.OriginalDefinition))
 			return;
 
 		var testMethodSymbol = semanticModel.GetDeclaredSymbol(testMethod, context.CancellationToken);
@@ -633,8 +643,8 @@ static void VerifyTheoryDataUsage(
 		var testMethodParameterSymbols = testMethodSymbol.Parameters;
 		var testMethodParameterSyntaxes = testMethod.ParameterList.Parameters;
 
-		if (testMethodParameterSymbols.Length > methodTypeArguments.Length
-			&& testMethodParameterSymbols.Skip(methodTypeArguments.Length).Any(p => !p.IsOptional && !p.IsParams))
+		if (testMethodParameterSymbols.Length > returnTypeArguments.Length
+			&& testMethodParameterSymbols.Skip(returnTypeArguments.Length).Any(p => !p.IsOptional && !p.IsParams))
 		{
 			var builder = ImmutableDictionary.CreateBuilder<string, string?>();
 			builder[Constants.Properties.MemberName] = memberName;
@@ -644,7 +654,7 @@ static void VerifyTheoryDataUsage(
 		}
 
 		if (testMethodParameterSymbols.Length > 0
-			&& testMethodParameterSymbols.Length < methodTypeArguments.Length
+			&& testMethodParameterSymbols.Length < returnTypeArguments.Length
 			&& !testMethodParameterSymbols.Last().IsParams)
 		{
 			var builder = ImmutableDictionary.CreateBuilder<string, string?>();
@@ -655,7 +665,7 @@ static void VerifyTheoryDataUsage(
 		}
 
 		int typeArgumentIdx = 0, parameterTypeIdx = 0;
-		for (; typeArgumentIdx < methodTypeArguments.Length && parameterTypeIdx < testMethodParameterSymbols.Length; typeArgumentIdx++)
+		for (; typeArgumentIdx < returnTypeArguments.Length && parameterTypeIdx < testMethodParameterSymbols.Length; typeArgumentIdx++)
 		{
 			var parameterSyntax = testMethodParameterSyntaxes[parameterTypeIdx];
 			if (parameterSyntax.Type is null)
@@ -670,33 +680,20 @@ static void VerifyTheoryDataUsage(
 					? paramsArraySymbol.ElementType
 					: parameter.Type;
 
-			var typeArgument = methodTypeArguments[typeArgumentIdx];
+			var typeArgument = returnTypeArguments[typeArgumentIdx];
 			if (typeArgument is null)
 				continue;
 
 			if (!parameterType.IsAssignableFrom(typeArgument))
-			{
-				var builder = ImmutableDictionary.CreateBuilder<string, string?>();
-				builder[Constants.Properties.ParameterIndex] = typeArgumentIdx.ToString();
-				builder[Constants.Properties.MemberName] = memberName;
-
-				ReportMemberMethodTheoryDataIncompatibleType(context, parameterSyntax.Type.GetLocation(), typeArgument, parameter, builder);
-			}
+				ReportMemberMethodTheoryDataIncompatibleType(context, parameterSyntax.Type.GetLocation(), typeArgument, namedMemberType, memberName, parameter);
 
 			// Nullability of value types is handled by the type compatibility test,
 			// but nullability of reference types isn't
-			if (parameterType.IsReferenceType && typeArgument.IsReferenceType)
-			{
-				if (parameterType.NullableAnnotation == NullableAnnotation.NotAnnotated
+			if (parameterType.IsReferenceType
+					&& typeArgument.IsReferenceType
+					&& parameterType.NullableAnnotation == NullableAnnotation.NotAnnotated
 					&& typeArgument.NullableAnnotation == NullableAnnotation.Annotated)
-				{
-					var builder = ImmutableDictionary.CreateBuilder<string, string?>();
-					builder[Constants.Properties.ParameterIndex] = typeArgumentIdx.ToString();
-					builder[Constants.Properties.MemberName] = memberName;
-
-					ReportMemberMethodTheoryDataNullability(context, parameterSyntax.Type.GetLocation(), typeArgument, parameter, builder);
-				}
-			}
+				ReportMemberMethodTheoryDataNullability(context, parameterSyntax.Type.GetLocation(), typeArgument, namedMemberType, memberName, parameter);
 
 			if (!parameter.IsParams)
 			{