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

deprecate NotNull #7247

Closed
scabug opened this Issue Mar 12, 2013 · 16 comments

Comments

Projects
None yet
2 participants
@scabug
Copy link

scabug commented Mar 12, 2013

We should deprecate NotNull and corral its implementation in the compiler. Its implementation has been unfinished since 2.5.

@scabug

This comment has been minimized.

Copy link

scabug commented Mar 12, 2013

@scabug

This comment has been minimized.

Copy link

scabug commented Mar 13, 2013

@paulp said:
The implementation has always been behind a -Y option, and for 2.10.0 I labeled it in an even more threatening fashion:

  -Ynotnull                           Enable (experimental and incomplete) scala.NotNull.

So as far as I'm concerned we should deprecate the empty scala.NotNull trait for 2.11.0 but we are within our rights to remove all of the implementation which underlies it immediately.

@scabug

This comment has been minimized.

Copy link

scabug commented Mar 13, 2013

@adriaanm said:
I was thinking the same thing.

@scabug

This comment has been minimized.

Copy link

scabug commented Mar 13, 2013

@paulp said:
I did better than think it.

@scabug

This comment has been minimized.

Copy link

scabug commented Mar 13, 2013

@paulp said:
scala/scala#2244 in case that last comment was overly vague.

@scabug scabug closed this Mar 18, 2013

@scabug

This comment has been minimized.

Copy link

scabug commented May 28, 2014

Mike Allen (mja) said:
I've just switched to Scala 2.11 and just realized that NotNull has been deprecated. In fact, it appears that it has been more than deprecated, since it's no longer possible to specify -Ynotnull as a compiler option. Thanks for the discussion and heads-up! ;-)

I understand and appreciate that this feature was experimental and never fully implemented. Nevertheless, I found it extremely valuable: the ability to enforce non-null references for certain types is something Scala can and should support. I don't particularly want to either trap null references, nor do I want to accommodate NullPointerException exceptions being thrown all over the place, so having base classes extend NotNull was a simpler, neater alternative - assuming that it actually worked, of course. Instead, a compile-time error that you cannot initialize a NotNull instance with null is elegant, simple and straightforward.

So, given that NotNull has been nuked, is there any proposed alternative that doesn't involve either utilizing a ton of check-not-null boilerplate or NullPointerException handling - including all the cumbersome overhead of extra unit tests to guarantee that null references are being handled correctly?

@scabug

This comment has been minimized.

Copy link

scabug commented May 30, 2014

Travis Calder (CriasSK) said (edited on May 30, 2014 9:02:43 PM UTC):
Just my two cents, but I feel like Option is what you're looking for. It solves all the NotNull problems much more effectively.

The only up-side to NotNull that I see is that you can mix it into a class definition directly, but when you're writing a class you don't know how users of that class will eventually intend to reference it. Mixing in NotNull at that point feels inappropriate.

On the other hand, at the declaration of the variable you could just as easily declare an Option[Foo], and simply never use null in your code ever. You'll still have to check at system-boundaries (IO, Java libraries, etc.) but you can easily wrap those in an Option(reference) and the Option class will automatically wrap it in a Some or None as appropriate.

This also gives you all of your map, flatMap, and filter operations, as well as your orElse method calls for when you want a default value.

Is there some area that an Option doesn't cover for you?

(Edit: Sorry, goofed the formatting)

@scabug

This comment has been minimized.

Copy link

scabug commented May 30, 2014

Mike Allen (mja) said:
Sorry, Travis, but Option doesn't even come close.

The need for NotNull is illustrated by the fact that any reference type can be null, but trapping null values or handling exceptions arising from dereferencing a null value - or even having to wrap references in Option - is a high overhead if there is no benefit whatsoever from having null values in the first place. Not only that, but you would (or should) have to write unit tests to verify that passing null references around doesn't cause collateral damage.

NotNull combined with -Ynotnull was intended to make assignment of null to a NotNull-derived type - as well as casting null reference values to such a type - illegal. Consider the following:

class SomeCannotPossiblyBeNullType extends NotNull {
  // ...
}

def doSomething (x: SomeCannotPossiblyBeNullType) = {
  // x is guaranteed not to be null, so dereferencing x will not cause an NPE to be thrown.
  // Also, there is also no need to trap whether x is null, or to wrap x in an Option.
  // Furthermore, I don't have to write a unit test to verify what happens if x is null. Can't happen.
  // ...
}

def willNotWork = {
  // ...
  val y: SomeCannotPossiblyBeNullType = null // <- results in a compiler error
  val z = somePossiblyNullReference // <- results in compiler error
  // ...
}

What you seem to be suggesting as an alternative is to wrap every single reference to such a class in an Option - that is, I have to use Option[SomeCannotPossiblyBeNullType] instead of simply SomeCannotPossiblyBeNullType which makes no sense to me whatsoever.

Ironically, this doesn't even solve the problem since you can initialize an Option[T] will null (it's a reference type after all):

val x: Option [String] = null
val s: = x.getOrElse ("x is null") // Whoops! Get an NPE!

so even Option would benefit from extending NotNull!

@scabug

This comment has been minimized.

Copy link

scabug commented May 30, 2014

Travis Calder (CriasSK) said:
Actually, my suggestion is the opposite of Option[SomeCannotPossiblyBeNullType].

That is, as I said you ban null from the interior of your system completely. It never exists, ever. If you have something that can never be null, you just reference it directly. You only use the Option in instances where you're not sure if you have it or not:

var x: SomeCanNeverBeNull = nonNullReference;
var y: Option[SomeCanNeverBeNull] = Option(nullableReference); // Oops, it actually was null, but we handled it cleanly

