search

ome / ome-common-java

Java library providing common functionality for all OME Java components
Other
2 stars 19 forks source link

Fix findbugs and activate in Travis #3

Open sbesson opened 8 years ago

sbesson commented 8 years ago

Follow-up task of https://github.com/ome/ome-common-java/pull/1, activating mvn findbugs:check in Travis requires to fix 30 bugs in the code base 

[INFO] --- findbugs-maven-plugin:3.0.4:check (default-cli) @ ome-common ---
[INFO] BugInstance size is 30
[INFO] Error size is 0
[INFO] Total bugs: 30
[INFO] loci.common.BZip2Handle.isBZip2File(String) may fail to clean up java.io.InputStream on checked exception [loci.common.BZip2Handle, loci.common.BZip2Handle, loci.common.BZip2Handle] Obligation to clean up resource created at BZip2Handle.java:[line 86] is not dischargedPath continues at BZip2Handle.java:[line 87]Path continues at BZip2Handle.java:[line 88] OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE
[INFO] loci.common.BZip2Handle.isBZip2File(String) ignores result of java.io.FileInputStream.read(byte[]) [loci.common.BZip2Handle] At BZip2Handle.java:[line 88] RR_NOT_CHECKED
[INFO] Nullcheck of ByteArrayHandle.buffer at line 112 of value previously dereferenced in loci.common.ByteArrayHandle.setLength(long) [loci.common.ByteArrayHandle, loci.common.ByteArrayHandle] At ByteArrayHandle.java:[line 109]Redundant null check at ByteArrayHandle.java:[line 112] RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE
[INFO] loci.common.ByteArrayHandle.read(ByteBuffer, int, int) ignores result of loci.common.ByteArrayHandle.read(byte[]) [loci.common.ByteArrayHandle] At ByteArrayHandle.java:[line 171] RR_NOT_CHECKED
[INFO] loci.common.ByteArrayHandle.readUTF() ignores result of loci.common.ByteArrayHandle.read(byte[]) [loci.common.ByteArrayHandle] At ByteArrayHandle.java:[line 362] RR_NOT_CHECKED
[INFO] loci.common.CRC.CRC_32_TABLE should be package protected [loci.common.CRC] At CRC.java:[line 64] MS_PKGPROTECT
[INFO] Switch statement found in loci.common.DateTools.convertDate(long, int, String, boolean) where default case is missing [loci.common.DateTools] At DateTools.java:[lines 144-158] SF_SWITCH_NO_DEFAULT
[INFO] loci.common.GZipHandle.isGZipFile(String) may fail to clean up java.io.InputStream on checked exception [loci.common.GZipHandle, loci.common.GZipHandle, loci.common.GZipHandle] Obligation to clean up resource created at GZipHandle.java:[line 83] is not dischargedPath continues at GZipHandle.java:[line 84]Path continues at GZipHandle.java:[line 85] OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE
[INFO] loci.common.GZipHandle.isGZipFile(String) ignores result of java.io.FileInputStream.read(byte[]) [loci.common.GZipHandle] At GZipHandle.java:[line 85] RR_NOT_CHECKED
[INFO] loci.common.IniList.flattenIntoHashMap() makes inefficient use of keySet iterator instead of entrySet iterator [loci.common.IniList] At IniList.java:[line 75] WMI_WRONG_MAP_ITERATOR
[INFO] loci.common.IniWriter.saveINI(IniList, String, boolean, boolean) makes inefficient use of keySet iterator instead of entrySet iterator [loci.common.IniWriter] At IniWriter.java:[line 87] WMI_WRONG_MAP_ITERATOR
[INFO] loci.common.Location.equals(Object) does not check for null argument [loci.common.Location] At Location.java:[lines 518-528] NP_EQUALS_SHOULD_HANDLE_NULL_ARGUMENT
[INFO] Should loci.common.Location$ListingsResult be a _static_ inner class? [loci.common.Location$ListingsResult] At Location.java:[lines 86-89] SIC_INNER_SHOULD_BE_STATIC
[INFO] loci.common.NIOFileHandle.defaultBufferSize should be package protected [loci.common.NIOFileHandle] At NIOFileHandle.java:[line 66] MS_PKGPROTECT
[INFO] loci.common.NIOFileHandle.defaultRWBufferSize should be package protected [loci.common.NIOFileHandle] At NIOFileHandle.java:[line 71] MS_PKGPROTECT
[INFO] loci.common.NIOFileHandle.readFully(byte[]) ignores result of loci.common.NIOFileHandle.read(byte[]) [loci.common.NIOFileHandle] At NIOFileHandle.java:[line 361] RR_NOT_CHECKED
[INFO] loci.common.NIOFileHandle.readFully(byte[], int, int) ignores result of loci.common.NIOFileHandle.read(byte[], int, int) [loci.common.NIOFileHandle] At NIOFileHandle.java:[line 367] RR_NOT_CHECKED
[INFO] loci.common.NIOInputStream.findString(boolean, int, String[]) may fail to close stream [loci.common.NIOInputStream] At NIOInputStream.java:[line 249] OS_OPEN_STREAM
[INFO] Unread public/protected field: loci.common.NIOInputStream.filename [loci.common.NIOInputStream] At NIOInputStream.java:[line 81] URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD
[INFO] Unused public or protected field: loci.common.NIOInputStream.channel [loci.common.NIOInputStream] In NIOInputStream.java UUF_UNUSED_PUBLIC_OR_PROTECTED_FIELD
[INFO] loci.common.RandomAccessInputStream.findString(boolean, int, String[]) may fail to close stream [loci.common.RandomAccessInputStream] At RandomAccessInputStream.java:[line 323] OS_OPEN_STREAM
[INFO] loci.common.RandomAccessInputStream.skipBits(long) ignores result of loci.common.RandomAccessInputStream.skipBytes(int) [loci.common.RandomAccessInputStream] At RandomAccessInputStream.java:[line 393] SR_NOT_CHECKED
[INFO] loci.common.ReflectedUniverse.setVar(String, boolean) invokes inefficient Boolean constructor; use Boolean.valueOf(...) instead [loci.common.ReflectedUniverse] At ReflectedUniverse.java:[line 333] DM_BOOLEAN_CTOR
[INFO] loci.common.ReflectedUniverse.getVar(String) invokes inefficient new Integer(String) constructor; use Integer.valueOf(String) instead [loci.common.ReflectedUniverse] At ReflectedUniverse.java:[line 395] DM_NUMBER_CTOR
[INFO] loci.common.ReflectedUniverse.getVar(String) invokes inefficient new Long(String) constructor; use Long.valueOf(String) instead [loci.common.ReflectedUniverse] At ReflectedUniverse.java:[line 399] DM_NUMBER_CTOR
[INFO] loci.common.ReflectedUniverse.setVar(String, byte) invokes inefficient new Byte(byte) constructor; use Byte.valueOf(byte) instead [loci.common.ReflectedUniverse] At ReflectedUniverse.java:[line 338] DM_NUMBER_CTOR
[INFO] new loci.common.ReflectedUniverse(URL[]) creates a java.net.URLClassLoader classloader, which should be performed within a doPrivileged block [loci.common.ReflectedUniverse] At ReflectedUniverse.java:[line 85] DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED
[INFO] Null passed for non-null parameter of new java.util.zip.ZipInputStream(InputStream) in loci.common.ZipHandle.resetStream() [loci.common.ZipHandle, loci.common.ZipHandle] Method invoked at ZipHandle.java:[line 175]Known null at ZipHandle.java:[line 170] NP_NULL_PARAM_DEREF
[INFO] loci.common.ZipHandle.isZipFile(String) ignores result of loci.common.IRandomAccess.read(byte[]) [loci.common.ZipHandle] At ZipHandle.java:[line 130] RR_NOT_CHECKED
[INFO] Incorrect lazy initialization of static field loci.common.services.ServiceFactory.defaultFactory in new loci.common.services.ServiceFactory() [loci.common.services.ServiceFactory] At ServiceFactory.java:[lines 78-79] LI_LAZY_INIT_STATIC
[INFO]
melissalinkert commented 6 years ago

