Enable lint and errorprone checks in Jdk11

[karllessard]

This PR enables lint checks from javac and Google's errorprone plugin. It's enabled only in JDK11 mode for now, since errorprone won't work with standard javac compiler of JDK8.

Right now I've enabled only default error and warning checks in errorprone since adding them all increase drastically the compilation time (see the list of all checks available here).

We'll readjust and add more checks if needed, plus clear all warnings that we already have in a separate PR.

[karllessard]

Ok closing this, I thought that forcing JDK11 in the quick-build but using the GitHub setup-java action could have helped somehow with the framework's javadoc issue but looks like not. I'll reopen it later if I find another way to prevent that annoying issue with GitHub Actions.

[Craigacp]

We might need to use setup-java v2 to pick the distribution. We can select Zulu, which is what was used up until a few months ago, maybe that will work?

[Craigacp]

Also maybe we could have the lint checks be a separate action? That way it doesn't matter as much what jdk they are run under.

[rnett]

Also maybe we could have the lint checks be a separate action? That way it doesn't matter as much what jdk they are run under.

I'll second this, it's nice to know if your build is failing b/c lint or test failures.

Also, would it be possible to have a maven goal to auto-format (i.e. fix issues)?

[karllessard]

I’m not sure how that would work in a separate action. Lint checks need to be ran, afaik, at compile time. So we’ll need to compile it twice?

@Craigacp , I think by default the setup-java is taking Zulu for Java 11 and you need to specify adopt for using it, I’ve tried both but it failed. I’ll try later by setting explicitly zulu if that fixes it.

@rnett I’m not aware of any plugin auto-formatting the code but haven’t look for it neither. Might be interesting but at the same time, I’d prefer that the PR we merge are already formatted properly instead of relying “blindly” on a plugin. We coule add some sort of “checkstyle” step though to flag bad formatting, the lint checks I have added are mostly for detecting coding mistakes

[rnett]

I’m not sure how that would work in a separate action. Lint checks need to be ran, afaik, at compile time. So we’ll need to compile it twice?

I guess so, I don't think that would be a huge deal in dev mode? It's just nice when you can tell if your PR is failing b/c tests, or b/c lint errors by looking at the GitHub actions, rather than having to dig through the log. I don't think you could compile and lint in one action and then have the test action depend on it, but that would be ideal.

I’m not aware of any plugin auto-formatting the code but haven’t look for it neither. Might be interesting but at the same time, I’d prefer that the PR we merge are already formatted properly instead of relying “blindly” on a plugin. We could add some sort of “checkstyle” step though to flag bad formatting, the lint checks I have added are mostly for detecting coding mistakes

Ah, yeah, I'm thinking of enforcing google-java-format, not error detection. Were you planning on doing that here or would it be a separate PR? Either way, I'm not proposing we do the auto-format as part of the CI, but a way to correctly format a file other than doing it manually would be nice, for people who don't have google-java-format in their IDE.

[karllessard]

Testing by setting explicitly zulu JDK11 distribution

[karllessard]

Ok so I finally got the build on JDK11 working again, by installing openjdk on centos manually. So it looks like @Craigacp and @rnett are agreeing on running the lints in a separate action, I'll do it. Also @Craigacp ask me to enable support for JDK16 according to these instructions, which I'll do as well.

[karllessard]

Also, would it be possible to have a maven goal to auto-format (i.e. fix issues)?

On this @rnett , I've just found a few interesting links related to auto-formatting on build (or even on Git push): https://github.com/google/google-java-format#third-party-integrations

[karllessard]

Ok so I've updated the PR, now lint checks are automatically enabled as soon as you run on JDK9+ (no matter if jdk11 profile is enabled).

They can be disabled explicitly by passing -Dlint.skip=true to the command line, which I do in the first step of the CI quick build before re-compiling without it. So lint errors will be raised distinctively.

[Craigacp]

LGTM