Findbugs updates

rozza · rozza · commit 2f4682917ae6 · 2015-05-22T16:47:00.000+01:00
Update findbugs to 3.0.1 Fix findbugs-exclude.xml Fix found bugs JAVA-1822 JAVA-1823
diff --git a/bson/src/main/org/bson/BsonDouble.java b/bson/src/main/org/bson/BsonDouble.java
@@ -39,7 +39,7 @@ public BsonDouble(final double value) {
 
     @Override
     public int compareTo(final BsonDouble o) {
-        return Double.valueOf(value).compareTo(o.value);
+        return Double.compare(value, o.value);
     }
 
     @Override
diff --git a/bson/src/main/org/bson/json/JsonReader.java b/bson/src/main/org/bson/json/JsonReader.java
@@ -823,7 +823,7 @@ private long visitDateTimeConstructor() {
             }
             if (pos == 1) {
                 return values[0];
-            } else if (pos < 3 && pos > 7) {
+            } else if (pos < 3 || pos > 7) {
                 throw new JsonParseException("JSON reader expected 1 or 3-7 integers but found %d.", pos);
             }
 
diff --git a/build.gradle b/build.gradle
@@ -195,7 +195,7 @@ configure(subprojects.findAll { it.name != 'util' && it.name != 'mongo-java-driv
     findbugs {
         excludeFilter = new File("$configDir/findbugs-exclude.xml")
         sourceSets = [sourceSets.main]
-        toolVersion = '3.0.0'
+        toolVersion = '3.0.1'
     }
 
     codenarc {
diff --git a/config/findbugs-exclude.xml b/config/findbugs-exclude.xml
@@ -7,56 +7,53 @@
         <Bug code="EI,EI2"/>
     </Match>
 
-    <Match>
-        <Package name="com.mongodb">
-            <Bug pattern="EQ_DOESNT_OVERRIDE_EQUALS"/>  <!-- Deliberately ignoring this, as many BSONObject subclasses don't do it -->
-        </Package>
-    </Match>
 
     <!-- these specific issues are deliberate design decisions -->
+    <!-- Deliberately ignoring this, as many BSONObject subclasses don't do it -->
     <Match>
-        <Class name="com.mongodb.connection.BaseCluster">
-            <Bug pattern="RV_RETURN_VALUE_IGNORED"/>  <!-- Deliberately ignoring return value of CountDownLatch.await -->
-        </Class>
+        <Package name="com.mongodb"/>
+        <Bug pattern="EQ_DOESNT_OVERRIDE_EQUALS"/>
     </Match>
 
+    <!-- Deliberately ignoring return value of CountDownLatch.await -->
     <Match>
-        <Class name="com.mongodb.gridfs.GridFS" />
-        <Method name="createFile" params="java.io.File" /> <!-- The underlying call to GridFSInputFile closes the file -->
-        <Bug pattern="OBL_UNSATISFIED_OBLIGATION"/>
+        <Class name="com.mongodb.connection.BaseCluster"/>
+        <Bug pattern="RV_RETURN_VALUE_IGNORED"/>
     </Match>
+
+    <!-- The underlying call to GridFSInputFile closes the file -->
     <Match>
-        <Class name="com.mongodb.DocumentCodec">
-            <Bug pattern="NM_SAME_SIMPLE_NAME_AS_SUPERCLASS"/>
-        </Class>
+        <Class name="com.mongodb.gridfs.GridFS"/>
+        <Method name="createFile" params="java.io.File"/>
+        <Bug pattern="OBL_UNSATISFIED_OBLIGATION"/>
     </Match>
+
     <Match>
-        <Class name="org.mongodb.DatabaseTestCase">
-            <Bug pattern="MS_PKGPROTECT"/>
-        </Class>
+        <Class name="com.mongodb.DocumentCodec"/>
+        <Bug pattern="NM_SAME_SIMPLE_NAME_AS_SUPERCLASS"/>
     </Match>
+
     <Match>
-        <Class name="org.mongodb.FunctionalSpecification">
-            <Bug pattern="MS_PKGPROTECT"/>
-        </Class>
+        <Class name="org.mongodb.DatabaseTestCase" />
+        <Bug pattern="MS_PKGPROTECT"/>
     </Match>
+
     <Match>
-        <Class name="com.mongodb.DatabaseTestCase">
-            <Bug pattern="MS_PKGPROTECT"/>
-        </Class>
+        <Class name="org.mongodb.FunctionalSpecification"/>
+        <Bug pattern="MS_PKGPROTECT"/>
     </Match>
+
     <Match>
-        <Class name="org.bson.types.StringRangeSet$NumberStringComparator">
-            <Bug pattern="SE_COMPARATOR_SHOULD_BE_SERIALIZABLE"/>
-        </Class>
+        <Class name="com.mongodb.DatabaseTestCase"/>
+        <Bug pattern="MS_PKGPROTECT"/>
     </Match>
 
-    <!-- Spock tests seem to fail the serial version ID test when stubbing.  Annoying. -->
     <Match>
-        <Class name="~.*\.*Specification.*"/>
-        <Bug pattern="SE_NO_SERIALVERSIONID,LI_LAZY_INIT_STATIC"/>
+        <Class name="org.bson.types.StringRangeSet$NumberStringComparator"/>
+        <Bug pattern="SE_COMPARATOR_SHOULD_BE_SERIALIZABLE"/>
     </Match>
 
+    <!-- Test exclusions -->
     <!-- All bugs in test classes, except for JUnit-specific bugs -->
     <Match>
         <Class name="~.*\.*Test"/>
@@ -65,4 +62,22 @@
         </Not>
     </Match>
 
+    <!-- Deliberate use of an unused field in the Person POJO class -->
+    <Match>
+        <Class name="com.mongodb.acceptancetest.crud.pojo.Person"/>
+        <Bug pattern="SS_SHOULD_BE_STATIC"/>
+    </Match>
+
+    <!-- Spock tests seem to fail the serial version ID test when stubbing.  Annoying. -->
+    <Match>
+        <Source name="~.*\.groovy"/>
+        <Bug pattern="SE_NO_SERIALVERSIONID,LI_LAZY_INIT_STATIC,EQ_UNUSUAL,IT_NO_SUCH_ELEMENT,RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT"/>
+    </Match>
+
+    <!-- Strange findbugs issues -->
+    <Match>
+        <Source name="~.*ClusterSettingsSpecification\.groovy"/>
+        <Bug pattern="RANGE_ARRAY_INDEX"/>
+    </Match>
+
 </FindBugsFilter>
diff --git a/driver-core/src/main/com/mongodb/WriteConcernResult.java b/driver-core/src/main/com/mongodb/WriteConcernResult.java
@@ -93,7 +93,7 @@ public boolean equals(final Object o) {
                 if (this == o) {
                     return true;
                 }
-                if (getClass() != o.getClass()) {
+                if (o == null || getClass() != o.getClass()) {
                     return false;
                 }
 
diff --git a/driver-core/src/test/functional/com/mongodb/ClusterFixture.java b/driver-core/src/test/functional/com/mongodb/ClusterFixture.java
@@ -162,14 +162,14 @@ public static AsyncReadWriteBinding getAsyncBinding(final Cluster cluster) {
         return new AsyncClusterBinding(cluster, ReadPreference.primary());
     }
 
-    public static Cluster getCluster() {
+    public static synchronized Cluster getCluster() {
         if (cluster == null) {
             cluster = createCluster(new SocketStreamFactory(getSocketSettings(), getSslSettings()));
         }
         return cluster;
     }
 
-    public static Cluster getAsyncCluster() {
+    public static synchronized Cluster getAsyncCluster() {
         if (asyncCluster == null) {
             asyncCluster = createCluster(getAsyncStreamFactory());
         }
diff --git a/driver-core/src/test/functional/com/mongodb/operation/ListCollectionsOperationSpecification.groovy b/driver-core/src/test/functional/com/mongodb/operation/ListCollectionsOperationSpecification.groovy
@@ -369,13 +369,10 @@ class ListCollectionsOperationSpecification extends OperationFunctionalSpecifica
 
     def cursorToListWithNext(BatchCursor cursor) {
         def list = []
-        try {
-            while (true) {
-                list += cursor.next()
-            }
-        } catch (NoSuchElementException e) {
-            return list
+        while (cursor.hasNext()) {
+            list += cursor.next()
         }
+        list
     }
 
     def cursorToListWithTryNext(BatchCursor cursor) {
diff --git a/driver-core/src/test/unit/com/mongodb/JsonPoweredTestHelper.java b/driver-core/src/test/unit/com/mongodb/JsonPoweredTestHelper.java
@@ -23,9 +23,11 @@
 
 import java.io.BufferedReader;
 import java.io.File;
-import java.io.FileReader;
+import java.io.FileInputStream;
 import java.io.IOException;
+import java.io.InputStreamReader;
 import java.net.URISyntaxException;
+import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -42,26 +44,31 @@ public static List<File> getTestFiles(final String resourcePath) throws URISynta
     }
 
     private static String getFileAsString(final File file) throws IOException {
-        BufferedReader reader = new BufferedReader(new FileReader(file));
         String line;
         StringBuilder stringBuilder = new StringBuilder();
         String ls = System.getProperty("line.separator");
-
-        while ((line = reader.readLine()) != null) {
-            stringBuilder.append(line);
-            stringBuilder.append(ls);
+        BufferedReader reader = new BufferedReader(new InputStreamReader(new FileInputStream(file), Charset.forName("UTF-8")));
+        try {
+            while ((line = reader.readLine()) != null) {
+                stringBuilder.append(line);
+                stringBuilder.append(ls);
+            }
+        } finally {
+            reader.close();
         }
-
         return stringBuilder.toString();
     }
 
     private static void addFilesFromDirectory(final File directory, final List<File> files) {
-        for (String fileName : directory.list()) {
-            File file = new File(directory, fileName);
-            if (file.isDirectory()) {
-                addFilesFromDirectory(file, files);
-            } else if (file.getName().endsWith(".json")) {
-                files.add(file);
+        String[] fileNames = directory.list();
+        if (fileNames != null) {
+            for (String fileName : fileNames) {
+                File file = new File(directory, fileName);
+                if (file.isDirectory()) {
+                    addFilesFromDirectory(file, files);
+                } else if (file.getName().endsWith(".json")) {
+                    files.add(file);
+                }
             }
         }
     }
diff --git a/driver/src/main/com/mongodb/FindIterableImpl.java b/driver/src/main/com/mongodb/FindIterableImpl.java
@@ -197,11 +197,5 @@ public TResult first() {
             BatchCursor<TResult> batchCursor = executor.execute(findFirstOperation, readPreference);
             return batchCursor.hasNext() ? batchCursor.next().iterator().next() : null;
         }
-
-        @Override
-        public FindOperationIterable batchSize(final int batchSize) {
-            batchSize(batchSize);
-            return this;
-        }
     }
 }
diff --git a/driver/src/main/org/bson/LazyBSONObject.java b/driver/src/main/org/bson/LazyBSONObject.java
@@ -170,8 +170,7 @@ Object readValue(final BsonBinaryReader reader) {
             case BINARY:
                 BsonBinary binary = reader.readBinaryData();
                 byte binaryType = binary.getType();
-                if (binaryType == BsonBinarySubType.BINARY.getValue()
-                    || binaryType == BsonBinarySubType.BINARY.getValue()) {
+                if (binaryType == BsonBinarySubType.BINARY.getValue() || binaryType == BsonBinarySubType.OLD_BINARY.getValue()) {
                     return binary.getData();
                 } else if (binaryType == BsonBinarySubType.UUID_LEGACY.getValue()) {
                     return new UUID(readLong(binary.getData(), 0), readLong(binary.getData(), 8));
diff --git a/driver/src/test/functional/com/mongodb/client/NameCodec.java b/driver/src/test/functional/com/mongodb/client/NameCodec.java
@@ -37,7 +37,7 @@ public void encode(final BsonWriter writer, final Name n, final EncoderContext e
     public Name decode(final BsonReader reader, final DecoderContext decoderContext) {
         reader.readStartDocument();
         String name = reader.readString("_id");
-        int count = new Double(reader.readDouble("value")).intValue();
+        int count = (int) reader.readDouble("value");
 
         reader.readEndDocument();
         return new Name(name, count);

Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ public BsonDouble(final double value) {`
`39`	`39`
`40`	`40`	`@Override`
`41`	`41`	`public int compareTo(final BsonDouble o) {`
`42`		`- return Double.valueOf(value).compareTo(o.value);`
	`42`	`+ return Double.compare(value, o.value);`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -823,7 +823,7 @@ private long visitDateTimeConstructor() {`
`823`	`823`	`}`
`824`	`824`	`if (pos == 1) {`
`825`	`825`	`return values[0];`
`826`		`- } else if (pos < 3 && pos > 7) {`
	`826`	`+ } else if (pos < 3 \|\| pos > 7) {`
`827`	`827`	`throw new JsonParseException("JSON reader expected 1 or 3-7 integers but found %d.", pos);`
`828`	`828`	`}`
`829`	`829`
Original file line number	Diff line number	Diff line change
`@@ -195,7 +195,7 @@ configure(subprojects.findAll { it.name != 'util' && it.name != 'mongo-java-driv`
`195`	`195`	`findbugs {`
`196`	`196`	`excludeFilter = new File("$configDir/findbugs-exclude.xml")`
`197`	`197`	`sourceSets = [sourceSets.main]`
`198`		`- toolVersion = '3.0.0'`
	`198`	`+ toolVersion = '3.0.1'`
`199`	`199`	`}`
`200`	`200`
`201`	`201`	`codenarc {`
Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,7 @@ public boolean equals(final Object o) {`
`93`	`93`	`if (this == o) {`
`94`	`94`	`return true;`
`95`	`95`	`}`
`96`		`- if (getClass() != o.getClass()) {`
	`96`	`+ if (o == null \|\| getClass() != o.getClass()) {`
`97`	`97`	`return false;`
`98`	`98`	`}`
`99`	`99`
Original file line number	Diff line number	Diff line change
`@@ -162,14 +162,14 @@ public static AsyncReadWriteBinding getAsyncBinding(final Cluster cluster) {`
`162`	`162`	`return new AsyncClusterBinding(cluster, ReadPreference.primary());`
`163`	`163`	`}`
`164`	`164`
`165`		`- public static Cluster getCluster() {`
	`165`	`+ public static synchronized Cluster getCluster() {`
`166`	`166`	`if (cluster == null) {`
`167`	`167`	`cluster = createCluster(new SocketStreamFactory(getSocketSettings(), getSslSettings()));`
`168`	`168`	`}`
`169`	`169`	`return cluster;`
`170`	`170`	`}`
`171`	`171`
`172`		`- public static Cluster getAsyncCluster() {`
	`172`	`+ public static synchronized Cluster getAsyncCluster() {`
`173`	`173`	`if (asyncCluster == null) {`
`174`	`174`	`asyncCluster = createCluster(getAsyncStreamFactory());`
`175`	`175`	`}`
Original file line number	Diff line number	Diff line change
`@@ -197,11 +197,5 @@ public TResult first() {`
`197`	`197`	`BatchCursor<TResult> batchCursor = executor.execute(findFirstOperation, readPreference);`
`198`	`198`	`return batchCursor.hasNext() ? batchCursor.next().iterator().next() : null;`
`199`	`199`	`}`
`200`		`-`
`201`		`- @Override`
`202`		`- public FindOperationIterable batchSize(final int batchSize) {`
`203`		`- batchSize(batchSize);`
`204`		`- return this;`
`205`		`- }`
`206`	`200`	`}`
`207`	`201`	`}`