Add DoesNotReturnIf to Assert.IsTrue/Assert.IsFalse and DoesNotReturn to Assert.Fail #1005
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1004
When adding these attributes, I did hesitate on the IsTrue/IsFalse overloads accepting nullable bools, because it does look odd to see
[DoesNotReturnIf(true)] bool? condition
right next to a check like
if (condition == true || condition == null)
knowing that for both
true
andnull
the method will not return - but only fortrue
will the nullability analysis kick in. Passingnull
will result in the call site still not knowing if the method returns or not so the user will still have to perform their own additional check.But my thought process was that the attribute is still accurate and still helps.
[DoesNotReturnIf(true)]
means just that - the method does not return if the parameter exactly matchestrue
. It does not say whether or not the method returns iffalse
ornull
are passed in.Even though in this case we know that
IsFalse(null)
will not return, there is not a way to represent that with the currentDoesNotReturnIf
attribute.