Repository: broadinstitute/picard
Issue: CollectRnaSeqMetrics Null Pointer Exception when RIBOSOMAL_INTERVALS are not given 

If the`RIBOSOMAL_INTERVALS` option is not given and `RRNA_FRAGMENT_PERCENTAGE=0`, then [this if statement is always true](https://github.com/broadinstitute/picard/blob/master/src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java#L168).  This leads to the following NPE:

```
Exception in thread "main" java.lang.NullPointerException
	at picard.analysis.directed.RnaSeqMetricsCollector$PerUnitRnaSeqMetricsCollector.acceptRecord(RnaSeqMetricsCollector.java:171)
	at picard.analysis.directed.RnaSeqMetricsCollector$PerUnitRnaSeqMetricsCollector.acceptRecord(RnaSeqMetricsCollector.java:98)
	at picard.metrics.MultiLevelCollector$AllReadsDistributor.acceptRecord(MultiLevelCollector.java:192)
	at picard.metrics.MultiLevelCollector.acceptRecord(MultiLevelCollector.java:315)
	at picard.analysis.CollectRnaSeqMetrics.acceptRead(CollectRnaSeqMetrics.java:164)
	at picard.analysis.SinglePassSamProgram.makeItSo(SinglePassSamProgram.java:138)
	at picard.analysis.SinglePassSamProgram.doWork(SinglePassSamProgram.java:77)
	at picard.cmdline.CommandLineProgram.instanceMain(CommandLineProgram.java:205)
	at picard.cmdline.PicardCommandLine.instanceMain(PicardCommandLine.java:94)
	at picard.cmdline.PicardCommandLine.main(PicardCommandLine.java:104)
```

A simple fix would be to not allow `RRNA_FRAGMENT_PERCENTAGE=0` if `RIBOSOMAL_INTERVALS=null`.

Changed files:
src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java, src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java, src/main/java/picard/analysis/CollectRnaSeqMetrics.java

diff --git a/src/main/java/picard/analysis/CollectRnaSeqMetrics.java b/src/main/java/picard/analysis/CollectRnaSeqMetrics.java
index dfeb67111..aed80f0bf 100644
--- a/src/main/java/picard/analysis/CollectRnaSeqMetrics.java
+++ b/src/main/java/picard/analysis/CollectRnaSeqMetrics.java
@@ -134,6 +134,15 @@ static final String USAGE_DETAILS = "<p>This tool takes a SAM/BAM file containin
         new CollectRnaSeqMetrics().instanceMainWithExit(argv);
     }
 
+    @Override
+    protected String[] customCommandLineValidation() {
+        // No ribosomal intervals file and rRNA fragment percentage = 0
+        if ( RIBOSOMAL_INTERVALS == null && RRNA_FRAGMENT_PERCENTAGE == 0 ) {
+            throw new PicardException("Must use a RIBOSOMAL_INTERVALS file if RRNA_FRAGMENT_PERCENTAGE = 0.0");
+        }
+        return super.customCommandLineValidation();
+    }
+
     @Override
     protected void setup(final SAMFileHeader header, final File samFile) {
 
@@ -143,7 +152,7 @@ static final String USAGE_DETAILS = "<p>This tool takes a SAM/BAM file containin
         LOG.info("Loaded " + geneOverlapDetector.getAll().size() + " genes.");
 
         final Long ribosomalBasesInitialValue = RIBOSOMAL_INTERVALS != null ? 0L : null;
-        final OverlapDetector<Interval> ribosomalSequenceOverlapDetector = RnaSeqMetricsCollector.makeOverlapDetector(samFile, header, RIBOSOMAL_INTERVALS);
+        final OverlapDetector<Interval> ribosomalSequenceOverlapDetector = RnaSeqMetricsCollector.makeOverlapDetector(samFile, header, RIBOSOMAL_INTERVALS, LOG);
 
         final HashSet<Integer> ignoredSequenceIndices = RnaSeqMetricsCollector.makeIgnoredSequenceIndicesSet(header, IGNORE_SEQUENCE);
 
diff --git a/src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java b/src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java
index 8bfb3300a..36083c5c5 100644
--- a/src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java
+++ b/src/main/java/picard/analysis/directed/RnaSeqMetricsCollector.java
@@ -6,6 +6,7 @@ import htsjdk.samtools.util.CoordMath;
 import htsjdk.samtools.util.Histogram;
 import htsjdk.samtools.util.Interval;
 import htsjdk.samtools.util.IntervalList;
+import htsjdk.samtools.util.Log;
 import htsjdk.samtools.util.OverlapDetector;
 import htsjdk.samtools.util.SequenceUtil;
 import picard.PicardException;
@@ -60,12 +61,15 @@ public class RnaSeqMetricsCollector extends SAMRecordMultiLevelCollector<RnaSeqM
         return new PerUnitRnaSeqMetricsCollector(sample, library, readGroup, ribosomalInitialValue);
     }
 
