Error Prone / NullAway support for JSpecify by bbakerman · Pull Request #196 · graphql-java/java-dataloader

Error Prone / NullAway support for JSpecify by bbakerman · Pull Request #196 · graphql-java/java-dataloader

related to #195

Putting ErrorProne / NullAway in place


		tasks.withType(JavaCompile) {
		options.release = 11
		options.errorprone {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

builds with v17 - targets v11

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how to achieve errorprone / nullaway checking without this

I took this from @bclozel examples : bclozel@f449972

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we are running the v17 compiler but using the "release" of JDK 11 including libraries

		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");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tooling understands Objects.requireNonNull but not out nonNull method

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just enough Kotlin - tests only

confirmed the build .jar has no Kotlin classes!

		java {
		toolchain {
		languageVersion = JavaLanguageVersion.of(11)
		languageVersion = JavaLanguageVersion.of(17)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs to run on 17 to get errorprone happy (compiled for 17)

but we target 11 later