diff --git a/src/TUnit.Core/Attributes/TestData/MethodDataSourceAttribute.cs b/src/TUnit.Core/Attributes/TestData/MethodDataSourceAttribute.cs index b05ee85c9d6..b61ff120ebe 100644 --- a/src/TUnit.Core/Attributes/TestData/MethodDataSourceAttribute.cs +++ b/src/TUnit.Core/Attributes/TestData/MethodDataSourceAttribute.cs @@ -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 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 @@ -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; @@ -513,7 +513,7 @@ internal InstanceMethodDataSourceAttribute ToInstanceVariant() // IEnumerable Rows() is used rather than an ambiguous match with IEnumerable Rows(), // 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? candidates = null; foreach (var method in targetType.GetMethods(BindingFlags)) @@ -524,17 +524,20 @@ 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); } [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? best = null; foreach (var method in targetType.GetMethods(BindingFlags)) { if (method.Name != MethodNameProvidingDataSource || method.IsGenericMethodDefinition) @@ -542,43 +545,18 @@ internal InstanceMethodDataSourceAttribute ToInstanceVariant() 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; } } - // 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) @@ -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; } diff --git a/tests/TUnit.Engine.Tests/Issue7108Tests.cs b/tests/TUnit.Engine.Tests/Issue7108Tests.cs index 4366830e015..298d2d58f02 100644 --- a/tests/TUnit.Engine.Tests/Issue7108Tests.cs +++ b/tests/TUnit.Engine.Tests/Issue7108Tests.cs @@ -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) ]); } diff --git a/tests/TUnit.TestProject/Bugs/_7108/DataSourceOverloadParityTests.cs b/tests/TUnit.TestProject/Bugs/_7108/DataSourceOverloadParityTests.cs index 35614b912e5..7581931fbf4 100644 --- a/tests/TUnit.TestProject/Bugs/_7108/DataSourceOverloadParityTests.cs +++ b/tests/TUnit.TestProject/Bugs/_7108/DataSourceOverloadParityTests.cs @@ -5,7 +5,7 @@ namespace TUnit.TestProject.Bugs._7108; /// /// A table of [MethodDataSource] overload scenarios. Source-gen mode picks the overload with /// ScoreDataSourceOverload in the generator, and reflection mode with the runtime binder and -/// MethodDataSourceAttribute.ResolveDataSourceMethodWithNullArgument. Each attribute is one row: +/// MethodDataSourceAttribute.ResolveDataSourceMethodByScore. 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 @@ -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(string expected, T value) { @@ -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) @@ -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);