ceylon / ceylon-model

DEPRECATED
Apache License 2.0
12 stars 3 forks source link

Support Java nullability annotations #8

Open lucaswerkmeister opened 9 years ago

lucaswerkmeister commented 9 years ago

Several frameworks have attempted to improve on Java’s nullability support by introducing annotations for something being @Nullable or @NotNull/@NonNull/@Nonnull. We should recognize these annotations, and warn or error if:

Behavior on unannotated elements should not change. Some frameworks define only one annotation because the other is the default, but we can’t detect that.

Test cases / examples: https://gist.github.com/lucaswerkmeister/07556232a9bd060dd593

Annotations to support (suggestion, potentially incomplete; list from Kotlin):

Alternatively, we could assume that any annotation named @Nullable, @NotNull, @NonNull, @Nonnull has this meaning. I think that’s probably a reasonable assumption.

lucaswerkmeister commented 9 years ago

(Issue moved here from ceylon/ceylon-spec#1424.)

FroMage commented 9 years ago

I am not sure about these warnings. IMO it should just change whether we consider those things optional or not. The same difference in Ceylon about X vs X?. Agreed @gavinking?

gavinking commented 9 years ago

IMO it should just change whether we consider those things optional or not.

Yes. This is what I have been arguing.

FroMage commented 9 years ago

OK fine.

jvasileff commented 9 years ago

If this can be disabled (perhaps by default) then I guess I don't care. Otherwise, my comment from the original issue:

Minor implementation note: assigning null returned from a function marked @NotNull to a Ceylon optional should not result in a runtime error.

And we probably shouldn't have compile time errors for:

if (exists x = getRequired()) { ... }

The thing is, annotations can't be trusted to be as correct as static types. My understanding is that this has been the experience for Objective C annotations for Swift.

FroMage commented 9 years ago

Well, how would you disable it? A compile-time flag means that every other module you use would also have to have disabled it.

I don't see why we should not trust these annotations. It's the same sort of trust as returning a List<X> which happens to be a List<Y> because Java can't check these things.

As for the other request, that's a question for @gavinking.

jvasileff commented 9 years ago

@FroMage I think you put way too much confidence in the code people generally write then! But, I just realized that as long as you aren't too aggressive with the null checks in the backend (first item above), there'd still be the escape hatch (for the second item):

if (exists x = (getRequired() of String?)) { ... }

An additional item would be the forced passing of a null to a non-Null Java method without having to write a helper in Java. But I don't really have any ideas for that one.

gavinking commented 9 years ago

@FroMage Are you sure you meant to assign this to 1.2?

FroMage commented 9 years ago

Well, not sure, it could be really fast.

FroMage commented 9 years ago

Not enough time for 1.2