diff --git a/java/src/org/broadinstitute/sting/utils/cmdLine/Argument.java b/java/src/org/broadinstitute/sting/utils/cmdLine/Argument.java index 5e951167f..454fc8d56 100755 --- a/java/src/org/broadinstitute/sting/utils/cmdLine/Argument.java +++ b/java/src/org/broadinstitute/sting/utils/cmdLine/Argument.java @@ -58,5 +58,5 @@ public @interface Argument { * @return A comma-separated string listing other arguments of which this * argument should be independent. */ - String exclusive() default ""; + String exclusiveOf() default ""; } diff --git a/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java b/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java index bd36dc604..545c1caa4 100755 --- a/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java +++ b/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java @@ -237,7 +237,7 @@ class ArgumentDefinition { /** * Is this argument exclusive of other arguments? */ - public final String exclusive; + public final String exclusiveOf; public final Class sourceClass; public final Field sourceField; @@ -256,7 +256,7 @@ class ArgumentDefinition { shortName = argument.shortName().trim().length() > 0 ? argument.shortName().trim() : null; doc = argument.doc(); required = argument.required() && !isFlag(); - exclusive = argument.exclusive().trim().length() > 0 ? argument.exclusive().trim() : null; + exclusiveOf = argument.exclusiveOf().trim().length() > 0 ? argument.exclusiveOf().trim() : null; } /** diff --git a/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentMatches.java b/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentMatches.java index d45d1e64d..c3c3892b0 100755 --- a/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentMatches.java +++ b/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentMatches.java @@ -120,6 +120,17 @@ public class ArgumentMatches implements Iterable { return matches; } + /** + * Find all successful matches (a 'successful' match is one paired with a definition). + */ + public Collection findSuccessfulMatches() { + Collection matches = new HashSet(); + for( ArgumentMatch argumentMatch: getUniqueMatches() ) { + if( argumentMatch.definition != null ) + matches.add( argumentMatch ); + } + return matches; + } } /** diff --git a/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java b/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java index 36c82b53d..09c511f1f 100755 --- a/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java +++ b/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java @@ -1,6 +1,7 @@ package org.broadinstitute.sting.utils.cmdLine; import org.broadinstitute.sting.utils.StingException; +import org.broadinstitute.sting.utils.Pair; import org.apache.log4j.Logger; import java.lang.reflect.Field; @@ -112,7 +113,8 @@ public class ParsingEngine { public enum ValidationType { MissingRequiredArgument, InvalidArgument, ValueMissingArgument, - TooManyValuesForArgument }; + TooManyValuesForArgument, + MutuallyExclusive }; /** * Validates the list of command-line argument matches. @@ -157,17 +159,35 @@ public class ParsingEngine { // Find arguments with too many values. if( !skipValidationOf.contains(ValidationType.TooManyValuesForArgument)) { Collection overvaluedArguments = new ArrayList(); - for( ArgumentMatch argumentMatch: argumentMatches ) { + for( ArgumentMatch argumentMatch: argumentMatches.findSuccessfulMatches() ) { // Warning: assumes that definition is not null (asserted by checks above). - if( argumentMatch.definition != null && - !argumentMatch.definition.isMultiValued() && - argumentMatch.values().size() > 1 ) + if( !argumentMatch.definition.isMultiValued() && argumentMatch.values().size() > 1 ) overvaluedArguments.add(argumentMatch); } if( !overvaluedArguments.isEmpty() ) throw new TooManyValuesForArgumentException(overvaluedArguments); } + + // Find sets of options that are supposed to be mutually exclusive. + if( !skipValidationOf.contains(ValidationType.MutuallyExclusive)) { + Collection> invalidPairs = new ArrayList>(); + for( ArgumentMatch argumentMatch: argumentMatches.findSuccessfulMatches() ) { + if( argumentMatch.definition.exclusiveOf != null ) { + for( ArgumentMatch conflictingMatch: argumentMatches.findSuccessfulMatches() ) { + // Skip over the current element. + if( argumentMatch == conflictingMatch ) + continue; + if( argumentMatch.definition.exclusiveOf.equals(conflictingMatch.definition.fullName) || + argumentMatch.definition.exclusiveOf.equals(conflictingMatch.definition.shortName)) + invalidPairs.add( new Pair(argumentMatch, conflictingMatch) ); + } + } + } + + if( !invalidPairs.isEmpty() ) + throw new ArgumentsAreMutuallyExclusiveException( invalidPairs ); + } } /** @@ -537,7 +557,24 @@ class TooManyValuesForArgumentException extends ParseException { private static String formatArguments( Collection arguments ) { StringBuilder sb = new StringBuilder(); for( ArgumentMatch argument: arguments ) - sb.append( String.format("Argument with name '%s' has to many values: %s", argument.label, Arrays.deepToString(argument.values().toArray())) ); + sb.append( String.format("Argument '%s' has to many values: %s", argument.label, Arrays.deepToString(argument.values().toArray())) ); return sb.toString(); } +} + +/** + * An exception indicating that mutually exclusive options have been passed in the same command line. + */ +class ArgumentsAreMutuallyExclusiveException extends ParseException { + public ArgumentsAreMutuallyExclusiveException( Collection> arguments ) { + super( formatArguments(arguments) ); + } + + private static String formatArguments( Collection> arguments ) { + StringBuilder sb = new StringBuilder(); + for( Pair argument: arguments ) + sb.append( String.format("Arguments '%s' and '%s' are mutually exclusive.", argument.first.definition.fullName, argument.second.definition.fullName ) ); + return sb.toString(); + } + } \ No newline at end of file diff --git a/java/test/org/broadinstitute/sting/utils/cmdLine/ParsingEngineTest.java b/java/test/org/broadinstitute/sting/utils/cmdLine/ParsingEngineTest.java index 0618763dd..ef9c87f07 100755 --- a/java/test/org/broadinstitute/sting/utils/cmdLine/ParsingEngineTest.java +++ b/java/test/org/broadinstitute/sting/utils/cmdLine/ParsingEngineTest.java @@ -425,4 +425,32 @@ public class ParsingEngineTest extends BaseTest { parsingEngine.validate( EnumSet.of(ParsingEngine.ValidationType.MissingRequiredArgument) ); } + @Test(expected=ArgumentsAreMutuallyExclusiveException.class) + public void testMutuallyExclusiveArguments() { + // Passing only foo should work fine... + String[] commandLine = new String[] {"--foo","5"}; + + parsingEngine.addArgumentSource( MutuallyExclusiveArgProvider.class ); + parsingEngine.parse( commandLine ); + parsingEngine.validate(); + + MutuallyExclusiveArgProvider argProvider = new MutuallyExclusiveArgProvider(); + parsingEngine.loadArgumentsIntoObject( argProvider ); + + Assert.assertEquals("Argument is not correctly initialized", 5, argProvider.foo.intValue() ); + + // But when foo and bar come together, danger! + commandLine = new String[] {"--foo","5","--bar","6"}; + + parsingEngine.parse( commandLine ); + parsingEngine.validate(); + } + + private class MutuallyExclusiveArgProvider { + @Argument(doc="foo",exclusiveOf="bar") + Integer foo; + + @Argument(doc="bar",required=false) + Integer bar; + } }