Skip to content

Commit

Permalink
xunit/xunit#2827: Passing array for params array incorrectly triggers…
Browse files Browse the repository at this point in the history
… xUnit1039
  • Loading branch information
bradwilson committed Nov 27, 2023
1 parent 81de769 commit f1e0908
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ public class TestClass {{
public static {memberType} Data;
[Xunit.MemberData(nameof(Data))]
public void TestMethod() {{ }}
public void TestMethod(int _) {{ }}
}}";

await Verify.VerifyAnalyzer(source);
Expand Down Expand Up @@ -925,6 +925,28 @@ public void TestMethod(int n) {{ }}
await Verify.VerifyAnalyzer(LanguageVersion.CSharp10, source, expected);
}

[Fact]
public async void FindWarning_WhenExtraTypeExistsPastArrayForParamsArray()
{
var source = @"
using Xunit;
public class TestClass {
public static TheoryData<int, string[], string> TestData = new TheoryData<int, string[], string>();
[MemberData(nameof(TestData))]
public void PuzzleOne(int _1, params string[] _2) { }
}";

var expected =
Verify
.Diagnostic("xUnit1038")
.WithSpan(7, 6, 7, 34)
.WithSeverity(DiagnosticSeverity.Error);

await Verify.VerifyAnalyzer(source, expected);
}

[Theory]
[MemberData(nameof(MemberSyntaxAndArgs))]
public async void FindWarning_IfHasValidTheoryDataMemberWithNotEnoughTypeParameters(
Expand Down Expand Up @@ -963,6 +985,61 @@ public static TheoryData<string, string, string> TypeWithMemberSyntaxAndArgs()
return result;
}

[Fact]
public async void DoesNotFindWarning_WhenPassingMultipleValuesForParamsArray()
{
var source = @"
using Xunit;
public class TestClass {
public static TheoryData<int, string, string> TestData = new TheoryData<int, string, string>();
[MemberData(nameof(TestData))]
public void PuzzleOne(int _1, params string[] _2) { }
}";

await Verify.VerifyAnalyzer(source);
}

[Fact]
public async void DoesNotFindWarning_WhenPassingArrayForParamsArray()
{
var source = @"
using Xunit;
public class TestClass {
public static TheoryData<int, string[]> TestData = new TheoryData<int, string[]>();
[MemberData(nameof(TestData))]
public void PuzzleOne(int _1, params string[] _2) { }
}";

await Verify.VerifyAnalyzer(source);
}

[Fact]
public async void FindWarning_WithExtraValueNotCompatibleWithParamsArray()
{
var source = @"
using Xunit;
public class TestClass {
public static TheoryData<int, string, int> TestData = new TheoryData<int, string, int>();
[MemberData(nameof(TestData))]
public void PuzzleOne(int _1, params string[] _2) { }
}";

var expected =
Verify
.Diagnostic("xUnit1039")
.WithSpan(8, 42, 8, 50)
.WithSeverity(DiagnosticSeverity.Error)
.WithArguments("int", "TestClass", "TestData", "_2");

await Verify.VerifyAnalyzer(source, expected);
}

[Theory]
[MemberData(nameof(TypeWithMemberSyntaxAndArgs))]
public async void FindWarning_IfHasValidTheoryDataMemberWithIncompatibleTypeParameters(
Expand Down
41 changes: 23 additions & 18 deletions src/xunit.analyzers/X1000/MemberDataShouldReferenceValidMember.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Reflection;
using System.Reflection.Metadata;
using System.Runtime.InteropServices.ComTypes;
using System.Threading;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
Expand Down Expand Up @@ -663,17 +660,6 @@ static void VerifyTheoryDataUsage(
return;
}

if (testMethodParameterSymbols.Length > 0
&& testMethodParameterSymbols.Length < returnTypeArguments.Length
&& !testMethodParameterSymbols.Last().IsParams)
{
var builder = ImmutableDictionary.CreateBuilder<string, string?>();
builder[Constants.Properties.MemberName] = memberName;

ReportMemberMethodTheoryDataExtraTypeArguments(context, attributeSyntax.GetLocation(), builder);
return;
}

int typeArgumentIdx = 0, parameterTypeIdx = 0;
for (; typeArgumentIdx < returnTypeArguments.Length && parameterTypeIdx < testMethodParameterSymbols.Length; typeArgumentIdx++)
{
Expand All @@ -695,7 +681,20 @@ static void VerifyTheoryDataUsage(
continue;

if (parameterType.Kind != SymbolKind.TypeParameter && !parameterType.IsAssignableFrom(typeArgument))
ReportMemberMethodTheoryDataIncompatibleType(context, parameterSyntax.Type.GetLocation(), typeArgument, namedMemberType, memberName, parameter);
{
bool report = true;

// The user might be providing the full array for 'params'; if they do, we need to move
// the parameter type index forward because it's been consumed by the array
if (parameter.IsParams && parameter.Type.IsAssignableFrom(typeArgument))
{
report = false;
parameterTypeIdx++;
}

if (report)
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
Expand All @@ -705,11 +704,17 @@ static void VerifyTheoryDataUsage(
&& typeArgument.NullableAnnotation == NullableAnnotation.Annotated)
ReportMemberMethodTheoryDataNullability(context, parameterSyntax.Type.GetLocation(), typeArgument, namedMemberType, memberName, parameter);

// Only move the parameter type index forward when the current parameter is not a 'params'
if (!parameter.IsParams)
{
// Stop moving parameterTypeIdx forward if the argument is a parameter array, regardless of xunit's support for it
parameterTypeIdx++;
}
}

if (typeArgumentIdx < returnTypeArguments.Length)
{
var builder = ImmutableDictionary.CreateBuilder<string, string?>();
builder[Constants.Properties.MemberName] = memberName;

ReportMemberMethodTheoryDataExtraTypeArguments(context, attributeSyntax.GetLocation(), builder);
}
}
}

0 comments on commit f1e0908

Please sign in to comment.