mirror of https://github.com/apache/kafka.git
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
This commit is contained in:
parent
0d535b167a
commit
f123d2f18c
16
README.md
16
README.md
|
@ -168,7 +168,7 @@ Please note for this to work you should create/update user maven settings (typic
|
||||||
./gradlew dependencyUpdates
|
./gradlew dependencyUpdates
|
||||||
|
|
||||||
### Running code quality checks ###
|
### 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 ####
|
||||||
Checkstyle enforces a consistent coding style in Kafka.
|
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
|
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.
|
subproject build directories. They are also are printed to the console. The build will fail if Checkstyle fails.
|
||||||
|
|
||||||
#### Findbugs ####
|
#### Spotbugs ####
|
||||||
Findbugs uses static analysis to look for bugs in the code.
|
Spotbugs uses static analysis to look for bugs in the code.
|
||||||
You can run findbugs using:
|
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
|
The spotbugs warnings will be found in `reports/spotbugs/main.html` and `reports/spotbugs/test.html` files in the subproject build
|
||||||
directories. Use -PxmlFindBugsReport=true to generate an XML report instead of an HTML one.
|
directories. Use -PxmlSpotBugsReport=true to generate an XML report instead of an HTML one.
|
||||||
|
|
||||||
### Common build options ###
|
### 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.
|
* `showStandardStreams`: shows standard out and standard error of the test JVM(s) on the console.
|
||||||
* `skipSigning`: skips signing of artifacts.
|
* `skipSigning`: skips signing of artifacts.
|
||||||
* `testLoggingEvents`: unit test events to be logged, separated by comma. For example `./gradlew -PtestLoggingEvents=started,passed,skipped,failed test`.
|
* `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 ###
|
### Running in Vagrant ###
|
||||||
|
|
||||||
|
|
31
build.gradle
31
build.gradle
|
@ -19,6 +19,9 @@ buildscript {
|
||||||
repositories {
|
repositories {
|
||||||
mavenCentral()
|
mavenCentral()
|
||||||
jcenter()
|
jcenter()
|
||||||
|
maven {
|
||||||
|
url "https://plugins.gradle.org/m2/"
|
||||||
|
}
|
||||||
}
|
}
|
||||||
apply from: file('gradle/buildscript.gradle'), to: buildscript
|
apply from: file('gradle/buildscript.gradle'), to: buildscript
|
||||||
|
|
||||||
|
@ -30,6 +33,7 @@ buildscript {
|
||||||
classpath 'com.github.jengelman.gradle.plugins:shadow:2.0.4'
|
classpath 'com.github.jengelman.gradle.plugins:shadow:2.0.4'
|
||||||
classpath 'org.owasp:dependency-check-gradle:3.2.1'
|
classpath 'org.owasp:dependency-check-gradle:3.2.1'
|
||||||
classpath "com.diffplug.spotless:spotless-plugin-gradle:3.10.0"
|
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 {
|
ext {
|
||||||
gradleVersion = "4.8.1"
|
gradleVersion = "4.10"
|
||||||
minJavaVersion = "8"
|
minJavaVersion = "8"
|
||||||
buildVersionFileName = "kafka-version.properties"
|
buildVersionFileName = "kafka-version.properties"
|
||||||
|
|
||||||
|
@ -145,9 +149,8 @@ subprojects {
|
||||||
apply plugin: 'maven'
|
apply plugin: 'maven'
|
||||||
apply plugin: 'signing'
|
apply plugin: 'signing'
|
||||||
apply plugin: 'checkstyle'
|
apply plugin: 'checkstyle'
|
||||||
|
if (!JavaVersion.current().isJava11Compatible())
|
||||||
if (!JavaVersion.current().isJava9Compatible())
|
apply plugin: "com.github.spotbugs"
|
||||||
apply plugin: 'findbugs'
|
|
||||||
|
|
||||||
sourceCompatibility = minJavaVersion
|
sourceCompatibility = minJavaVersion
|
||||||
targetCompatibility = minJavaVersion
|
targetCompatibility = minJavaVersion
|
||||||
|
@ -364,18 +367,20 @@ subprojects {
|
||||||
}
|
}
|
||||||
test.dependsOn('checkstyleMain', 'checkstyleTest')
|
test.dependsOn('checkstyleMain', 'checkstyleTest')
|
||||||
|
|
||||||
if (!JavaVersion.current().isJava9Compatible()) {
|
if (!JavaVersion.current().isJava11Compatible()) {
|
||||||
findbugs {
|
spotbugs {
|
||||||
toolVersion = "3.0.1"
|
// 3.1.6 has a regression that breaks our build, seems to be https://github.com/spotbugs/spotbugs/pull/688
|
||||||
excludeFilter = file("$rootDir/gradle/findbugs-exclude.xml")
|
toolVersion = '3.1.5'
|
||||||
|
excludeFilter = file("$rootDir/gradle/spotbugs-exclude.xml")
|
||||||
ignoreFailures = false
|
ignoreFailures = false
|
||||||
}
|
}
|
||||||
test.dependsOn('findbugsMain')
|
test.dependsOn('spotbugsMain')
|
||||||
|
|
||||||
tasks.withType(FindBugs) {
|
tasks.withType(com.github.spotbugs.SpotBugsTask) {
|
||||||
reports {
|
reports {
|
||||||
xml.enabled(project.hasProperty('xmlFindBugsReport'))
|
// Continue supporting `xmlFindBugsReport` for compatibility
|
||||||
html.enabled(!project.hasProperty('xmlFindBugsReport'))
|
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 ->
|
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
|
task.enabled = false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -942,7 +942,7 @@ public class ConfigDef {
|
||||||
@Override
|
@Override
|
||||||
public void ensureValid(String name, Object value) {
|
public void ensureValid(String name, Object value) {
|
||||||
if (value == null) {
|
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");
|
throw new ConfigException(name, "null", "entry must be non null");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -22,6 +22,7 @@ import org.slf4j.Logger;
|
||||||
import org.slf4j.LoggerFactory;
|
import org.slf4j.LoggerFactory;
|
||||||
|
|
||||||
import java.io.IOException;
|
import java.io.IOException;
|
||||||
|
import java.io.InputStream;
|
||||||
import java.nio.file.Files;
|
import java.nio.file.Files;
|
||||||
import java.nio.file.Paths;
|
import java.nio.file.Paths;
|
||||||
import java.util.Map;
|
import java.util.Map;
|
||||||
|
@ -64,7 +65,9 @@ public class PropertyFileLoginModule implements LoginModule {
|
||||||
if (!credentialPropertiesMap.containsKey(fileName)) {
|
if (!credentialPropertiesMap.containsKey(fileName)) {
|
||||||
Properties credentialProperties = new Properties();
|
Properties credentialProperties = new Properties();
|
||||||
try {
|
try {
|
||||||
credentialProperties.load(Files.newInputStream(Paths.get(fileName)));
|
try (InputStream inputStream = Files.newInputStream(Paths.get(fileName))) {
|
||||||
|
credentialProperties.load(inputStream);
|
||||||
|
}
|
||||||
credentialPropertiesMap.putIfAbsent(fileName, credentialProperties);
|
credentialPropertiesMap.putIfAbsent(fileName, credentialProperties);
|
||||||
} catch (IOException e) {
|
} catch (IOException e) {
|
||||||
log.error("Error loading credentials file ", e);
|
log.error("Error loading credentials file ", e);
|
||||||
|
|
|
@ -180,7 +180,7 @@ class ClientQuotaManager(private val config: ClientQuotaManagerConfig,
|
||||||
delayQueueSensor.add(metrics.metricName("queue-size",
|
delayQueueSensor.add(metrics.metricName("queue-size",
|
||||||
quotaType.toString,
|
quotaType.toString,
|
||||||
"Tracks the size of the delay queue"), new Total())
|
"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() {
|
private def start() {
|
||||||
throttledChannelReaper.start()
|
throttledChannelReaper.start()
|
||||||
}
|
}
|
||||||
|
|
|
@ -15,12 +15,12 @@
|
||||||
limitations under the License.
|
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.
|
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 supress.
|
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>
|
<FindBugsFilter>
|
||||||
<Match>
|
<Match>
|
||||||
|
@ -44,8 +44,8 @@ For a detailed description of findbugs bug categories, see http://findbugs.sourc
|
||||||
</Match>
|
</Match>
|
||||||
|
|
||||||
<Match>
|
<Match>
|
||||||
<!-- Findbugs tends to work a little bit better with Java than with Scala. We suppress
|
<!-- Spotbugs 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
|
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.
|
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
|
NP_LOAD_OF_KNOWN_NULL_VALUE: The variable referenced at this point is known to be null
|
|
@ -17,4 +17,4 @@
|
||||||
# This script is used for verifying changes in Jenkins. In order to provide faster feedback, the tasks are ordered so
|
# 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
|
# 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.
|
# 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…
Reference in New Issue