From 0390acc36c031473d6205263af2c49e330563307 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 28 Jan 2019 10:47:02 +0000 Subject: [PATCH 01/14] SpotBugs --- build.gradle | 13 +++++++++++++ gradle/spotbugs-exclude.xml | 23 +++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 gradle/spotbugs-exclude.xml diff --git a/build.gradle b/build.gradle index 7edc16bd0b..7562a0aff5 100644 --- a/build.gradle +++ b/build.gradle @@ -13,6 +13,7 @@ plugins { id 'com.palantir.git-version' version '0.11.0' id 'com.github.johnrengelman.shadow' version '4.0.4' id 'com.github.maiflai.scalatest' version '0.23' + id 'com.github.spotbugs' version '1.6.9' } repositories { @@ -159,6 +160,18 @@ task sourcesJar(type: Jar) { archiveClassifier.set('sources') } +spotbugs { + reportLevel = 'high' + excludeFilter = file('gradle/spotbugs-exclude.xml') +} + +tasks.withType(com.github.spotbugs.SpotBugsTask) { + reports { + xml.enabled = false + html.enabled = true + } +} + publishing { publications { htsjdk(MavenPublication) { diff --git a/gradle/spotbugs-exclude.xml b/gradle/spotbugs-exclude.xml new file mode 100644 index 0000000000..89d185d81a --- /dev/null +++ b/gradle/spotbugs-exclude.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file From fc3dec40158da3cf1cc9a90c22c821fc05c437a0 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 10:43:11 +0000 Subject: [PATCH 02/14] Ignore clone/Cloneable warning --- gradle/spotbugs-exclude.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gradle/spotbugs-exclude.xml b/gradle/spotbugs-exclude.xml index 89d185d81a..8d79765bf4 100644 --- a/gradle/spotbugs-exclude.xml +++ b/gradle/spotbugs-exclude.xml @@ -1,10 +1,18 @@ + + + + + + + + From f6309fcaecd1a358cf80a8c92b1f8e28ee6d651d Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 10:44:54 +0000 Subject: [PATCH 03/14] MergingSamRecordIterator and SortingCollection are not actually Serializable. --- src/main/java/htsjdk/samtools/MergingSamRecordIterator.java | 5 +---- src/main/java/htsjdk/samtools/util/SortingCollection.java | 5 +---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/main/java/htsjdk/samtools/MergingSamRecordIterator.java b/src/main/java/htsjdk/samtools/MergingSamRecordIterator.java index 45d002e3e5..ece5735f78 100644 --- a/src/main/java/htsjdk/samtools/MergingSamRecordIterator.java +++ b/src/main/java/htsjdk/samtools/MergingSamRecordIterator.java @@ -25,7 +25,6 @@ import htsjdk.samtools.util.CloseableIterator; -import java.io.Serializable; import java.util.Collection; import java.util.Map; import java.util.PriorityQueue; @@ -209,9 +208,7 @@ public SAMFileHeader getMergedHeader() { * sequence dictionary. I hate the fact that this extends SAMRecordCoordinateComparator, but it avoids * more copy & paste. */ - private class MergedSequenceDictionaryCoordinateOrderComparator extends SAMRecordCoordinateComparator implements Serializable { - private static final long serialVersionUID = 1L; - + private class MergedSequenceDictionaryCoordinateOrderComparator extends SAMRecordCoordinateComparator { @Override public int fileOrderCompare(final SAMRecord samRecord1, final SAMRecord samRecord2) { final int referenceIndex1 = getReferenceIndex(samRecord1); diff --git a/src/main/java/htsjdk/samtools/util/SortingCollection.java b/src/main/java/htsjdk/samtools/util/SortingCollection.java index cec8a74c93..dd47602d29 100644 --- a/src/main/java/htsjdk/samtools/util/SortingCollection.java +++ b/src/main/java/htsjdk/samtools/util/SortingCollection.java @@ -29,7 +29,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.Serializable; import java.lang.reflect.Array; import java.nio.file.Files; import java.nio.file.Path; @@ -645,9 +644,7 @@ class PeekFileRecordIterator extends PeekIterator { } } - class PeekFileRecordIteratorComparator implements Comparator, Serializable { - private static final long serialVersionUID = 1L; - + class PeekFileRecordIteratorComparator implements Comparator { @Override public int compare(final PeekFileRecordIterator lhs, final PeekFileRecordIterator rhs) { final int result = comparator.compare(lhs.peek(), rhs.peek()); From 452bdf56995ed7e5d97e0b38fff8b7fdb3abc687 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 10:47:48 +0000 Subject: [PATCH 04/14] Remove unnecessary check (it's known that i3 > -1 at this point) --- src/main/java/htsjdk/samtools/cram/compression/rans/E14.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/htsjdk/samtools/cram/compression/rans/E14.java b/src/main/java/htsjdk/samtools/cram/compression/rans/E14.java index c483834579..bed11a83ad 100644 --- a/src/main/java/htsjdk/samtools/cram/compression/rans/E14.java +++ b/src/main/java/htsjdk/samtools/cram/compression/rans/E14.java @@ -43,7 +43,7 @@ static int compress(final ByteBuffer in, final RansEncSymbol[][] syms, // Deal with the remainder l3 = 0xFF & in.get(in_size - 1); for (i3 = in_size - 2; i3 > 4 * isz4 - 2 && i3 >= 0; i3--) { - final int c3 = 0xFF & in.get(i3 > -1 ? i3 : 0); + final int c3 = 0xFF & in.get(i3); rans3 = Encoding.RansEncPutSymbol(rans3, ptr, syms[c3][l3]); l3 = c3; } From 212414ee2c570dbf697757378934d018e8ba852a Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 10:58:26 +0000 Subject: [PATCH 05/14] Deprecate IOUtils#VCF_EXTENSIONS since it is a public mutable array. --- gradle/spotbugs-exclude.xml | 6 ++++++ src/main/java/htsjdk/samtools/util/IOUtil.java | 9 ++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/gradle/spotbugs-exclude.xml b/gradle/spotbugs-exclude.xml index 8d79765bf4..8bbf9234a4 100644 --- a/gradle/spotbugs-exclude.xml +++ b/gradle/spotbugs-exclude.xml @@ -13,6 +13,12 @@ + + + + + + diff --git a/src/main/java/htsjdk/samtools/util/IOUtil.java b/src/main/java/htsjdk/samtools/util/IOUtil.java index b863000b32..a908ab2e29 100755 --- a/src/main/java/htsjdk/samtools/util/IOUtil.java +++ b/src/main/java/htsjdk/samtools/util/IOUtil.java @@ -70,7 +70,14 @@ public class IOUtil { public static final String COMPRESSED_VCF_INDEX_EXTENSION = ".tbi"; /** Possible extensions for VCF files and related formats. */ - public static final String[] VCF_EXTENSIONS = {VCF_FILE_EXTENSION, COMPRESSED_VCF_FILE_EXTENSION, BCF_FILE_EXTENSION}; + public static final List VCF_EXTENSIONS_LIST = Collections.unmodifiableList(Arrays.asList(VCF_FILE_EXTENSION, COMPRESSED_VCF_FILE_EXTENSION, BCF_FILE_EXTENSION)); + + /** + * Possible extensions for VCF files and related formats. + * @deprecated Use {@link #VCF_EXTENSIONS_LIST} instead. + */ + @Deprecated + public static final String[] VCF_EXTENSIONS = VCF_EXTENSIONS_LIST.toArray(new String[0]); public static final String INTERVAL_LIST_FILE_EXTENSION = IntervalList.INTERVAL_LIST_FILE_EXTENSION; From a8656bdf66fd2e103033c7df18e135030be7c884 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 11:38:36 +0000 Subject: [PATCH 06/14] Ignore gc warning --- gradle/spotbugs-exclude.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/gradle/spotbugs-exclude.xml b/gradle/spotbugs-exclude.xml index 8bbf9234a4..b116e915b3 100644 --- a/gradle/spotbugs-exclude.xml +++ b/gradle/spotbugs-exclude.xml @@ -19,6 +19,16 @@ + + + + + + + + + + From 3ab170b5d983ce874f9cba68e0f17c63237a6816 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 11:45:25 +0000 Subject: [PATCH 07/14] Ignore Thread.start() warning in constructor of AbstractAsyncWriter --- gradle/spotbugs-exclude.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gradle/spotbugs-exclude.xml b/gradle/spotbugs-exclude.xml index b116e915b3..d4c4035be2 100644 --- a/gradle/spotbugs-exclude.xml +++ b/gradle/spotbugs-exclude.xml @@ -29,6 +29,12 @@ + + + + + + From c672089c6dade24fabf092fcc72fe6bd503a5b5f Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 11:53:46 +0000 Subject: [PATCH 08/14] Ignore redundant null check in htsjdk.variant.vcf.AbstractVCFCodec --- gradle/spotbugs-exclude.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gradle/spotbugs-exclude.xml b/gradle/spotbugs-exclude.xml index d4c4035be2..225c7d1e80 100644 --- a/gradle/spotbugs-exclude.xml +++ b/gradle/spotbugs-exclude.xml @@ -35,6 +35,13 @@ + + + + + + + From 6482343c161ec1862206eee554a6b53d94053605 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 11:59:41 +0000 Subject: [PATCH 09/14] Clarify that inherited (not outer) method hasNext() is being called by TupleIterator --- .../java/htsjdk/tribble/readers/AsciiLineReaderIterator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/htsjdk/tribble/readers/AsciiLineReaderIterator.java b/src/main/java/htsjdk/tribble/readers/AsciiLineReaderIterator.java index 5db3b91a90..a36797e21f 100644 --- a/src/main/java/htsjdk/tribble/readers/AsciiLineReaderIterator.java +++ b/src/main/java/htsjdk/tribble/readers/AsciiLineReaderIterator.java @@ -75,7 +75,7 @@ public String peek() { private class TupleIterator extends AbstractIterator> implements LocationAware { public TupleIterator() { - hasNext(); // Initialize the iterator, which appears to be a requirement of the parent class. TODO: Really? + super.hasNext(); // Initialize the iterator, which appears to be a requirement of the parent class. TODO: Really? } @Override From a5abccd02987d144648c4c72a41ae1f06e49c740 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 12:03:55 +0000 Subject: [PATCH 10/14] Ignore warning in HTTPHelper about writing to a static field from an instance. --- gradle/spotbugs-exclude.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gradle/spotbugs-exclude.xml b/gradle/spotbugs-exclude.xml index 225c7d1e80..5c6dcb6613 100644 --- a/gradle/spotbugs-exclude.xml +++ b/gradle/spotbugs-exclude.xml @@ -35,6 +35,12 @@ + + + + + + From 09cd583ab84041dfa3210b8693ef77f59133e44e Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 12:25:52 +0000 Subject: [PATCH 11/14] Ignore test for floating point equality in Histogram and LinearIndex --- gradle/spotbugs-exclude.xml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/gradle/spotbugs-exclude.xml b/gradle/spotbugs-exclude.xml index 5c6dcb6613..8b46d5728c 100644 --- a/gradle/spotbugs-exclude.xml +++ b/gradle/spotbugs-exclude.xml @@ -48,6 +48,15 @@ + + + + + + + + + From 2e7f566053d701b7d049d15b92720bf87add934f Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 12:37:53 +0000 Subject: [PATCH 12/14] Fix NPE --- .../java/htsjdk/samtools/TextualBAMIndexWriter.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/main/java/htsjdk/samtools/TextualBAMIndexWriter.java b/src/main/java/htsjdk/samtools/TextualBAMIndexWriter.java index da418fd2d5..836d0e0f5b 100644 --- a/src/main/java/htsjdk/samtools/TextualBAMIndexWriter.java +++ b/src/main/java/htsjdk/samtools/TextualBAMIndexWriter.java @@ -71,14 +71,14 @@ private void writeHeader() { @Override public void writeReference(final BAMIndexContent content) { - final int reference = content.getReferenceSequence(); - if (content == null) { - writeNullContent(reference); + writeNullContent(); count++; return; } + final int reference = content.getReferenceSequence(); + if (reference != count){ throw new SAMException("Reference on content is " + reference + " but expecting reference " + count); } @@ -162,6 +162,11 @@ private void writeChunkMetaData(final int reference, final BAMIndexMetaData meta } } + + private void writeNullContent() { + pw.println("Reference has n_bin=0"); + pw.println("Reference has n_intv=0"); + } private void writeNullContent(final int reference) { pw.println("Reference " + reference + " has n_bin=0"); From a2ef2073a4d9ae6cea1b50a7b1dddf22b4cc6ff1 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 11 Mar 2019 11:08:01 +0000 Subject: [PATCH 13/14] Preserve existing behaviour of TextualBAMIndexWriter --- src/main/java/htsjdk/samtools/TextualBAMIndexWriter.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/htsjdk/samtools/TextualBAMIndexWriter.java b/src/main/java/htsjdk/samtools/TextualBAMIndexWriter.java index 836d0e0f5b..ffc424ec53 100644 --- a/src/main/java/htsjdk/samtools/TextualBAMIndexWriter.java +++ b/src/main/java/htsjdk/samtools/TextualBAMIndexWriter.java @@ -72,9 +72,7 @@ private void writeHeader() { public void writeReference(final BAMIndexContent content) { if (content == null) { - writeNullContent(); - count++; - return; + throw new NullPointerException("BAMIndexContent cannot be null"); } final int reference = content.getReferenceSequence(); From 07510fcd743386ca6fc23905b2411f8d1f46171e Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Mon, 18 Mar 2019 16:30:25 -0400 Subject: [PATCH 14/14] run spotBugs on travis --- .travis.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 14dc650c21..8ca84626fa 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,9 +22,13 @@ matrix: env: TEST_TYPE=EXTERNAL_APIS - jdk: oraclejdk8 env: TEST_TYPE=FTP + - jdk: openjdk8 + env: SPOT_BUGS=true script: - - if [[ $TEST_TYPE == "FTP" ]]; then + - if [[ $SPOT_BUGS == "true" ]]; then + ./gradlew spotBugsMain spotBugsTest; + elif [[ $TEST_TYPE == "FTP" ]]; then ./gradlew testFTP jacocoTestReport; elif [[ $TEST_TYPE == "EXTERNAL_APIS" ]]; then ./gradlew testExternalApis jacocoTestReport;