Fix issue where non-required file inputs can throw a NullPointerException

rather than a UserException when an the input argument is specified without
an argument value. 
The magnitude of code required to fix this points to a need to give the
command-line argument system a good spring cleaning.


git-svn-id: file:///humgen/gsa-scr1/gsa-engineering/svn_contents/trunk@4714 348d0f76-0448-11de-a6fe-93d51630548a

java/src/org/broadinstitute/sting/commandline/ArgumentTypeDescriptor.java

@@ -322,7 +322,7 @@ class SimpleArgumentTypeDescriptor extends ArgumentTypeDescriptor {
                     // if their argument has no value and there's no default, throw a missing argument value exception.
                     // TODO: Clean this up so that null values never make it to this point.  To fix this, we'll have to clean up the implementation of -U.
                 else if (value == null)
-                    throw new MissingArgumentValueException(Collections.singleton(createDefaultArgumentDefinition(source)));
+                    throw new MissingArgumentValueException(createDefaultArgumentDefinition(source));
                 else
                     throw new UnknownEnumeratedValueException(createDefaultArgumentDefinition(source),value);
             } else {

java/src/org/broadinstitute/sting/commandline/MissingArgumentValueException.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2010, The Broad Institute
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+package org.broadinstitute.sting.commandline;
+
+import org.broadinstitute.sting.utils.Utils;
+
+import java.util.Collection;
+
+/**
+ * Specifies that a value was missing when attempting to populate an argument.
+ */
+public class MissingArgumentValueException extends ArgumentException {
+    public MissingArgumentValueException( ArgumentDefinition... missingArguments ) {
+        super( formatArguments(missingArguments) );
+    }
+
+    private static String formatArguments( ArgumentDefinition... missingArguments ) {
+        StringBuilder sb = new StringBuilder();
+        for( ArgumentDefinition missingArgument: missingArguments ) {
+            if( missingArgument.shortName != null )
+                sb.append( String.format("%nValue for argument with name '--%s' (-%s) is missing.", missingArgument.fullName, missingArgument.shortName) );
+            else
+                sb.append( String.format("%nValue for argument with name '--%s' is missing.", missingArgument.fullName) );
+            if(missingArgument.validOptions != null)
+                sb.append( String.format("  Valid options are (%s).", Utils.join(",",missingArgument.validOptions)));
+        }
+        return sb.toString();
+    }
+}

java/src/org/broadinstitute/sting/commandline/ParsingEngine.java

@@ -490,25 +490,6 @@ class MissingArgumentException extends ArgumentException {
     }
 }
 
-class MissingArgumentValueException extends ArgumentException {
-    public MissingArgumentValueException( Collection<ArgumentDefinition> missingArguments ) {
-        super( formatArguments(missingArguments) );
-    }
-
-    private static String formatArguments( Collection<ArgumentDefinition> missingArguments ) {
-        StringBuilder sb = new StringBuilder();
-        for( ArgumentDefinition missingArgument: missingArguments ) {
-            if( missingArgument.shortName != null )
-                sb.append( String.format("%nValue for argument with name '--%s' (-%s) is missing.", missingArgument.fullName, missingArgument.shortName) );
-            else
-                sb.append( String.format("%nValue for argument with name '--%s' is missing.", missingArgument.fullName) );
-            if(missingArgument.validOptions != null)
-                sb.append( String.format("  Valid options are (%s).", Utils.join(",",missingArgument.validOptions)));
-        }
-        return sb.toString();
-    }
-}
-
 /**
  * An exception for undefined arguments.
  */

java/src/org/broadinstitute/sting/gatk/io/stubs/OutputStreamArgumentTypeDescriptor.java

@@ -28,6 +28,7 @@ package org.broadinstitute.sting.gatk.io.stubs;
 import org.broadinstitute.sting.commandline.*;
 import org.broadinstitute.sting.gatk.GenomeAnalysisEngine;
 import org.broadinstitute.sting.utils.exceptions.DynamicClassResolutionException;
+import org.broadinstitute.sting.utils.exceptions.ReviewedStingException;
 
 import java.io.OutputStream;
 import java.io.File;
