Add Findbugs (#618)

* Add Findbugs

* Fix existing findbugs errors

Author: AustinShalit

.travis.yml

@@ -30,7 +30,7 @@ install:
   - '[ "${TRAVIS_PULL_REQUEST}" = "false" ] && ./gradlew :ui:jfxNative --stacktrace || ./gradlew --stacktrace '
 
 script:
-  - ./gradlew checkstyleMain checkstyleTest pmdMain pmdTest jacocoTestReport jacocoRootReport test --stacktrace -Pheadless=true -PlogTests
+  - ./gradlew checkstyleMain checkstyleTest pmdMain pmdTest findbugsMain findbugsTest jacocoTestReport jacocoRootReport test --stacktrace -Pheadless=true -PlogTests
 
 after_success:
   - if [[ "$TRAVIS_OS_NAME" != "osx" ]]; then codecov         ; fi

appveyor.yml

@@ -6,7 +6,7 @@ build_script:
 
 # to run your custom scripts instead of automatic tests
 test_script:
-  - gradlew.bat checkstyleMain checkstyleTest pmdMain pmdTest jacocoTestReport jacocoRootReport test --stacktrace -Pheadless=true -PlogTests
+  - gradlew.bat checkstyleMain checkstyleTest pmdMain pmdTest findbugsMain findbugsTest jacocoTestReport jacocoRootReport test --stacktrace -Pheadless=true -PlogTests
 
 platform:
   - x64

build.gradle

@@ -1,3 +1,5 @@
+import groovy.xml.XmlUtil
+
 buildscript {
     repositories {
         jcenter()
@@ -85,13 +87,14 @@ def getVersionSimple = { ->
     }
 }
 
-configure(allprojects - project(':ui:linuxLauncher')) {
+configure(subprojects - project(':ui:linuxLauncher')) {
     apply plugin: 'java'
     apply plugin: 'application'
     apply plugin: 'jacoco'
     apply plugin: 'net.ltgt.errorprone'
     apply plugin: 'checkstyle'
     apply plugin: 'pmd'
+    apply plugin: 'findbugs'
 
     configurations.errorprone {
         resolutionStrategy.force 'com.google.errorprone:error_prone_core:2.0.9'
@@ -119,6 +122,30 @@ configure(allprojects - project(':ui:linuxLauncher')) {
         ruleSets = []
     }
 
+    findbugs {
+        sourceSets = [sourceSets.main]
+        excludeFilter = new File(rootDir, "findBugsSuppressions.xml")
+        effort = "max"
+    }
+
+    ext.printReportSafe = { xmlReport ->
+        if (xmlReport.exists()) {
+            def bugs = (new XmlParser().parse(xmlReport)).BugInstance
+            bugs.each { System.out.println(new XmlUtil().serialize(it)) }
+        }
+    }
+
+    task findbugsMainReport << {
+        printReportSafe(findbugsMain.reports.getXml().destination)
+    }
+
+    task findbugsTestReport << {
+        printReportSafe(findbugsTest.reports.getXml().destination)
+    }
+
+    findbugsMain.finalizedBy findbugsMainReport
+    findbugsTest.finalizedBy findbugsTestReport
+
     repositories {
         mavenCentral()
         jcenter()
@@ -129,6 +156,7 @@ configure(allprojects - project(':ui:linuxLauncher')) {
 
 
     dependencies {
+        compile group: 'com.google.code.findbugs', name: 'annotations', version: '3.0.1'
         testCompile group: 'net.jodah', name: 'concurrentunit', version: '0.4.2'
         testCompile group: 'org.hamcrest', name: 'hamcrest-all', version: '1.3'
         testCompile group: 'junit', name: 'junit', version: '4.12'

core/src/main/java/edu/wpi/grip/core/OperationDescription.java

@@ -9,6 +9,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
 

core/src/main/java/edu/wpi/grip/core/operations/composite/FilterContoursOperation.java

@@ -191,7 +191,7 @@ public void perform() {
         continue;
       }
 
-      final double ratio = bb.width() / bb.height();
+      final double ratio = (double) bb.width() / (double) bb.height();
       if (ratio < minRatio || ratio > maxRatio) {
         continue;
       }

core/src/main/java/edu/wpi/grip/core/operations/composite/PublishVideoOperation.java

@@ -9,6 +9,8 @@
 
 import com.google.common.collect.ImmutableList;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 import org.bytedeco.javacpp.BytePointer;
 import org.bytedeco.javacpp.IntPointer;
 
@@ -133,6 +135,8 @@ public class PublishVideoOperation implements Operation {
   };
 
   @SuppressWarnings("JavadocMethod")
+  @SuppressFBWarnings(value = "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD",
+      justification = "Do not need to synchronize inside of a constructor")
   public PublishVideoOperation(InputSocket.Factory inputSocketFactory) {
     if (numSteps != 0) {
       throw new IllegalStateException("Only one instance of PublishVideoOperation may exist");
@@ -141,7 +145,7 @@ public PublishVideoOperation(InputSocket.Factory inputSocketFactory) {
         false));
     this.qualitySocket = inputSocketFactory.create(SocketHints.Inputs
         .createNumberSliderSocketHint("Quality", 80, 0, 100));
-    numSteps = numSteps + 1;
+    numSteps++;
 
     serverThread = new Thread(runServer, "Camera Server");
     serverThread.setDaemon(true);

core/src/main/java/edu/wpi/grip/core/operations/composite/ValveOperation.java

@@ -16,7 +16,7 @@
 
 public class ValveOperation implements Operation {
 
-  public static OperationDescription DESCRIPTION =
+  public static final OperationDescription DESCRIPTION =
       OperationDescription.builder()
           .name("Valve")
           .summary("Toggle an output socket on or off using a boolean")

core/src/main/java/edu/wpi/grip/core/operations/network/http/DataHandler.java

@@ -12,6 +12,8 @@
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 import org.eclipse.jetty.server.Request;
 
 import java.io.IOException;
@@ -69,6 +71,8 @@ public final class DataHandler extends PedanticHandler {
   }
 
   @Override
+  @SuppressFBWarnings(value = "UW_UNCOND_WAIT",
+      justification = "A bug in FindBugs.  There is a condidtion (outside of the try)")
   protected void handleIfPassed(String target,
                                 Request baseRequest,
                                 HttpServletRequest request,

core/src/main/java/edu/wpi/grip/core/util/SafeShutdown.java

@@ -1,5 +1,7 @@
 package edu.wpi.grip.core.util;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
 import javax.annotation.Nullable;
 
 /**
@@ -16,8 +18,11 @@ public final class SafeShutdown {
      * flagging a shutdown
      * that we can't control.
      */
+
     Runtime.getRuntime().addShutdownHook(new Thread() {
       @Override
+      @SuppressFBWarnings(value = "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD",
+          justification = "Static variable is volatile")
       public void run() {
         SafeShutdown.stopping = true;
       }

core/src/test/java/edu/wpi/grip/core/ConnectionTest.java

@@ -69,7 +69,7 @@ public void testPipelineSaysConnectionIsInvalid() {
     );
   }
 
-  private class MockPipeline extends Pipeline {
+  private static class MockPipeline extends Pipeline {
   }