Compiling on Module Path with -Werror and -Xlint:all

[jjohannes]

This is about getting an error like the following when compiling with a module-info.java and the compiler flags -Xlint:all and -Werror:

> Compilation failed; see the compiler output below.
log4j-api-2.24.3.jar(/org/apache/logging/log4j/message/ParameterizedMessage.class): warning: Cannot find annotation method 'replacement()' in type 'InlineMe': class file for com.google.errorprone.annotations.InlineMe not found
log4j-api-2.24.3.jar(/org/apache/logging/log4j/message/ParameterizedMessage.class): warning: Cannot find annotation method 'imports()' in type 'InlineMe'
log4j-api-2.24.3.jar(/org/apache/logging/log4j/status/StatusData.class): warning: Cannot find annotation method 'value()' in type 'SuppressFBWarnings': class file for edu.umd.cs.findbugs.annotations.SuppressFBWarnings not found
log4j-api-2.24.3.jar(/org/apache/logging/log4j/status/StatusData.class): warning: Cannot find annotation method 'justification()' in type 'SuppressFBWarnings'
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/SystemPropertiesPropertySource.class): warning: Cannot find annotation method 'value()' in type 'ServiceProvider': class file for aQute.bnd.annotation.spi.ServiceProvider not found
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/SystemPropertiesPropertySource.class): warning: Cannot find annotation method 'resolution()' in type 'ServiceProvider'
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/Activator.class): warning: Cannot find annotation method 'value()' in type 'Headers': class file for org.osgi.annotation.bundle.Headers not found
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/Activator.class): warning: Cannot find annotation method 'name()' in type 'Header': class file for org.osgi.annotation.bundle.Header not found
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/Activator.class): warning: Cannot find annotation method 'value()' in type 'Header'
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/Activator.class): warning: Cannot find annotation method 'name()' in type 'Header'
log4j-api-2.24.3.jar(/org/apache/logging/log4j/util/Activator.class): warning: Cannot find annotation method 'value()' in type 'Header'
error: warnings found and -Werror specified
1 error
11 warnings

The error is triggered by using classes from Log4j that use annotations on public members from the annotation libraries indicated in the error message. There are two layers of missing dependencies that cause this:

1. The annotation library's Jar file (e.g. com.google.errorprone:error_prone_annotations:2.36.0) is not on the Classpath/Module Path
2. The annotation library's Java Module (e.g. com.google.errorprone.annotations) is not defined as require static in the corresponding module-info.class of Log4j

For (1) it's debatable if it can be fixed in a good way, as POM does not have a scope that resembles Gradle's compileOnlyApi scope: visible transitively but ONLY on the compile classpath.

For (2) you can argue that it is a bug, because if the code of Log4j would be compiled on the Module Path (i.e. with the compiler checking the module-info.java) it would not compile.

Workarounds for users

(1)

• Adding the missing dependencies directly as compileOnly (<provided>) to your own project.
• Patching the POM metadata with a rule (possible in Gradle)

(2)

• Calling the compiler with additional --add-reads arguments
• Patching the module-info.class itself (possible in Gradle with plugin)

This is a minimal example showing all the annotation libraries that need to be "patched in":
https://github.com/jjohannes/gradle-demos/blob/main/log4j-metadata/build.gradle.kts

Possible improvements in Log4j

