Merge branch 'master' of ssh://gsa1/humgen/gsa-scr1/gsa-engineering/git/unstable

build.xml

@@ -520,7 +520,7 @@
             <fileset dir="${java.classes}">
               <include name="**/utils/codecs/**/*.class"/>
               <include name="**/utils/variantcontext/**/*.class"/>
-              <include name="**/UserException*.class"/>
+              <include name="org/broadinstitute/sting/utils/exceptions/**"/>
 	      <include name="org/broadinstitute/sting/utils/help/DocumentedGATKFeature.class"/>
             </fileset>
         </jar>

public/java/src/org/broadinstitute/sting/commandline/ArgumentDefinitions.java

@@ -174,7 +174,8 @@ public class ArgumentDefinitions implements Iterable<ArgumentDefinition> {
 
     static DefinitionMatcher VerifiableDefinitionMatcher = new DefinitionMatcher() {
         public boolean matches( ArgumentDefinition definition, Object key ) {
-            return definition.validation != null;
+            // We can perform some sort of validation for anything that isn't a flag.
+            return !definition.isFlag;
         }        
     };
 }

public/java/src/org/broadinstitute/sting/commandline/ArgumentMatch.java

@@ -44,7 +44,7 @@ public class ArgumentMatch implements Iterable<ArgumentMatch> {
     public final String label;
 
     /**
-     * Maps indicies of command line arguments to values paired with that argument.
+     * Maps indices of command line arguments to values paired with that argument.
      */
     public final SortedMap<Integer,List<String>> indices = new TreeMap<Integer,List<String>>();
 

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

@@ -41,6 +41,11 @@ import java.util.*;
  * A parser for Sting command-line arguments.
  */
 public class ParsingEngine {
+    /**
+     * The loaded argument sources along with their back definitions.
+     */
+    private Map<ArgumentDefinition,ArgumentSource> argumentSourcesByDefinition = new HashMap<ArgumentDefinition,ArgumentSource>();
+
     /**
      * A list of defined arguments against which command lines are matched.
      * Package protected for testing access.
@@ -107,8 +112,13 @@ public class ParsingEngine {
      */
     public void addArgumentSource( String sourceName, Class sourceClass ) {
         List<ArgumentDefinition> argumentsFromSource = new ArrayList<ArgumentDefinition>();
-        for( ArgumentSource argumentSource: extractArgumentSources(sourceClass) )
+        for( ArgumentSource argumentSource: extractArgumentSources(sourceClass) ) {
-            argumentsFromSource.addAll( argumentSource.createArgumentDefinitions() );
+            List<ArgumentDefinition> argumentDefinitions = argumentSource.createArgumentDefinitions();
+            for(ArgumentDefinition argumentDefinition: argumentDefinitions) {
+                argumentSourcesByDefinition.put(argumentDefinition,argumentSource);
+                argumentsFromSource.add( argumentDefinition );
+            }
+        }
         argumentDefinitions.add( new ArgumentDefinitionGroup(sourceName, argumentsFromSource) );
     }
 
@@ -199,16 +209,25 @@ public class ParsingEngine {
                 throw new InvalidArgumentException( invalidArguments );
         }
 
-        // Find invalid argument values (arguments that fail the regexp test.
+        // Find invalid argument values -- invalid arguments are either completely missing or fail the specified 'validation' regular expression.
         if( !skipValidationOf.contains(ValidationType.InvalidArgumentValue) ) {
             Collection<ArgumentDefinition> verifiableArguments = 
                     argumentDefinitions.findArgumentDefinitions( null, ArgumentDefinitions.VerifiableDefinitionMatcher );
             Collection<Pair<ArgumentDefinition,String>> invalidValues = new ArrayList<Pair<ArgumentDefinition,String>>();
             for( ArgumentDefinition verifiableArgument: verifiableArguments ) {
                 ArgumentMatches verifiableMatches = argumentMatches.findMatches( verifiableArgument );
+                // Check to see whether an argument value was specified.  Argument values must be provided
+                // when the argument name is specified and the argument is not a flag type.
+                for(ArgumentMatch verifiableMatch: verifiableMatches) {
+                    ArgumentSource argumentSource = argumentSourcesByDefinition.get(verifiableArgument);
+                    if(verifiableMatch.values().size() == 0 && !verifiableArgument.isFlag && argumentSource.createsTypeDefault())
+                        invalidValues.add(new Pair<ArgumentDefinition,String>(verifiableArgument,null));
+                }
+
+                // Ensure that the field contents meet the validation criteria specified by the regular expression.
                 for( ArgumentMatch verifiableMatch: verifiableMatches ) {
                     for( String value: verifiableMatch.values() ) {
-                        if( !value.matches(verifiableArgument.validation) )
+                        if( verifiableArgument.validation != null && !value.matches(verifiableArgument.validation) )
                             invalidValues.add( new Pair<ArgumentDefinition,String>(verifiableArgument, value) );
                     }
                 }
@@ -515,10 +534,14 @@ class InvalidArgumentValueException extends ArgumentException {
     private static String formatArguments( Collection<Pair<ArgumentDefinition,String>> invalidArgumentValues ) {
         StringBuilder sb = new StringBuilder();
         for( Pair<ArgumentDefinition,String> invalidValue: invalidArgumentValues ) {
-            sb.append( String.format("%nArgument '--%s' has value of incorrect format: %s (should match %s)",
+            if(invalidValue.getSecond() == null)
-                                     invalidValue.first.fullName,
+                sb.append( String.format("%nArgument '--%s' requires a value but none was provided",
-                                     invalidValue.second,
+                                         invalidValue.first.fullName) );
-                                     invalidValue.first.validation) );
+            else
+                sb.append( String.format("%nArgument '--%s' has value of incorrect format: %s (should match %s)",
+                        invalidValue.first.fullName,
+                        invalidValue.second,
+                        invalidValue.first.validation) );
         }
         return sb.toString();
     }

public/java/src/org/broadinstitute/sting/gatk/datasources/reads/SAMDataSource.java

@@ -893,6 +893,7 @@ public class SAMDataSource {
          * Custom representation of interval bounds.
          * Makes it simpler to track current position. 
          */
+        private int[] intervalContigIndices;
         private int[] intervalStarts;
         private int[] intervalEnds;
 
@@ -917,12 +918,14 @@ public class SAMDataSource {
             if(foundMappedIntervals) {
                 if(keepOnlyUnmappedReads)
                     throw new ReviewedStingException("Tried to apply IntervalOverlapFilteringIterator to a mixed of mapped and unmapped intervals.  Please apply this filter to only mapped or only unmapped reads");
+                this.intervalContigIndices = new int[intervals.size()];
                 this.intervalStarts = new int[intervals.size()];
                 this.intervalEnds = new int[intervals.size()];
                 int i = 0;
                 for(GenomeLoc interval: intervals) {
-                    intervalStarts[i] = (int)interval.getStart();
+                    intervalContigIndices[i] = interval.getContigIndex();
-                    intervalEnds[i] = (int)interval.getStop();
+                    intervalStarts[i] = interval.getStart();
+                    intervalEnds[i] = interval.getStop();
                     i++;
                 }
             }
@@ -961,11 +964,10 @@ public class SAMDataSource {
             while(nextRead == null && (keepOnlyUnmappedReads || currentBound < intervalStarts.length)) {
                 if(!keepOnlyUnmappedReads) {
                     // Mapped read filter; check against GenomeLoc-derived bounds.
-                    if(candidateRead.getAlignmentEnd() >= intervalStarts[currentBound] ||
+                    if(readEndsOnOrAfterStartingBound(candidateRead)) {
-                            (candidateRead.getReadUnmappedFlag() && candidateRead.getAlignmentStart() >= intervalStarts[currentBound])) {
+                        // This read ends after the current interval begins.
-                        // This read ends after the current interval begins (or, if unmapped, starts within the bounds of the interval.
                         // Promising, but this read must be checked against the ending bound.
-                        if(candidateRead.getAlignmentStart() <= intervalEnds[currentBound]) {
+                        if(readStartsOnOrBeforeEndingBound(candidateRead)) {
                             // Yes, this read is within both bounds.  This must be our next read.
                             nextRead = candidateRead;
                             break;
@@ -993,6 +995,37 @@ public class SAMDataSource {
                 candidateRead = iterator.next();
             }
         }
+
+        /**
+         * Check whether the read lies after the start of the current bound.  If the read is unmapped but placed, its
+         * end will be distorted, so rely only on the alignment start.
+         * @param read The read to position-check.
+         * @return True if the read starts after the current bounds.  False otherwise.
+         */
+        private boolean readEndsOnOrAfterStartingBound(final SAMRecord read) {
+            return
+                    // Read ends on a later contig, or...
+                    read.getReferenceIndex() > intervalContigIndices[currentBound] ||
+                    // Read ends of this contig...
+                    (read.getReferenceIndex() == intervalContigIndices[currentBound] &&
+                            // either after this location, or...
+                            (read.getAlignmentEnd() >= intervalStarts[currentBound] ||
+                            // read is unmapped but positioned and alignment start is on or after this start point.
+                            (read.getReadUnmappedFlag() && read.getAlignmentStart() >= intervalStarts[currentBound])));
+        }
+
+        /**
+         * Check whether the read lies before the end of the current bound.
+         * @param read The read to position-check.
+         * @return True if the read starts after the current bounds.  False otherwise.
+         */
+        private boolean readStartsOnOrBeforeEndingBound(final SAMRecord read) {
+            return
+                    // Read starts on a prior contig, or...
+                    read.getReferenceIndex() < intervalContigIndices[currentBound] ||
+                    // Read starts on this contig and the alignment start is registered before this end point.
+                   (read.getReferenceIndex() == intervalContigIndices[currentBound] && read.getAlignmentStart() <= intervalEnds[currentBound]);
+        }
     }
 
     /**