Browse Source

KAFKA-5887; Replace findBugs with spotBugs and upgrade to Gradle 4.10

findBugs is abandoned, it doesn't work with Java 9 and the Gradle plugin will be deprecated in
Gradle 5.0: https://github.com/gradle/gradle/pull/6664

spotBugs is actively maintained and it supports Java 8, 9 and 10. Java 11 is not supported yet,
but it's likely to happen soon.

Also fixed a file leak in Connect identified by spotbugs.

Manually tested spotBugsMain, jarAll and importing kafka in IntelliJ and running
a build in the IDE.

Author: Ismael Juma <ismael@juma.me.uk>

Reviewers: Colin Patrick McCabe <colin@cmccabe.xyz>, Dong Lin <lindong28@gmail.com>

Closes #5625 from ijuma/kafka-5887-spotbugs
pull/5625/merge
Ismael Juma 6 years ago committed by Dong Lin
parent
commit
f123d2f18c
  1. 16
      README.md
  2. 31
      build.gradle
  3. 2
      clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java
  4. 5
      connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/PropertyFileLoginModule.java
  5. 2
      core/src/main/scala/kafka/server/ClientQuotaManager.scala
  6. 12
      gradle/spotbugs-exclude.xml
  7. 2
      jenkins.sh

16
README.md

