You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by GitBox <gi...@apache.org> on 2019/01/11 13:14:55 UTC

[GitHub] eolivelli closed pull request #763: ZOOKEEPER-3223: Configure Spotbugs

eolivelli closed pull request #763: ZOOKEEPER-3223: Configure Spotbugs
URL: https://github.com/apache/zookeeper/pull/763
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/build.xml b/build.xml
index a119a0d594..bd488f0a24 100644
--- a/build.xml
+++ b/build.xml
@@ -28,6 +28,7 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
     <!-- ====================================================== -->
     <property name="slf4j.version" value="1.7.25"/>
     <property name="commons-cli.version" value="1.2"/>
+    <property name="spotbugsannotations.version" value="3.1.8"/>
 
     <property name="wagon-http.version" value="2.4"/>
     <property name="maven-ant-tasks.version" value="2.1.3"/>
diff --git a/excludeFindBugsFilter.xml b/excludeFindBugsFilter.xml
new file mode 100644
index 0000000000..c836911dbe
--- /dev/null
+++ b/excludeFindBugsFilter.xml
@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<FindBugsFilter>
+    <!-- this work work on JDK11  https://github.com/spotbugs/spotbugs-maven-plugin/issues/92 -->
+    <Bug pattern="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE"/>
+
+    <!-- this problem is to be addressed in ZOOKEEPER-3227 -->
+    <Bug pattern="DM_DEFAULT_ENCODING"/>
+
+    <!-- not really a problem -->
+    <Bug pattern="DM_EXIT"/>
+
+</FindBugsFilter>
+
diff --git a/ivy.xml b/ivy.xml
index 035bb4f748..691c8cf5fa 100644
--- a/ivy.xml
+++ b/ivy.xml
@@ -46,6 +46,7 @@
     <dependency org="org.slf4j" name="slf4j-api" rev="${slf4j.version}"/>
     <dependency org="org.slf4j" name="slf4j-log4j12" rev="${slf4j.version}" transitive="false"/>
     <dependency org="commons-cli" name="commons-cli" rev="${commons-cli.version}" />
+    <dependency org="com.github.spotbugs" name="spotbugs-annotations" rev="${spotbugsannotations.version}" />
   
     <dependency org="org.apache.maven.wagon" name="wagon-http" rev="${wagon-http.version}"
                 conf="mvn-ant-task->default"/>
diff --git a/pom.xml b/pom.xml
index 719e69ab47..8417213885 100755
--- a/pom.xml
+++ b/pom.xml
@@ -273,6 +273,7 @@
     <kerby.version>1.1.0</kerby.version>
     <bouncycastle.version>1.56</bouncycastle.version>
     <commons-collections.version>3.2.2</commons-collections.version>
+    <spotbugsannotations.version>3.1.8</spotbugsannotations.version>
   </properties>
 
   <dependencyManagement>
@@ -405,6 +406,13 @@
         <artifactId>jline</artifactId>
         <version>${jline.version}</version>
       </dependency>
+      <dependency>
+         <groupId>com.github.spotbugs</groupId>
+         <artifactId>spotbugs-annotations</artifactId>
+         <version>${spotbugsannotations.version}</version>
+         <scope>provided</scope>
+         <optional>true</optional>
+        </dependency>
     </dependencies>
   </dependencyManagement>
 
@@ -451,6 +459,14 @@
           <artifactId>clover-maven-plugin</artifactId>
           <version>4.3.1</version>
         </plugin>
+        <plugin>
+          <groupId>com.github.spotbugs</groupId>
+          <artifactId>spotbugs-maven-plugin</artifactId>
+          <version>3.1.8</version>
+          <configuration>
+            <excludeFilterFile>excludeFindBugsFilter.xml</excludeFilterFile>
+          </configuration>
+        </plugin>
       </plugins>
     </pluginManagement>
 
@@ -478,7 +494,11 @@
           </execution>
         </executions>
       </plugin>
-    </plugins>
+      <plugin>
+        <groupId>com.github.spotbugs</groupId>
+        <artifactId>spotbugs-maven-plugin</artifactId>
+      </plugin>
+      </plugins>
   </build>
 
   <reporting>
diff --git a/zookeeper-jute/pom.xml b/zookeeper-jute/pom.xml
index 5c8071de85..3582d453da 100755
--- a/zookeeper-jute/pom.xml
+++ b/zookeeper-jute/pom.xml
@@ -145,6 +145,14 @@
           </execution>
         </executions>
       </plugin>
+      <plugin>
+        <!-- spotbugs does not make sense for generated code -->
+        <groupId>com.github.spotbugs</groupId>
+        <artifactId>spotbugs-maven-plugin</artifactId>
+        <configuration>
+            <skip>true</skip>
+        </configuration>
+      </plugin>
     </plugins>
   </build>
 
diff --git a/zookeeper-server/pom.xml b/zookeeper-server/pom.xml
index 3c71c0503d..2cbc491fda 100755
--- a/zookeeper-server/pom.xml
+++ b/zookeeper-server/pom.xml
@@ -34,6 +34,12 @@
   <description>ZooKeeper server</description>
 
   <dependencies>
