google / jsinterop-generator

Generates Java annotated with JsInterop from JavaScript extern sources
Apache License 2.0
78 stars 24 forks source link

Add nullability annotations on generated code #25

Open realityforge opened 5 years ago

realityforge commented 5 years ago

Closure externs already have the concept of nullable and non-nullable values. Some types are implicitly nullable and some are implicitly nonnull although these can be explicitly overridden. See Types-in-the-Closure-Type-System for further details.

When translating the externs into java code this nullability is not modelled in the java code. There seems to be @JsNonNull and @JsNullable annotations that may be able to be used. Another possibility is to use @javax.annotation.Nonnull and @javax.annotation.Nullable which would integrate much better with existing java tools. Alternatively they could both be applied where appropriate.

Adding the javax.annotation annotation almost instantly improves the experience when editing in an IDE like IDEA which will highlight where the developer has failed to check for null or conversely, checks for null without cause.

The one disadvantage is that if the upstream closure compiler externs are incorrectly annotated (and some of them are), it will confuse the developer and it may force us to fix the upstream externs.

In the future I plan to tackle this issue but no set timeline

tbroyer commented 5 years ago

javax.annotation shouldn't be used nowadays. org.checkerframework.checker.nullness.qual.Nullable is a better replacement (or possibly org.jetbrains.annotations.Nullable from org.jetbrains:annotations, or edu.umd.cs.findbugs.annotations.Nullable from com.github.spotbugs:spotbugs-annotations)