feat(assertions): IsEqualTo with implicitly-convertible wrappers (#5720)

[thomhurst]

Closes #5720

Summary

• Adds generic IsEqualTo<TValue, TOther> and IsNotEqualTo<TValue, TOther> overloads that detect a user-defined implicit operator at runtime (cached per type-pair) and compare the converted value. This restores Assert.That(productCode).IsEqualTo("Example") for wrapper Value Objects and generalises it to int / decimal / custom wrappers.
• Defined as partials of the source-generated EqualsAssertionExtensions / NotEqualsAssertionExtensions so [OverloadResolutionPriority(-1)] actually disambiguates calls where source and expected types match (priority is only consulted between overloads in the same containing type).
• AssertionExtensionGenerator now emits public static partial class to host these partials.
• Two F# tests call the int-specific extension method directly because F# does not honour OverloadResolutionPriority.

Test plan

• New Issue5720Tests cover string and int wrapper Value Objects (positive, negative, null, same-type-still-wins)
• All 2030 TUnit.Assertions.Tests pass (net10.0)
• Source-generator snapshot tests pass on net8.0 / net9.0 / net10.0 (snapshots updated for the partial keyword)
• PublicAPI snapshot tests pass on net8.0 / net9.0 / net10.0; Net4_7 snapshot updated by hand (no host runtime locally)
• F# test project compiles and links

[codacy-production]

Not up to standards ⛔

🔴 Issues 2 minor

Alerts:
⚠ 2 issues (≤ 0 issues of at least minor severity)

Results:
2 new issues

Category	Results
CodeStyle	2 minor

View in Codacy

🟢 Metrics 31 complexity

Metric	Results
Complexity	31

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Code Review: IsEqualTo with implicitly-convertible wrappers

Summary

The PR adds generic IsEqualTo<TValue, TOther> and IsNotEqualTo<TValue, TOther> overloads that discover an implicit conversion operator at runtime via reflection (cached per type-pair), then compares the converted value. The source generator is updated to emit partial classes so hand-written partials can live alongside generated ones, allowing [OverloadResolutionPriority(-1)] to work within the same type.

Overall the design approach is sound — caching reflection, using partial classes for OverloadResolutionPriority, and applying [RequiresUnreferencedCode] properly. However there are a few issues to address before merge.

---

Issues

Bug: FindImplicitOperator fallback searches in the wrong direction

File: TUnit.Assertions/Extensions/ImplicitConversionEqualityExtensions.cs

var op = FindImplicitOperator(fromType, toType)
         ?? FindImplicitOperator(toType, fromType);   // <-- logically wrong

FindImplicitOperator(definedOn, targetType) returns a method defined on definedOn whose ReturnType == targetType. The first call correctly finds an operator on TFrom that converts to TTo. The second call is intended to handle the C# rule that implicit operators may be declared on either participant type — but FindImplicitOperator(toType, fromType) looks for a method on TTo with ReturnType == TFrom, which is the reverse conversion (TTo → TFrom), not the forward one (TFrom → TTo defined on TTo).

The correct second search should look for a method on TTo that has ReturnType == TTo and a single parameter of type TFrom:

// Correct: find "public static implicit operator TTo(TFrom value)" defined on TTo
private static MethodInfo? FindImplicitOperatorOnTarget(Type definedOn, Type fromType)
{
    foreach (var method in definedOn.GetMethods(BindingFlags.Public | BindingFlags.Static))
    {
        if (method.Name != "op_Implicit") continue;
        if (method.ReturnType != definedOn) continue;  // ReturnType == TTo
        var parameters = method.GetParameters();
        if (parameters.Length == 1 && parameters[0].ParameterType == fromType)
            return method;
    }
    return null;
}

This bug is invisible in the current test suite because all test operators are defined on the source type, but it will silently fail for the legitimate C# pattern of defining the conversion on the target type.

---

Inconsistency: IsNotEqualTo<TValue,TOther> parameter is not nullable

File: TUnit.Assertions/Extensions/ImplicitConversionEqualityExtensions.cs

The companion IsEqualTo<TValue,TOther> takes TOther? expected (nullable), but IsNotEqualTo takes a non-nullable TOther. This means Assert.That(x).IsNotEqualTo((string?)null) won't compile where Assert.That(x).IsEqualTo((string?)null) does (and is tested). The parameter should be TOther? notExpected for consistency, with a corresponding null test case added.

---

F# template files not updated — will produce broken projects

File: TUnit.Templates/content/TUnit.FSharp/Tests.fs

The F# template files still use the old call pattern. F# does not honour [OverloadResolutionPriority], so after this PR introduces a second applicable overload in EqualsAssertionExtensions, new projects created from the F# template will fail to compile with an ambiguity error. The project's own test files were patched (with explicit type qualification), but the shipped templates were missed. This will affect every user who creates a new F# TUnit project.

---

Minor / Non-blocking Notes

Cache write pattern — The current code uses a manual TryGetValue + assignment pattern. Under contention, two threads can both miss the cache check and both call BuildConverter. This is benign (equivalent delegates), but the idiomatic pattern Cache.GetOrAdd(key, _ => BuildConverter<TFrom, TTo>()) is more consistent with how PropertyCache is used elsewhere in the codebase.

MethodInfo.Invoke boxes value types — op.Invoke(null, [value]) boxes value-type arguments on every assertion call even though the delegate is cached. An Expression.Lambda<Func<TFrom,TTo>>(...).Compile() would avoid this, but requires [RequiresDynamicCode] making it incompatible with AOT. Worth a brief comment explaining the trade-off so future maintainers don't blindly switch to expression compilation without adding the AOT annotation.

Missing error-path test — There's no test for the case where no implicit operator exists. The error-throwing lambda is cached, and the resulting InvalidOperationException surfaces inside assertion evaluation rather than at the call site, which may produce a confusing failure message. A test confirming the error is reported clearly would help.

---

Positives

• The partial class approach for co-locating hand-written and generated overloads is the correct design for OverloadResolutionPriority to work.
• Caching the reflection lookup per type-pair is the right performance mitigation.
• [RequiresUnreferencedCode] is consistently applied throughout.
• The null-guard in the converter lambda correctly handles nullable reference types.
• Snapshot and PublicAPI tests are fully updated including the net4.7 case.
• Test coverage for primary use cases (string wrapper, int wrapper, positive/negative, null, same-type) is solid.

---

Recommendation: REQUEST_CHANGES

The FindImplicitOperator fallback logic is incorrect for the "operator defined on target type" case — a real bug that current tests don't exercise. The IsNotEqualTo non-nullable parameter is a usability inconsistency. The F# template files are not updated and will produce broken projects for new users. These three issues should be fixed before merge.

[claude]

Follow-up Review: All Previous Blocking Issues Resolved ✅

All three blocking issues from the prior review (commit 33b0b9ef) have been addressed in the current HEAD:

• FindImplicitOperator direction bug — Fixed. The 3-arg signature FindImplicitOperator(toType, fromType, toType) correctly searches for op_Implicit on the target type returning toType with a parameter of fromType.
• IsNotEqualTo nullability — Fixed. The parameter is now TOther? notExpected.
• F# template files — Fixed. Both TUnit.FSharp/Tests.fs and TUnit.AspNet.FSharp/TestProject/Tests.fs call the type-specific extension directly with explanatory comments.

---

Remaining Observations

Minor: Cache write pattern is not race-safe against wasted work

// Current:
if (Cache.TryGetValue(key, out var cached))
    return (Func<TFrom?, TTo?>)cached;
var converter = BuildConverter<TFrom, TTo>();
Cache[key] = converter;
return converter;

Under concurrent first-call pressure for the same type pair, multiple threads can each run BuildConverter and overwrite each other — safe because all converters are equivalent, but wasteful. The idiomatic pattern is:

return (Func<TFrom?, TTo?>)Cache.GetOrAdd(key, _ => BuildConverter<TFrom, TTo>());

GetOrAdd still allows a race on the factory call (it doesn't lock), but the returned value is always the winner's, and future readers always hit the cache. The intent to "build once" is also more clearly expressed. This is non-blocking.

Minor: No error-path test for missing implicit operator

There is no test verifying that calling IsEqualTo when no implicit conversion operator exists throws InvalidOperationException with the diagnostic message. The message is good:

No implicit conversion operator from 'X' to 'Y' was found. IsEqualTo / IsNotEqualTo with a different argument type requires 'X' to define 'public static implicit operator Y(X value)'.

A test for this path would protect the error message from unintentional changes and confirm the feature behaves sensibly for misuse. Non-blocking but recommended.

Noted: F# fluent API regression

The F# template change from:

do! check(Assert.That(result).IsEqualTo(c))

to:

do! check(IntEqualsAssertionExtensions.IsEqualTo(Assert.That(result), c))

…is verbose and breaks the fluent experience for F# users. This is a pre-existing limitation (F# ignores [OverloadResolutionPriority]) that is now more visible because the PR introduces the ambiguity. There is no clean solution within this PR's scope — it's a known F# language constraint — but it's worth tracking as a known rough edge.

---

Positives

• The partial class design for co-locating the overload with the source-generated one is the correct architectural choice for [OverloadResolutionPriority] to work.
• The cache lookup is per-type-pair and lazily populated — correct performance trade-off.
• [RequiresUnreferencedCode] and [DynamicallyAccessedMembers] are applied correctly and consistently; the [UnconditionalSuppressMessage] on FindImplicitOperator is properly justified.
• The AOT comment explaining the MethodInfo.Invoke boxing trade-off vs Expression.Lambda is exactly the right level of documentation.
• Snapshot and PublicAPI tests updated across all TFMs including net4.7.
• Test coverage for the primary use cases (string wrapper, int wrapper, operator on target type, nulls, IsNotEqualTo) is solid.

Verdict: Approve. The core design is sound, all previous blockers are resolved, and the remaining items are minor and non-blocking.

[claude]

All three previous blocking issues resolved. Approving — see inline comment for remaining minor observations.

[claude]

Code Review: Final Pass ✅

This is a follow-up to the two prior review rounds. All previous blocking issues and minor observations from those rounds have been addressed in the current HEAD (). Here is a fresh assessment of the final state.

---

Previous Observations — Status

Issue	Status
FindImplicitOperator direction bug	✅ Fixed — correct 3-arg signature
IsNotEqualTo nullability (TOther notExpected → TOther? notExpected)	✅ Fixed
F# template files calling type-specific extension	✅ Fixed with explanatory comments
Cache.GetOrAdd instead of manual TryGetValue/set	✅ Fixed
Error-path test for missing implicit operator	✅ Fixed — IsEqualTo_NoImplicitOperator_Surfaces_Diagnostic + IsNotEqualTo_...

---

Fresh Assessment

Core implementation () is clean and correct:

• The with is idiomatic. The keyword is valid here — generic type parameters are not captured variables, they are part of the generic instantiation.
• Null path in the converter lambda is correct for both reference types and struct-based : boxing a with produces a boxed , which unboxes correctly.
• on the parameter of protects public operators from trimming at analysis time. The with a clear justification is appropriate.
• The comment explaining why was not used (would require , breaking Native AOT) is valuable — keep it.
• The null-forgiving operator in works correctly at runtime; the test covers that path.

Source generator change (): emitting is necessary and correct. This is not a breaking change — only adds the ability to split a class across files; it removes no API surface.

Test coverage () is thorough:

• Positive and negative cases for both string and int wrappers
• Operator-on-target-type fallback
• Null source vs non-null expected and vice versa
• Same-type still uses the original overload (critical regression guard)
• Missing-operator diagnostic surfaced correctly

F# workaround (, template snapshots): the direct call to with an explanatory comment is the right approach given that F# does not honour .

---

One Minor Observation (Non-Blocking)

** in the hand-written partials** uses a single interpolated string:

The source-generated code uses three separate calls to avoid the intermediate allocation. The difference is negligible in assertion paths, but if you ever want strict consistency with the generated style, the three-call pattern would match. Not worth a change on its own.

---

Summary

The implementation is well-designed and production-ready. The -class approach for is the correct solution to the overload disambiguation problem, the reflection cache is safe and efficient, and the AOT story is handled correctly with appropriate annotations. The test suite is comprehensive.

No further changes required — this is good to merge.

[claude]

Code Review: Final Pass ✅

This is a follow-up to the two prior review rounds. All previous blocking issues and minor observations from those rounds have been addressed in the current HEAD (bb27ef48). Here is a fresh assessment of the final state.

---

Previous Observations — Status

Issue	Status
FindImplicitOperator direction bug	✅ Fixed — correct 3-arg signature
IsNotEqualTo nullability (TOther notExpected → TOther? notExpected)	✅ Fixed
F# template files calling type-specific extension	✅ Fixed with explanatory comments
Cache.GetOrAdd instead of manual TryGetValue/set	✅ Fixed
Error-path test for missing implicit operator	✅ Fixed — IsEqualTo_NoImplicitOperator_Surfaces_Diagnostic + IsNotEqualTo_...

---

Fresh Assessment

Core implementation (ImplicitConversionEqualityExtensions.cs) is clean and correct:

• The ConcurrentDictionary.GetOrAdd with static _ => BuildConverter<TFrom, TTo>() is idiomatic. The static keyword is valid here — generic type parameters are not captured variables; they are part of the generic instantiation.
• The null path in the converter lambda is correct for both reference types and struct-based Nullable<T>: boxing a Nullable<T> with HasValue = true produces a boxed T, which MethodInfo.Invoke unboxes correctly.
• [DynamicallyAccessedMembers(PublicMethods)] on the definedOn parameter of FindImplicitOperator protects public operators from trimming at analysis time. The [UnconditionalSuppressMessage] with a clear justification is appropriate.
• The comment explaining why Expression.Lambda(...).Compile() was not used (would require [RequiresDynamicCode], breaking Native AOT) is valuable — keep it.
• The notExpected! null-forgiving operator in IsNotEqualTo works correctly at runtime; the test IsNotEqualTo_NonNull_Wrapper_Against_Null_Primitive_Passes verifies that path.

Source generator change (AssertionExtensionGenerator.cs): emitting public static partial class is necessary and correct. This is not a breaking change — partial only adds the ability to split a class across files; it removes no API surface.

Test coverage (Issue5720Tests.cs) is thorough:

• Positive and negative cases for both string and int wrappers
• Operator-on-target-type fallback
• Null source vs non-null expected and vice versa
• Same-type still uses the original overload (critical regression guard)
• Missing-operator diagnostic surfaced correctly

F# workaround (AsyncTests.fs, template snapshots): the direct call to IntEqualsAssertionExtensions.IsEqualTo(...) with an explanatory comment is the right approach given that F# does not honour [OverloadResolutionPriority].

---

One Minor Observation (Non-Blocking)

ExpressionBuilder.Append in the hand-written partials uses a single interpolated string:

source.Context.ExpressionBuilder.Append($".IsEqualTo({expectedExpression})");

The source-generated code uses three separate Append calls to avoid the intermediate allocation. The difference is negligible in assertion paths, but if consistency with the generated style matters, the three-call pattern would match. Not worth a change on its own.

---

Summary

The implementation is well-designed and production-ready. The partial-class approach for [OverloadResolutionPriority] is the correct solution to the overload disambiguation problem, the reflection cache is safe and efficient, and the AOT story is handled correctly with appropriate annotations. The test suite is comprehensive.

No further changes required — this is good to merge.