feat: impprove type inference for some collection assertions

[thomhurst]

No description provided.

[claude]

Claude finished @thomhurst's task —— View job

---

Code Review Complete ✅

Summary

This PR improves type inference for collection assertions by removing redundant [AssertionExtension] attributes from three assertion classes. The functionality remains available through better-typed instance methods in CollectionAssertionBase.

Changes Reviewed

Primary Changes

• Removed [AssertionExtension] attributes from:
  • CollectionIsEmptyAssertion ("IsEmpty")
  • CollectionIsNotEmptyAssertion ("IsNotEmpty")
  • CollectionCountAssertion ("HasCount")

Impact

• Before: Users had both extension methods AND instance methods available
• After: Only instance methods available (through CollectionAssertionBase<TCollection, TItem>)
• Result: Better type inference, reduced API surface duplication

Supporting Changes

• API verification files updated to reflect removal of extension classes
• Speed comparison test updated: WithInclusiveBounds() → Inclusive() (unrelated cleanup)

Code Quality Assessment

✅ Strengths

1. Cleaner API Design: Eliminates confusing duplication where both extension methods and instance methods provided identical functionality
2. Better Type Inference: Instance methods on CollectionAssertionBase provide superior type inference since they preserve both TCollection and TItem type parameters
3. Consistent Architecture: Aligns with the framework's pattern of providing collection operations as instance methods
4. Proper Documentation: All affected instance methods are well-documented with examples
5. Snapshot Tests Updated: API verification tests properly updated, following repository guidelines

✅ Technical Implementation

• No Breaking Changes: Functionality remains identical, just accessed through instance methods
• Performance: No performance impact - same underlying assertion classes used
• Type Safety: Enhanced type safety through better generic type preservation
• Maintainability: Reduces code generation complexity and potential conflicts

Security & Performance Considerations

✅ Security

• No security implications - pure refactoring of API surface
• No changes to core assertion logic

✅ Performance

• Neutral performance impact
• Reduces generated code size slightly
• Same execution paths used

Test Coverage

✅ Testing

• Speed comparison tests updated correctly
• API surface tests properly reflect changes
• Core functionality unchanged, existing tests remain valid

Adherence to Repository Guidelines

✅ TUnit Guidelines Met

• ✅ Snapshot tests accepted (API verification files updated)
• ✅ No dual-mode issues (change affects source generation only)
• ✅ Modern C# patterns maintained
• ✅ Performance-first approach (reduced generated code)
• ✅ Thread safety unchanged

Recommendations

✅ Approved Changes

This PR successfully improves the API design without breaking functionality. The changes align well with the framework's architecture and improve developer experience through better type inference.

Minor Observations

1. Typo in Title: "impprove" → "improve" (minor)
2. Unrelated Change: The WithInclusiveBounds() → Inclusive() change appears unrelated but is a positive cleanup

Conclusion

Recommendation: ✅ APPROVE

This is a well-executed refactoring that improves API consistency and type inference without breaking changes. The implementation follows repository guidelines and maintains all existing functionality while reducing API surface complexity.

[tigran-sargsyan-w]

Great improvement to collection assertion type inference — the diff is small and targeted. ✅
A couple of quick nits before merge:

PR title typo: “impprove” → “improve.”

PR description: please add a brief note on what inference changed and confirm there’s no public API change (PublicAPI snapshots only show deletions from verification files).

Tests: consider adding/pointing to a test that covers a non-trivial IEnumerable (e.g., custom iterables) to demonstrate the improved inference; I see a tiny update under tools/speed-comparison/UnifiedTests/AssertionTests.cs, but an assertion focused test would help future regressions.

Looks good overall! 🎯