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

Consider 'value is null' instead of 'value == null' for the "Add null check" codefix #24237

Closed
jamesqo opened this issue Jan 15, 2018 · 6 comments · Fixed by #35215
Closed

Consider 'value is null' instead of 'value == null' for the "Add null check" codefix #24237

jamesqo opened this issue Jan 15, 2018 · 6 comments · Fixed by #35215
Assignees
Labels
Area-IDE Developer Community The issue was originally reported on https://developercommunity.visualstudio.com Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Milestone

Comments

@jamesqo
Copy link
Contributor

jamesqo commented Jan 15, 2018

It's possible that there could be a custom operator defined for ==, which could be slower than a simple pointer comparison. is null reads better and is guaranteed to be just a pointer comparison.

This may warrant looking into the user's code style to see if they already have a lot of == nulls or is nulls, to see which codefix should be offered to maintain consistency.

@MaStr11
Copy link
Contributor

MaStr11 commented Jan 15, 2018

This is minor (and more of a detail that needs to be considered once this gets implemented), but is null isn't a 1:1 replacement for == null. This code

    public static void M<T>(T value)
    {
        if (value is null) { } //error CS0403: Cannot convert null to type parameter 'T' because it could be a non-nullable value type.
    }

does not compile, while value == null would. See also #23581.

@gafter
Copy link
Member

gafter commented Jan 15, 2018

We should change the language to permit value is null for an unconstrained generic.

@MaStr11
Copy link
Contributor

MaStr11 commented Jan 15, 2018

For the sake of completeness, there are some more inconsistencies here (but some might be unresolvable):

public class C {
    //unconstraint
    public void UnconstraintIs<T>(T v) {
        var b = v is null; //error CS0403: Cannot convert null to type parameter 'T' because it could be a non-nullable value type. Consider using 'default(T)' instead.
    }
    public void UnconstraintNull<T>(T v) {
        var b = v == null; // Fine
    }

    //class
    public void RefconstraintIs<T>(T v) where T:class {
        var b = v is null; //Fine
    }
    public void RefconstraintNull<T>(T v)  where T:class {
        var b = v == null; //Fine
    }
	
    //struct
    public void ValueconstraintIs<T>(T v) where T:struct {
        var b = v is null; //error CS0403: Cannot convert null to type parameter 'T' because it could be a non-nullable value type. Consider using 'default(T)' instead.
    }
    public void ValueconstraintNull<T>(T v)  where T:struct {
        var b = v == null; //error CS0019: Operator '==' cannot be applied to operands of type 'T' and '<null>'
    }
    public void ValueconstraintRefEquals<T>(T v)  where T:struct {
        var b = ReferenceEquals(v, null); // Fine
    }
}

sharplab.io

Public Class C
    Public Sub UnconstraintIs(Of T)(v As T)
        Dim flag As Boolean = v Is Nothing
    End Sub

    Public Sub RefconstraintIs(Of T As Class)(v As T)
        Dim flag As Boolean = v Is Nothing
    End Sub

    Public Sub ValueconstraintIs(Of T As Structure)(v As T)
        Dim flag As Boolean = v Is Nothing ' error BC30020: 'Is' operator does not accept operands of type 'T'. Operands must be reference or nullable types.
    End Sub

    Public Sub ValueconstraintReferenceEquals(Of T As Structure)(v As T)
        Dim flag As Boolean = ReferenceEquals(v, Nothing)
    End Sub
End Class

Sharplab.io

@jcouv jcouv added the Area-IDE label Jan 15, 2018
@CyrusNajmabadi
Copy link
Member

It's possible that there could be a custom operator defined for ==, which could be slower than a simple pointer comparison. is null reads better and is guaranteed to be just a pointer comparison.

Imo, if the user has an overloaded operator == then we should be respecting that. I think using "== null" is appropriate for that feature.

@jinujoseph jinujoseph added this to the 15.7 milestone Jan 29, 2018
@jinujoseph jinujoseph modified the milestones: 15.7, Unknown Feb 20, 2018
@jinujoseph jinujoseph added the Developer Community The issue was originally reported on https://developercommunity.visualstudio.com label Sep 16, 2018
@jinujoseph
Copy link
Contributor

jinujoseph commented Sep 16, 2018

also reported here

@onyxmaster
Copy link
Contributor

Could this be implemented and configured using an option in .editorconfig? Autogenerating code increases productivity (and I'm talking not only about this specific code fix, but also about auto-generated constructors), yet if one wants to enforce the is null/is object pattern for null checks in the codebase, it just won't work.
P.S. https://twitter.com/jaredpar/status/1115019017297596416 @jaredpar

@jinujoseph jinujoseph modified the milestones: Backlog, 16.2.P1 Apr 23, 2019
@jinujoseph jinujoseph added the 4 - In Review A fix for the issue is submitted for review. label Apr 23, 2019
@jinujoseph jinujoseph added Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented and removed 4 - In Review A fix for the issue is submitted for review. labels Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Developer Community The issue was originally reported on https://developercommunity.visualstudio.com Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants