Consider removing spotbugs-annotations dependency
spotbugs-annotations brings two nullability annotations which might accidentally cause wrong import being selected by the developer.
As far as I understand, the only purpose of spotbugs-annotations in log4j is to have @SuppressFBWarnings, however it does not really require a dependency for it. SpotBugs is happy to have a class in any package.
-
Here's a sample in log4j code: https://github.com/apache/logging-log4j2/blob/4b7065b8b3818cabb88cf1d553da33897b733413/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/SocketAppenderReconnectTest.java#L24
-
log4j2 ships Gradle Metadata that includes
spotbugs-annotations: https://repo1.maven.org/maven2/org/apache/logging/log4j/log4j-api/2.25.2/log4j-api-2.25.2.moduleIn other words, Gradle projects that depend on
log4j2do getspotbugs-annotationsdependencies. In fact, it would make sense if log4j really used the annotations: if the project required a dependency to be present during compilation, then the one who depend on such a module should also have those dependency on the compile classpath. The justification is that annotations (e.g.@Nullable) do impact the API of a module, and, say Kotlin compiler might properly infer if the parameter type is nullable or not based on the annotation. If the annotation is not present on the compile classpath, then the compilation might fail.Note: this is exactly the reason why Guava keeps
@Nullableannotation on theircompilescope.
Apache JMeter uses log4j2 for logging, and it turns out that log4j2 brings spotbugs-annotations and its @Nullable annotations which makes it inconvenient to maintain code.
I know I can configure IDE to avoid unwanted imports, however, it is IDE-specifc.
What do you think of removing spotbugs-annotations dependency altogether?
PS I asked SpotBugs developers to deprecate their nullable annotations, so the clients get warnings at least, however, they insist that they can deprecate their nullability annotations only when they get full support for JSpecify: https://github.com/spotbugs/spotbugs/issues/3811
Frankly, I do not believe SpotBugs will get full JSpecify support in the nearest future (why spending time on it provided there's NullAway and friends?)
Prelude to removing it entirely like https://github.com/apache/jmeter/pull/6574 ?
I do not care if log4j wants using spotbugs. However, I do not like how log4j brings spotbugs-annotations to jmeter compile classpath.
Just in case, this PR does not prevent Log4j from using SpotBugs if they still want it.
@vlsi, I've discussed the issue with @ppkarwasz and we've decided to indeed remove the com.github.spotbugs:spotbugs-annotations dependency. We might follow a slightly different route then in #3985 – I will share further details there.
Fixed by #3985.