Error Prone / NullAway support for JSpecify #196
Conversation
|
|
||
| tasks.withType(JavaCompile) { | ||
| options.release = 11 | ||
| options.errorprone { |
There was a problem hiding this comment.
builds with v17 - targets v11
There was a problem hiding this comment.
Hmmm .. I don't think we can do that: build with 17 would allow to use APIs which are in 17, but not in 11, which would result in errors when run with 11.
There was a problem hiding this comment.
Not sure how to achieve errorprone / nullaway checking without this
I took this from @bclozel examples : bclozel@f449972
There was a problem hiding this comment.
It should be safe as the "-release" option is tailored for that:
When using the --release option, only the supported documented API for that release may be used; you cannot use any options to break encapsulation to access any internal classes.
There was a problem hiding this comment.
https://docs.gradle.org/current/userguide/building_java_projects.html#sec:compiling_with_release
Using release property is possible starting from Java 10.
Selecting a Java release makes sure that compilation is done with the configured language level and against the JDK APIs from that Java version.
There was a problem hiding this comment.
So we are running the v17 compiler but using the "release" of JDK 11 including libraries
| @@ -142,7 +144,7 @@ private static DataLoaderOptions setInInstrumentation(DataLoaderOptions options, | |||
| */ | |||
| public DataLoaderRegistry register(DataLoader<?, ?> dataLoader) { | |||
| String name = dataLoader.getName(); | |||
| assertState(name != null, () -> "The DataLoader must have a non null name"); | |||
| Objects.requireNonNull(name, "The DataLoader must have a non null name"); | |||
There was a problem hiding this comment.
The tooling understands Objects.requireNonNull but not out nonNull method
There was a problem hiding this comment.
You can teach tooling about this custom assertion method.
Spring Framework does that with a custom @Contract annotation, used on the relevant methods.
In a nutshell, two possibilities:
- use methods like
Objects.requireNonNull, already known by tooling - OR create a custom contract annotation, annotate the relevant methods in your codebase with it and teach NullAway about this annotation.
There was a problem hiding this comment.
Thanks - for the record the reason we have a custom Assert class is that back in Java 6 days - Objects.requireNonNull() didnt exist and we wanted near zero dependendencies - so we made out own
Same in graphql-java btw
| * as expected in Kotlin land. We don't intend to ue Kotlin in our tests | ||
| * or to deliver Kotlin code in the java | ||
| */ | ||
| class KotlinExamples { |
There was a problem hiding this comment.
Just enough Kotlin - tests only
confirmed the build .jar has no Kotlin classes!
| } | ||
|
|
||
| java { | ||
| toolchain { | ||
| languageVersion = JavaLanguageVersion.of(11) | ||
| languageVersion = JavaLanguageVersion.of(17) |
There was a problem hiding this comment.
needs to run on 17 to get errorprone happy (compiled for 17)
but we target 11 later
… to old assertions
related to #195
Putting ErrorProne / NullAway in place