diff --git a/TUnit.Assertions.Tests/Bugs/Issue6296Tests.cs b/TUnit.Assertions.Tests/Bugs/Issue6296Tests.cs new file mode 100644 index 0000000000..333e0ec261 --- /dev/null +++ b/TUnit.Assertions.Tests/Bugs/Issue6296Tests.cs @@ -0,0 +1,88 @@ +namespace TUnit.Assertions.Tests.Bugs; + +/// +/// Regression tests for GitHub issue #6296: +/// await Assert.That(guid).IsEqualTo(guid) failed to compile with +/// CS0121: The call is ambiguous between IsEqualTo<TValue, TOther> and +/// IsEqualTo<TValue> for consumers building with the .NET 8 SDK. +/// +/// The cross-type IsEqualTo<TValue, TOther> overload (added for value-object +/// ergonomics, see ) is equally applicable to the same-type +/// overload when the expected value is the same type as the source (Guid vs Guid). It was +/// previously deprioritized only by [OverloadResolutionPriority], which is honored +/// solely by the Roslyn that ships with the .NET 9 SDK and later — NOT by LangVersion. The +/// .NET 8 SDK's compiler ignores it, so the call stayed ambiguous regardless of LangVersion. +/// +/// The fix gives the cross-type overloads a trailing params CrossTypeOverloadMarker[], +/// making them applicable only in expanded form so they lose to the normal-form same-type +/// overload via the compiler-version-independent "normal beats expanded" tie-break. The marker +/// element type (rather than object) has no accessible constructor, so a stray trailing +/// argument — e.g. IsEqualTo("x", StringComparer.Ordinal) on a value-object source — is a +/// compile error instead of being silently swallowed and discarded (PR #6313 review). The body +/// of each test below would not compile before the fix when built with the .NET 8 SDK — +/// successful compilation IS the regression assertion; the runtime assertions confirm the +/// same-type overload (default equality), not the implicit-conversion overload, is selected. +/// +public class Issue6296Tests +{ + [Test] + public async Task Guid_IsEqualTo_SameType_IsNotAmbiguous() + { + var value = Guid.Parse("11111111-1111-1111-1111-111111111111"); + var same = Guid.Parse("11111111-1111-1111-1111-111111111111"); + + await Assert.That(value).IsEqualTo(same); + } + + [Test] + public async Task Guid_IsNotEqualTo_SameType_IsNotAmbiguous() + { + var value = Guid.Parse("11111111-1111-1111-1111-111111111111"); + var other = Guid.Parse("22222222-2222-2222-2222-222222222222"); + + await Assert.That(value).IsNotEqualTo(other); + } + + [Test] + public async Task Int_IsEqualTo_SameType_IsNotAmbiguous() + { + await Assert.That(42).IsEqualTo(42); + } + + [Test] + public async Task DateTime_IsEqualTo_SameType_IsNotAmbiguous() + { + var now = new DateTime(2026, 6, 24, 0, 0, 0, DateTimeKind.Utc); + + await Assert.That(now).IsEqualTo(now); + } + + [Test] + public async Task Guid_IsEqualTo_SameType_Fails_When_Different() + { + var value = Guid.Parse("11111111-1111-1111-1111-111111111111"); + var other = Guid.Parse("22222222-2222-2222-2222-222222222222"); + + await Assert.That(async () => await Assert.That(value).IsEqualTo(other)) + .Throws(); + } + + // The expanded-form marker on the cross-type overload must not steal a genuine + // same-type-with-comparer call: the second positional argument is an IEqualityComparer, + // which would land in the marker's params array if the marker were `params object[]`. + // CrossTypeOverloadMarker has no accessible ctor, so this binds to the real same-type + // comparer overload and the comparer is actually applied (PR #6313 review, finding #1). + [Test] + public async Task String_IsEqualTo_SameType_WithComparer_BindsComparerOverload() + { + await Assert.That("VALUE").IsEqualTo("value", StringComparer.OrdinalIgnoreCase); + } + + [Test] + public async Task String_IsEqualTo_SameType_WithComparer_Fails_When_ComparerRejects() + { + await Assert.That(async () => + await Assert.That("VALUE").IsEqualTo("value", StringComparer.Ordinal)) + .Throws(); + } +} diff --git a/TUnit.Assertions/Extensions/ImplicitConversionEqualityExtensions.cs b/TUnit.Assertions/Extensions/ImplicitConversionEqualityExtensions.cs index 70bdb4ed0e..acb8ba98c7 100644 --- a/TUnit.Assertions/Extensions/ImplicitConversionEqualityExtensions.cs +++ b/TUnit.Assertions/Extensions/ImplicitConversionEqualityExtensions.cs @@ -1,7 +1,29 @@ -// These overloads rely on [OverloadResolutionPriority] (honored only by C# 13+) to lose to the -// source-generated same-type IsEqualTo / IsNotEqualTo. TUnit raises consumer LangVersion so the -// attribute is honored on every target (see TUnit.Assertions.props). Issues #5765, #6276, #6280. +// These cross-type overloads must lose to the source-generated same-type IsEqualTo / IsNotEqualTo +// when the expected argument is the same type as the source (e.g. Guid vs Guid), where both are +// applicable. Two layered mechanisms achieve that: +// +// 1. The trailing `params CrossTypeOverloadMarker[]` makes each overload applicable only in +// EXPANDED form, so a same-type call binds to the normal-form same-type overload via the C# +// "normal beats expanded" tie-break (§12.6.4.2). This is honored by EVERY Roslyn version, +// including the .NET 8 SDK's compiler, which is the only mechanism that fixes #6296. +// 2. [OverloadResolutionPriority(-1)] is kept as a redundant signal for completeness on modern +// compilers, but it is NOT load-bearing — it is honored only by Roslyn 4.12+ (.NET 9 SDK and +// later), so consumers pinned to the .NET 8 SDK via global.json never see its effect. +// +// Why CrossTypeOverloadMarker and not `params object[]`: an `object[]` marker is applicable in +// expanded form to ANY trailing argument, so a call like +// `Assert.That(productCode).IsEqualTo("x", StringComparer.OrdinalIgnoreCase)` would silently bind +// here and DISCARD the comparer — a false-pass footgun for a test framework (PR #6313 review). +// CrossTypeOverloadMarker has no accessible constructor, so callers can never supply an element: +// the overload stays applicable only with ZERO trailing args (the tie-break case), and any stray +// trailing argument is a compile error again, exactly as it was before #6296. +// +// History: #5765 / #6276 / #6280 tried to fix this purely via ORP + a LangVersion bump in +// TUnit.Assertions.props. That bump is a no-op for the real failure because ORP honoring tracks +// the build SDK's Roslyn version, not LangVersion — see #6296. The `params` tie-break is what +// makes this compiler-version-independent. The unused `_` parameter is never read. using System.Collections.Concurrent; +using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Runtime.CompilerServices; @@ -13,9 +35,9 @@ namespace TUnit.Assertions.Extensions; /// /// Adds IsEqualTo overloads accepting an expected value of a different type when the /// source type defines an implicit conversion operator to that type. Defined as a partial of -/// the source-generated EqualsAssertionExtensions so it lives in the same containing -/// type as the original overload — required for -/// to disambiguate calls where both overloads are applicable (e.g. enum or value-type sources). +/// the source-generated EqualsAssertionExtensions so it shares the containing type. The +/// trailing params array on each overload keeps it from displacing the same-type overload +/// for same-type calls (e.g. enum or value-type sources) — see the file header for the why. /// /// Enables ergonomic comparison of wrapper Value Objects against their wrapped primitive, /// e.g. await Assert.That(productCode).IsEqualTo("Example") when @@ -34,7 +56,10 @@ public static partial class EqualsAssertionExtensions public static EqualsAssertion IsEqualTo( this IAssertionSource source, TOther? expected, - [CallerArgumentExpression(nameof(expected))] string? expectedExpression = null) + [CallerArgumentExpression(nameof(expected))] string? expectedExpression = null, + // Expanded-form-only marker so a same-type call loses to the same-type overload; never read. + // Element type has no accessible ctor, so stray trailing args stay a compile error. See file header. + params CrossTypeOverloadMarker[] _) { var converter = ImplicitConversionCache.GetConverter(); source.Context.ExpressionBuilder.Append($".IsEqualTo({expectedExpression})"); @@ -62,7 +87,10 @@ public static partial class NotEqualsAssertionExtensions public static NotEqualsAssertion IsNotEqualTo( this IAssertionSource source, TOther? notExpected, - [CallerArgumentExpression(nameof(notExpected))] string? notExpectedExpression = null) + [CallerArgumentExpression(nameof(notExpected))] string? notExpectedExpression = null, + // Expanded-form-only marker so a same-type call loses to the same-type overload; never read. + // Element type has no accessible ctor, so stray trailing args stay a compile error. See file header. + params CrossTypeOverloadMarker[] _) { var converter = ImplicitConversionCache.GetConverter(); source.Context.ExpressionBuilder.Append($".IsNotEqualTo({notExpectedExpression})"); @@ -71,6 +99,20 @@ public static NotEqualsAssertion IsNotEqualTo( } } +/// +/// Marker element type for the trailing params on the cross-type +/// IsEqualTo / IsNotEqualTo overloads. Its only purpose is to make those overloads +/// applicable solely in expanded form so a same-type call loses to the same-type overload +/// (see ). It has no accessible constructor, so callers can +/// never pass an element — the overloads bind only with zero trailing arguments, and any stray +/// trailing argument remains a compile error rather than being silently discarded. Never instantiated. +/// +[EditorBrowsable(EditorBrowsableState.Never)] +public sealed class CrossTypeOverloadMarker +{ + private CrossTypeOverloadMarker() { } +} + /// /// Caches user-defined implicit conversion operators from TFrom to TTo. /// Reflection lookup is performed once per type pair and the resulting delegate is reused. diff --git a/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet10_0.verified.txt b/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet10_0.verified.txt index ab8556bcd1..6c79349f71 100644 --- a/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet10_0.verified.txt +++ b/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet10_0.verified.txt @@ -3203,6 +3203,7 @@ namespace .Extensions protected override .<.> CheckAsync(.<.Cookie> metadata) { } protected override string GetExpectation() { } } + public sealed class CrossTypeOverloadMarker { } public static class CultureInfoAssertionExtensions { [.("Trimming", "IL2091", Justification="Generic type parameter is only used for property access, not instantiation")] @@ -4357,7 +4358,7 @@ namespace .Extensions [.("Looks up implicit conversion operators via reflection. Trimming may remove user-d" + "efined operators.")] [.(-1)] - public static . IsEqualTo(this . source, TOther? expected, [.("expected")] string? expectedExpression = null) { } + public static . IsEqualTo(this . source, TOther? expected, [.("expected")] string? expectedExpression = null, params .[] _) { } } public static class EquatableAssertionExtensions { @@ -5310,7 +5311,7 @@ namespace .Extensions [.("Looks up implicit conversion operators via reflection. Trimming may remove user-d" + "efined operators.")] [.(-1)] - public static . IsNotEqualTo(this . source, TOther? notExpected, [.("notExpected")] string? notExpectedExpression = null) { } + public static . IsNotEqualTo(this . source, TOther? notExpected, [.("notExpected")] string? notExpectedExpression = null, params .[] _) { } } public static class NotEquivalentToAssertionExtensions { diff --git a/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet8_0.verified.txt b/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet8_0.verified.txt index 010201b3c4..7ea0eac6d1 100644 --- a/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet8_0.verified.txt +++ b/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet8_0.verified.txt @@ -3168,6 +3168,7 @@ namespace .Extensions protected override .<.> CheckAsync(.<.Cookie> metadata) { } protected override string GetExpectation() { } } + public sealed class CrossTypeOverloadMarker { } public static class CultureInfoAssertionExtensions { [.("Trimming", "IL2091", Justification="Generic type parameter is only used for property access, not instantiation")] @@ -4301,7 +4302,7 @@ namespace .Extensions public static . IsEqualTo(this . source, TValue? expected, . comparer, [.("expected")] string? expectedExpression = null, [.("comparer")] string? comparerExpression = null) { } [.("Looks up implicit conversion operators via reflection. Trimming may remove user-d" + "efined operators.")] - public static . IsEqualTo(this . source, TOther? expected, [.("expected")] string? expectedExpression = null) { } + public static . IsEqualTo(this . source, TOther? expected, [.("expected")] string? expectedExpression = null, params .[] _) { } } public static class EquatableAssertionExtensions { @@ -5230,7 +5231,7 @@ namespace .Extensions public static . IsNotEqualTo(this . source, TValue notExpected, .? comparer = null, [.("notExpected")] string? notExpectedExpression = null, [.("comparer")] string? comparerExpression = null) { } [.("Looks up implicit conversion operators via reflection. Trimming may remove user-d" + "efined operators.")] - public static . IsNotEqualTo(this . source, TOther? notExpected, [.("notExpected")] string? notExpectedExpression = null) { } + public static . IsNotEqualTo(this . source, TOther? notExpected, [.("notExpected")] string? notExpectedExpression = null, params .[] _) { } } public static class NotEquivalentToAssertionExtensions { diff --git a/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet9_0.verified.txt b/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet9_0.verified.txt index af2f4e2b1f..3329f0025d 100644 --- a/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet9_0.verified.txt +++ b/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.DotNet9_0.verified.txt @@ -3203,6 +3203,7 @@ namespace .Extensions protected override .<.> CheckAsync(.<.Cookie> metadata) { } protected override string GetExpectation() { } } + public sealed class CrossTypeOverloadMarker { } public static class CultureInfoAssertionExtensions { [.("Trimming", "IL2091", Justification="Generic type parameter is only used for property access, not instantiation")] @@ -4357,7 +4358,7 @@ namespace .Extensions [.("Looks up implicit conversion operators via reflection. Trimming may remove user-d" + "efined operators.")] [.(-1)] - public static . IsEqualTo(this . source, TOther? expected, [.("expected")] string? expectedExpression = null) { } + public static . IsEqualTo(this . source, TOther? expected, [.("expected")] string? expectedExpression = null, params .[] _) { } } public static class EquatableAssertionExtensions { @@ -5310,7 +5311,7 @@ namespace .Extensions [.("Looks up implicit conversion operators via reflection. Trimming may remove user-d" + "efined operators.")] [.(-1)] - public static . IsNotEqualTo(this . source, TOther? notExpected, [.("notExpected")] string? notExpectedExpression = null) { } + public static . IsNotEqualTo(this . source, TOther? notExpected, [.("notExpected")] string? notExpectedExpression = null, params .[] _) { } } public static class NotEquivalentToAssertionExtensions { diff --git a/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.Net4_7.verified.txt b/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.Net4_7.verified.txt index 2c799fc620..dedaac056f 100644 --- a/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.Net4_7.verified.txt +++ b/TUnit.PublicAPI/Tests.Assertions_Library_Has_No_API_Changes.Net4_7.verified.txt @@ -2845,6 +2845,7 @@ namespace .Extensions protected override .<.> CheckAsync(.<.Cookie> metadata) { } protected override string GetExpectation() { } } + public sealed class CrossTypeOverloadMarker { } public static class CultureInfoAssertionExtensions { public static ._IsEnglish_Assertion IsEnglish(this . source) @@ -3822,7 +3823,7 @@ namespace .Extensions { public static . IsEqualTo(this . source, TValue? expected, [.("expected")] string? expectedExpression = null) { } public static . IsEqualTo(this . source, TValue? expected, . comparer, [.("expected")] string? expectedExpression = null, [.("comparer")] string? comparerExpression = null) { } - public static . IsEqualTo(this . source, TOther? expected, [.("expected")] string? expectedExpression = null) { } + public static . IsEqualTo(this . source, TOther? expected, [.("expected")] string? expectedExpression = null, params .[] _) { } } public static class EquatableAssertionExtensions { @@ -4651,7 +4652,7 @@ namespace .Extensions public static class NotEqualsAssertionExtensions { public static . IsNotEqualTo(this . source, TValue notExpected, .? comparer = null, [.("notExpected")] string? notExpectedExpression = null, [.("comparer")] string? comparerExpression = null) { } - public static . IsNotEqualTo(this . source, TOther? notExpected, [.("notExpected")] string? notExpectedExpression = null) { } + public static . IsNotEqualTo(this . source, TOther? notExpected, [.("notExpected")] string? notExpectedExpression = null, params .[] _) { } } public static class NotEquivalentToAssertionExtensions {