-
-
Notifications
You must be signed in to change notification settings - Fork 108
fix: handle IgnoringType for types nested in ValueTypes #4322
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When a type is stored inside a ValueTuple (or any value type implementing IEquatable<T>), the tuple was treated as a "primitive" type and compared directly with Equals() instead of recursively comparing members. This caused IgnoringType to be bypassed for types nested within tuples. The fix adds a ContainsIgnoredGenericArgument() check that recursively inspects generic type arguments. When a generic value type contains an ignored type, the comparison falls through to structural comparison which properly handles the ignored type. Fixes #4320 Co-Authored-By: Claude Opus 4.5 <[email protected]>
SummaryFixes a bug where Critical IssuesNone found ✅ SuggestionsPerformance: Hot path optimizationThe new // Current: Always calls the method
if (TypeHelper.IsPrimitiveOrWellKnownType(actualType) && !ContainsIgnoredGenericArgument(actualType))
// Suggested: Only check generic types
if (TypeHelper.IsPrimitiveOrWellKnownType(actualType) &&
!(actualType.IsGenericType && _ignoredTypes.Count > 0 && ContainsIgnoredGenericArgument(actualType)))This avoids the method call overhead for non-generic types and when no types are being ignored (the common case). The early exits are already in Alternatively, consider caching results or only checking value types that implement IEquatable (since that's the only category from TypeHelper that would include tuples). Consider edge case: What about generic classes implementing IEquatable?The fix specifically targets value types (which include ValueTuple), but could a similar issue occur with a generic class that implements Verdict✅ APPROVE - No critical issues. The fix correctly solves the reported bug with comprehensive tests. The performance suggestion is optional optimization. |
Summary
IgnoringTypebeing bypassed when the ignored type is nested inside aValueTupleor other value type implementingIEquatable<T>ContainsIgnoredGenericArgument()helper method that recursively checks if a generic type contains ignored typesProblem
When a type is stored inside a
ValueTuple(like(IgnoreMe, IgnoreMe)), the tuple is treated as a "primitive" type byTypeHelper.IsPrimitiveOrWellKnownType()because it implementsIEquatable<T>. This causes the comparison to useEquals()directly instead of recursively comparing members, bypassing theShouldIgnoreTypecheck entirely.Solution
Before treating a value type as primitive, check if it's a generic type containing any ignored types as generic arguments. If so, fall through to structural comparison which properly handles the ignored types.
Test plan
IgnoringType_In_Tuple_Properties_Are_Ignored- Basic tuple caseIgnoringType_In_Nested_Tuple_Properties_Are_Ignored- Nested tuplesIgnoringType_In_Mixed_Tuple_Still_Compares_Non_Ignored_Parts- Verifies non-ignored parts are still comparedIgnoringType_In_Mixed_Tuple_Passes_When_NonIgnored_Parts_Match- Mixed tuple with matching non-ignored valuesIgnoringTypeEquivalentTestspass (12 tests)CollectionStructuralEquivalenceTestspass (20 tests)EquivalentAssertionTestspass (29 tests)Fixes #4320
🤖 Generated with Claude Code