Skip to content

Adjust NUnit1029 to account for TestCaseSource support for params and optional arguments #958

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stevenaw wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Adjust NUnit1029 to account for TestCaseSource support for params and optional arguments #958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
60a55b9
Add a test which passes now but will fail once we make our change
stevenaw Nov 18, 2025
eb23a58
testcase params and optional support
stevenaw Nov 18, 2025
8da80c2
Review fixes.
manfred-brands Nov 24, 2025
3028d64
NUnit 3.14 doesn't support `Is.Default`
manfred-brands Nov 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 97 additions & 0 deletions src/nunit.analyzers.tests/TestCaseSourceUsage/TestCaseSourceUsesStringAnalyzerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,103 @@ public class InnerClass
RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic.WithMessage(message), testCode, fixedCode);
}

[Test]
public void NoWarningWhenNumberOfParametersOfTestWillFitIntoParamsArray()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
[TestFixture]
public class NoWarningWhenNumberOfParametersOfTestWillFitIntoParamsArray
{
[TestCaseSource(nameof(TestData))]
public void ShortName(params int[] x)
{
Assert.That(x.Length, Is.GreaterThanOrEqualTo(0));
}

static IEnumerable<int> TestData()
{
for (int i = 1; i <= 3; i++)
{
yield return i;
}
}
}", additionalUsings: "using System.Collections.Generic;");

RoslynAssert.Valid(analyzer, testCode);
}

[Test]
public void NoWarningWhenNumberOfParametersOfTestWillFitIntoParamsArrayForGenericMethod()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
[TestFixture]
public class NoWarningWhenNumberOfParametersOfTestWillFitIntoParamsArrayForGenericMethod
{
[TestCaseSource(nameof(TestData))]
public void ShortName<T>(params T[] x)
{
Assert.That(x.Length, Is.GreaterThanOrEqualTo(0));
}

static IEnumerable<int> TestData()
{
for (int i = 1; i <= 3; i++)
{
yield return i;
}
}
}", additionalUsings: "using System.Collections.Generic;");
Comment on lines 583 to 599
Copy link
Member

@manfred-brands manfred-brands Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NUnit itself fails to run this test with Unable to determine type arguments for method.

I always extract the code from the string to see if NUnit will actually run it.


RoslynAssert.Valid(analyzer, testCode);
}

[Test]
public void NoWarningWhenNumberOfParametersOfTestWillFitIntoOptionalParams()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
[TestFixture]
public class NoWarningWhenNumberOfParametersOfTestWillFitIntoOptionalParams
{
[TestCaseSource(nameof(TestData))]
public void ShortName(int a, int b = 2)
{
Assert.That(a, Is.GreaterThanOrEqualTo(b));
}

static IEnumerable<int> TestData()
{
for (int i = 1; i <= 3; i++)
{
yield return i;
}
}
}", additionalUsings: "using System.Collections.Generic;");

RoslynAssert.Valid(analyzer, testCode);
}

[Test]
public void NoWarningWhenNumberOfParametersOfTestWillFitIntoParamsArrayFlatArgs()
{
var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
[TestFixture]
public class NoWarningWhenNumberOfParametersOfTestWillFitIntoParamsArrayFlatArgs
{
[TestCaseSource(nameof(TestData))]
public static void TestLotsOfNonRequiredParams(params int[] z)
{
Assert.That(z, Is.Not.Empty);
}

static IEnumerable<int[]> TestData()
{
yield return new int[] { 2, 4, 6, 8 };
}
}", additionalUsings: "using System.Collections.Generic;");

RoslynAssert.Valid(analyzer, testCode);
}

[Test]
public void AnalyzeWhenNumberOfParametersOfTestIsLessThanProvidedByTestCaseSource()
{
Expand Down
25 changes: 14 additions & 11 deletions src/nunit.analyzers/TestCaseSourceUsage/TestCaseSourceUsesStringAnalyzer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ private static void AnalyzeAttribute(
var hasCancelAfterAttribute = testMethod.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, cancelAfterType)) ||
testMethod.ContainingType.GetAttributes().Any(a => SymbolEqualityComparer.Default.Equals(a.AttributeClass, cancelAfterType));

var (methodRequiredParameters, methodOptionalParameters, _) = testMethod.GetParameterCounts(hasCancelAfterAttribute, cancellationTokenType);
var (methodRequiredParameters, methodOptionalParameters, methodParamsParameters) = testMethod.GetParameterCounts(hasCancelAfterAttribute, cancellationTokenType);

if (elementType.SpecialType != SpecialType.System_String && (elementType.SpecialType == SpecialType.System_Object || elementType.IsIEnumerable(out _) ||
IsOrDerivesFrom(elementType, context.SemanticModel.Compilation.GetTypeByMetadataName(NUnitFrameworkConstants.FullNameOfTypeTestCaseParameters))))
Expand All @@ -257,7 +257,7 @@ private static void AnalyzeAttribute(
// The object could hide an array, possibly with a variable number of elements: TestCaseData.Argument.
// Potentially we could examine the body of the TestCaseSource to see if we can determine the exact amount.
// For complex method that is certainly beyond the scope of this.
if (methodRequiredParameters + methodOptionalParameters < 1 && !testMethod.IsGenericMethod)
if (methodRequiredParameters + methodOptionalParameters + methodParamsParameters < 1 && !testMethod.IsGenericMethod)
{
context.ReportDiagnostic(Diagnostic.Create(
mismatchInNumberOfTestMethodParameters,
Expand All @@ -268,15 +268,7 @@ private static void AnalyzeAttribute(
}
else
{
if (methodRequiredParameters != 1)
{
context.ReportDiagnostic(Diagnostic.Create(
mismatchInNumberOfTestMethodParameters,
syntaxNode.GetLocation(),
1,
methodRequiredParameters));
}
else
if (methodRequiredParameters == 1)
{
IParameterSymbol testMethodParameter = testMethod.Parameters.First();

Expand All @@ -290,6 +282,17 @@ private static void AnalyzeAttribute(
testMethodParameter.Name));
}
}
else if (methodRequiredParameters > 1 ||
(methodRequiredParameters + methodOptionalParameters + methodParamsParameters == 0))
{
// more than one required parameter is always a mismatch
// zero parameters of any kind is also a mismatch
context.ReportDiagnostic(Diagnostic.Create(
Copy link
Member

@manfred-brands manfred-brands Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a new test case that validates this.

Copy link
Member Author

@stevenaw stevenaw Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @manfred-brands
Do you mean this comment?

// more than one required parameter is always a mismatch
// zero parameters of any kind is also a mismatch

That comment was added by me to clarify the existing logic for myself prior to and after refactoring the logic a bit. I had thought they're already covered through existing tests here:

Unless, would you prefer additional cases with 0 and two required args which also include optional args in the signature like this?

[TestCaseSource(↓nameof(TestData))]
        public void ShortName(int x, int y, int z = 2)

mismatchInNumberOfTestMethodParameters,
syntaxNode.GetLocation(),
1,
methodRequiredParameters));
}
}
}
}
Expand Down