From ec0261275b0bc93c10df21efd0f049acb0ff2363 Mon Sep 17 00:00:00 2001 From: hanna Date: Tue, 5 May 2009 22:08:00 +0000 Subject: [PATCH] Lots of command line argument validation. Catches all common validation problems, including missing required arguments, invalid arguments, and several types of misplaced argument value errors. Still pending: - Help system. - Mutually exclusive arguments. - Design includes too many classes per file. git-svn-id: file:///humgen/gsa-scr1/gsa-engineering/svn_contents/trunk@597 348d0f76-0448-11de-a6fe-93d51630548a --- .../sting/utils/cmdLine/Argument.java | 42 +++- .../utils/cmdLine/ArgumentDefinitions.java | 231 ++++++++++++++---- .../sting/utils/cmdLine/ArgumentMatches.java | 95 +++++-- .../sting/utils/cmdLine/ParsingEngine.java | 202 ++++++++++++--- .../utils/cmdLine/ParsingEngineTest.java | 170 ++++++++++++- 5 files changed, 640 insertions(+), 100 deletions(-) diff --git a/java/src/org/broadinstitute/sting/utils/cmdLine/Argument.java b/java/src/org/broadinstitute/sting/utils/cmdLine/Argument.java index eb28eec85..7479796e0 100755 --- a/java/src/org/broadinstitute/sting/utils/cmdLine/Argument.java +++ b/java/src/org/broadinstitute/sting/utils/cmdLine/Argument.java @@ -12,17 +12,57 @@ import java.lang.annotation.Target; * User: hanna * Date: Mar 24, 2009 * Time: 11:11:36 AM - * To change this template use File | Settings | File Templates. + */ +/** + * Annotates fields in objects that should be used as command-line arguments. + * Any field annotated with @Argument can appear as a command-line parameter. */ @Documented @Inherited @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.FIELD}) public @interface Argument { + /** + * The full name of the command-line argument. Full names should be + * prefixed on the command-line with a double dash (--). + * @return Selected full name, or "" to use the default. + */ String fullName() default ""; + + /** + * Specified short name of the command. Short names should be prefixed + * with a single dash. Argument values can directly abut single-char + * short names or be separated from them by a space. + * @return Selected short name, or "" for none. + */ String shortName() default ""; + + /** + * Documentation for the command-line argument. Should appear when the + * --help argument is specified. + * @return Doc string associated with this command-line argument. + */ String doc() default ""; + + /** + * Is this command-line argument required. The application should exit + * printing help if this command-line argument is not specified. + * @return True if the argument is required. False otherwise. + */ boolean required() default true; + + /** + * Should this command-line argument be exclusive of others. Should be + * a comma-separated list of names of arguments of which this should be + * independent. + * @return A comma-separated string listing other arguments of which this + * argument should be independent. + */ String exclusive() default ""; + + /** + * What is the default value for this argument type. + * @return Default value of this argument type. + */ String defaultValue() default ""; } diff --git a/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java b/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java index 29ec5bb35..69181df8c 100755 --- a/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java +++ b/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentDefinitions.java @@ -1,8 +1,13 @@ package org.broadinstitute.sting.utils.cmdLine; +import org.broadinstitute.sting.utils.StingException; + import java.lang.reflect.Field; -import java.util.HashMap; -import java.util.Map; +import java.util.Set; +import java.util.HashSet; +import java.util.Collection; +import java.util.List; +import java.util.ArrayList; /** * Created by IntelliJ IDEA. @@ -25,26 +30,7 @@ class ArgumentDefinitions { /** * Backing data set of argument stored by short name and long name. */ - private Map argumentsByShortName = new HashMap(); - private Map argumentsByLongName = new HashMap(); - - /** - * Returns the argument with the given short name. - * @param shortName Argument short name. - * @return The argument definition, or null if nothing matches. - */ - public ArgumentDefinition getArgumentWithShortName( String shortName ) { - return argumentsByShortName.get( shortName ); - } - - /** - * Returns the argument with the given short name. - * @param longName Argument long name. - * @return The argument definition, or null if nothing matches. - */ - public ArgumentDefinition getArgumentWithLongName( String longName ) { - return argumentsByLongName.get( longName ); - } + private Set argumentDefinitions = new HashSet(); /** * Adds an argument to the this argument definition list. @@ -54,24 +40,154 @@ class ArgumentDefinitions { */ public void add( Argument argument, Class sourceClass, Field sourceField ) { ArgumentDefinition definition = new ArgumentDefinition( argument, sourceClass, sourceField ); - String fullName = argument.fullName().trim(); - String shortName = argument.shortName().trim(); - - if( fullName.length() == 0 ) + if( definition.fullName.length() == 0 ) { throw new IllegalArgumentException( "Argument cannot have 0-length fullname." ); + } - argumentsByLongName.put( fullName, definition ); - if( shortName.length() != 0 ) - argumentsByShortName.put( shortName, definition ); + if( hasArgumentDefinition( definition.fullName, FullNameDefinitionMatcher ) ) + throw new StingException("Duplicate definition of argument with full name: " + definition.fullName); + if( hasArgumentDefinition( definition.shortName, ShortNameDefinitionMatcher ) ) + throw new StingException("Duplicate definition of argument with short name: " + definition.shortName); + + argumentDefinitions.add( definition ); } + /** + * Are there any argument definitions matching the given property? + * @param property Property to find. + * @param matcher Method of matching a given property. + * @return True if one or multiple argument definitions match; false otherwise. + */ + public boolean hasArgumentDefinition( Object property, DefinitionMatcher matcher ) { + return findArgumentDefinitions( property, matcher ).size() > 0; + } + + /** + * Find the given definition matching this property. + * @param property Property to find. + * @param matcher Method of matching a given property. + * @return The ArgumentDefinition matching the given property. Null if none matches. + * @throws IllegalArgumentException if multiple arguments match this definition. + */ + public ArgumentDefinition findArgumentDefinition( Object property, DefinitionMatcher matcher ) { + Collection selectedDefinitions = findArgumentDefinitions( property, matcher ); + if( selectedDefinitions.size() > 1 ) + throw new IllegalArgumentException("Multiple argument definitions match the selected property: " + property); + + if( selectedDefinitions.size() == 0 ) + return null; + + return selectedDefinitions.iterator().next(); + } + + /** + * Find all argument definitions matching a certain category. + * @param property Property to inspect. + * @param matcher Test to see whether property matches. + * @return All argument definitions matching a certain object. + */ + public Collection findArgumentDefinitions( Object property, DefinitionMatcher matcher ) { + Set selectedArgumentDefinitions = new HashSet(); + for( ArgumentDefinition argumentDefinition: argumentDefinitions ) { + if( matcher.matches( argumentDefinition, property ) ) + selectedArgumentDefinitions.add( argumentDefinition ); + } + return selectedArgumentDefinitions; + } + + /** + * Match the full name of a definition. + */ + public static DefinitionMatcher FullNameDefinitionMatcher = new DefinitionMatcher() { + public boolean matches( ArgumentDefinition definition, Object key ) { + if( definition.fullName == null ) + return key == null; + else + return definition.fullName.equals( key ); + } + }; + + /** + * Match the short name of a definition. + */ + public static DefinitionMatcher ShortNameDefinitionMatcher = new DefinitionMatcher() { + public boolean matches( ArgumentDefinition definition, Object key ) { + if( definition.shortName == null ) + return key == null; + else + return definition.shortName.equals( key ); + } + }; + + public static AliasProvider ShortNameAliasProvider = new AliasProvider() { + /** + * Short names can come in the form -Ofoo.txt, -O foo.txt, or -out (multi-character short name). + * Given the argument name and built-in provided, see if these can be formed into some other argument + * name. + * @param argument Name of the argument, as parsed. For a short name, will be a single letter. + * @param value Value of the argument, as parsed. + * @return Any potential aliases for the given shortname. + */ + public List getAliases( String argument, String value ) { + List aliases = new ArrayList(); + aliases.add(argument+value); + aliases.add(argument); + return aliases; + } + + /** + * Is the value part of the given alias, or something separate that should be treated as an argument value. + * @param alias The alias to use. + * @param argument The parsed argument. + * @param value The parsed value. + * @return True if this alias should be used instead of the given value. + */ + public boolean doesAliasConsumeValue( String alias, String argument, String value ) { + return alias.equals(argument + value); + } + }; + + /** + * Find all required definitions. + */ + public static class RequiredDefinitionMatcher implements DefinitionMatcher { + public boolean matches( ArgumentDefinition definition, Object key ) { + if( !(key instanceof Boolean) ) + throw new IllegalArgumentException("RequiredDefinitionMatcher requires boolean key"); + return definition.required == (Boolean)key; + } + } } /** * A specific argument definition. Maps one-to-one with a field in some class. */ class ArgumentDefinition { - public final Argument argument; + /** + * Full name of the argument. Must have a value. + */ + public final String fullName; + + /** + * Short name of the argument. Can be null. + */ + public final String shortName; + + /** + * Doc string for the argument. Displayed in help. + */ + public final String doc; + + /** + * Is this argument required? + */ + public final boolean required; + + /** + * Is this argument exclusive of other arguments? + */ + public final String exclusive; + public final Class sourceClass; public final Field sourceField; @@ -82,27 +198,56 @@ class ArgumentDefinition { * @param sourceField Source field for the argument, extracted from the sourceClass. */ public ArgumentDefinition( Argument argument, Class sourceClass, Field sourceField ) { - this.argument = argument; + fullName = argument.fullName().trim().length() > 0 ? argument.fullName().trim() : sourceField.getName().toLowerCase(); + shortName = argument.shortName().trim().length() > 0 ? argument.shortName().trim() : null; + doc = argument.doc(); + required = argument.required(); + exclusive = argument.exclusive().trim().length() > 0 ? argument.exclusive().trim() : null; + this.sourceClass = sourceClass; this.sourceField = sourceField; } + + /** + * Can this argument support multiple values, or just one? + * @return True if the argument supports multiple values. + */ + public boolean isMultiValued() { + Class argumentType = sourceField.getType(); + return Collection.class.isAssignableFrom(argumentType) || sourceField.getType().isArray(); + } } /** - * A general purpose accessor interface for ArgumentDefinitions. + * A Comparator-esque interface for finding argument definitions within a collection. */ interface DefinitionMatcher { - ArgumentDefinition get( ArgumentDefinitions argumentDefinitions, String key ); + /** + * Does the given definition match the provided key? + * @param definition The definition to inspect. + * @param key The value to match. + * @return True if the key matches the definition, false otherwise. + */ + boolean matches( ArgumentDefinition definition, Object key ); } -class FullNameDefinitionMatcher implements DefinitionMatcher { - public ArgumentDefinition get( ArgumentDefinitions argumentDefinitions, String key ) { - return argumentDefinitions.getArgumentWithLongName( key ); - } -} +/** + * A way to get alternate names for the argument given the recognized name and value. + */ +interface AliasProvider { + /** + * Give all alternate names for the given argument / value pair. The aliases should + * be returned in 'preferred order'. + * @param argument The argument. + * @param value The value. + * @return All possible names. + */ + List getAliases( String argument, String value ); -class ShortNameDefinitionMatcher implements DefinitionMatcher { - public ArgumentDefinition get( ArgumentDefinitions argumentDefinitions, String key ) { - return argumentDefinitions.getArgumentWithShortName( key ); - } -} \ No newline at end of file + /** + * True if this alias 'consumes' the value, meaning that the argument + value together + * represent some other alias. + * @return True if the value should still be used. False otherwise. + */ + boolean doesAliasConsumeValue( String alias, String argument, String value ); +} diff --git a/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentMatches.java b/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentMatches.java index af08dfb18..6058c03f1 100755 --- a/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentMatches.java +++ b/java/src/org/broadinstitute/sting/utils/cmdLine/ArgumentMatches.java @@ -7,6 +7,8 @@ import java.util.TreeMap; import java.util.Map; import java.util.Set; import java.util.HashSet; +import java.util.Collection; +import java.util.HashMap; /** * Created by IntelliJ IDEA. * User: mhanna @@ -33,22 +35,27 @@ public class ArgumentMatches implements Iterable { */ Map argumentMatches = new TreeMap(); + /** + * Provide a place to put command-line argument values that don't seem to belong to + * any particular command-line option. + */ + public ArgumentMatch MissingArgument = new ArgumentMatch(); + void mergeInto( ArgumentMatch match ) { boolean definitionExists = false; // Clone the list of argument matches to avoid ConcurrentModificationExceptions. - Set uniqueMatches = getUniqueMatches(); - for( ArgumentMatch argumentMatch: uniqueMatches ) { - if( argumentMatch.definition.equals(match.definition) ) { + for( ArgumentMatch argumentMatch: getUniqueMatches() ) { + if( argumentMatch.definition == match.definition ) { argumentMatch.mergeInto( match ); - for( int index: match.indices ) + for( int index: match.indices.keySet() ) argumentMatches.put( index, argumentMatch ); definitionExists = true; } } if( !definitionExists ) { - for( int index: match.indices ) + for( int index: match.indices.keySet() ) argumentMatches.put( index, match ); } } @@ -89,6 +96,30 @@ public class ArgumentMatches implements Iterable { private Set getUniqueMatches() { return new HashSet( argumentMatches.values() ); } + + /** + * Does the match collection have a match for this argument definition. + * @param definition Definition to match. + * @return True if a match exists; false otherwise. + */ + public boolean hasMatch( ArgumentDefinition definition ) { + return findMatches( definition ).size() > 0; + } + + /** + * Return all argument matches of this definition. + * @param definition Argument definition to match. + * @return List of all matches. + */ + public Collection findMatches( ArgumentDefinition definition ) { + Collection matches = new HashSet(); + for( ArgumentMatch argumentMatch: getUniqueMatches() ) { + if( argumentMatch.definition == definition ) + matches.add( argumentMatch ); + } + return matches; + } + } /** @@ -101,18 +132,27 @@ class ArgumentMatch { public final ArgumentDefinition definition; /** - * Index into the string of arguments where this match was found. + * The text that's been matched, as it appears in the command line arguments. */ - public final Set indices = new HashSet(); + public final String label; /** - * The values associated with this parameter. + * Maps indicies of command line arguments to values paired with that argument. */ - public final List values = new ArrayList(); + public final Map> indices = new HashMap>(); - public ArgumentMatch( ArgumentDefinition definition, int index ) { + /** + * Create a new argument match, defining its properties later. Used to create invalid arguments. + */ + public ArgumentMatch() { + definition = null; + label = null; + } + + public ArgumentMatch( String label, ArgumentDefinition definition, int index ) { + this.label = label; this.definition = definition; - indices.add(index); + indices.put(index,null); } /** @@ -121,15 +161,40 @@ class ArgumentMatch { * @param other The other match to merge into. */ public void mergeInto( ArgumentMatch other ) { - indices.addAll(other.indices); - values.addAll(other.values); + indices.putAll(other.indices); } /** * Associate a value with this merge maapping. + * @param index index of the command-line argument to which this value is mated. * @param value Text representation of value to add. */ - public void addValue( String value ) { - this.values.add(value); + public void addValue( int index, String value ) { + if( !indices.containsKey(index) || indices.get(index) == null ) + indices.put(index, new ArrayList() ); + indices.get(index).add(value); + } + + /** + * Does this argument already have a value at the given site? + * Arguments are only allowed to be single-valued. + * @param index Index at which to check for values. + * @return True if the argument has a value at the given site. False otherwise. + */ + public boolean hasValueAtSite( int index ) { + return indices.get(index) != null && indices.get(index).size() >= 1; + } + + /** + * Return the values associated with this argument match. + * @return A collection of the string representation of these value. + */ + public List values() { + List values = new ArrayList(); + for( int index: indices.keySet() ) { + if( indices.get(index) != null ) + values.addAll(indices.get(index)); + } + return values; } } \ No newline at end of file diff --git a/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java b/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java index 9a37af4d0..be3f2cddc 100755 --- a/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java +++ b/java/src/org/broadinstitute/sting/utils/cmdLine/ParsingEngine.java @@ -14,6 +14,7 @@ import java.util.regex.Matcher; import java.util.ArrayList; import java.util.List; import java.util.Collection; +import java.util.Arrays; /** * Created by IntelliJ IDEA. @@ -35,8 +36,9 @@ import java.util.Collection; public class ParsingEngine { /** * A list of defined arguments against which command lines are matched. + * Package protected for testing access. */ - private ArgumentDefinitions argumentDefinitions = new ArgumentDefinitions(); + ArgumentDefinitions argumentDefinitions = new ArgumentDefinitions(); /** * Techniques for parsing and for argument lookup. @@ -46,12 +48,13 @@ public class ParsingEngine { /** * our log, which we want to capture anything from org.broadinstitute.sting */ - protected static Logger logger = Logger.getLogger(ArgumentParser.class); + protected static Logger logger = Logger.getLogger(ArgumentParser.class); public ParsingEngine() { - parsingMethods.add( new ParsingMethod(Pattern.compile("\\s*--([\\w\\.]+)\\s*"), new FullNameDefinitionMatcher()) ); - parsingMethods.add( new ParsingMethod(Pattern.compile("\\s*-([\\w\\.]+)\\s*"), new ShortNameDefinitionMatcher()) ); - parsingMethods.add( new ParsingMethod(Pattern.compile("\\s*-([\\w\\.])([\\w\\.]+)\\s*"), new ShortNameDefinitionMatcher()) ); + parsingMethods.add( new ParsingMethod(Pattern.compile("\\s*--([\\w\\.]+)\\s*"), ArgumentDefinitions.FullNameDefinitionMatcher) ); + parsingMethods.add( new ParsingMethod(Pattern.compile("\\s*-([\\w\\.])([\\w\\.]*)\\s*"), + ArgumentDefinitions.ShortNameDefinitionMatcher, + ArgumentDefinitions.ShortNameAliasProvider) ); } /** @@ -62,9 +65,9 @@ public class ParsingEngine { */ public void addArgumentSources( Class... sources ) { for( Class source: sources ) { - Field[] fields = source.getFields(); + Field[] fields = source.getDeclaredFields(); for( Field field: fields ) { - Argument argument = field.getAnnotation(Argument.class); + Argument argument = field.getAnnotation(Argument.class); if(argument != null) argumentDefinitions.add( argument, source, field ); } @@ -88,10 +91,39 @@ public class ParsingEngine { /** * Validates the list of command-line argument matches. On - * failure ...TBD... + * failure throws an exception with detailed info about the particular failures. */ public void validate( ArgumentMatches argumentMatches ) { - + // Find missing required arguments. + Collection requiredArguments = argumentDefinitions.findArgumentDefinitions( true, new ArgumentDefinitions.RequiredDefinitionMatcher() ); + Collection missingArguments = new ArrayList(); + for( ArgumentDefinition requiredArgument: requiredArguments ) { + if( !argumentMatches.hasMatch(requiredArgument) ) + missingArguments.add( requiredArgument ); + } + + if( missingArguments.size() > 0 ) + throw new MissingArgumentException( missingArguments ); + + // Find invalid arguments. Invalid arguments will have a null argument definition. + Collection invalidArguments = argumentMatches.findMatches(null); + if( invalidArguments.size() > 0 ) + throw new InvalidArgumentException( invalidArguments ); + + // Find values without an associated mate. + if( argumentMatches.MissingArgument.values().size() > 0 ) + throw new InvalidArgumentValueException( argumentMatches.MissingArgument ); + + // Find arguments with too many values. + Collection overvaluedArguments = new ArrayList(); + for( ArgumentMatch argumentMatch: argumentMatches ) { + // Warning: assumes that definition is not null (asserted by checks above). + if( !argumentMatch.definition.isMultiValued() && argumentMatch.values().size() > 1 ) + overvaluedArguments.add(argumentMatch); + } + + if( !overvaluedArguments.isEmpty() ) + throw new TooManyValuesForArgumentException(overvaluedArguments); } /** @@ -104,8 +136,8 @@ public class ParsingEngine { ArgumentDefinition definition = match.definition; if( object.getClass().equals(definition.sourceClass) ) { try { - if( !isArgumentBoolean(definition) ) - definition.sourceField.set( object, constructFromString( definition.sourceField, match.values ) ); + if( !isArgumentFlag(definition) ) + definition.sourceField.set( object, constructFromString( definition.sourceField, match.values() ) ); else definition.sourceField.set( object, true ); } @@ -117,7 +149,12 @@ public class ParsingEngine { } } - private boolean isArgumentBoolean( ArgumentDefinition definition ) { + /** + * Returns true if the argument is a flag (a 0-valued argument). + * @param definition Argument definition. + * @return True if argument is a flag; false otherwise. + */ + private boolean isArgumentFlag( ArgumentDefinition definition ) { return (definition.sourceField.getType() == Boolean.class) || (definition.sourceField.getType() == Boolean.TYPE); } @@ -145,8 +182,8 @@ public class ParsingEngine { throw new IllegalArgumentException( "Token is not recognizable as an argument: " + token ); for( ParsingMethod parsingMethod: parsingMethods ) { - if( parsingMethod.hasMatch( argumentDefinitions, token ) ) - return parsingMethod.findMatch( argumentDefinitions, token, position ); + if( parsingMethod.matches( argumentDefinitions, token ) ) + return parsingMethod.match( argumentDefinitions, token, position ); } // No parse results found. @@ -179,15 +216,20 @@ public class ParsingEngine { * @param tokens The command-line input. */ private void fitValuesToArguments( ArgumentMatches argumentMatches, String[] tokens ) { - ArgumentMatch lastMatched = null; - for( int i = 0; i < tokens.length; i++ ) { - if( argumentMatches.hasMatch(i) ) { - lastMatched = argumentMatches.getMatch(i); + // If this is the site of a successfully matched argument, pass it over. + if( argumentMatches.hasMatch(i) ) continue; - } - - lastMatched.addValue( tokens[i] ); + + // tokens[i] must be an argument value. Match it with the previous argument. + String value = tokens[i]; + int argumentSite = i - 1; + + // If the argument is present and doesn't already have a value associated with the given site, add the value. + if( argumentMatches.hasMatch(argumentSite) && !argumentMatches.getMatch(argumentSite).hasValueAtSite(argumentSite)) + argumentMatches.getMatch(argumentSite).addValue( argumentSite, value ); + else + argumentMatches.MissingArgument.addValue( i, value ); } } @@ -303,18 +345,24 @@ public class ParsingEngine { private class ParsingMethod { public final Pattern pattern; public final DefinitionMatcher definitionMatcher; + public final AliasProvider aliasProvider; public ParsingMethod( Pattern pattern, DefinitionMatcher definitionMatcher ) { + this( pattern, definitionMatcher, null ); + } + + public ParsingMethod( Pattern pattern, DefinitionMatcher definitionMatcher, AliasProvider aliasProvider ) { this.pattern = pattern; this.definitionMatcher = definitionMatcher; + this.aliasProvider = aliasProvider; } - public boolean hasMatch( ArgumentDefinitions definitions, String token ) { + public boolean matches( ArgumentDefinitions definitions, String token ) { Matcher matcher = pattern.matcher(token); - return matcher.matches() && definitionMatcher.get( definitions, matcher.group(1) ) != null; + return matcher.matches(); } - public ArgumentMatch findMatch( ArgumentDefinitions definitions, String token, int position ) { + public ArgumentMatch match( ArgumentDefinitions definitions, String token, int position ) { Matcher matcher = pattern.matcher(token); // Didn't match? Must be bad input. @@ -323,19 +371,107 @@ public class ParsingEngine { // If the argument is valid, parse out the argument and value (if present). String argument = matcher.group(1); - String value = matcher.groupCount() > 1 ? matcher.group(2) : null; + String value = null; + if( matcher.groupCount() > 1 && matcher.group(2).trim().length() > 0) + value = matcher.group(2).trim(); - // Try to find a matching argument. If found, label that as the match. - ArgumentDefinition argumentDefinition = definitionMatcher.get( definitions, argument ); + // If an alias provider has been provided, determine the possible list of argument names that this + // argument / value pair can represent. + ArgumentDefinition bestMatchArgumentDefinition = null; + if( aliasProvider != null ) { + List aliases = aliasProvider.getAliases( argument, value ); + String bestAlias = null; - if( argumentDefinition != null ) { - ArgumentMatch argumentMatch = new ArgumentMatch( argumentDefinition, position ); - if( value != null ) - argumentMatch.addValue( value ); - return argumentMatch; + for( String alias: aliases ) { + if( definitions.findArgumentDefinition(alias,definitionMatcher) != null ) { + bestAlias = alias; + bestMatchArgumentDefinition = definitions.findArgumentDefinition(alias,definitionMatcher); + break; + } + } + + // Couldn't find anything appropriate? The aliases should be in best-to-worst order, so + if( bestAlias == null ) { + bestAlias = aliases.get(0); + } + + if( aliasProvider.doesAliasConsumeValue(bestAlias,argument,value) ) value = null; + argument = bestAlias; } + else + bestMatchArgumentDefinition = definitions.findArgumentDefinition( argument, definitionMatcher ); - throw new IllegalArgumentException( String.format("Unable to find match for token %s", token) ); + // Try to find a matching argument. If found, label that as the match. If not found, add the argument + // with a null definition. + ArgumentMatch argumentMatch = new ArgumentMatch( argument, bestMatchArgumentDefinition, position ); + if( value != null ) + argumentMatch.addValue( position, value ); + return argumentMatch; } } } + +/** + * An exception indicating that some required arguments are missing. + */ +class MissingArgumentException extends StingException { + public MissingArgumentException( Collection missingArguments ) { + super( formatArguments(missingArguments) ); + } + + private static String formatArguments( Collection missingArguments ) { + StringBuilder sb = new StringBuilder(); + for( ArgumentDefinition missingArgument: missingArguments ) + sb.append( String.format("Argument with name '%s' is missing.", missingArgument.fullName) ); + return sb.toString(); + } +} + +/** + * An exception for undefined arguments. + */ +class InvalidArgumentException extends StingException { + public InvalidArgumentException( Collection invalidArguments ) { + super( formatArguments(invalidArguments) ); + } + + private static String formatArguments( Collection invalidArguments ) { + StringBuilder sb = new StringBuilder(); + for( ArgumentMatch invalidArgument: invalidArguments ) + sb.append( String.format("Argument with name '%s' isn't defined.", invalidArgument.label) ); + return sb.toString(); + } +} + +/** + * An exception for values that can't be mated with any argument. + */ +class InvalidArgumentValueException extends StingException { + public InvalidArgumentValueException( ArgumentMatch invalidValues ) { + super( formatArguments(invalidValues) ); + } + + private static String formatArguments( ArgumentMatch invalidValues ) { + StringBuilder sb = new StringBuilder(); + for( int index: invalidValues.indices.keySet() ) + for( String value: invalidValues.indices.get(index) ) + sb.append( String.format("Invalid argument value '%s' at position %d", value, index) ); + return sb.toString(); + } +} + +/** + * An exception indicating that too many values have been provided for the given argument. + */ +class TooManyValuesForArgumentException extends StingException { + public TooManyValuesForArgumentException( Collection arguments ) { + super( formatArguments(arguments) ); + } + + 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())) ); + 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 7c770f968..5620b9f97 100755 --- a/java/test/org/broadinstitute/sting/utils/cmdLine/ParsingEngineTest.java +++ b/java/test/org/broadinstitute/sting/utils/cmdLine/ParsingEngineTest.java @@ -1,6 +1,7 @@ package org.broadinstitute.sting.utils.cmdLine; import org.broadinstitute.sting.BaseTest; +import org.broadinstitute.sting.utils.StingException; import org.junit.Test; import org.junit.Before; import org.junit.Assert; @@ -64,6 +65,26 @@ public class ParsingEngineTest extends BaseTest { Assert.assertEquals("Argument is not correctly initialized", "na12878.bam", argProvider.inputFile ); } + @Test + public void multiCharShortNameArgumentTest() { + final String[] commandLine = new String[] {"-out","out.txt"}; + + parsingEngine.addArgumentSources( MultiCharShortNameArgProvider.class ); + ArgumentMatches argumentMatches = parsingEngine.parse( commandLine ); + parsingEngine.validate(argumentMatches); + + MultiCharShortNameArgProvider argProvider = new MultiCharShortNameArgProvider(); + parsingEngine.loadArgumentsIntoObject( argProvider, argumentMatches); + + Assert.assertEquals("Argument is not correctly initialized", "out.txt", argProvider.outputFile ); + } + + + private class MultiCharShortNameArgProvider { + @Argument(shortName="out") + public String outputFile; + } + @Test public void longNameArgumentTest() { final String[] commandLine = new String[] {"--input_file", "na12878.bam"}; @@ -127,7 +148,6 @@ public class ParsingEngineTest extends BaseTest { Assert.assertEquals("2nd filename is incorrect", "bar.txt", argProvider.inputFile[1] ); } - private class MultiValueArgProvider { @Argument(fullName="input_file",shortName="I") public String[] inputFile; @@ -183,12 +203,146 @@ public class ParsingEngineTest extends BaseTest { public List integers; } + @Test(expected=MissingArgumentException.class) + public void requiredArgTest() { + final String[] commandLine = new String[0]; - // To test - // misc first element - // multiple trailing values - // differing input types - // spurious arguments with in conjuction with immediate setters "-Ifoo.txt bar.txt" - // required but missing arguments - // invalid arguments + parsingEngine.addArgumentSources( RequiredArgProvider.class ); + ArgumentMatches argumentMatches = parsingEngine.parse( commandLine ); + parsingEngine.validate( argumentMatches ); + } + + private class RequiredArgProvider { + @Argument(required=true) + public Integer value; + } + + @Test + public void unrequiredArgTest() { + final String[] commandLine = new String[0]; + + parsingEngine.addArgumentSources( UnrequiredArgProvider.class ); + ArgumentMatches argumentMatches = parsingEngine.parse( commandLine ); + parsingEngine.validate( argumentMatches ); + + UnrequiredArgProvider argProvider = new UnrequiredArgProvider(); + parsingEngine.loadArgumentsIntoObject( argProvider, argumentMatches); + + Assert.assertNull( "Value was unrequired and unspecified; contents should be null", argProvider.value ); + } + + private class UnrequiredArgProvider { + @Argument(required=false) + public Integer value; + } + + @Test(expected=InvalidArgumentException.class) + public void invalidArgTest() { + final String[] commandLine = new String[] { "--foo" }; + + parsingEngine.addArgumentSources( UnrequiredArgProvider.class ); + ArgumentMatches argumentMatches = parsingEngine.parse( commandLine ); + parsingEngine.validate( argumentMatches ); + } + + @Test(expected=StingException.class) + public void duplicateLongNameTest() { + parsingEngine.addArgumentSources( DuplicateLongNameProvider.class ); + } + + private class DuplicateLongNameProvider { + @Argument(fullName="myarg") + public Integer foo; + + @Argument(fullName="myarg") + public Integer bar; + } + + @Test(expected=StingException.class) + public void duplicateShortNameTest() { + parsingEngine.addArgumentSources( DuplicateShortNameProvider.class ); + } + + + private class DuplicateShortNameProvider { + @Argument(shortName="myarg") + public Integer foo; + + @Argument(shortName="myarg") + public Integer bar; + } + + @Test(expected=InvalidArgumentValueException.class) + public void missingArgumentNameTest() { + final String[] commandLine = new String[] {"foo.txt"}; + + parsingEngine.addArgumentSources( NoArgProvider.class ); + ArgumentMatches argumentMatches = parsingEngine.parse( commandLine ); + parsingEngine.validate(argumentMatches); + } + + private class NoArgProvider { + + } + + @Test(expected=InvalidArgumentValueException.class) + public void extraValueTest() { + final String[] commandLine = new String[] {"-Ifoo.txt", "bar.txt"}; + + parsingEngine.addArgumentSources( InputFileArgProvider.class ); + ArgumentMatches argumentMatches = parsingEngine.parse( commandLine ); + parsingEngine.validate(argumentMatches); + } + + @Test(expected=MissingArgumentException.class) + public void multipleInvalidArgTest() { + final String[] commandLine = new String[] {"-N1", "-N2", "-N3"}; + + parsingEngine.addArgumentSources( RequiredArgProvider.class ); + ArgumentMatches argumentMatches = parsingEngine.parse( commandLine ); + parsingEngine.validate( argumentMatches ); + } + + @Test(expected=TooManyValuesForArgumentException.class) + public void invalidArgCountTest() { + final String[] commandLine = new String[] {"--value","1","--value","2","--value","3"}; + + parsingEngine.addArgumentSources( RequiredArgProvider.class ); + ArgumentMatches argumentMatches = parsingEngine.parse( commandLine ); + parsingEngine.validate( argumentMatches ); + } + + @Test + public void packageProtectedArgTest() { + final String[] commandLine = new String[] {"--foo", "1"}; + + parsingEngine.addArgumentSources( PackageProtectedArgProvider.class ); + ArgumentMatches argumentMatches = parsingEngine.parse( commandLine ); + parsingEngine.validate(argumentMatches); + + PackageProtectedArgProvider argProvider = new PackageProtectedArgProvider(); + parsingEngine.loadArgumentsIntoObject( argProvider, argumentMatches); + + Assert.assertEquals("Argument is not correctly initialized", 1, argProvider.foo.intValue() ); + } + + private class PackageProtectedArgProvider { + @Argument + Integer foo; + } + + @Test + public void correctDefaultArgNameTest() { + parsingEngine.addArgumentSources( CamelCaseArgProvider.class ); + + DefinitionMatcher matcher = ArgumentDefinitions.FullNameDefinitionMatcher; + ArgumentDefinition definition = parsingEngine.argumentDefinitions.findArgumentDefinition("myarg", matcher); + + Assert.assertNotNull("Invalid default argument name assigned", definition ); + } + + private class CamelCaseArgProvider { + @Argument + Integer myArg; + } }