Description
The recently introduced nullability analysis feature in C# 8 is expressive enough to capture cases where a method that is known to not return can be annotated as such (for instance if it is throwing).
See the following for reference: https://github.com/dotnet/roslyn/blob/3cef228f7a9c1991fd8e1e3fa59403d2ae682a96/docs/features/nullable-reference-types.md
Specifically: cs/dereferenced-value-may-be-null (https://lgtm.com/rules/1506091056882/)
With that, certain lgtm alerts around nullability can be false positives. Once such example is the following:
https://lgtm.com/projects/g/dotnet/corefx/latest/files/src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoNotNullable.cs?sort=name&dir=ASC&mode=heatmap
Note that a null check already exists which calls a method that will throw.
https://github.com/dotnet/runtime/blob/c44edc19fc6c4792b002ac115e72d2f7805ade6d/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs#L40-L47
The simplified pattern that we sometimes use in super perf sensitive code (where the readability trade off makes sense) is the following:
public int Foo(string s)
{
if (s == null)
{
CallThrowHelper(); // This method does not return;
}
return s.Length; // This is guaranteed to not be null.
}
[DoesNotReturn]
private void CallThrowHelper()
{
throw new ArgumentNullException();
}Even outside of throwing, I can imagine this type of nullability analysis being leveraged by any C# developer and hence the lgtm alerts false positive rate might become higher.