Skip to content

Fix All().Satisfy() compile error with nullable value types#4685

Merged
thomhurst merged 1 commit intomainfrom
copilot/fix-all-satisfy-bug-double-type
Feb 6, 2026
Merged

Fix All().Satisfy() compile error with nullable value types#4685
thomhurst merged 1 commit intomainfrom
copilot/fix-all-satisfy-bug-double-type

Conversation

Copy link
Contributor

Copilot AI commented Feb 6, 2026

All().Satisfy() fails to compile when the mapped property is a nullable value type (e.g., double?). The Satisfy lambda expects Assertion<TMapped>? as return type, but IsNotNull() on double? returns NotNullAssertion<double> (i.e., Assertion<double>), which is not assignable to Assertion<double?>.

// This fails to compile
await Assert.That(items)
    .All()
    .Satisfy(x => x.Price, c => c.IsNotNull()); // Price is double?

Approach

Uses a generic TAssertion type parameter with where TAssertion : IAssertion constraint on the Satisfy methods. This preserves type safety (the compiler still enforces that the lambda returns an assertion) while allowing type-transforming assertions like IsNotNull() on nullable value types.

Changes

  • CollectionAssertions.cs: Added TAssertion type parameter with IAssertion constraint to CollectionAllSatisfyHelper.Satisfy overloads. The Satisfy methods wrap the typed lambda into Func<..., IAssertion?> when constructing the assertion classes. Internal storage in CollectionAllSatisfyAssertion and CollectionAllSatisfyMappedAssertion changed from Func<..., Assertion<T>?> to Func<..., IAssertion?>.
  • NullAssertion.cs: NotNullAssertion.CheckAsync now checks metadata.Exception — required because the nullable-to-non-nullable Map throws InvalidOperationException on null, and the exception was being silently ignored since default(double) is never null.
  • SatisfiesTests.cs: Added tests for All().Satisfy() with double? members (both pass and fail cases, direct and mapped).
  • Public API snapshots: Updated all 4 .verified.txt files.

Fixes #4684

Copilot AI changed the title [WIP] Fix All().Satisfy for double? member properties Fix All().Satisfy() compile error with nullable value types Feb 6, 2026
Copilot AI requested a review from thomhurst February 6, 2026 13:53
@thomhurst thomhurst marked this pull request as ready for review February 6, 2026 13:53
@claude
Copy link
Contributor

claude bot commented Feb 6, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.


Review Summary:

This PR cleanly fixes the compilation error with nullable value types in All().Satisfy() by:

  1. Widening the assertion return type from Assertion<T>? to IAssertion? - This is an elegant architectural improvement that provides the necessary flexibility while maintaining backward compatibility (since Assertion<T> implements IAssertion).

  2. Adding exception handling in NotNullAssertion to properly detect null-related failures when mapping nullable-to-non-nullable types.

  3. Comprehensive test coverage for both passing and failing scenarios with nullable value types.

  4. Properly updated snapshot files following TUnit's testing conventions.

The changes are focused, well-tested, and maintain the project's quality standards.

@claude
Copy link
Contributor

claude bot commented Feb 6, 2026


Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

The PR correctly fixes the type compatibility issue with nullable value types in All().Satisfy() by:

  1. Widening the assertion function signature from Assertion? to IAssertion? (consistent with existing patterns like MappedSatisfiesAssertion)
  2. Adding proper exception handling in NotNullAssertion to catch null mapping failures
  3. Updating API snapshots appropriately with .verified.txt files
  4. Including comprehensive test coverage for the fix

The changes are well-structured and follow TUnit's development guidelines.


Use generic TAssertion constraint (where TAssertion : IAssertion) on
Satisfy methods instead of widening to IAssertion? directly. This
preserves type safety while allowing type-transforming assertions like
IsNotNull() on double? (which returns Assertion<double>, not
Assertion<double?>).

- Add TAssertion type parameter to CollectionAllSatisfyHelper.Satisfy
- Store IAssertion? internally in assertion classes
- Fix NotNullAssertion.CheckAsync to handle exceptions from nullable
  value type Map operations
- Add tests for All().Satisfy() with double? types
- Update public API snapshots

Fixes #4684

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@thomhurst thomhurst force-pushed the copilot/fix-all-satisfy-bug-double-type branch from 968c3f7 to c175f4f Compare February 6, 2026 14:31
@thomhurst thomhurst merged commit b5e74ea into main Feb 6, 2026
12 of 13 checks passed
@thomhurst thomhurst deleted the copilot/fix-all-satisfy-bug-double-type branch February 6, 2026 16:19
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]: All().Satisfy doesn't work with member of double? types

2 participants