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: gradle/gradle#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 <[email protected]>

Reviewers: Colin Patrick McCabe <[email protected]>, Dong Lin <[email protected]>

Closes apache#5625 from ijuma/kafka-5887-spotbugs

README.md

@@ -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:
 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 -
 * `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 ###
 

build.gradle

@@ -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 {
     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 {
 }
 
 ext {
-  gradleVersion = "4.8.1"
+  gradleVersion = "4.10"
   minJavaVersion = "8"
   buildVersionFileName = "kafka-version.properties"
 
@@ -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 {
   }
   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 {
 }
 
 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
   }
 }

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

@@ -942,7 +942,7 @@ public static class NonNullValidator implements Validator {
         @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");
             }
         }

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

@@ -22,6 +22,7 @@
 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 void initialize(Subject subject, CallbackHandler callbackHandler, Map<Str
         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);

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

@@ -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()
   }

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

@@ -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
     </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

jenkins.sh

@@ -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 "$@"