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
75 changes: 32 additions & 43 deletions src/TUnit.Core/Attributes/TestData/MethodDataSourceAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -474,12 +474,12 @@ internal InstanceMethodDataSourceAttribute ToInstanceVariant()
// GetData(object) vs GetData(string) — where a plain IsAssignableFrom scan would match both and
// throw on SingleOrDefault.
//
// When any argument is null it has no runtime type, so it cannot be expressed in the binder's
// Type[]. The overloads are then scored the way the source generator scores them (ScoreArguments):
// When an argument is null, or the binder finds no exact-arity match, the overloads are scored
// the way the source generator scores them (ScoreArguments):
// a null needs a reference or Nullable<T> parameter, other arguments may use numeric widening,
// missing trailing parameters must be optional or a CancellationToken, and exact-count overloads and
// exact argument types rank higher. A single best overload is used. Otherwise we fall back to the
// caller's name-only lookup.
// exact argument types rank higher. Ties prefer more specific supplied parameter types, then the
// first declaration, as in the generator. If none match, use the caller's name-only lookup.
//
// With zero arguments the empty Type[] flows through the same binder path and selects the
// parameterless overload directly — including when the name is shared with other-arity
Expand All @@ -500,7 +500,7 @@ internal InstanceMethodDataSourceAttribute ToInstanceVariant()
if (argumentType is null)
{
// A null argument has no runtime type the binder can match on.
return ResolveDataSourceMethodWithNullArgument(targetType, arguments);
return ResolveDataSourceMethodByScore(targetType, arguments);
}

argumentTypes[i] = argumentType;
Expand All @@ -513,7 +513,7 @@ internal InstanceMethodDataSourceAttribute ToInstanceVariant()
// IEnumerable<int> Rows() is used rather than an ambiguous match with IEnumerable<T> Rows<T>(),
// as the source generator does.
#if NET
return targetType.GetMethod(MethodNameProvidingDataSource, genericParameterCount: 0, BindingFlags, binder: null, argumentTypes, modifiers: null);
var resolvedMethod = targetType.GetMethod(MethodNameProvidingDataSource, genericParameterCount: 0, BindingFlags, binder: null, argumentTypes, modifiers: null);
#else
List<MethodBase>? candidates = null;
foreach (var method in targetType.GetMethods(BindingFlags))
Expand All @@ -524,61 +524,39 @@ internal InstanceMethodDataSourceAttribute ToInstanceVariant()
}
}

return candidates is null
var resolvedMethod = candidates is null
? null
: (MethodInfo?)Type.DefaultBinder.SelectMethod(BindingFlags, candidates.ToArray(), argumentTypes, modifiers: null);
#endif
// The binder requires exact arity. Match omitted optional and CancellationToken parameters
// with the same scoring used for null arguments and by the source generator.
return resolvedMethod ?? ResolveDataSourceMethodByScore(targetType, arguments);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}

[UnconditionalSuppressMessage("Trimming", "IL2070", Justification = "Method data sources require runtime discovery. AOT users should use Factory property.")]
private MethodInfo? ResolveDataSourceMethodWithNullArgument([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)] Type targetType, object?[] arguments)
private MethodInfo? ResolveDataSourceMethodByScore([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicMethods | DynamicallyAccessedMemberTypes.NonPublicMethods)] Type targetType, object?[] arguments)
{
var bestScore = -1;
var best = new List<MethodInfo>();
MethodInfo? best = null;
foreach (var method in targetType.GetMethods(BindingFlags))
{
if (method.Name != MethodNameProvidingDataSource || method.IsGenericMethodDefinition)
{
continue;
}

var score = ScoreArguments(method.GetParameters(), arguments);
if (score < 0 || score < bestScore)
var parameters = method.GetParameters();
var score = ScoreArguments(parameters, arguments);
if (score > bestScore
|| (score == bestScore && score >= 0 && IsMoreSpecific(parameters, best!.GetParameters(), arguments.Length)))
{
continue;
}

if (score > bestScore)
{
best.Clear();
bestScore = score;
}

best.Add(method);
}

// On a tie, the overload with the more specific parameter types wins, as with the binder and the source
// generator: Rows(string?, long) beats Rows(string?, object) for { null, 20 }.
foreach (var candidate in best)
{
var mostSpecific = true;
foreach (var other in best)
{
if (!ReferenceEquals(candidate, other) && !IsMoreSpecific(candidate.GetParameters(), other.GetParameters(), arguments.Length))
{
mostSpecific = false;
break;
}
}

if (mostSpecific)
{
return candidate;
best = method;
Comment thread
thomhurst marked this conversation as resolved.
Resolved
}
}

// Still ambiguous, or nothing accepts the arguments: the caller's name-only lookup reports it
return null;
// Keep the first declaration on an otherwise equal tie, just as the source generator does.
return best;
}