@ -168,7 +168,7 @@ Please note for this to work you should create/update user maven settings (typic @@ -168,7 +168,7 @@ Please note for this to work you should create/update user maven settings (typic
./gradlew dependencyUpdates
### Running code quality checks ###
There are two code quality analysis tools that we regularly run, findbugs and checkstyle.
There are two code quality analysis tools that we regularly run, spotbugs and checkstyle.
#### Checkstyle ####
Checkstyle enforces a consistent coding style in Kafka.
@ -179,14 +179,14 @@ You can run checkstyle using: @@ -179,14 +179,14 @@ You can run checkstyle using:
The checkstyle warnings will be found in `reports/checkstyle/reports/main.html` and `reports/checkstyle/reports/test.html` files in the
subproject build directories. They are also are printed to the console. The build will fail if Checkstyle fails.
#### Findbugs ####
Findbugs uses static analysis to look for bugs in the code.
You can run findbugs using:
#### Spotbugs ####
Spotbugs uses static analysis to look for bugs in the code.
You can run spotbugs using:
./gradlew findbugsMain findbugsTest -x test
./gradlew spotbugsMain spotbugsTest -x test
The findbugs warnings will be found in `reports/findbugs/main.html` and `reports/findbugs/test.html` files in the subproject build
directories. Use -PxmlFindBugsReport=true to generate an XML report instead of an HTML one.
The spotbugs warnings will be found in `reports/spotbugs/main.html` and `reports/spotbugs/test.html` files in the subproject build
directories. Use -PxmlSpotBugsReport=true to generate an XML report instead of an HTML one.
### Common build options ###
@ -198,7 +198,7 @@ The following options should be set with a `-P` switch, for example `./gradlew - @@ -198,7 +198,7 @@ The following options should be set with a `-P` switch, for example `./gradlew -
* `showStandardStreams`: shows standard out and standard error of the test JVM(s) on the console.
* `skipSigning`: skips signing of artifacts.
* `testLoggingEvents`: unit test events to be logged, separated by comma. For example `./gradlew -PtestLoggingEvents=started,passed,skipped,failed test`.
* `xmlFindBugsReport`: enable XML reports for findBugs. This also disables HTML reports as only one can be enabled at a time.
* `xmlSpotBugsReport`: enable XML reports for spotBugs. This also disables HTML reports as only one can be enabled at a time.
### Running in Vagrant ###

31
build.gradle

@ -19,6 +19,9 @@ buildscript { @@ -19,6 +19,9 @@ buildscript {
repositories {
mavenCentral()
jcenter()
maven {
url "https://plugins.gradle.org/m2/"
}
}
apply from: file('gradle/buildscript.gradle'), to: buildscript
@ -30,6 +33,7 @@ buildscript { @@ -30,6 +33,7 @@ buildscript {
classpath 'com.github.jengelman.gradle.plugins:shadow:2.0.4'
classpath 'org.owasp:dependency-check-gradle:3.2.1'
classpath "com.diffplug.spotless:spotless-plugin-gradle:3.10.0"
classpath "gradle.plugin.com.github.spotbugs:spotbugs-gradle-plugin:1.6.3"
}
}
@ -85,7 +89,7 @@ allprojects { @@ -85,7 +89,7 @@ allprojects {
}
ext {
gradleVersion = "4.8.1"
gradleVersion = "4.10"
minJavaVersion = "8"
buildVersionFileName = "kafka-version.properties"
@ -145,9 +149,8 @@ subprojects { @@ -145,9 +149,8 @@ subprojects {
apply plugin: 'maven'
apply plugin: 'signing'
apply plugin: 'checkstyle'
if (!JavaVersion.current().isJava9Compatible())
apply plugin: 'findbugs'
if (!JavaVersion.current().isJava11Compatible())
apply plugin: "com.github.spotbugs"
sourceCompatibility = minJavaVersion
targetCompatibility = minJavaVersion
@ -364,18 +367,20 @@ subprojects { @@ -364,18 +367,20 @@ subprojects {
}
test.dependsOn('checkstyleMain', 'checkstyleTest')
if (!JavaVersion.current().isJava9Compatible()) {
findbugs {
toolVersion = "3.0.1"
excludeFilter = file("$rootDir/gradle/findbugs-exclude.xml")
if (!JavaVersion.current().isJava11Compatible()) {
spotbugs {
// 3.1.6 has a regression that breaks our build, seems to be https://github.com/spotbugs/spotbugs/pull/688
toolVersion = '3.1.5'
excludeFilter = file("$rootDir/gradle/spotbugs-exclude.xml")
ignoreFailures = false
}
test.dependsOn('findbugsMain')
test.dependsOn('spotbugsMain')
tasks.withType(FindBugs) {
tasks.withType(com.github.spotbugs.SpotBugsTask) {
reports {
xml.enabled(project.hasProperty('xmlFindBugsReport'))
html.enabled(!project.hasProperty('xmlFindBugsReport'))
// Continue supporting `xmlFindBugsReport` for compatibility
xml.enabled(project.hasProperty('xmlSpotBugsReport') || project.hasProperty('xmlFindBugsReport'))
html.enabled(!project.hasProperty('xmlSpotBugsReport') && !project.hasProperty('xmlFindBugsReport'))
}
}
}
@ -408,7 +413,7 @@ subprojects { @@ -408,7 +413,7 @@ subprojects {
}
gradle.taskGraph.whenReady { taskGraph ->
taskGraph.getAllTasks().findAll { it.name.contains('findbugsScoverage') || it.name.contains('findbugsTest') }.each { task ->
taskGraph.getAllTasks().findAll { it.name.contains('spotbugsScoverage') || it.name.contains('spotbugsTest') }.each { task ->
task.enabled = false
}
}

2
clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java

@ -942,7 +942,7 @@ public class ConfigDef { @@ -942,7 +942,7 @@ public class ConfigDef {
@Override
public void ensureValid(String name, Object value) {
if (value == null) {
// Pass in the string null to avoid the findbugs warning
// Pass in the string null to avoid the spotbugs warning
throw new ConfigException(name, "null", "entry must be non null");
}
}

5
connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/PropertyFileLoginModule.java

@ -22,6 +22,7 @@ import org.slf4j.Logger; @@ -22,6 +22,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Map;
@ -64,7 +65,9 @@ public class PropertyFileLoginModule implements LoginModule { @@ -64,7 +65,9 @@ public class PropertyFileLoginModule implements LoginModule {
if (!credentialPropertiesMap.containsKey(fileName)) {
Properties credentialProperties = new Properties();
try {
credentialProperties.load(Files.newInputStream(Paths.get(fileName)));
try (InputStream inputStream = Files.newInputStream(Paths.get(fileName))) {
credentialProperties.load(inputStream);
}
credentialPropertiesMap.putIfAbsent(fileName, credentialProperties);
} catch (IOException e) {
log.error("Error loading credentials file ", e);

2
core/src/main/scala/kafka/server/ClientQuotaManager.scala

@ -180,7 +180,7 @@ class ClientQuotaManager(private val config: ClientQuotaManagerConfig, @@ -180,7 +180,7 @@ class ClientQuotaManager(private val config: ClientQuotaManagerConfig,
delayQueueSensor.add(metrics.metricName("queue-size",
quotaType.toString,
"Tracks the size of the delay queue"), new Total())
start() // Use start method to keep findbugs happy
start() // Use start method to keep spotbugs happy
private def start() {
throttledChannelReaper.start()
}

12
gradle/findbugs-exclude.xml → gradle/spotbugs-exclude.xml

@ -15,12 +15,12 @@ @@ -15,12 +15,12 @@
limitations under the License.
-->
<!-- Findbugs filtering.
<!-- Spotbugs filtering.
Findbugs is a static code analysis tool run as part of the "check" phase of the build.
This file dictates which categories of bugs and individual false positives that we supress.
Spotbugs is a static code analysis tool run as part of the "check" phase of the build.
This file dictates which categories of bugs and individual false positives that we suppress.
For a detailed description of findbugs bug categories, see http://findbugs.sourceforge.net/bugDescriptions.html
For a detailed description of spotbugs bug categories, see https://spotbugs.readthedocs.io/en/latest/bugDescriptions.html
-->
<FindBugsFilter>
<Match>
@ -44,8 +44,8 @@ For a detailed description of findbugs bug categories, see http://findbugs.sourc @@ -44,8 +44,8 @@ For a detailed description of findbugs bug categories, see http://findbugs.sourc
</Match>
<Match>
<!-- Findbugs tends to work a little bit better with Java than with Scala. We suppress
some categories of bug reports when using Scala, since findbugs generates huge
<!-- Spotbugs tends to work a little bit better with Java than with Scala. We suppress
some categories of bug reports when using Scala, since spotbugs generates huge
numbers of false positives in some of these categories when examining Scala code.
NP_LOAD_OF_KNOWN_NULL_VALUE: The variable referenced at this point is known to be null

2
jenkins.sh

@ -17,4 +17,4 @@ @@ -17,4 +17,4 @@
# This script is used for verifying changes in Jenkins. In order to provide faster feedback, the tasks are ordered so
# that faster tasks are executed in every module before slower tasks (if possible). For example, the unit tests for all
# the modules are executed before the integration tests.
./gradlew clean compileJava compileScala compileTestJava compileTestScala spotlessScalaCheck checkstyleMain checkstyleTest findbugsMain unitTest rat integrationTest --no-daemon --continue -PxmlFindBugsReport=true -PtestLoggingEvents=started,passed,skipped,failed "$@"
./gradlew clean compileJava compileScala compileTestJava compileTestScala spotlessScalaCheck checkstyleMain checkstyleTest spotbugsMain unitTest rat integrationTest --no-daemon --continue -PxmlSpotBugsReport=true -PtestLoggingEvents=started,passed,skipped,failed "$@"

Loading…
Cancel
Save