You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by ke...@apache.org on 2019/03/01 04:00:47 UTC
[beam] branch master updated: [BEAM-6554] Migrate from findbugs to
spotbugs
This is an automated email from the ASF dual-hosted git repository.
kenn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git
The following commit(s) were added to refs/heads/master by this push:
new 71d70fa [BEAM-6554] Migrate from findbugs to spotbugs
new 086d037 Merge pull request #7704: [BEAM-6554] Migrate from findbugs to spotbugs
71d70fa is described below
commit 71d70fa58328d84a69d6c026748df7fcee19f3bf
Author: Kenneth Knowles <ke...@apache.org>
AuthorDate: Thu Jan 31 20:16:12 2019 -0800
[BEAM-6554] Migrate from findbugs to spotbugs
---
buildSrc/build.gradle | 1 +
.../org/apache/beam/gradle/BeamModulePlugin.groovy | 27 ++++++++++++----------
.../core/SplittableProcessElementInvoker.java | 13 ++++++++---
.../core/metrics/ExecutionStateTracker.java | 21 ++++++++++-------
.../fnexecution/environment/DockerCommand.java | 5 ++++
sdks/java/extensions/sql/build.gradle | 5 ++++
6 files changed, 49 insertions(+), 23 deletions(-)
diff --git a/buildSrc/build.gradle b/buildSrc/build.gradle
index d281146..020000c 100644
--- a/buildSrc/build.gradle
+++ b/buildSrc/build.gradle
@@ -35,6 +35,7 @@ dependencies {
compile gradleApi()
compile localGroovy()
compile 'com.github.jengelman.gradle.plugins:shadow:4.0.3'
+ compile 'gradle.plugin.com.github.spotbugs:spotbugs-gradle-plugin:1.6.9' // Enable spotbugs
runtime "net.ltgt.gradle:gradle-apt-plugin:0.20" // Enable a Java annotation processor
runtime "com.google.protobuf:protobuf-gradle-plugin:0.8.5" // Enable proto code generation
diff --git a/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy b/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
index b64893b..7c50a52 100644
--- a/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
+++ b/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
@@ -27,16 +27,14 @@ import org.gradle.api.Project
import org.gradle.api.artifacts.Configuration
import org.gradle.api.file.FileTree
import org.gradle.api.plugins.quality.Checkstyle
-import org.gradle.api.plugins.quality.FindBugs
import org.gradle.api.publish.maven.MavenPublication
import org.gradle.api.tasks.Exec
import org.gradle.api.tasks.JavaExec
-import org.gradle.api.tasks.javadoc.Javadoc
import org.gradle.api.tasks.bundling.Jar
import org.gradle.api.tasks.compile.JavaCompile
+import org.gradle.api.tasks.javadoc.Javadoc
import org.gradle.api.tasks.testing.Test
import org.gradle.testing.jacoco.tasks.JacocoReport
-
/**
* This plugin adds methods to configure a module with Beam's defaults, called "natures".
*
@@ -713,14 +711,19 @@ class BeamModulePlugin implements Plugin<Project> {
// These dependencies are needed to avoid error-prone warnings on package-info.java files,
// also to include the annotations to suppress warnings.
//
- // findbugs-annotations artifact is licensed under LGPL and cannot be included in the
+ // spotbugs-annotations artifact is licensed under LGPL and cannot be included in the
// Apache Beam distribution, but may be relied on during build.
// See: https://www.apache.org/legal/resolved.html#prohibited
- def findbugs_annotations = "com.google.code.findbugs:annotations:3.0.1"
- compileOnly findbugs_annotations
- testCompileOnly findbugs_annotations
- annotationProcessor findbugs_annotations
- testAnnotationProcessor findbugs_annotations
+ def spotbugs_annotations = "com.github.spotbugs:spotbugs-annotations:3.1.11"
+ def jcip_annotations = "net.jcip:jcip-annotations:1.0"
+ compileOnly spotbugs_annotations
+ compileOnly jcip_annotations
+ testCompileOnly spotbugs_annotations
+ testCompileOnly jcip_annotations
+ annotationProcessor spotbugs_annotations
+ annotationProcessor jcip_annotations
+ testAnnotationProcessor spotbugs_annotations
+ testAnnotationProcessor jcip_annotations
}
// Add the optional and provided configurations for dependencies
@@ -781,12 +784,12 @@ class BeamModulePlugin implements Plugin<Project> {
// Enables a plugin which performs code analysis for common bugs.
// This plugin is configured to only analyze the "main" source set.
if (configuration.enableFindbugs) {
- project.apply plugin: 'findbugs'
- project.findbugs {
+ project.apply plugin: 'com.github.spotbugs'
+ project.spotbugs {
excludeFilter = project.rootProject.file('sdks/java/build-tools/src/main/resources/beam/findbugs-filter.xml')
sourceSets = [sourceSets.main]
}
- project.tasks.withType(FindBugs) {
+ project.tasks.withType(com.github.spotbugs.SpotBugsTask) {
reports {
html.enabled = !project.jenkins.isCIBuild
xml.enabled = project.jenkins.isCIBuild
diff --git a/runners/core-java/src/main/java/org/apache/beam/runners/core/SplittableProcessElementInvoker.java b/runners/core-java/src/main/java/org/apache/beam/runners/core/SplittableProcessElementInvoker.java
index 81f0bd4..b3958e8 100644
--- a/runners/core-java/src/main/java/org/apache/beam/runners/core/SplittableProcessElementInvoker.java
+++ b/runners/core-java/src/main/java/org/apache/beam/runners/core/SplittableProcessElementInvoker.java
@@ -17,8 +17,9 @@
*/
package org.apache.beam.runners.core;
-import static org.apache.beam.vendor.guava.v20_0.com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.beam.vendor.guava.v20_0.com.google.common.base.Preconditions.checkArgument;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import javax.annotation.Nullable;
import org.apache.beam.sdk.transforms.DoFn;
import org.apache.beam.sdk.transforms.reflect.DoFnInvoker;
@@ -38,13 +39,19 @@ public abstract class SplittableProcessElementInvoker<
private final DoFn.ProcessContinuation continuation;
private final @Nullable Instant futureOutputWatermark;
+ @SuppressFBWarnings(
+ value = "NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE",
+ justification = "Spotbugs incorrectly thinks continuation is marked @Nullable")
public Result(
@Nullable RestrictionT residualRestriction,
DoFn.ProcessContinuation continuation,
@Nullable Instant futureOutputWatermark) {
- this.continuation = checkNotNull(continuation);
+ checkArgument(continuation != null, "continuation must not be null");
+ this.continuation = continuation;
if (continuation.shouldResume()) {
- checkNotNull(residualRestriction);
+ checkArgument(
+ residualRestriction != null,
+ "residual restriction must not be null if continuation indicate it should resume");
}
this.residualRestriction = residualRestriction;
this.futureOutputWatermark = futureOutputWatermark;
diff --git a/runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java b/runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java
index d9ab245..a559507 100644
--- a/runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java
+++ b/runners/core-java/src/main/java/org/apache/beam/runners/core/metrics/ExecutionStateTracker.java
@@ -223,29 +223,34 @@ public class ExecutionStateTracker implements Comparable<ExecutionStateTracker>
* Indicates that the execution thread has entered the {@code newState}. Returns a {@link
* Closeable} that should be called when that state is completed.
*
- * <p>This must be the only place where the variable numTransitions is updated, and always called
- * from the execution thread.
+ * <p>This must be the only place where incTransitions is called, and always called from the
+ * execution thread.
*/
- @SuppressWarnings("NonAtomicVolatileUpdate")
- @SuppressFBWarnings(
- value = "VO_VOLATILE_INCREMENT",
- justification = "Intentional for performance.")
public Closeable enterState(ExecutionState newState) {
// WARNING: This method is called in the hottest path, and must be kept as efficient as
// possible. Avoid blocking, synchronizing, etc.
final ExecutionState previous = currentState;
currentState = newState;
newState.onActivate(true);
- numTransitions++;
+ incTransitions();
return () -> {
currentState = previous;
- numTransitions++;
+ incTransitions();
if (previous != null) {
previous.onActivate(false);
}
};
}
+ @SuppressWarnings("NonAtomicVolatileUpdate")
+ // Helper method necessary due to https://github.com/spotbugs/spotbugs/issues/724
+ @SuppressFBWarnings(
+ value = "VO_VOLATILE_INCREMENT",
+ justification = "Intentional for performance.")
+ private void incTransitions() {
+ numTransitions++;
+ }
+
/** Return the number of transitions that have been observed by this state tracker. */
public long getNumTransitions() {
return numTransitions;
diff --git a/runners/java-fn-execution/src/main/java/org/apache/beam/runners/fnexecution/environment/DockerCommand.java b/runners/java-fn-execution/src/main/java/org/apache/beam/runners/fnexecution/environment/DockerCommand.java
index b510722..cbd65be 100644
--- a/runners/java-fn-execution/src/main/java/org/apache/beam/runners/fnexecution/environment/DockerCommand.java
+++ b/runners/java-fn-execution/src/main/java/org/apache/beam/runners/fnexecution/environment/DockerCommand.java
@@ -19,6 +19,7 @@ package org.apache.beam.runners.fnexecution.environment;
import static org.apache.beam.vendor.guava.v20_0.com.google.common.base.Preconditions.checkArgument;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
@@ -37,6 +38,10 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/** A docker command wrapper. Simplifies communications with the Docker daemon. */
+// Suppressing here due to https://github.com/spotbugs/spotbugs/issues/724
+@SuppressFBWarnings(
+ value = "OS_OPEN_STREAM",
+ justification = "BufferedReader wraps stream we don't own and should not close")
class DockerCommand {
private static final Logger LOG = LoggerFactory.getLogger(DockerCommand.class);
diff --git a/sdks/java/extensions/sql/build.gradle b/sdks/java/extensions/sql/build.gradle
index 6db4477..b077f8d 100644
--- a/sdks/java/extensions/sql/build.gradle
+++ b/sdks/java/extensions/sql/build.gradle
@@ -115,6 +115,11 @@ dependencies {
// Dependencies that are bundled in when we bundle Calcite
permitUsedUndeclared "org.codehaus.janino:janino:3.0.9"
permitUsedUndeclared "org.codehaus.janino:commons-compiler:3.0.9"
+
+ // Dependencies where one or the other appears "used" depending on classpath,
+ // but it doesn't matter which is used
+ permitUsedUndeclared "com.google.code.findbugs:jsr305:3.0.2"
+ permitUnusedDeclared "net.jcip:jcip-annotations:1.0"
}
// Copy Caclcite templates and our own template into the build directory