ppkarwasz commented on issue #3110: URL: https://github.com/apache/logging-log4j2/issues/3110#issuecomment-2423586754
Hi @lbergelson, It's ironic that adding annotations for third-party static code analysis tools, causes warnings in Oracle's `-Xlint` tool. My recommendation would be to add `-classfile` to your [custom `-Xlint` option](https://docs.oracle.com/en/java/javase/17/docs/specs/man/javac.html#option-Xlint-custom). As far as I could find in my IDE, there are only **two** warnings in the whole `javac` compiler code that use that category. They are both in [`c.s.t.javac.jvm.ClassReader`](https://github.com/openjdk/jdk/blob/master/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java) and concern: 1. Missing annotations at compile time. This includes annotations with a retention of `CLASS`, which are not used at runtime, like the ones you are reporting. 2. The presence in a classfile of attributes from a future Java version, e.g. the `sealed` attribute in a class that uses Java 8 bytecode. I can not think of a natural situation, when this could happen. If you disable `classfile`, you won't miss anything important. > This was very surprising to me. Have you considered changing the annotation dependencies to be included. Or they processed out of the published jar? We have considered both solutions and each one comes at a cost. Currently we are using the following annotations, which all have their specificity: - OSGi annotations (like [`@Version`](https://docs.osgi.org/javadoc/osgi.annotation/8.0.0/org/osgi/annotation/versioning/Version.html)) and [BND annotations](https://bnd.bndtools.org/chapters/230-manifest-annotations.html). These are license (or dual licensed) under `Apache-2.0` and have a retention of `CLASS`. OSGi users (admittedly a small group) **need** them in the classfiles. - Annotations for the [SpotBugs](https://spotbugs.github.io/) static analysis tool. They are [licensed under LGPL-2.1](https://github.com/spotbugs/spotbugs/issues/3144), so inclusion in some products might be problematic (I am not a lawyer). On the other hand I don't know if Log4j users can benefit from them. They have a retention of `CLASS`. - Annotations for the [Error Prone](https://errorprone.info/) static analysis tool. These are definitively useful for a large group of users (see the [`@InlineMe` annotation](https://errorprone.info/docs/inlineme) we also use in our code base). Some of them have a retention of `RUNTIME`, but I believe we only use those with a retention of `CLASS`. - Nullability annotations from [JSpecify](https://jspecify.dev/). This seems to be the first standard for nullability annotations supported by multiple vendors and it could really benefit the users. For some reason they have a retention of `RUNTIME`. ### Adding annotations to the `compile/implementation` scope We certainly could not add Spotbugs annotations to our `compile` scope, at least [our lawyers say so](https://www.apache.org/legal/resolved). Some of these annotations would be very useful to Log4j API users (e.g. JSpecify), but in general the PMC is quite reticent to add **any** dependency to the `compile` scope for `log4j-api`. We only have OSGi dependencies in the `provided` scope (which corresponds to Gradle's `compileOnly` configuration and is not transitive). For Log4j Core there are even less reasons to add the (compile-time only) annotations to the `compile` scope, since most users have Log4j Core in the `runtime` scope (`runtimeOnly` for Gradle). These dependencies would end up in the applications even if users do not use them. ### Removing annotations from the class files The problem with removing annotations from the class files is that AFAIK, there is no tool for that! Admittedly it must be a couple of hundred lines of ASM 9 code, but we didn't have time to develop it yet. I have opened apache/logging-log4j-tools#153. If you are willing to contribute such a tool, we love PRs! :heart: As I stated earlier, not all annotations are eligible for removal (I think JSpecify, the OSGi `@Version` and Error Prone `@InlineMe` should stay), but `@BaselineIgnore` does not give any useful information to the compiler. We only use it to make breaking changes according to Java, which are not breaking changes according to the documentation: e.g. removing a `public` class that was **always** documented as `private`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