private static bool IsMoreSpecific(ParameterInfo[] candidate, ParameterInfo[] other, int argumentCount)
Expand All @@ -601,10 +579,21 @@ private static bool IsMoreSpecific(ParameterInfo[] candidate, ParameterInfo[] ot
continue;
}

var otherUnderlying = Nullable.GetUnderlyingType(otherType) ?? otherType;
var candidateNullable = Nullable.GetUnderlyingType(candidateType);
var otherNullable = Nullable.GetUnderlyingType(otherType);
var otherUnderlying = otherNullable ?? otherType;
// Specificity compares parameter types, not boxed argument conversions. Nullable numeric
// conversions are lifted, and decimal conversions count even though Invoke cannot widen to decimal.
var numericSource = candidateNullable ?? candidateType;
var hasNumericConversion = (candidateNullable is null || otherNullable is not null)
&& (IsNumericWidening(numericSource, otherUnderlying)
|| (otherUnderlying == typeof(decimal) && !numericSource.IsEnum
&& Type.GetTypeCode(numericSource) is TypeCode.SByte or TypeCode.Byte
or TypeCode.Int16 or TypeCode.UInt16 or TypeCode.Char or TypeCode.Int32
or TypeCode.UInt32 or TypeCode.Int64 or TypeCode.UInt64));
if (!otherType.IsAssignableFrom(candidateType)
&& candidateType != otherUnderlying
&& !IsNumericWidening(candidateType, otherUnderlying))
&& !hasNumericConversion)
{
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions tests/TUnit.Engine.Tests/Issue7108Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ await RunTestsWithFilter(
"/*/TUnit.TestProject.Bugs._7108/DataSourceOverloadParityTests/*",
[
result => result.ResultSummary.Outcome.ShouldBe("Completed"),
result => result.ResultSummary.Counters.Total.ShouldBe(31),
result => result.ResultSummary.Counters.Passed.ShouldBe(31),
result => result.ResultSummary.Counters.Total.ShouldBe(37),
result => result.ResultSummary.Counters.Passed.ShouldBe(37),
result => result.ResultSummary.Counters.Failed.ShouldBe(0)
]);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ namespace TUnit.TestProject.Bugs._7108;
/// <summary>
/// A table of [MethodDataSource] overload scenarios. Source-gen mode picks the overload with
/// <c>ScoreDataSourceOverload</c> in the generator, and reflection mode with the runtime binder and
/// <c>MethodDataSourceAttribute.ResolveDataSourceMethodWithNullArgument</c>. Each attribute is one row:
/// <c>MethodDataSourceAttribute.ResolveDataSourceMethodByScore</c>. Each attribute is one row:
/// its first argument names the overload both modes must choose, and every overload yields
/// (expected, its own signature). Issue7108Tests runs the table in both modes, so a divergence between
/// the two resolvers fails the row in the mode that drifted. The test methods are not generic, so the
Expand Down Expand Up @@ -159,6 +159,36 @@ public class DataSourceOverloadParityTests
yield return (expected, "NullToNullable(string, int?)");
}

public static IEnumerable<(string, string)> NullableNumericTie(string expected, long? value)
{
yield return (expected, "NullableNumericTie(string, long?)");
}

public static IEnumerable<(string, string)> NullableNumericTie(string expected, int? value)
{
yield return (expected, "NullableNumericTie(string, int?)");
}

public static IEnumerable<(string, string)> NullableNumericTieNarrowFirst(string expected, int? value)
{
yield return (expected, "NullableNumericTieNarrowFirst(string, int?)");
}