That is, you seem to be assuming that nullable is your default and you're flagging non-null cases. My understanding of idiomatic Scala code was that you should consider all variables non-nullable, and flag values that might not be present as Option.

If you do your null-checks at system boundaries (IO, etc.) and when calling Java libraries, a null should literally never enter your system. You shouldn't have to write unit tests around them, or boiler-plate null-checking code, because they just don't exist.

YMMV, and my take might be wrong, but I never get a NullPointerException inside a system unless I forget a check at the system boundary.

@scabug

This comment has been minimized.

Copy link

scabug commented May 30, 2014

Mike Allen (mja) said:
Sure you can ban null from the interior of your system (still, enforcement is more complex than using NotNull). I detest null and never, ever use it (aside from when forced to by Java). Also, I know how to use Option, thank you very much. ;-)

I would guess that, since no-one is infallible, even you occasionally forget to check for null at a system boundary and get NPEs - and then proceed to waste precious development time diagnosing, fixing and testing for them. (Unit testing for null is, therefore, very much required at the system boundary.)

All the same, what if, like me, you're designing an API? In that case, nullable is the default. You have zero enforcement over banning use of null in your user's code. Every single time a reference value is passed to that API, you have to consider the possibility that the value might be null - even if there's no logical need for it (a prime example being an Option instance). System boundaries can be big! The task can be made a lot simpler if you can enforce that certain types be non-null through the compiler - and that's what NotNull (used to) provide.

@scabug

This comment has been minimized.

Copy link

scabug commented May 31, 2014

Travis Calder (CriasSK) said:
Sorry, didn't mean to imply you didn't know how to use Option. I misinterpreted the reason why you thought I was suggesting wrapping every SomeCannotPossiblyBeNullType instead of just the ones at system boundaries that might be null.

It sounds like we're mostly on the same page though, with just a few different default assumptions separating our different approaches.

My answer to your "what if" is that it depends on the API and what a failure might cause. If the failures would be critical and damaging, such as aborting in the middle of a vital transaction, I would null-check at boundaries. Otherwise, I would classify that as a failure of the API user, and not bother checking. (In my case, that user will probably be "future me" most of the time. I accept that.)

That's just my approach, it's worked for me so far.
The only reason I said anything at all is that I also detest null, and having both NotNull and Option feels inconsistent to me.

Then again, if I got my way I'd also disallow usage of null at the compiler-level unless enabled by a flag (since Java interop needs it). I'm secretly hoping that since Java 8 now has its own Option, maybe they'll remove null completely in a future version. One can wish.

@scabug

This comment has been minimized.

Copy link

scabug commented Jun 2, 2014

@adriaanm said:

I understand and appreciate that this feature was experimental and never fully implemented. Nevertheless, I found it extremely valuable: the ability to enforce non-null references for certain types is something Scala can and should support.

I apologize for the lack of communication on this, but these two sentences are at odds: since this feature was never implemented (at all, basically), it was giving you a false sense of security. In the spirit of simplifying Scala, we've deprecated everything of which we've known for a while that it was unmaintained/broken/.... (See also: the CPS plugin. We didn't mark specialization as deprecated, even though we should've.)

@scabug

This comment has been minimized.

Copy link

scabug commented Jun 2, 2014

Mike Allen (mja) said:
Adriaan, thanks for your response. As for the feature not working "at all", I tested it and found it worked pretty well, but I accepted that it was incomplete and that there might be a surprise or two in store. There are other experimental features in Scala 2.10 that were in common use (e.g. macros), so I assumed that you guys were working on finishing the job. C'est la Vie.

So, you've removed it. Fine - I accept that. However, going back to my original comment, is there any proposed alternative functionality (provide references that are guaranteed not to be null)?

Also, I'm curious: what were the difficulties/holes in implementing this feature?

@scabug

This comment has been minimized.

Copy link

scabug commented Jun 2, 2014

@paulp said:
Part of the difficulty is the same difficulty which makes it easy to deliver null to AnyVal and Nothing, both of which should exclude it: uninitialized references And scala allowing overridden and abstract vals makes that problem largely insoluble if you want to keep separate compilation.

@scabug

This comment has been minimized.

Copy link

scabug commented Jun 2, 2014

@adriaanm said:
The original implementation only caught blatant assignments of null to a NotNull type. As Paul alluded to, tracking non-nullness is a form of effect tracking, which is much trickier than regular type checking because it's flow sensitive (e.g., val x = if(foo) notNull else null; ... ; if(foo) methodExpectingNotNull(x) is a common pattern that requires flow sensitivity/full dependent types to type check). More generally, it likely requires whole-program analysis to get good results (but that still doesn't solve how to do java interop).

There are no plans to replace NotNull at the moment, though some Scala 3.x release may gain effect tracking, or some lint-like tool could do a static analysis.

Here are two relevant papers that should give you a sense of how it could work: http://research.microsoft.com/en-us/um/people/leino/papers/krml109.pdf, http://www.cs.cornell.edu/andru/papers/masked-types.pdf (nullness interferes with initialization)

@scabug

This comment has been minimized.

Copy link

scabug commented Jun 2, 2014

Mike Allen (mja) said:
Paul, Adriaan, thanks to you both for your comments. Not for the first time, it appears to be a much more complex issue than I had assumed. Maybe it's easier just to deprecate null? ;-) In any case, I'll be sure to follow-up your links: guaranteed non-null reference types would seem to be a natural fit with Scala and would sure make life easier for a lot of us.

Also, as for static checking, I should point out (for anyone interested) that Scalastyle includes a test for the use of null - highly recommended!

@scabug scabug added the has PR label Apr 7, 2017

@scabug scabug added this to the 2.11.0-M3 milestone Apr 7, 2017

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