Skip to content

Conversation

@thomhurst
Copy link
Owner

Summary

  • Fixes IsEquivalentTo for ValueTuples and structs containing reference type properties
  • Removes the IEquatable<T> optimization that caused value types to use Equals() instead of structural comparison
  • Adds 46 comprehensive tests covering various scenarios

Problem

Previously, TypeHelper.IsPrimitiveOrWellKnownType() treated any value type implementing IEquatable<T> as a "primitive" type, which meant it used Equals() instead of structural comparison. This caused issues with:

  • ValueTuple<T1, T2, ...> containing records with array properties
  • Any struct with reference type fields that need deep comparison

For example, this test would fail:

public record Thing(string Name, int[] Numbers);

var foo = new Thing("Foo", [1, 2, 3]);
var bar = new Thing("Foo", [1, 2, 3]);

// Would FAIL because ValueTuple.Equals() delegates to record.Equals()
// which uses reference equality for arrays
await Assert.That((foo, bar)).IsEquivalentTo((bar, foo));

Solution

Removed the IEquatable<T> check for value types. IsEquivalentTo should always perform structural comparison:

  • For actual primitives (int, string, DateTime, etc.) - handled explicitly as primitives
  • For composite types (structs) - compare their fields/properties structurally
  • For types like Vector2 - structural comparison of X/Y yields the same result as Equals() anyway

Users who want Equals() behavior should use IsEqualTo() instead.

Test plan

  • All 46 new tests pass covering:
    • ValueTuple with various arities (2-7 elements)
    • Nested ValueTuples (multiple levels deep)
    • Structs with reference properties
    • Structs implementing IEquatable<T> (verifies structural comparison is used, not Equals())
    • Nullable references in structs
    • Empty collections in tuples/structs
    • Complex mixed scenarios (lists of tuples, arrays of structs, dictionaries)
  • All existing equivalency tests pass (41 tests)

Fixes #4358

🤖 Generated with Claude Code

…l value types

Previously, value types implementing IEquatable<T> (including ValueTuple) were
compared using Equals() instead of structural comparison. This caused issues
when structs contained reference type properties (e.g., arrays) that weren't
compared structurally.

- Remove IEquatable<T> optimization from TypeHelper.IsPrimitiveOrWellKnownType
- IsEquivalentTo now always compares struct fields/properties structurally
- Users who want Equals() behavior should use IsEqualTo() instead
- Add 46 comprehensive tests covering ValueTuple and struct equivalency

Fixes #4358

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@thomhurst
Copy link
Owner Author

Summary

Fixes IsEquivalentTo to use structural comparison for all value types, resolving issues with ValueTuples and structs containing reference properties.

Critical Issues

None found ✅

Analysis

This PR correctly addresses issue #4358 by removing the IEquatable optimization for value types in the TypeHelper.IsPrimitiveOrWellKnownType() method.

What Changed

The PR removes lines 90-98 from TypeHelper.cs that treated any value type implementing IEquatable as a "primitive" type. This was causing ValueTuples and structs with reference type fields to use Equals() instead of structural comparison.

Why This Is Correct

  1. Root Cause: ValueTuple<T1, T2> implements IEquatable<ValueTuple<T1, T2>>, which triggered the old optimization. When the tuple contained records with array properties, ValueTuple.Equals() delegated to the record's Equals(), which uses reference equality for arrays.

  2. Solution: By removing this optimization, IsEquivalentTo now performs structural comparison for ALL value types (except explicitly listed primitives), which correctly handles:

    • ValueTuples with any content (records, arrays, nested structures)
    • Custom structs with reference type fields
    • Nested value types
  3. Performance Impact: Minimal. For actual primitives like Vector2, structural comparison of X/Y properties yields identical results to Equals() anyway. The TypeHelper method is only called during assertion execution (not a hot path during test discovery).

Test Coverage

Excellent - 46 comprehensive tests covering:

  • All ValueTuple arities (2-7 elements)
  • Nested tuples (multiple levels)
  • Structs with reference properties
  • Structs implementing IEquatable (verifying structural comparison wins)
  • Empty collections, nullable references
  • Complex mixed scenarios

TUnit Rules Compliance

Dual-Mode: Not applicable (assertions are post-discovery)
Snapshot Testing: Not applicable (no generator or public API changes)
No VSTest: No VSTest dependencies added
Performance: Change affects assertion execution only, not hot paths
AOT Compatible: Existing [DynamicallyAccessedMembers] annotations remain intact

Code Quality

  • Clear inline comments explaining the design decision (TUnit.Assertions/Conditions/Helpers/TypeHelper.cs:90-96 in the diff)
  • Users wanting Equals() behavior can use IsEqualTo() instead
  • ImplementsSelfEquatable() method removed entirely (dead code cleanup)
  • Tests are well-organized with clear section comments

Verdict

APPROVE - This is a correct bug fix with excellent test coverage that follows all TUnit development principles.

@thomhurst thomhurst merged commit 451aad2 into main Jan 13, 2026
12 of 13 checks passed
@thomhurst thomhurst deleted the fix/issue-4358-isequivalentto-valuetuple branch January 13, 2026 19:49
This was referenced Jan 13, 2026
This was referenced Jan 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: IsEquivalentTo broken for value types

2 participants