Update ResilienceProperties to correctly handle null values #2300
Conversation
Adds logic handling a case when the value being requested is null
| } | ||
| else if (val == null) | ||
| { | ||
| value = default!; |
There was a problem hiding this comment.
We have to use a null-forgiving operator here, otherwise - the compiler generates a warning https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-messages/nullable-warnings#nonnullable-reference-not-initialized.
| @@ -20,10 +20,18 @@ public sealed class ResilienceProperties | |||
| /// <returns>True, if a property was retrieved.</returns> | |||
| public bool TryGetValue<TValue>(ResiliencePropertyKey<TValue> key, [MaybeNullWhen(false)] out TValue value) | |||
There was a problem hiding this comment.
Assuming a null value is permissible in the first place, doesn't that mean this annotation is wrong? Feels to me that if a null value is legal then we shouldn't need to use ! as the compiler should already consider it a valid possibility.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2300 +/- ##
=======================================
Coverage ? 85.39%
=======================================
Files ? 313
Lines ? 7461
Branches ? 1126
=======================================
Hits ? 6371
Misses ? 745
Partials ? 345
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
The PR adds logic to handling cases when the value being requested from
ResiliencePropertiesis null.Fixes #2299