+    <dependency>
+      <groupId>com.github.spotbugs</groupId>
+      <artifactId>spotbugs-annotations</artifactId>
+      <scope>provided</scope>
+      <optional>true</optional>
+    </dependency>
     <dependency>
       <groupId>org.hamcrest</groupId>
       <artifactId>hamcrest-all</artifactId>
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
index ef53edf0a8..db2b4866af 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.BufferedReader;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -97,6 +98,7 @@
  * connected to as needed.
  *
  */
+@SuppressFBWarnings({"EI_EXPOSE_REP", "EI_EXPOSE_REP2"})
 public class ClientCnxn {
     private static final Logger LOG = LoggerFactory.getLogger(ClientCnxn.class);
 
@@ -479,6 +481,7 @@ public void queueCallback(AsyncCallback cb, int rc, String path,
             waitingEvents.add(new LocalCallback(cb, rc, path, ctx));
         }
 
+       @SuppressFBWarnings("JLM_JSR166_UTILCONCURRENT_MONITORENTER")
        public void queuePacket(Packet packet) {
           if (wasKilled) {
              synchronized (waitingEvents) {
@@ -495,6 +498,7 @@ public void queueEventOfDeath() {
         }
 
         @Override
+        @SuppressFBWarnings("JLM_JSR166_UTILCONCURRENT_MONITORENTER")
         public void run() {
            try {
               isRunning = true;
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java
index f685e32e2a..97aa28a7c7 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java
@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.ArrayList;
 import java.util.Collections;
 
@@ -118,18 +119,21 @@
         /**
          * This is a completely open ACL .
          */
+        @SuppressFBWarnings(value = "MS_MUTABLE_COLLECTION", justification = "Cannot break API")
         public final ArrayList<ACL> OPEN_ACL_UNSAFE = new ArrayList<ACL>(
                 Collections.singletonList(new ACL(Perms.ALL, ANYONE_ID_UNSAFE)));
 
         /**
          * This ACL gives the creators authentication id's all permissions.
          */
+        @SuppressFBWarnings(value = "MS_MUTABLE_COLLECTION", justification = "Cannot break API")
         public final ArrayList<ACL> CREATOR_ALL_ACL = new ArrayList<ACL>(
                 Collections.singletonList(new ACL(Perms.ALL, AUTH_IDS)));
 
         /**
          * This ACL gives the world the ability to read.
          */
+        @SuppressFBWarnings(value = "MS_MUTABLE_COLLECTION", justification = "Cannot break API")
         public final ArrayList<ACL> READ_ACL_UNSAFE = new ArrayList<ACL>(
                 Collections
                         .singletonList(new ACL(Perms.READ, ANYONE_ID_UNSAFE)));
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataNode.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataNode.java
index 0859aab22c..0e4ed73970 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataNode.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataNode.java
@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.server;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.util.HashSet;
 import java.util.Set;
@@ -36,6 +37,7 @@
  * array of ACLs, a stat object, and a set of its children's paths.
  * 
  */
+@SuppressFBWarnings("EI_EXPOSE_REP2")
 public class DataNode implements Record {
     /** the data for this datanode */
     byte data[];
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java
index f5d58ae8be..d4c6c8086e 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java
@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.server;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.zookeeper.CreateMode;
 
 import java.util.Collections;
@@ -210,6 +211,8 @@ public static void validateServerId(long serverId) {
      * @param ttl  ttl
      * @throws IllegalArgumentException if the ttl is not valid for the mode
      */
+    @SuppressFBWarnings(value = "RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT",
+            justification = "toEphemeralOwner may throw IllegalArgumentException")
     public static void validateTTL(CreateMode mode, long ttl) {
         if (mode.isTTL()) {
             TTL.toEphemeralOwner(ttl);
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnLog.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnLog.java
index 4f0ef2a95e..a3c870268c 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnLog.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnLog.java
@@ -155,7 +155,7 @@ public static void setPreallocSize(long size) {
      * @param serverStats used to update fsyncThresholdExceedCount
      */
     @Override
-    public void setServerStats(ServerStats serverStats) {
+    public synchronized void setServerStats(ServerStats serverStats) {
         this.serverStats = serverStats;
     }
 
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java
index 0f8c9c1ec8..2892868260 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java
@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.server.quorum;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.net.DatagramPacket;
 import java.net.DatagramSocket;
@@ -459,6 +460,8 @@ public void run() {
                 }
             }
 
+            @SuppressFBWarnings(value = "RV_RETURN_VALUE_IGNORED",
+                    justification = "tryAcquire result not chacked, but it is not an issue")
             private void process(ToSend m) {
                 int attempts = 0;
                 byte zeroes[];
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
index e87f3591b7..a505c006b8 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.server.quorum;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.LinkedBlockingQueue;
@@ -316,6 +317,7 @@ public void doWork() throws RequestProcessorException {
         }
     }
 
+    @SuppressFBWarnings("NN_NAKED_NOTIFY")
     synchronized private void wakeup() {
         notifyAll();
     }
@@ -333,6 +335,7 @@ public void commit(Request request) {
         }
     }
 
+    @Override
     public void processRequest(Request request) {
         if (stopped) {
             return;


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services