-    public static OverlapDetector<Interval> makeOverlapDetector(final File samFile, final SAMFileHeader header, final File ribosomalIntervalsFile) {
+    public static OverlapDetector<Interval> makeOverlapDetector(final File samFile, final SAMFileHeader header, final File ribosomalIntervalsFile, final Log log) {
 
-        OverlapDetector<Interval> ribosomalSequenceOverlapDetector = new OverlapDetector<Interval>(0, 0);
+        final OverlapDetector<Interval> ribosomalSequenceOverlapDetector = new OverlapDetector<Interval>(0, 0);
         if (ribosomalIntervalsFile != null) {
 
             final IntervalList ribosomalIntervals = IntervalList.fromFile(ribosomalIntervalsFile);
+            if (ribosomalIntervals.size() == 0) {
+                log.warn("The RIBOSOMAL_INTERVALS file, " + ribosomalIntervalsFile.getAbsolutePath() + " does not contain intervals");
+            }
             try {
                 SequenceUtil.assertSequenceDictionariesEqual(header.getSequenceDictionary(), ribosomalIntervals.getHeader().getSequenceDictionary());
             } catch (SequenceUtil.SequenceListsDifferException e) {
diff --git a/src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java b/src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java
index fd25a8503..84da166c8 100644
--- a/src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java
+++ b/src/test/java/picard/analysis/CollectRnaSeqMetricsTest.java
@@ -29,12 +29,15 @@ import htsjdk.samtools.util.Interval;
 import htsjdk.samtools.util.IntervalList;
 import htsjdk.samtools.util.StringUtil;
 import org.testng.Assert;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
+import picard.PicardException;
 import picard.cmdline.CommandLineProgramTest;
 import picard.annotation.RefFlatReader.RefFlatColumns;
 
 import java.io.File;
 import java.io.FileReader;
+import java.io.IOException;
 import java.io.PrintStream;
 
 public class CollectRnaSeqMetricsTest extends CommandLineProgramTest {
@@ -112,6 +115,65 @@ public class CollectRnaSeqMetricsTest extends CommandLineProgramTest {
         Assert.assertEquals(metrics.PCT_R2_TRANSCRIPT_STRAND_READS, 0.666667);
     }
 
+    @DataProvider(name = "rRnaIntervalsFiles")
+    public static Object[][] rRnaIntervalsFiles() throws IOException {
+        return new Object[][] {
+                {null},
+                {File.createTempFile("tmp.rRna.", ".interval_list")}
+        };
+    }
+
+    @Test(dataProvider = "rRnaIntervalsFiles")
+    public void testNoIntevalsNoFragPercentage(final File rRnaIntervalsFile) throws Exception {
+        final SAMRecordSetBuilder builder = new SAMRecordSetBuilder(true, SAMFileHeader.SortOrder.coordinate);
+
+        // Add a header but no intervals
+        if ( rRnaIntervalsFile != null  ) {
+            final IntervalList rRnaIntervalList = new IntervalList(builder.getHeader());
+            rRnaIntervalList.write(rRnaIntervalsFile);
+            rRnaIntervalsFile.deleteOnExit();
+        }
+
+        // Create some alignments
+        final String sequence = "chr1";
+        final String ignoredSequence = "chrM";
+
+        // Set seed so that strandedness is consistent among runs.
+        builder.setRandomSeed(0);
+        final int sequenceIndex = builder.getHeader().getSequenceIndex(sequence);
+        builder.addPair("pair1", sequenceIndex, 45, 475);
+        builder.addFrag("ignoredFrag", builder.getHeader().getSequenceIndex(ignoredSequence), 1, false);
+
+        final File samFile = File.createTempFile("tmp.collectRnaSeqMetrics.", ".sam");
+        samFile.deleteOnExit();
+
+        final SAMFileWriter samWriter = new SAMFileWriterFactory().makeSAMWriter(builder.getHeader(), false, samFile);
+        for (final SAMRecord rec : builder.getRecords()) samWriter.addAlignment(rec);
+        samWriter.close();
+
+        // Generate the metrics.
+        final File metricsFile = File.createTempFile("tmp.", ".rna_metrics");
+        metricsFile.deleteOnExit();
+
+        final String rRnaIntervalsPath = rRnaIntervalsFile != null ? rRnaIntervalsFile.getAbsolutePath() : null;
+        final String[] args = new String[]{
+                "INPUT=" + samFile.getAbsolutePath(),
+                "OUTPUT=" + metricsFile.getAbsolutePath(),
+                "REF_FLAT=" + getRefFlatFile(sequence).getAbsolutePath(),
+                "RIBOSOMAL_INTERVALS=" + rRnaIntervalsPath,
+                "RRNA_FRAGMENT_PERCENTAGE=" + 0.0,
+                "STRAND_SPECIFICITY=SECOND_READ_TRANSCRIPTION_STRAND",
+                "IGNORE_SEQUENCE=" + ignoredSequence
+        };
+        try {
+            Assert.assertEquals(runPicardCommandLine(args), 0);
+        } catch(final Exception e) {
+            if (rRnaIntervalsFile != null) {
+                Assert.assertEquals(e.getCause().getClass(), PicardException.class);
+            }
+        }
+    }
+
     @Test
     public void testMultiLevel() throws Exception {
         final String sequence = "chr1";