Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
23 changes: 23 additions & 0 deletions docs/docs/reference/tunit0076.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# TUnit0076: Data source Arguments fit more than one overload equally well

**Severity:** Error

The `Arguments` of a `[MethodDataSource]` select which overload of the data source method is called. TUnit0076 reports `Arguments` that fit two or more overloads equally well, where no overload is more specific than the others. A C# call with the same arguments would fail with CS0121 for the same reason.

For example, with the overloads `Tie(string expected, IComparable value)` and `Tie(string expected, IConvertible value)`, the attribute `[MethodDataSource(nameof(Tie), Arguments = new object[] { "a", 1 })]` is ambiguous: `int` implements both interfaces, and neither interface converts to the other.

## Why it is an error

Without the diagnostic, the two execution modes disagree. Reflection mode asks the runtime binder for the overload that matches the argument types, and the binder throws `AmbiguousMatchException`. Source-generated mode would call one of the overloads instead.

## How to fix it

Make one overload the best match for the `Arguments`:

- Rename one of the overloads, so the attribute names the method it means.
- Change a parameter type so that one overload is more specific, for example `int` instead of `IConvertible`.
- Remove the overload that the data source does not need.

## Scope

The rule follows the runtime binder's rules. It compares only overloads with one parameter per argument, and it accepts identity, reference, boxing and numeric widening conversions. It does not check `Arguments` that contain `null` or overloads that are generic methods.
1 change: 1 addition & 0 deletions docs/sidebars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ const sidebars: SidebarsConfig = {
'reference/environment-variables',
'reference/test-configuration',
'reference/tunit0075',
'reference/tunit0076',
{
type: 'doc',
id: 'guides/best-practices',
Expand Down
1 change: 1 addition & 0 deletions src/TUnit.Analyzers/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ TUnit0062 | Usage | Warning | CancellationToken must be the last parameter
TUnit0073 | Usage | Error | Missing polyfill types required by TUnit
TUnit0074 | Usage | Error | Hook attribute is redundant on an override
TUnit0075 | Usage | Warning | Use the hook cancellation token for setup
TUnit0076 | Usage | Error | Data source Arguments fit more than one overload equally well

### Removed Rules

Expand Down
1 change: 1 addition & 0 deletions src/TUnit.Analyzers/DiagnosticIds.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,5 @@ public static class DiagnosticIds
public const string MissingPolyfillPackage = "TUnit0073";
public const string RedundantHookAttributeOnOverride = "TUnit0074";
public const string UseHookCancellationToken = "TUnit0075";
public const string AmbiguousMethodDataSourceOverload = "TUnit0076";
}
9 changes: 9 additions & 0 deletions src/TUnit.Analyzers/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -534,4 +534,13 @@
<data name="TUnit0075Title" xml:space="preserve">
<value>Use the hook cancellation token for setup</value>
</data>
<data name="TUnit0076Description" xml:space="preserve">
<value>The `Arguments` of a data source must select a single overload of the data source method. When they fit two or more overloads equally well, the overload to call is ambiguous, as a C# call with the same arguments would be (CS0121).</value>
</data>
<data name="TUnit0076MessageFormat" xml:space="preserve">
<value>The `Arguments` fit more than one overload of '{0}' equally well: {1}</value>
</data>
<data name="TUnit0076Title" xml:space="preserve">
<value>Data source Arguments fit more than one overload equally well</value>
</data>
</root>
3 changes: 3 additions & 0 deletions src/TUnit.Analyzers/Rules.cs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ public static class Rules
public static readonly DiagnosticDescriptor UseHookCancellationToken =
CreateDescriptor(DiagnosticIds.UseHookCancellationToken, UsageCategory, DiagnosticSeverity.Warning);

public static readonly DiagnosticDescriptor AmbiguousMethodDataSourceOverload =
CreateDescriptor(DiagnosticIds.AmbiguousMethodDataSourceOverload, UsageCategory, DiagnosticSeverity.Error);

private static DiagnosticDescriptor CreateDescriptor(string diagnosticId, string category, DiagnosticSeverity severity,
string[]? customTags = null, string? helpLinkUri = null)
{
Expand Down
156 changes: 155 additions & 1 deletion src/TUnit.Analyzers/TestDataAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public class TestDataAnalyzer : ConcurrentDiagnosticAnalyzer
Rules.MatrixDataSourceAttributeRequired,
Rules.TooManyArguments,
Rules.InstanceMethodSource,
Rules.PotentialEmptyDataSource
Rules.PotentialEmptyDataSource,
Rules.AmbiguousMethodDataSourceOverload
);

protected override void InitializeInternal(AnalysisContext context)
Expand Down Expand Up @@ -462,6 +463,19 @@ private void CheckMethodDataSource(SymbolAnalysisContext context,
return;
}

var ambiguousOverloads = FindAmbiguousOverloads(context.Compilation, methodSymbols, methodName!, argumentsNamedArgument);
if (ambiguousOverloads is not null)
{
context.ReportDiagnostic(
Diagnostic.Create(
Rules.AmbiguousMethodDataSourceOverload,
attribute.GetLocation(),
methodName,
string.Join(", ", ambiguousOverloads.Select(m => m.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat))))
);
return;
}

