Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why RCS1098 is enabled by default? #183

Open
AlexeiScherbakov opened this issue Oct 17, 2017 · 3 comments
Open

Why RCS1098 is enabled by default? #183

AlexeiScherbakov opened this issue Oct 17, 2017 · 3 comments

Comments

@jnyrup
Copy link
Contributor

jnyrup commented Oct 17, 2017

While it often true that null == a and ReferenceEquals(a, null) are interchangeable, it is not always the case.

null == a and ReferenceEquals(a, null) are interchangeable if a is of type object or no public static bool operator == has not been overridden i the type of a.

Here's a class with all possible overrides of operator ==

public class Test
{
    public static bool operator ==(Test x, Test y) => true;
    public static bool operator !=(Test x, Test y) => !(x == y);

    public static bool operator ==(Test x, object y) => true;
    public static bool operator !=(Test x, object y) => !(x == y);

    public static bool operator ==(object x, Test y) => true;
    public static bool operator !=(object x, Test y) => !(x == y);
}

And here you can see in the comments, which operator == is chosen depending on the types.

var a = new Test();

if (null == a) { } // Test == Test
if (a == null) { } // Test == Test

if (null == (object)a) { } // object == object
if (a == (object)null) { } // Test == object

if ((object)null == a) { } // object == Test
if ((object)a == null) { } // object == object

As your links point out having null on the left hand side (sometimes known as Yoda conditions) mostly has its benefits in C-style languages.

@josefpihrt
Copy link
Collaborator

Yoda condition is not relevant because this analyzer is applicable only for == and != operators and not for assignment.

Also as Jon Hanna points out "a non-symmetric == is always a bug".

So now we can agree that changing null == x to x == null is only about style and not about semantic change.

Disabling this analyzer by default is worth consideration but before doing that can someone provide c# project (repository) that consistently use null == x over x == null?

@tamlin-mike
Copy link

While I can't point to any such repo, I can suggest a possibly cleaner way to do object null check, using is null.

bool IsNullTest1(String s) { return (object)s == null; }
bool IsNullTest2(String s) { return s is null; }

At least the compiler I tested with is generating the exactly same code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants