It's really important to note that not only are the nullability checks not bullet proof, but while they're designed to discourage callers from sending null references, they do nothing to prevent it. Code can still compile that sends a null to this method, and there isn't any runtime validation of the parameter values themselves.
If you’re certain that all callers will be using C# 8’s nullability context—e.g., this is an internal method—and you’re really diligent about resolving all warnings from Roslyn’s static flow analysis (e.g., you’ve configured your build server to treat them as errors) then you’re correct that these null checks are redundant.
As noted in the migration guide, however, any external code that isn’t using C# nullability context will be completely oblivious to this:
The new syntax doesn't provide runtime checking. External code might circumvent the compiler's flow analysis.
Given that, it’s generally considered a best practice to continue to provide guard clauses and other nullability checks in any public or protected members.
In fact, if you use Microsoft’s Code Analysis package—which I’d recommend—it will warn you to use a guard clause in this exact situation. They considered removing this for code in C# 8’s nullability context, but decided to maintain it due to the above concerns.
When you get these warnings from Code Analysis, you can wrap your code in a null check, as you've done here. But you can also throw an exception. In fact, you could throw another NullReferenceException—though that's definitely not recommended. In a case like this, you should instead throw an ArgumentNullException, and pass the name of the parameter to the constructor:
void Blah(SomeClass a) {
if (a == null) {
throw new ArgumentNullException(nameof(a));
}
…
}
This is much preferred over throwing a NullReferenceException at the source because it communicates to callers what they can do to avoid this scenario by explicitly naming the exact parameter (in this case) that was passed as a null. That's more useful than just getting a NullReferenceException—and, possibly a reference to your internal code—where the exception occurred.
Critically, this exception isn't meant to help you debug your code—that's what Code Analysis is doing for you. Instead, it's demonstrating that you've already identified the potential dereference of a null value, and you've accounted for it at the source.
Note: These guard clauses can add a lot of clutter to your code. My preference is to create a reusable internal utility that handles this via a single line. Alternatively, a single-line shorthand for the above code is:
void Blah(SomeClass a) {
_ = a?? throw new ArgumentNullException(nameof(a));
}
This is a really roundabout way of answering your original question, which is how to detect the presence of null checks made unnecessary by C#’s non-nullable reference types.
The short answer is that you can’t; at this point, Roselyn’s static flow analysis is focused on identifying the possibility of dereferencing null objects, not detecting potentially extraneous checks.
The long answer, though, as outlined above, is that you shouldn’t; until Microsoft adds runtime validation, or mandates the nullability context, those null checks continue to provide value.