public static IEnumerable<(string, string)> NullableNumericTieNarrowFirst(string expected, long? value)
{
yield return (expected, "NullableNumericTieNarrowFirst(string, long?)");
}

public static IEnumerable<(string, string)> NullableDecimalTie(string expected, decimal? value)
{
yield return (expected, "NullableDecimalTie(string, decimal?)");
}

public static IEnumerable<(string, string)> NullableDecimalTie(string expected, int? value)
{
yield return (expected, "NullableDecimalTie(string, int?)");
}

// A generic overload is declared first: its T parameter cannot take an Arguments value in either mode
public static IEnumerable<(string, string)> GenericOverload<T>(string expected, T value)
{
Expand All @@ -185,6 +215,26 @@ public class DataSourceOverloadParityTests
yield return (expected, "Token(string, int, CancellationToken)");
}

public static IEnumerable<(string, string)> TiedTokens(string expected, CancellationToken token)
{
yield return (expected, "TiedTokens(string, CancellationToken)");
}

public static IEnumerable<(string, string)> TiedTokens(string expected, CancellationToken first, CancellationToken second)
{
yield return (expected, "TiedTokens(string, CancellationToken, CancellationToken)");
}

public static IEnumerable<(string, string)> TiedTokensLongerFirst(string expected, CancellationToken first, CancellationToken second)
{
yield return (expected, "TiedTokensLongerFirst(string, CancellationToken, CancellationToken)");
}

public static IEnumerable<(string, string)> TiedTokensLongerFirst(string expected, CancellationToken token)
{
yield return (expected, "TiedTokensLongerFirst(string, CancellationToken)");
}

// C# converts an int to decimal or long?, but reflection's binder and MethodInfo.Invoke cannot, so the object
// overload declared first wins in both modes
public static IEnumerable<(string, string)> ObjectOrDecimal(string expected, object value)
Expand Down Expand Up @@ -271,11 +321,17 @@ public async Task Chosen_Overload(string expected, string chosen)
[MethodDataSource(nameof(NullToNullable), Arguments = new object?[] { "NullToNullable(string, int?)", null })]
[MethodDataSource(nameof(NullTie), Arguments = new object?[] { "NullTie(string, string?, long)", null, 1 })]
[MethodDataSource(nameof(NullObjectOrNullableLong), Arguments = new object?[] { "NullObjectOrNullableLong(string, string?, object)", null, 1 })]
[MethodDataSource(nameof(NullableNumericTie), Arguments = new object?[] { "NullableNumericTie(string, int?)", null })]
[MethodDataSource(nameof(NullableNumericTieNarrowFirst), Arguments = new object?[] { "NullableNumericTieNarrowFirst(string, int?)", null })]
[MethodDataSource(nameof(NullableDecimalTie), Arguments = new object?[] { "NullableDecimalTie(string, int?)", null })]
[MethodDataSource(nameof(NullOptional), Arguments = new object?[] { "NullOptional(string, string?, int)", null })]
[MethodDataSource(nameof(Optional), Arguments = new object[] { "Optional(string, string, int)", "a" })]
[MethodDataSource(nameof(Token), Arguments = new object[] { "Token(string, int, CancellationToken)", 1 })]
[MethodDataSource(nameof(TiedTokens), Arguments = new object[] { "TiedTokens(string, CancellationToken)" })]
[MethodDataSource(nameof(TiedTokensLongerFirst), Arguments = new object[] { "TiedTokensLongerFirst(string, CancellationToken, CancellationToken)" })]
// An override called through its base declaration passes its own default, as reflection does, not the base's
[MethodDataSource(typeof(DataSourceOverloadParityDerivedProvider), nameof(DataSourceOverloadParityDerivedProvider.OverriddenDefault), Arguments = new object?[] { "Derived.OverriddenDefault(string, string?, int = 2)", null })]
[MethodDataSource(typeof(DataSourceOverloadParityDerivedProvider), nameof(DataSourceOverloadParityDerivedProvider.OverriddenDefault), Arguments = new object[] { "Derived.OverriddenDefault(string, string?, int = 2)", "non-null" })]
public async Task Chosen_Overload_With_Missing_Or_Null_Arguments(string expected, string chosen)
{
await Assert.That(chosen).IsEqualTo(expected);
Expand Down
Loading