Use is operator for null check instead of ==

[hishamco]

Enhance readability and reduces the chance of null-related bugs

[Piedone]

Oh no. Why change 800 files for this? :) I had some discussion with somebody for a line changed somewhere under a PR, but that's one thing, changing every instance in the codebase is another ballpark.

What's the benefit, exactly? How does it enhances readability and reduces the change of null-related bugs? Is dashboardFeature is null really more readable than dashboardFeature == null when we use == and != for every other equality check, and have all C-type languages since the dawn of time? Can you provide some evidence of the benefit?

Pattern matching is great, but there is no "pattern" here to match. All these changes are just literal == -> is and != -> is not changes (and FWIW, I've seen actual use cases for pattern matching missed in there where the expression was longer).

[hishamco]

All these changes are just literal == -> is and != -> is not changes

is and is not much readable & cleaner for null equality nothing else

[Piedone]

Please reply to this:

How does it enhances readability and reduces the change of null-related bugs? Is dashboardFeature is null really more readable than dashboardFeature == null when we use == and != for every other equality check, and have all C-type languages since the dawn of time? Can you provide some evidence of the benefit?

[hishamco]

To be honest, the JIT ASM for both are the same, but for the readers is null more readable than ==, maybe I'm a VB guy :) there's no example in mind right now, but I might think about one

[Piedone]

Maybe you should be more of a C# guy ;).

[hishamco]

Have a look at the examples below:

int i = 1;
if (i is null)
{
    Console.WriteLine("i is null");
}

this will not compile because i is non-nullable, while in the second example

int i = 1;
if (i == null)
{
    Console.WriteLine("i is null");
}

it will compile and the condition is false

Think for this and you will know what I mean :)

[Piedone]

That's a good point, but since this PR compiles, it doesn't apply to any of the changes in it. The second example also emits this warning BTW, that in our case still breaks the build: https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs0472?f1url=%3FappId%3Droslyn%26k%3Dk(CS0472)

[hishamco]

that in our case still breaks the build

I didn't understand which build had been broken.

[Piedone]

If you write the second example, it causes a build warning. This will fail the CI build.

[MikeAlhayek]

@hishamco I also agree that this isn't needed.

Also today @sebastienros said this isn't needed. Maybe we should not then?

[hishamco]

No problem, we could close it then :)