Hi devs,
Our build reports 120 javac warnings on generated code. We cannot fix any of them, and they are what stops us from turning on -Werror.
So I’d like us to stop linting generated sources.
What we get today
Measured on master (18.8.0-SNAPSHOT, JDK 21):
| Generator | Module | Warnings |
|---|---|---|
| SableCC | xwiki-platform-query-jpql-parser |
100 rawtypes |
| JavaCC | xwiki-rendering-wikimodel |
6 this-escape, 6 static |
| XJC | xwiki-commons-repository-model |
4 dep-ann, 4 deprecation |
Only 4 of the 120 are deprecations. Raw types are 83% of it. So this is about lint checks in general, not just deprecations.
Note: you only see these with the Develocity build cache disabled. With it on, compile is served from cache and reports 0 javac warnings.
Why we cannot fix them
- SableCC predates generics and is unmaintained. It generates
ArrayList list = new ArrayList();. No generator option changes that. - JavaCC generates the
*TokenManagerclass body itself. There is nowhere to put a@SuppressWarnings. - XJC copies
@deprecatedfrom our XSD into javadoc but does not emit the@Deprecatedannotation. That gives the 4dep-annwarnings. It also gives the 4deprecationones, and that part is worth spelling out, because “just fix the call site” looks right but is not.
Here is the generated call site:
@Override
public Extension withFeatures(String... values) {
for (String value : values) { getFeatures().add(value); }
return this;
}
features is deprecated since 8.0M1, but it is part of the Extension Repository REST format, so it has to stay in the XSD for old clients. withFeatures() exists only to fill it, so there is nothing else it could call. And javac only warns here because XJC did not annotate withFeatures() either. With the annotation, javac would stay silent, since it does not warn about deprecated use inside deprecated code.
Why it matters
xwiki-commons-pom has carried this TODO for years:
At some point, we should turn on failing on warnings since they can be real errors and they keep growing on our build logs
We cannot turn on -Werror while the build has warnings nobody can fix. Clearing generated code is the prerequisite for it.
We also already made this choice for our other linters:
xwiki-platform-query-jpql-parserexcludes the generated packages from Checkstyle.xwiki-commons-repository-modelandxwiki-rendering-wikimodelsetxwiki.checkstyle.skip, with the comment “Only contain generated sources for which we don’t control the code style”.xwiki-commons-pomsilences Sonar’sjava:S1133for legacy modules.
This just makes javac agree with them.
One trap, which I think we should agree on explicitly
-Xlint applies to a whole javac call, not to a source path. So switching a category off “for a module” also stops checking that module’s hand-written code. wikimodel has 102 hand-written files against 42 generated ones, and 2 real this-escape sites in the hand-written ones that we do want reported.
So the fix should not be a module-wide -Xlint:-xxx, except in modules that contain only generated code. Instead, give the generated source root its own maven-compiler-plugin execution with -Xlint:none and restrict the default execution to src/main/java. This works even when the generated and hand-written sources reference each other (wikimodel), by passing the hand-written root on -sourcepath with -implicit:none.
I have this working on all 3 modules: the 120 warnings are gone, hand-written sources keep -Xlint:all (I checked by introducing a raw type on purpose, it is still reported), and class output and test results are unchanged.
See Loading... for the jira issue and links to the 3 PRs (commons, rendering and platform).
WDYT?
Thanks
