Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
27 changes: 27 additions & 0 deletions docs/docs/reference/tunit0076.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# 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 usually fails 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, which differ from C# overload resolution in a few places:

- It compares only overloads with one parameter per argument, and skips overloads with `ref`, `in` or `out` parameters, parameters typed by a generic method's type parameter, and private methods declared on a base class, because the runtime lookup never selects them.
- It accepts identity, reference and boxing conversions, `T` to `Nullable<T>`, and the runtime's primitive widening conversions. These are narrower than C#'s implicit numeric conversions: `int` does not widen to `decimal`, `nint` or `nuint`. They also allow `byte` to `char`, which C# does not.
- It does not check `Arguments` that contain `null`, or enum arguments passed to primitive parameters.
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
242 changes: 241 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, type, 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,230 @@ 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,
ITypeSymbol targetType,
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
// Reflection never returns private members inherited from a base type, even with FlattenHierarchy.
if (method.DeclaredAccessibility == Accessibility.Private
&& !SymbolEqualityComparer.Default.Equals(method.ContainingType.OriginalDefinition, targetType.OriginalDefinition))
{
continue;
}

// A ref, in or out parameter has a by-ref runtime type (T&), which never matches the argument's type.
if (method.Parameters.Any(p => p.RefKind != RefKind.None))
{
continue;
}

// The binder does not infer type arguments: a parameter typed by a method type parameter never matches,
// while a generic method whose parameters do not use its type parameters competes like any other overload.
if (method.IsGenericMethod && method.Parameters.Any(p => ReferencesMethodTypeParameter(p.Type)))
{
continue;
}

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;
}

private static bool ReferencesMethodTypeParameter(ITypeSymbol type) => type switch
{
ITypeParameterSymbol typeParameter => typeParameter.TypeParameterKind == TypeParameterKind.Method,
IArrayTypeSymbol array => ReferencesMethodTypeParameter(array.ElementType),
IPointerTypeSymbol pointer => ReferencesMethodTypeParameter(pointer.PointedAtType),
INamedTypeSymbol named => named.TypeArguments.Any(ReferencesMethodTypeParameter),
_ => false,
};

// Mirrors the default binder's applicability check (and Type.IsAssignableFrom in its specificity check):
// - a primitive parameter accepts only the CLR's primitive widening table, which is narrower than C#'s
// implicit numeric conversions (no int -> decimal, no conversions to nint/nuint) and also allows byte -> char;
// - Nullable<T> is assignable from T, and Nullable<T> itself is assignable only to object and ValueType;
// - everything else uses reference and boxing conversions. User-defined conversions exist only in C#.
// An enum argument for a primitive parameter is accepted by the runtime (through its underlying type) but not
// here, so those overloads are skipped: that can miss an ambiguity but never reports a false one for them.
private static bool IsBinderConversion(Compilation compilation, ITypeSymbol from, ITypeSymbol to)
{
if (SymbolEqualityComparer.Default.Equals(from, to))
{
return true;
}

if (IsClrPrimitive(to.SpecialType))
{
return CanWidenPrimitive(from.SpecialType, to.SpecialType);
}

if (to.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T)
{
return to is INamedTypeSymbol { TypeArguments.Length: 1 } nullable
&& SymbolEqualityComparer.Default.Equals(nullable.TypeArguments[0], from);
}

if (from.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T)
{
return to.SpecialType is SpecialType.System_Object or SpecialType.System_ValueType;
}

var conversion = compilation.ClassifyConversion(from, to);
return conversion.IsImplicit && (conversion.IsReference || conversion.IsBoxing);
}

private static bool IsClrPrimitive(SpecialType type) => type is SpecialType.System_Boolean
or SpecialType.System_Char
or SpecialType.System_SByte
or SpecialType.System_Byte
or SpecialType.System_Int16
or SpecialType.System_UInt16
or SpecialType.System_Int32
or SpecialType.System_UInt32
or SpecialType.System_Int64
or SpecialType.System_UInt64
or SpecialType.System_Single
or SpecialType.System_Double
or SpecialType.System_IntPtr
or SpecialType.System_UIntPtr;

// The runtime's primitive widening table (DefaultBinder.CanChangePrimitive), excluding identity.
private static bool CanWidenPrimitive(SpecialType from, SpecialType to) => from switch
{
SpecialType.System_Char => to is SpecialType.System_UInt16 or SpecialType.System_UInt32 or SpecialType.System_Int32
or SpecialType.System_UInt64 or SpecialType.System_Int64 or SpecialType.System_Single or SpecialType.System_Double,
SpecialType.System_SByte => to is SpecialType.System_Int16 or SpecialType.System_Int32 or SpecialType.System_Int64
or SpecialType.System_Single or SpecialType.System_Double,
SpecialType.System_Byte => to is SpecialType.System_Char or SpecialType.System_UInt16 or SpecialType.System_Int16
or SpecialType.System_UInt32 or SpecialType.System_Int32 or SpecialType.System_UInt64 or SpecialType.System_Int64
or SpecialType.System_Single or SpecialType.System_Double,
SpecialType.System_Int16 => to is SpecialType.System_Int32 or SpecialType.System_Int64 or SpecialType.System_Single
or SpecialType.System_Double,
SpecialType.System_UInt16 => to is SpecialType.System_UInt32 or SpecialType.System_Int32 or SpecialType.System_UInt64
or SpecialType.System_Int64 or SpecialType.System_Single or SpecialType.System_Double,
SpecialType.System_Int32 => to is SpecialType.System_Int64 or SpecialType.System_Single or SpecialType.System_Double,
SpecialType.System_UInt32 => to is SpecialType.System_UInt64 or SpecialType.System_Int64 or SpecialType.System_Single
or SpecialType.System_Double,
SpecialType.System_Int64 or SpecialType.System_UInt64 => to is SpecialType.System_Single or SpecialType.System_Double,
SpecialType.System_Single => to is SpecialType.System_Double,
_ => false,
};
}
Loading
Loading