if (dataSourceMethod.ReturnsVoid)
{
context.ReportDiagnostic(
Expand Down Expand Up @@ -1077,4 +1091,144 @@ private static bool IsIAsyncEnumerable(ITypeSymbol type, Compilation compilation

return false;
}

// Reflection mode picks the data source overload with Type.GetMethod(name, flags, binder: null, argumentTypes, modifiers: null),
// so this mirrors the default binder: only overloads with exactly one parameter per argument are candidates, and
// when none of them is at least as specific as all the others, the binder throws AmbiguousMatchException.
// Source-gen mode would silently pick one of them instead, so report the ambiguity at compile time.
// Returns the tied overloads, or null when the Arguments select a single overload (or cannot be checked).
private static List<IMethodSymbol>? FindAmbiguousOverloads(Compilation compilation,
IMethodSymbol[] methodSymbols,
string methodName,
TypedConstant argumentsNamedArgument)
{
ITypeSymbol[] argumentTypes;
if (argumentsNamedArgument.Kind == TypedConstantKind.Array)
{
// A null argument has no runtime type, so reflection mode falls back to a name-only lookup.
if (argumentsNamedArgument.Values.Any(x => x.IsNull || x.Type is null))
{
return null;
}

argumentTypes = argumentsNamedArgument.Values.Select(x => x.Type!).ToArray();
}
else
{
argumentTypes = Array.Empty<ITypeSymbol>();
}

var candidates = new List<IMethodSymbol>();
foreach (var method in methodSymbols)
{
// An override is the same runtime method as the member it overrides, which is also in the list.
if (method.Name != methodName
|| method.Parameters.Length != argumentTypes.Length
|| method.IsOverride)
Comment thread
thomhurst marked this conversation as resolved.
Resolved
Comment thread
thomhurst marked this conversation as resolved.
Resolved
{
continue;
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
// Inferring type arguments is outside what the binder does, so leave generic overloads alone.
if (method.IsGenericMethod)
{
return null;
}

var applicable = true;
for (var i = 0; i < argumentTypes.Length; i++)
{
if (!IsBinderConversion(compilation, argumentTypes[i], method.Parameters[i].Type))
Comment thread
thomhurst marked this conversation as resolved.
Resolved
{
applicable = false;
break;
}
}

if (applicable)
{
candidates.Add(method);
}
}

if (candidates.Count < 2)
{
return null;
}

foreach (var candidate in candidates)
{
if (candidates.All(other => ReferenceEquals(candidate, other) || IsAtLeastAsSpecific(compilation, candidate, other, argumentTypes)))
{
return null;
}
}

return candidates;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

// Mirrors DefaultBinder.FindMostSpecificMethod: an overload wins when it is more specific for at least one
// parameter and less specific for none. Overloads with the same parameter types (a member hidden with `new`)
// are resolved by the binder in favour of the most derived declaration, so they never tie.
private static bool IsAtLeastAsSpecific(Compilation compilation, IMethodSymbol candidate, IMethodSymbol other, ITypeSymbol[] argumentTypes)
{
var otherWins = false;

for (var i = 0; i < argumentTypes.Length; i++)
{
var candidateType = candidate.Parameters[i].Type;
var otherType = other.Parameters[i].Type;

if (SymbolEqualityComparer.Default.Equals(candidateType, otherType))
{
continue;
}

switch (FindMostSpecificType(compilation, candidateType, otherType, argumentTypes[i]))
{
case 1:
break;
case 2:
otherWins = true;
break;
default:
return false;
}
}

return !otherWins;
}

// Mirrors DefaultBinder.FindMostSpecificType: 1 when the first type is more specific, 2 when the second is, 0 for a tie.
private static int FindMostSpecificType(Compilation compilation, ITypeSymbol first, ITypeSymbol second, ITypeSymbol argumentType)
{
if (SymbolEqualityComparer.Default.Equals(first, argumentType))
{
return 1;
}

if (SymbolEqualityComparer.Default.Equals(second, argumentType))
{
return 2;
}

var firstFromSecond = IsBinderConversion(compilation, second, first);
var secondFromFirst = IsBinderConversion(compilation, first, second);

if (firstFromSecond == secondFromFirst)
{
return 0;
}

return firstFromSecond ? 2 : 1;
}

// The conversions the default binder accepts: identity, reference and boxing (Type.IsAssignableFrom), and
// primitive widening. User-defined and nullable conversions exist only in C#.
private static bool IsBinderConversion(Compilation compilation, ITypeSymbol from, ITypeSymbol to)
{
var conversion = compilation.ClassifyConversion(from, to);
return conversion.IsImplicit
&& (conversion.IsIdentity || conversion.IsReference || conversion.IsBoxing || conversion.IsNumeric);
Comment thread
thomhurst marked this conversation as resolved.
Resolved
}
}
142 changes: 142 additions & 0 deletions tests/TUnit.Analyzers.Tests/MethodDataSourceAnalyzerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1970,4 +1970,146 @@ public class MyClass<T>(T seed) where T : struct
"""
);
}

[Test]
public async Task Arguments_That_Fit_Two_Overloads_Equally_Are_Flagged()
{
await Verifier
.VerifyAnalyzerAsync(
"""
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using TUnit.Core;

public class MyClass
{
[{|#0:MethodDataSource(nameof(Tie), Arguments = new object[] { "IComparable", 1 })|}]
[Test]
public Task MyTest(string expected, string chosen) => Task.CompletedTask;

public static IEnumerable<(string, string)> Tie(string expected, IComparable value) { yield return (expected, "IComparable"); }
public static IEnumerable<(string, string)> Tie(string expected, IConvertible value) { yield return (expected, "IConvertible"); }
}
""",

Verifier.Diagnostic(Rules.AmbiguousMethodDataSourceOverload)
.WithLocation(0)
.WithArguments("Tie", "MyClass.Tie(string, IComparable), MyClass.Tie(string, IConvertible)")
);
}

[Test]
public async Task Arguments_That_Fit_Overloads_In_Opposite_Directions_Are_Flagged()
{
await Verifier
.VerifyAnalyzerAsync(
"""
using System;
using System.Collections.Generic;
using TUnit.Core;

public class MyClass
{
[{|#0:MethodDataSource(nameof(Data), Arguments = new object[] { "a", "b" })|}]
[Test]
public void MyTest(int value)
{
}

public static IEnumerable<int> Data(string first, object second) => [1];
public static IEnumerable<int> Data(object first, string second) => [2];
}
""",

Verifier.Diagnostic(Rules.AmbiguousMethodDataSourceOverload)
.WithLocation(0)
.WithArguments("Data", "MyClass.Data(string, object), MyClass.Data(object, string)")
);
}

[Test]
public async Task Arguments_With_A_More_Specific_Overload_Are_Not_Flagged()
{
await Verifier
.VerifyAnalyzerAsync(
"""
using System;
using System.Collections.Generic;
using TUnit.Core;

public class MyClass
{
[MethodDataSource(nameof(Data), Arguments = new object[] { "a" })]
[MethodDataSource(nameof(Data), Arguments = new object[] { 1 })]
[MethodDataSource(nameof(Numbers), Arguments = new object[] { 1 })]
[Test]
public void MyTest(int value)
{
}

public static IEnumerable<int> Data(object value) => [1];
public static IEnumerable<int> Data(string value) => [2];
public static IEnumerable<int> Data(IComparable value) => [3];

public static IEnumerable<int> Numbers(long value) => [1];
public static IEnumerable<int> Numbers(double value) => [2];
}
"""
);
}

[Test]
public async Task Overloads_With_Other_Parameter_Counts_Are_Not_Flagged()
{
await Verifier
.VerifyAnalyzerAsync(
"""
using System;
using System.Collections.Generic;
using TUnit.Core;

public class MyClass
{
[MethodDataSource(nameof(Data), Arguments = new object[] { 1 })]
[Test]
public void MyTest(int value)
{
}

public static IEnumerable<int> Data(IComparable value) => [1];
public static IEnumerable<int> Data(IConvertible value, int other) => [2];
}
"""
);
}

[Test]
public async Task Overridden_Data_Source_Methods_Are_Not_Flagged()
{
await Verifier
.VerifyAnalyzerAsync(
"""
using System;
using System.Collections.Generic;
using TUnit.Core;

public class BaseClass
{
public virtual IEnumerable<int> Data(int value) => [1];
}

public class MyClass : BaseClass
{
[MethodDataSource(nameof(Data), Arguments = new object[] { 1 })]
[Test]
public void MyTest(int value)
{
}

public override IEnumerable<int> Data(int value) => [2];
}
"""
);
}
}
Loading