From 1406487290947e6fe0b516813b73033dddc4a8c5 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 28 Jan 2019 10:47:02 +0000 Subject: [PATCH 01/13] 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 9834e8e36e..4327b1393c 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 + } +} + /** * Sign non-snapshot releases with our secret key. This should never need to be invoked directly. */ 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 7d9b10687ef566ecae52df17e3008d73e6d54e66 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 10:43:11 +0000 Subject: [PATCH 02/13] 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 e303f25b3defc34080acb205de26d22d461e4f9e Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 10:44:54 +0000 Subject: [PATCH 03/13] 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 5505225b7af12e63b95071371c7286fbf6de7b36 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 10:47:48 +0000 Subject: [PATCH 04/13] 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 a4d0694dbc075744eb8137b9bbeba5f8b79916b3 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 10:58:26 +0000 Subject: [PATCH 05/13] 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 30cedc34a9..1a88b65992 100755 --- a/src/main/java/htsjdk/samtools/util/IOUtil.java +++ b/src/main/java/htsjdk/samtools/util/IOUtil.java @@ -94,7 +94,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 1645881f14ac326466d327813e902ff24cb15cd0 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 11:38:36 +0000 Subject: [PATCH 06/13] 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 c790d0554e19b7e542f2c1bec5a863e2989bd66d Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 11:45:25 +0000 Subject: [PATCH 07/13] 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 766943f1b1a73702acd789859190fc1ba80c9508 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 11:53:46 +0000 Subject: [PATCH 08/13] 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 297081bfe869fd238a36bfad280d78d3a5e12800 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 11:59:41 +0000 Subject: [PATCH 09/13] 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 eb82996bdd2d92f518258bafcf11ff2067032b47 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 12:03:55 +0000 Subject: [PATCH 10/13] 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 22c5babbd086a4d5a84146eb2f4dd9c2a313291c Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 12:25:52 +0000 Subject: [PATCH 11/13] 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 158f05e10c3059aa334fb1b5828a2a2a372a5201 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 25 Feb 2019 12:37:53 +0000 Subject: [PATCH 12/13] 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 b66ee44a72388c0bf29c63aa737874bca75785e7 Mon Sep 17 00:00:00 2001 From: Tom White Date: Mon, 11 Mar 2019 11:08:01 +0000 Subject: [PATCH 13/13] 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();