Add CallerArgumentExpression overloads to Assert.IsTrue/IsFalse/IsNull/IsNotNull#5851
Add CallerArgumentExpression overloads to Assert.IsTrue/IsFalse/IsNull/IsNotNull#5851Evangelink merged 11 commits intodev/v4from
Conversation
| /// Thrown if <paramref name="value"/> is not null. | ||
| /// </exception> | ||
| public static void IsNull(object? value, string? message) | ||
| public static void IsNull(object? value, [CallerArgumentExpression(nameof(value))] string message = "") |
There was a problem hiding this comment.
We can discuss about the design, what I had in mind was to still allow the user to set a custom message but for us to use the capture mechanism to enrich the default message (always displayed - or with some verbosity setting in case).
There was a problem hiding this comment.
I don't know if we should have an analyzer to ensure people don't set this value or if we should use the trick from @sharwell (bait and switch).
There was a problem hiding this comment.
I could be missing something, but I don't see how that trick can prevent users from passing the value. If the overload is available in the assembly passed to Csc, then it's available.
The trick is useful if we want to preserve binary compatibility and keep Assert.IsNull(object) but let the compiler prefer Assert.IsNull(object, /*caller arg expression*/ string = ""), by passing an assembly that only has the latter to Csc, but at run time, we keep the assembly that has both overloads.
Youssef1313
force-pushed
the
dev/ygerges/callerarg-round1
branch
from
91ab65c to
0e864ba
Compare
Youssef1313
force-pushed
the
dev/ygerges/callerarg-round1
branch
from
0e864ba to
3782fab
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/v4 #5851 +/- ##
==========================================
+ Coverage 62.99% 63.05% +0.05%
==========================================
Files 574 574
Lines 31434 31417 -17
==========================================
+ Hits 19803 19811 +8
+ Misses 11631 11606 -25
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Evangelink
left a comment
Evangelink
left a comment
There was a problem hiding this comment.
Not a big fan of the current message but let's merge as-is and I'll suggest a bigger assertion message rename.
3 participants
Part of #1154
Related to #1285