feat: update StringEqualsAssertion to support nullable strings

[thomhurst]

Fixes #3643

[claude]

Claude finished @thomhurst's task —— View job

---

Code Review for PR #3648

Reviewing the changes to add nullable string support to StringEqualsAssertion.

Review Tasks:

• Analyze StringEqualsAssertion.cs changes
• Review new NullableStringEqualityTests.cs for coverage and quality
• Check public API changes for appropriateness
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Provide final feedback and recommendations

Analysis Complete ✅

Summary: This PR addresses issue #3643 by updating StringEqualsAssertion to properly support nullable strings (string?) without requiring workarounds like ?? string.Empty.

Key Changes:

• Changed generic type parameter from Assertion<string> to Assertion<string?>
• Updated constructor and method signatures to accept AssertionContext<string?>
• Updated CheckAsync method parameter to EvaluationMetadata<string?>
• Added comprehensive test coverage for nullable string scenarios

🟢 Positive Feedback

Code Quality & Best Practices:

• Clean implementation: The changes are minimal and surgical, only updating the necessary type signatures
• Follows TUnit patterns: Maintains consistency with existing assertion patterns and fluent API design
• Proper null handling: The existing logic already handles null values correctly in all transformation methods (_trimming, _ignoringWhitespace, _nullAndEmptyEquality)
• Modern C# usage: Good use of nullable reference types and proper null-conditional operators

Test Coverage:

• Comprehensive scenarios: Excellent test coverage including:
  • Both null vs both non-null comparisons
  • Null vs non-null mismatch cases
  • All string-specific modifiers with nullable strings
  • Combined modifier scenarios
  • Method return value scenarios
• Clear test names: Test method names clearly describe the scenario being tested
• Good documentation: Comments explain the purpose and relate back to issue Assertion On Nullable String Actual Value Requires Non-Nullable String Expected Value #3643

API Design:

• Backward compatible: The change is purely additive - existing string usage will continue to work
• Type-safe: Proper nullable annotations ensure compile-time null safety
• Consistent: Extension methods properly updated to support IValueSource<string?>

🟡 Minor Observations

Performance Considerations:

• No performance impact: The changes are purely type-level, with no runtime performance implications
• Existing optimizations preserved: All the performance optimizations for string processing (Span usage, conditional compilation) remain intact

Security Considerations:

• No security concerns: This is a type signature change with no new attack vectors
• Safe null handling: All existing null checks and safe string operations are preserved

🟢 Final Recommendation

APPROVE - This is a well-implemented, focused change that:

1. Solves the stated problem: Users can now use nullable strings directly without workarounds
2. Maintains code quality: Clean, minimal changes that follow established patterns
3. Has excellent test coverage: Comprehensive tests cover all scenarios and edge cases
4. Is backward compatible: No breaking changes to existing functionality
5. Follows TUnit conventions: Proper snapshot updates for public API changes

Verification:

• ✅ Public API snapshots properly updated across all target frameworks
• ✅ Implementation handles all nullable string scenarios correctly
• ✅ Tests verify behavior with all existing string assertion modifiers
• ✅ No performance or security regressions introduced

This PR is ready for merge! 🚀

---

[claude]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[claude]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[claude]

Claude encountered an error —— View job

---

Code Review for PR #3648

Reviewing the changes to add nullable string support to StringEqualsAssertion.

Review Tasks:

• Analyze StringEqualsAssertion.cs changes
• Review new NullableStringEqualityTests.cs for coverage and quality
• Check public API changes for appropriateness
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Provide final feedback and recommendations

Analysis Complete ✅

Summary: This PR addresses issue #3643 by updating StringEqualsAssertion to properly support nullable strings (string?) without requiring workarounds like ?? string.Empty.

Key Changes:

• Changed generic type parameter from Assertion<string> to Assertion<string?>
• Updated constructor and method signatures to accept AssertionContext<string?>
• Updated CheckAsync method parameter to EvaluationMetadata<string?>
• Added comprehensive test coverage for nullable string scenarios

🟢 Positive Feedback

Code Quality & Best Practices:

• Clean implementation: The changes are minimal and surgical, only updating the necessary type signatures
• Follows TUnit patterns: Maintains consistency with existing assertion patterns and fluent API design
• Proper null handling: The existing logic already handles null values correctly in all transformation methods (_trimming, _ignoringWhitespace, _nullAndEmptyEquality)
• Modern C# usage: Good use of nullable reference types and proper null-conditional operators

Test Coverage:

• Comprehensive scenarios: Excellent test coverage including:
  • Both null vs both non-null comparisons
  • Null vs non-null mismatch cases
  • All string-specific modifiers with nullable strings
  • Combined modifier scenarios
  • Method return value scenarios
• Clear test names: Test method names clearly describe the scenario being tested
• Good documentation: Comments explain the purpose and relate back to issue Assertion On Nullable String Actual Value Requires Non-Nullable String Expected Value #3643

API Design:

• Backward compatible: The change is purely additive - existing string usage will continue to work
• Type-safe: Proper nullable annotations ensure compile-time null safety
• Consistent: Extension methods properly updated to support IValueSource<string?>

🟡 Minor Observations

Performance Considerations:

• No performance impact: The changes are purely type-level, with no runtime performance implications
• Existing optimizations preserved: All the performance optimizations for string processing (Span usage, conditional compilation) remain intact

Security Considerations:

