From f123d2f18c55b1cf2edb452aeb87e6ad0743c292 Mon Sep 17 00:00:00 2001 From: Ismael Juma Date: Mon, 10 Sep 2018 13:14:00 -0700 Subject: [PATCH] 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 Reviewers: Colin Patrick McCabe , Dong Lin Closes #5625 from ijuma/kafka-5887-spotbugs --- README.md | 16 +++++----- build.gradle | 31 +++++++++++-------- .../apache/kafka/common/config/ConfigDef.java | 2 +- .../extension/PropertyFileLoginModule.java | 5 ++- .../kafka/server/ClientQuotaManager.scala | 2 +- ...dbugs-exclude.xml => spotbugs-exclude.xml} | 12 +++---- jenkins.sh | 2 +- 7 files changed, 39 insertions(+), 31 deletions(-) rename gradle/{findbugs-exclude.xml => spotbugs-exclude.xml} (97%) diff --git a/README.md b/README.md index 228581426b968..22ef66fc9951e 100644 --- a/README.md +++ b/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 ### diff --git a/build.gradle b/build.gradle index 9ebdfe9633e5d..c9663abe4c53c 100644 --- a/build.gradle +++ b/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 } } diff --git a/clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java b/clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java index 3868ed501e5b3..7669f1dd99942 100644 --- a/clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java +++ b/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"); } } diff --git a/connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/PropertyFileLoginModule.java b/connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/PropertyFileLoginModule.java index 101c6f49d02ff..8a013330808b8 100644 --- a/connect/basic-auth-extension/src/main/java/org/apache/kafka/connect/rest/basic/auth/extension/PropertyFileLoginModule.java +++ b/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 - @@ -44,8 +44,8 @@ For a detailed description of findbugs bug categories, see http://findbugs.sourc -