(1) This was already discussed here. The dependencies marked as <provided> here would need to be switched to <compile>. But that has the unwanted effect that ALL users will get them as unnecessary RUNTIME dependencies.
There is the option to publish additional Gradle Metadata that supports the compileOnlyApi scope. If you are interested, I can make a contribution for that as I maintain the Maven plugin for publishing such metadata with Maven (it's used by Jackson for example). But I can understand if you don't want to have additional complexity in your build setup.

(2) Maybe you can check if the additional requires could be added to all module-info:

    requires static transitive com.google.errorprone.annotations;
    requires static transitive com.github.spotbugs.annotations;
    requires static transitive biz.aQute.bnd.annotation;
    requires static transitive org.osgi.annotation.bundle;

*requires static works also, but conceptually requires static transitive feels more correct as the annotations are transitively visible.

---

All of this can also be avoided by turning of the classfiles line check of javac...

I can understand if there is nothing you can/want to do at this point. Then, feel free to close this issue.
Having this written down can be helpful for users who run into this in any case.
If you want to do something I can help with, let me know.

[ppkarwasz]

@jjohannes,

(1) This was already discussed here. The dependencies marked as <provided> here would need to be switched to <compile>. But that has the unwanted effect that ALL users will get them as unnecessary RUNTIME dependencies. There is the option to publish additional Gradle Metadata that supports the compileOnlyApi scope. If you are interested, I can make a contribution for that as I maintain the Maven plugin for publishing such metadata with Maven (it's used by Jackson for example). But I can understand if you don't want to have additional complexity in your build setup.

Please submit a PR, 🥺 . I found out about your gradle-module-metadata-maven-plugin a couple of weeks ago, but I didn't have the time to check how it works.

(2) Maybe you can check if the additional requires could be added to all module-info:

    requires static transitive com.google.errorprone.annotations;
    requires static transitive com.github.spotbugs.annotations;
    requires static transitive biz.aQute.bnd.annotation;
    requires static transitive org.osgi.annotation.bundle;

*requires static works also, but conceptually requires static transitive feels more correct as the annotations are transitively visible.

A requires static transitive statement has some unwanted effects. When you compile using the module path, JPMS recursively enumerates all the modules marked as transitive and fails if one is missing. After your changes Gradle users will not be affected by this problem, but Maven users will. See #2929 for example.

[jjohannes]

Thanks for the quick response @ppkarwasz.

I'll prepare a PR for the Gradle Metadata in the next days.

A requires static transitive statement has some unwanted effects.

Ah yes. I did not think about that. Luckily, if you would add them as requires static it is stille enough for the lint check to work (see my reproducer):

    requires static com.google.errorprone.annotations;
    requires static com.github.spotbugs.annotations;
    requires static biz.aQute.bnd.annotation;
    requires static org.osgi.annotation.bundle;

This would be in line with the requires static org.jspecify you already have and it should do no harm. If you agree and if you give me a pointer how this would be added, I can also provide a PR for that.

[ppkarwasz]

This would be in line with the requires static org.jspecify you already have and it should do no harm. If you agree and if you give me a pointer how this would be added, I can also provide a PR for that.

We use the BND JPMS Libraries to generate our module-info.class. I am not sure what is the exact logic it applies to annotations to decide whether they are included or not. I suspect that only annotation with a retention of RUNTIME are included. Unless I am mistaken, only JSpecify is retained at runtime, so it is the only one in module-info.class.

IMHO annotations that are not visible at runtime do not need to be mentioned in the module-info.class, but the opinion is not universally shared (see JDK-8342833 for some counter arguments).

[jjohannes]

My understanding of the argument why annotations with CLASS retention should be visible transitively (in certain cases) at compile time is: If, for instance, a public member of a Log4j class is annotated, it can be argued that that annotation is part of the public API at compile time – even though it's gone at runtime. If I now use tooling at compile time that visits the public API of all things used on the compile classpath, it may encounter this annotated method. Then it may be (at least theoreticaly) important that the class behind the annotation is available. That's what, to my understanding, the -Xlint:classfile check is validating: that all annotations on public members used during compilations are available. (Similar to how the compiler would fail if a superclass in the hierarchy is missing even though our own code does not interact with it directly.)

As OSGi is only about runtime, it's no surprise that BND does not care about this as well. It took me some time and source code reading / debugging of BND's JPMS support, but I found the switch to add additional require static entries. As said above, I think these are harmless to add, as the compiler won't complain in general if they are missing. And at runtime, they are not of importance anyway.

Only having both things – Gradle Metadata for compileOnlyApi dependencies and the additional require static entries – will solve the case I outlined in the description completely.

Here is a PR that proposes to add both: #3450