@@ -64,11 +65,13 @@ public class OutputStreamArgumentTypeDescriptor extends ArgumentTypeDescriptor {
 
     @Override
     public boolean createsTypeDefault(ArgumentSource source) {
-        return true;
+        return source.isRequired();
     }
 
     @Override
     public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source) {
+        if(!source.isRequired())
+            throw new ReviewedStingException("BUG: tried to create type default for argument type descriptor that can't support a type default.");
         OutputStreamStub stub = new OutputStreamStub(defaultOutputStream);
         engine.addOutput(stub);
         return createInstanceOfClass(source.field.getType(),stub);        
@@ -79,6 +82,11 @@ public class OutputStreamArgumentTypeDescriptor extends ArgumentTypeDescriptor {
         ArgumentDefinition definition = createDefaultArgumentDefinition(source);
         String fileName = getArgumentValue( definition, matches );
 
+        // This parser has been passed a null filename and the GATK is not responsible for creating a type default for the object;
+        // therefore, the user must have failed to specify a type default
+        if(fileName == null && !source.isRequired())
+            throw new MissingArgumentValueException(definition);
+
         OutputStreamStub stub = new OutputStreamStub(new File(fileName));
 
         engine.addOutput(stub);

java/src/org/broadinstitute/sting/gatk/io/stubs/SAMFileWriterArgumentTypeDescriptor.java

@@ -29,6 +29,7 @@ import org.broadinstitute.sting.commandline.*;
 import org.broadinstitute.sting.gatk.GenomeAnalysisEngine;
 import org.broadinstitute.sting.gatk.io.StingSAMFileWriter;
 import net.sf.samtools.SAMFileWriter;
+import org.broadinstitute.sting.utils.exceptions.ReviewedStingException;
 import org.broadinstitute.sting.utils.exceptions.UserException;
 
 import java.lang.annotation.Annotation;
@@ -83,11 +84,13 @@ public class SAMFileWriterArgumentTypeDescriptor extends ArgumentTypeDescriptor
 
     @Override
     public boolean createsTypeDefault(ArgumentSource source) {
-        return true;
+        return source.isRequired();
     }
 
     @Override
     public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source) {
+        if(!source.isRequired())
+            throw new ReviewedStingException("BUG: tried to create type default for argument type descriptor that can't support a type default.");
         SAMFileWriterStub stub = new SAMFileWriterStub(engine,defaultOutputStream);
         engine.addOutput(stub);
         return stub;
@@ -95,9 +98,13 @@ public class SAMFileWriterArgumentTypeDescriptor extends ArgumentTypeDescriptor
 
     @Override
     public Object parse( ParsingEngine parsingEngine, ArgumentSource source, Class type, ArgumentMatches matches )  {
-        String writerFileName = getArgumentValue( createBAMArgumentDefinition(source), matches );
+        ArgumentDefinition bamArgumentDefinition = createBAMArgumentDefinition(source);
-        if( writerFileName == null )
+        String writerFileName = getArgumentValue( bamArgumentDefinition, matches );
-            throw new UserException.CommandLineException("SAM file compression was supplied, but no associated writer was supplied with it.");
+
+        // This parser has been passed a null filename and the GATK is not responsible for creating a type default for the object;
+        // therefore, the user must have failed to specify a type default
+        if(writerFileName == null && !source.isRequired())
+            throw new MissingArgumentValueException(bamArgumentDefinition);
 
         SAMFileWriterStub stub = new SAMFileWriterStub(engine, new File(writerFileName));
 

java/src/org/broadinstitute/sting/gatk/io/stubs/VCFWriterArgumentTypeDescriptor.java

@@ -104,11 +104,13 @@ public class VCFWriterArgumentTypeDescriptor extends ArgumentTypeDescriptor {
      */
     @Override
     public boolean createsTypeDefault(ArgumentSource source) {
-        return true;
+        return source.isRequired();
     }
 
     @Override
     public Object createTypeDefault(ParsingEngine parsingEngine,ArgumentSource source) {
+        if(!source.isRequired())
+            throw new ReviewedStingException("BUG: tried to create type default for argument type descriptor that can't support a type default.");        
         VCFWriterStub stub = new VCFWriterStub(engine, defaultOutputStream, false, argumentSources, false);
         engine.addOutput(stub);
         return stub;
@@ -123,10 +125,16 @@ public class VCFWriterArgumentTypeDescriptor extends ArgumentTypeDescriptor {
      */
     @Override
     public Object parse( ParsingEngine parsingEngine, ArgumentSource source, Class type, ArgumentMatches matches )  {
+        ArgumentDefinition defaultArgumentDefinition = createDefaultArgumentDefinition(source);
         // Get the filename for the genotype file, if it exists.  If not, we'll need to send output to out.
-        String writerFileName = getArgumentValue(createDefaultArgumentDefinition(source),matches);
+        String writerFileName = getArgumentValue(defaultArgumentDefinition,matches);
         File writerFile = writerFileName != null ? new File(writerFileName) : null;
 
+        // This parser has been passed a null filename and the GATK is not responsible for creating a type default for the object;
+        // therefore, the user must have failed to specify a type default
+        if(writerFile == null && !source.isRequired())
+            throw new MissingArgumentValueException(defaultArgumentDefinition);
+
         // Should we compress the output stream?
         boolean compress = writerFileName != null && SUPPORTED_ZIPPED_SUFFIXES.contains(getFileSuffix(writerFileName));