• No security concerns: This is a type signature change with no new attack vectors
• Safe null handling: All existing null checks and safe string operations are preserved

🟢 Final Recommendation

APPROVE - This is a well-implemented, focused change that:

1. Solves the stated problem: Users can now use nullable strings directly without workarounds
2. Maintains code quality: Clean, minimal changes that follow established patterns
3. Has excellent test coverage: Comprehensive tests cover all scenarios and edge cases
4. Is backward compatible: No breaking changes to existing functionality
5. Follows TUnit conventions: Proper snapshot updates for public API changes

Verification:

• ✅ Public API snapshots properly updated across all target frameworks
• ✅ Implementation handles all nullable string scenarios correctly
• ✅ Tests verify behavior with all existing string assertion modifiers
• ✅ No performance or security regressions introduced

This PR is ready for merge! 🚀

---

[claude]

Claude finished @thomhurst's task —— View job

---

Code Review for PR #3648

Reviewing the changes to add nullable string support to StringEqualsAssertion.

Review Tasks:

• Analyze StringEqualsAssertion.cs changes
• Review new NullableStringEqualityTests.cs for coverage and quality
• Review new NullabilityWarningTests.cs
• Check public API changes for appropriateness
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Provide final feedback and recommendations

Analysis Complete ✅

Summary: This PR addresses issue #3643 by updating StringEqualsAssertion to properly support nullable strings (string?) without requiring workarounds like ?? string.Empty.

Key Changes:

• Changed generic type parameter from Assertion<string> to Assertion<string?> in StringEqualsAssertion<TActual>
• Updated constructor and method signatures to accept AssertionContext<TActual>
• Updated CheckAsync method parameter to EvaluationMetadata<TActual>
• Added comprehensive test coverage for nullable string scenarios
• Updated public API snapshots across all target frameworks

🟢 Positive Feedback

Code Quality & Best Practices:

• Clean implementation: The changes are minimal and surgical, only updating the necessary type signatures
• Follows TUnit patterns: Maintains consistency with existing assertion patterns and fluent API design
• Proper null handling: The existing logic already handles null values correctly in all transformation methods (_trimming, _ignoringWhitespace, _nullAndEmptyEquality)
• Modern C# usage: Good use of nullable reference types and proper null-conditional operators
• Type safety: Uses generic TActual parameter allowing both string and string? support

Test Coverage:

• Comprehensive scenarios: Excellent test coverage including:
  • Both null vs both non-null comparisons
  • Null vs non-null mismatch cases
  • All string-specific modifiers with nullable strings (IgnoringCase, WithTrimming, IgnoringWhitespace, WithNullAndEmptyEquality)
  • Combined modifier scenarios
  • Method return value scenarios
  • Mixed nullability scenarios
• Clear test names: Test method names clearly describe the scenario being tested
• Good documentation: Comments explain the purpose and relate back to issue Assertion On Nullable String Actual Value Requires Non-Nullable String Expected Value #3643
• Dedicated nullability warning tests: NullabilityWarningTests.cs specifically validates that all combinations work without compiler warnings

API Design:

• Backward compatible: The change is purely additive - existing string usage will continue to work seamlessly
• Type-safe: Proper nullable annotations ensure compile-time null safety
• Consistent: Extension methods properly updated to support IValueSource<string?> through generic type parameter
• Proper overload resolution: Uses OverloadResolutionPriority attributes in generated extensions to ensure correct method selection

Source Generator Integration:

• Well-integrated: The AssertionExtensionGenerator.cs properly handles the new generic signature
• Maintains existing patterns: Uses the same generation logic while supporting the new nullable type parameter

🟡 Minor Observations

Performance Considerations:

• No performance impact: The changes are purely type-level, with no runtime performance implications
• Existing optimizations preserved: All the performance optimizations for string processing (Span usage on .NET 8+, conditional compilation) remain intact

Security Considerations:

• No security concerns: This is a type signature change with no new attack vectors
• Safe null handling: All existing null checks and safe string operations are preserved

Public API Changes:

• Appropriate scope: The API changes in public snapshot files show the expected updates to StringEqualsAssertion<TActual> and related extension methods
• Properly versioned: Changes are reflected across all target framework snapshots (.NET Framework 4.7, .NET 8.0, .NET 9.0, .NET 10.0)

🟢 Final Recommendation

APPROVE ✅ - This is a well-implemented, focused change that:

1. Solves the stated problem: Users can now use nullable strings directly without workarounds like ?? string.Empty
2. Maintains code quality: Clean, minimal changes that follow established TUnit patterns
3. Has excellent test coverage: Comprehensive tests cover all scenarios including edge cases and nullability warning validation
4. Is backward compatible: No breaking changes to existing functionality - all existing string assertions continue to work
5. Follows TUnit conventions: Proper snapshot updates for public API changes, dual-mode compatibility maintained
6. Type-safe implementation: Uses modern C# nullable reference types correctly

Verification Checklist:

• ✅ Public API snapshots properly updated across all target frameworks
• ✅ Implementation handles all nullable string scenarios correctly
• ✅ Tests verify behavior with all existing string assertion modifiers
• ✅ No performance or security regressions introduced
• ✅ Backward compatibility maintained for existing code
• ✅ Nullability warnings eliminated as intended

This PR is ready for merge! 🚀

The implementation demonstrates excellent attention to detail, comprehensive testing, and follows all TUnit development best practices. The change successfully addresses the original issue while maintaining the high quality standards of the codebase.

---