Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

In C# 8, how do I detect impossible null checks?

I've started using nullable reference types in C# 8. So far, I'm loving the improvement except for one small thing.

I'm migrating an old code base, and it's filled with a lot of redundant or unreachable code, something like:

void Blah(SomeClass a) {
  if (a == null) {
    // this should be unreachable, since a is not nullable
  }
}

Unfortunately, I don't see any warning settings that can flag this code for me! Was this an oversight by Microsoft, or am I missing something?

I also use ReSharper, but none of its warning settings appear to capture this either. Has anybody else found a solution to this?

Edit: I'm aware that technically this is still reachable because the nullability checks aren't bulletproof. That's not really the point. In a situation like this, where I declare a paramater as NOT nullable, it is a usually a mistake to check if it's null. In the rare event that null gets passed in as a non-nullable type, I'd prefer to see the NullReferenceException and track down the offending code that passed in null by mistake.

like image 917
Adam Wall Avatar asked Apr 02 '20 20:04

Adam Wall


People also ask

What does << mean in C?

<< is the left shift operator. It is shifting the number 1 to the left 0 bits, which is equivalent to the number 1 .

What does %d do in C?

In C programming language, %d and %i are format specifiers as where %d specifies the type of variable as decimal and %i specifies the type as integer. In usage terms, there is no difference in printf() function output while printing a number using %d or %i but using scanf the difference occurs.

What is && operator in C?

The && (logical AND) operator indicates whether both operands are true. If both operands have nonzero values, the result has the value 1 . Otherwise, the result has the value 0 . The type of the result is int . Both operands must have an arithmetic or pointer type.

What is and symbol in C?

The & symbol is used as an operator in C++. It is used in 2 different places, one as a bitwise and operator and one as a pointer address of operator.


1 Answers

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.

like image 67
Jeremy Caney Avatar answered Oct 04 '22 16:10

Jeremy Caney