Fix nullable warnings when using Member() on nullable properties#5191
Fix nullable warnings when using Member() on nullable properties#5191
Conversation
…lable warnings Change Expression<Func<TObject, TMember>> to Expression<Func<TObject, TMember?>> in Member and HasProperty overloads so that selectors on nullable properties (e.g. string?) no longer produce CS8603/CS8620/CS8619 warnings. Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com>
|
@copilot can we make sure we have sufficient test coverage for: And any other you can think of |
…tructs, nullable structs, enums, collections, and chaining Co-authored-by: thomhurst <30480171+thomhurst@users.noreply.github.com> Agent-Logs-Url: https://github.com/thomhurst/TUnit/sessions/abcf6a00-83b2-4c2d-be1a-acf22a4cf702
Added
|
There was a problem hiding this comment.
Review: Fix nullable warnings when using Member() on nullable properties
Overall: Clean, minimal fix for a real pain point. The approach is correct and the test coverage is excellent.
The Fix
Changing the selector parameter from Expression<Func<TObject, TMember>> to Expression<Func<TObject, TMember?>> is the right solution here. It exploits how C# type inference works:
- For reference types (
string?): the compiler still infersTMember = string, accepts the nullable selector, and the assertions lambda remains typed toIAssertionSource<string>— eliminating CS8603. - For value types (
int?): the compiler infersTMember = intfrom theNullable<int>↔TMember?unification, keeping assertions typed toIAssertionSource<int>.
This is backwards-compatible: existing callers with non-nullable properties continue to work unchanged.
Test Coverage
The 30+ test cases covering reference types, value types, structs, enums, nullable variants, chaining, and HasProperty are comprehensive. The explicit "Failure" cases (Member_NonNullableString_Failure, Member_NonNullableInt_Failure) that verify the assertion message are a nice touch.
Pre-existing Performance Issue (not introduced here, but worth flagging)
Inside the Map lambda in all three overloads, memberSelector.Compile() is called on each evaluation:
var memberContext = parentContext.Map<TMember>(obj =>
{
if (obj == null) throw ...;
var compiled = memberSelector.Compile(); // recompiles every time Map runs
return compiled(obj);
});Expression compilation is expensive. This should be hoisted outside the lambda:
var compiled = memberSelector.Compile(); // compile once
var memberContext = parentContext.Map<TMember>(obj =>
{
if (obj == null) throw ...;
return compiled(obj);
});This applies to all 5 methods touched in this PR (3 in AssertionExtensions.cs, 2 in PropertyAssertionExtensions.cs). Given TUnit's performance-first philosophy, this is worth addressing — though it predates this PR.
Minor Gap
There's no test for HasProperty(x => x.NullableName, null) (asserting a property is null via the expected-value overload). Not a blocker, just a potential edge case to consider documenting or testing.
Member(x => x.Schema, schema => schema.IsEqualTo("dbo"))on astring?property produces CS8603 because the compiler infersTMember = stringbut the selector lambda returnsstring?, which doesn't matchExpression<Func<TObject, string>>.Changed the selector parameter from
Expression<Func<TObject, TMember>>toExpression<Func<TObject, TMember?>>in:AssertionExtensions.cs— all 3Memberoverloads (<TObject, TMember, TTransformed>,<TObject, TMember>strongly-typed,<TObject, TMember>object-returning)PropertyAssertionExtensions.cs— bothHasPropertyoverloads (delegates toMemberinternally)With
TMember?in the selector, the compiler still infersTMember = stringwhile accepting astring?return. The assertions lambda staysFunc<IAssertionSource<TMember>, Assertion<TMember>>— i.e.Func<IAssertionSource<string>, Assertion<string>>— which matchesStringEqualsAssertion : Assertion<string>without any nullability mismatch.Original prompt
💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.