See also https://trello.com/c/UP6SA0d3/31-findbugs-301 and https://trello.com/c/eKY4jQVH/180-findbugs-evaluate-spotbugs. The current version of findbugs-maven-plugin (3.0.5) uses Findbugs 3.0.1: https://github.com/gleclaire/findbugs-maven-plugin/blob/findbugs-maven-plugin-3.0.5/pom.xml#L121

Many of the bugs noted above would be ignored by https://github.com/openmicroscopy/bioformats/blob/develop/excludebugs.xml. Before going too far with this, it might be worth discussing to what extent we want to have unified exclude rules for all repositories.

jburel commented 6 months ago

Considering how old findbugs is (latest release was in 06 March 2015) if we should consider a different bug finder system

melissalinkert commented 6 months ago

Spotbugs would be one option, as it's a continuation of Findbugs. Adding this to the pom.xml and running mvn spotbugs:check seems to work:

      <plugin>
        <groupId>com.github.spotbugs</groupId>
        <artifactId>spotbugs-maven-plugin</artifactId>
        <version>4.8.4.0</version>
        <configuration>
          <threshold>high</threshold>
        </configuration>
        <dependencies>
          <dependency>
            <groupId>com.github.spotbugs</groupId>
            <artifactId>spotbugs</artifactId>
            <version>4.8.4</version>
          </dependency>
        </dependencies>
      </plugin>

Omitting the configuration block will fail with a bunch of medium-priority warnings (similar to the original issue description).