beam-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From k...@apache.org
Subject [beam] branch master updated: [BEAM-6554] Migrate from findbugs to spotbugs
Date Fri, 01 Mar 2019 04:00:47 GMT
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 <kenn@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


Mime
View raw message