Add support for ignoring specific types in equivalency assertions

[thomhurst]

… and enhance tests for type ignoring functionality

Fixes #2614

[claude]

Claude finished @thomhurst's task —— View job

---

Code Review Progress

Reviewing PR #3154: Add support for ignoring specific types in equivalency assertions

Todo List:

• Read TUnit development guidelines from CLAUDE.md
• Analyze the new test file for coverage and quality
• Review assertion builder wrapper changes
• Examine core comparison logic modifications
• Check public API changes for consistency
• Evaluate performance considerations
• Assess security implications
• Provide comprehensive feedback

---

Code Review Feedback

This PR adds a valuable feature for ignoring specific types during equivalency assertions. The implementation is well-structured and follows TUnit's coding standards. Here's my detailed analysis:

✅ Strengths

1. Comprehensive Test Coverage: The new test file IgnoringTypeEquivalentTests.cs provides excellent coverage:

   • Tests both generic (IgnoringType<T>()) and non-generic (IgnoringType(Type)) APIs
   • Covers nullable types, DateTimeOffset, multiple types, and fields
   • Tests both positive and negative assertion scenarios
   • Includes edge cases like ignoring types that don't exist in the objects

2. Consistent API Design: The new IgnoringType methods follow the same fluent pattern as existing IgnoringMember functionality:

   • Generic and non-generic overloads for flexibility
   • Proper CallerArgumentExpression usage for better debugging
   • Identical implementation in both EquivalentTo and NotEquivalentTo wrappers

3. Smart Type Matching Logic: The ShouldIgnoreType method in Compare.cs:298-314 handles several important scenarios:

   • Nullable type unwrapping (Nullable.GetUnderlyingType)
   • Exact type matching and inheritance hierarchy checks
   • Both IsAssignableFrom directions for comprehensive type matching

4. AOT-Friendly Implementation: Proper use of DynamicallyAccessedMembersAttribute maintains AOT compatibility

⚠️ Areas for Improvement

1. Performance Consideration: In Compare.cs:176-184 and 236-244, the ShouldIgnoreType check is called for every field/property. Consider optimizing this:

   // Current: O(n*m) where n=members, m=ignored types
   // Consider: Pre-compute ignored types set for O(1) lookup

2. Type Matching Logic: The current implementation at Compare.cs:309-313 might be overly broad:

   return typesToIgnore.Any(ignoredType => 
       type == ignoredType || 
       underlyingType == ignoredType ||
       type.IsAssignableFrom(ignoredType) ||      // ⚠️ Might match unintended types
       underlyingType.IsAssignableFrom(ignoredType)); // ⚠️ Same concern

   The bidirectional IsAssignableFrom check could lead to unexpected behavior. Consider if this is the intended semantics.

3. Missing Null Checks: ShouldIgnoreType doesn't handle null arrays gracefully. While the length check protects against most issues, explicit null checking would be more robust.

🔒 Security Assessment

No security concerns identified. The feature only affects assertion behavior and doesn't introduce any unsafe operations or external dependencies.

📈 Performance Impact

• Discovery/Execution: Minimal impact on test discovery
• Assertion Performance: Small overhead per field/property comparison, but acceptable for the functionality gained
• Memory: Negligible - only stores Type[] references

🎯 Suggestions

1. Optimize Type Checking:

   private static readonly ConcurrentDictionary<(Type[], Type), bool> _typeIgnoreCache = new();
   
   private static bool ShouldIgnoreType(Type type, Type[] typesToIgnore)
   {
       if (typesToIgnore.Length == 0) return false;
       
       return _typeIgnoreCache.GetOrAdd((typesToIgnore, type), static key => 
       {
           var (types, checkType) = key;
           var underlyingType = Nullable.GetUnderlyingType(checkType) ?? checkType;
           return types.Any(ignoredType => 
               checkType == ignoredType || 
               underlyingType == ignoredType ||
               checkType.IsAssignableFrom(ignoredType) ||
               underlyingType.IsAssignableFrom(ignoredType));
       });
   }

2. Consider Inheritance Semantics: Document or test the inheritance behavior more explicitly. For example, should ignoring object ignore all reference types?

3. Minor Code Style: Consider using HashSet<Type> instead of Type[] for better lookup performance when the ignored types list is large.

✅ Compliance Check

• Behavioral Parity: ✅ No source generator changes needed - reflection-only feature
• Public API: ✅ Snapshots correctly updated across all target frameworks
• Modern C#: ✅ Uses collection expressions, proper async patterns
• Threading: ✅ No shared state concerns
• AOT Compatibility: ✅ Proper attributes for reflection usage

🎉 Overall Assessment

This is a well-implemented feature that adds valuable functionality to TUnit's assertion library. The code follows TUnit's standards, has comprehensive tests, and maintains performance characteristics. The minor suggestions above are optimizations rather than blocking issues.

Recommendation: ✅ Approve - Ready to merge with optional performance optimizations to be considered in future iterations.

---