Open up isNotNull, adding it to the top-level functions for the rest of us

[abelbraaksma]

Allow usage of isNotNull as counterpart for isNull

The function isNotNull already exists. I propose to remove internal so that we can use it and that we help people write better code when struggling with null and isNull.

Pros and Cons

The advantages of making this adjustment to F# are:

• Discovery for beginners: on Slack and on SO often people wonder how to do isNull and the counterpart
• Lengthy discussions exist on the abuse of the wrong x <> null, we should strive to ban that, and isNotNull can help
• Linter currently suggests use of isNull when it sees x == null or x <> null, which leads to debate in the latter case: Lint suggests "not (isNull x)" over "x <> null" fsprojects/FSharpLint#439
• It's just too tempting to write the cleaner-looking x <> null than not(isNull x). Having isNotNull will help guide in the right direction (see this lengthy Slack chat where I unsuccessfully try to explain why using x <> null is bad
• While we're fixing an optimization issue with not(isNull x), if less code is emitted, a larger chance exists that the optimizer catches more cases, and method bodies can remain under the JIT threshold for inlining

The disadvantages of making this adjustment to F# are

• One more function in the global space
• ... dunno

Extra information

Estimated cost (XS, S, M, L, XL, XXL): XXXXS

Related suggestions: #99

Affidavit (please submit!)

Please tick this by placing a cross in the box:

• This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
• I have searched both open and closed suggestions on this site and believe this is not a duplicate:
  • this is a duplicate, but I would like to ask the community to reconsider and make an exception for isNotNull, or name it isNonNull, if that reads better. Both are present currently in the source.
• This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.
  • I don't think it is fundamental, but if discussion really shouldn't be reopened because of the 2017 decision, then I'll settle with the status quo.

Please tick all that apply:

• This is not a breaking change to the F# language design
• I or my company would be willing to help implement and/or test this

[charlesroddie]

The function isNotNull already exists.

The latest decision is to remove it from FSharp.Core as it has the form f >> not where f already exists. I agree with this as not is unproblematic and if it becomes problematic that is a bug to be fixed.

So anything null-related should now be considered in the light of https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1060-nullable-reference-types.md, as anything that can be exposed as null should be an NRT .

Perhaps NRT.IsNull and NRT.IsNotNull would be OK as parallels to Option.IsSome and Option.IsNone. With the extra qualification, it may be more acceptable to add the function.

x <> null is bad

The RFC says = and <> are valid way to check for null. So if this not actually valid, then we should tweak the RFC. This code does seem to cause problems and I would support anythig that makes equality simpler.

[abelbraaksma]

The RFC says = and <> are valid way to check for null. So if this not actually valid, then we should tweak the RFC.

@charlesroddie The operators can be overloaded, custom equality can have been defined. This is true for any CLR language, and therefore, the general rule of thumb is not to use that. C# added x is null, F# the isNull function to do this unambiguously.

And if you would argue that it shouldn't matter if it's overloaded or not, using is null prevents a method call, and so does isNull.

C# 8 introduced x is object and x is { }. Both are tests for non-null. If they considered it a good use case, adding isNotNull may seem like less of a stretch.

I agree with this as not is unproblematic and if it becomes problematic that is a bug to be fixed.

The function not actually was problematic, and partially still is. It led to very slow code (it should be exactly the same speed as isNull, but was a magnitude slower). I've meanwhile fixed this, which does make it a slightly lesser issue to have this function in or not.

[Lanayx]

isNotNull would be extremely useful, since it's used in code much more often than isNull

[charlesroddie]

All the linked issues are covered by https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1060-nullable-reference-types.md . Should this issue be closed as a result @abelbraaksma ?

In particular when this is implemented, things that can be null in .Net can be deconstructed using the syntax:

let len (str: string?) =
    match str with
    | null -> -1
    | NonNull s -> s.Length

If there is anything missing in the RFC, best to add to the discussion thread linked to there.

[Lanayx]

@charlesroddie I don't think the RFC you mentioned is relevant, the pattern that is used very often when integrating with nullable objects is the following

if isNotNull someObject then
    doWork()

What you are suggesting is reversing the flow, which is less readable

match someObject with
| null -> ()
| NonNull _ -> doWork()

[abelbraaksma]

Should this issue be closed as a result

@charlesroddie, it's probably going to be closed because there's pushback, though the referred issue, introducing NonNull may just as well be seen as an argument to have both functions here (why introduce NonNull and not the functional equivalent isNotNull?). I still feel it's a benign change and we shouldn't require people to write this themselves all the time.

[dsyme]

I agree with @charlesroddie that this would only be actioned in the context of https://github.com/fsharp/fslang-design/blob/master/RFCs/FS-1060-nullable-reference-types.md

[abelbraaksma]

Fair enough. We should take it up there.

[abelbraaksma]

This is not completed as suggested above. Closing instead as not planned.