Add unreachable code warning when comparing non-nullable type variables to null

[vsfeedback]

Migrated from Visual Studio IDE UserVoice forum

The C# linter (or whatever it's called) should warn the programmer of an unreachable code if they try to compare a non-nullable type variable to null.

After all, it's a non-nullable type, which means that it can never reference to null, isn't that right?

Example:

if (new Point(0, 0) == null) MessageBox.Show("Hello World!");

This issue has been moved from https://developercommunity.visualstudio.com/content/idea/351685/add-unreachable-code-warning-when-comparing-non-nu.html VSTS ticketId: 700919 These are the original issue comments:

Fiona Niu[MSFT] on 10/11/2018, 01:44 AM (24 days ago):

Thank you for taking the time to provide your suggestion. We will do some preliminary checks to make sure we can proceed further. You will hear from us in about two weeks on our next steps.

Kendra Havens [MSFT] on 10/31/2018, 04:05 PM (3 days ago):

Your suggestion has been queued up for prioritization. Feature suggestions are prioritized based on the value to our broader developer community and the product roadmap. We may not be able to pursue this one immediately, but we will continue to monitor it up to 90 days for community input

[Neme12]

duplicate of #30196?

@gafter If the warning was implemented in a warning wave, would the unreachable code analysis just fall out from that? Or is it required for the expression to be a constant value for control flow analysis to recognize this? I assume that would require a language change, right?

[gafter]

After all, it's a non-nullable type, which means that it can never reference to null, isn't that right?

No, that is not right. This suggestion is a pretty bad idea. Public APIs that take parameters that are not supposed to be null should both annotate the parameter to convey that information, and also check (with an if statement or something) that the actual value passed in at runtime is not null, and throw an ArgumentNullException if it is. This is the way we expect people to write public APIs.

There are many ways that null values can seep in unexpectedly. The simplest one is reading a public field that is declared to be non-null, but has not been initialized yet. There is also the ! operator, for example in an expression such as null!.

[Neme12]

I might not have understood the question. Whoever the OP is, can you please clarify whether you're referring to non-nullable value types or "non-nullable" reference types with respect to the new nullable reference types feature for C# 8?

[Neme12]

I thought Point was referring to System.Drawing.Point, which is a struct.

[gafter]

If OP was referring to non-nullable value types (such as int), then we cannot produce unreachable code warnings for testing them against null because unreachable code warnings are specified by the C# language specification, and this is not one of the situations that are specified to produce such warnings. However, we can warn that the test is "always false", and we do have plans (#30196) to enhance our existing detection of such conditions.

[jmarolf]

I believe this is the "Warn when comparing a struct to null" case. Which we do not warn about at all today.

[jcouv]

This issue indeed seems to be about comparing structs to null. I removed the "nullable reference types" label and marked as "warning waves" instead.

[gafter]

This is a dup of https://github.com/dotnet/roslyn/issues/30196