build: check vectors code with error-prone

[rmuir]

Previously these sources were excluded. But these are where we'd like to have the most checks.

Along with the PR on #14916 I'm finally able to fail the build statically on my bad printf string:

> Task :lucene:core:compileMain24Java
/home/rmuir/workspace/lucene/lucene/core/src/java24/org/apache/lucene/internal/vectorization/PanamaVectorizationProvider.java:52: error: [FormatString] missing argument for format specifier '%s'
        String.format(
                     ^
    (see https://errorprone.info/bugpattern/FormatString)
1 error

> Task :lucene:core:compileMain24Java FAILED

[github-actions]

This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR.

[dweiss]

I think the reason for this exclusion is that error prone didn't handle newer java versions... so if we have a java25+ source folder for mr-jar, it may still be applicable.

I wonder if we should just move the existing java24 folder sources into the main location and keep the exclusion here? Or maybe add a separate build option that will explicitly say up to which Java version errorprone can be included/ is compatible with?

[uschindler]

i think this was added when the MR-JAR code required newer JDK to be installed in former times. This is now obsolete as we have APIJARs and on top at moment we compile with default compiler (24).

[uschindler]

I think the reason for this exclusion is that error prone didn't handle newer java versions... so if we have a java25+ source folder for mr-jar, it may still be applicable.

I wonder if we should just move the existing java24 folder sources into the main location and keep the exclusion here? Or maybe add a separate build option that will explicitly say up to which Java version errorprone can be included/ is compatible with?

The exclusion had another reason. Nowadays we compile the MR-JAR part with default compiler always, in former times we stripped preview flags afterwars and required newer compiler for later java versions. This is no longer the case, the MR-JAR parts are compiled with module patching and default compiler, so the exclusion is obsolete.

No problem in removing the check it is a relic, separating the Java classes should still be done.

[uschindler]

The separate sourceSet is important also to not let incubator classes enter public APIs. The code separattion makes it isolated (and we have the stub compilation so when later versions change we have no issue and can enable MR-JAR again.

[rmuir]

I figured there was a reason checks were disabled: I think even if we have to disable these checks in stable branches, it would be valuable if we can keep them functional for main.

That's where the "front lines" are for new vectors code or modifications in pull requests.

[uschindler]

Yes the reason was at very early times that we needed to compile with newer JDKs (19, 20, 21) while the main branch at that time was on 11 or 17. This lead to incomptibilities so we left it out.

In the current stable branch 10.x we can backport that (should not hurt, just try it out). Reason is: We compile with same compiler and only use stub jars.

I fact, it is my fault that I forgot to reenable the check when I changed to apijars for compilation.

+1 to backport this.

[github-actions]

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

[uschindler]

Hi @rmuir,
did we forget to merge this?
Uwe

[github-actions]

This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR.

[github-actions]

This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR.

[uschindler]

Seems to pass after merging in all changes on main. Will merge soon.