diff --git a/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/genotyper/UnifiedGenotyper.java b/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/genotyper/UnifiedGenotyper.java index 938787907..40f3271a1 100644 --- a/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/genotyper/UnifiedGenotyper.java +++ b/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/genotyper/UnifiedGenotyper.java @@ -212,7 +212,7 @@ public class UnifiedGenotyper extends LocusWalker, Unif * If specified, all available annotations in the group will be applied. See the VariantAnnotator -list argument to view available groups. * Keep in mind that RODRequiringAnnotations are not intended to be used as a group, because they require specific ROD inputs. */ - @Argument(fullName="group", shortName="G", doc="One or more classes/groups of annotations to apply to variant calls", required=false) + @Argument(fullName="group", shortName="G", doc="One or more classes/groups of annotations to apply to variant calls. The single value 'none' removes the default group", required=false) protected String[] annotationClassesToUse = { "Standard" }; // the calculation arguments diff --git a/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/haplotypecaller/HaplotypeCaller.java b/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/haplotypecaller/HaplotypeCaller.java index 2b51e6776..6a4222c09 100644 --- a/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/haplotypecaller/HaplotypeCaller.java +++ b/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/haplotypecaller/HaplotypeCaller.java @@ -320,7 +320,7 @@ public class HaplotypeCaller extends ActiveRegionWalker, In * Which annotations to add to the output VCF file. See the VariantAnnotator -list argument to view available annotations. */ @Advanced - @Argument(fullName="annotation", shortName="A", doc="One or more specific annotations to apply to variant calls", required=false) + @Argument(fullName="annotation", shortName="A", doc="One or more specific annotations to apply to variant calls. The single value 'none' removes the default annotations", required=false) protected List annotationsToUse = new ArrayList<>(Arrays.asList(new String[]{"ClippingRankSumTest", "DepthPerSampleHC"})); /** @@ -334,7 +334,7 @@ public class HaplotypeCaller extends ActiveRegionWalker, In /** * Which groups of annotations to add to the output VCF file. See the VariantAnnotator -list argument to view available groups. */ - @Argument(fullName="group", shortName="G", doc="One or more classes/groups of annotations to apply to variant calls", required=false) + @Argument(fullName="group", shortName="G", doc="One or more classes/groups of annotations to apply to variant calls. The single value 'none' removes the default group", required=false) protected String[] annotationClassesToUse = { "Standard" }; @ArgumentCollection diff --git a/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/variantutils/GenotypeGVCFs.java b/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/variantutils/GenotypeGVCFs.java index 2374e06f2..ae683993d 100644 --- a/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/variantutils/GenotypeGVCFs.java +++ b/protected/gatk-tools-protected/src/main/java/org/broadinstitute/gatk/tools/walkers/variantutils/GenotypeGVCFs.java @@ -133,7 +133,7 @@ public class GenotypeGVCFs extends RodWalker annotationsToUse = new ArrayList<>(Arrays.asList(new String[]{"InbreedingCoeff", "FisherStrand", "QualByDepth", "ChromosomeCounts", "GenotypeSummaries"})); /** diff --git a/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/tools/walkers/haplotypecaller/HaplotypeCallerParallelIntegrationTest.java b/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/tools/walkers/haplotypecaller/HaplotypeCallerParallelIntegrationTest.java index 8a91e649e..cd203cf38 100644 --- a/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/tools/walkers/haplotypecaller/HaplotypeCallerParallelIntegrationTest.java +++ b/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/tools/walkers/haplotypecaller/HaplotypeCallerParallelIntegrationTest.java @@ -71,7 +71,7 @@ public class HaplotypeCallerParallelIntegrationTest extends WalkerTest { WalkerTestSpec spec = new WalkerTestSpec( "-T HaplotypeCaller --pcr_indel_model NONE -R " + b37KGReference + " --no_cmdline_in_header -I " + privateTestDir + "PCRFree.2x250.Illumina.20_10_11.bam -o %s " + - " -L 20:10,000,000-10,100,000 -G none -A -contamination 0.0 -nct " + nct, 1, + " -L 20:10,000,000-10,100,000 -G none -A none -contamination 0.0 -nct " + nct, 1, Arrays.asList(md5)); executeTest("HC test parallel HC with NCT with nct " + nct, spec); } diff --git a/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/utils/nanoScheduler/NanoSchedulerIntegrationTest.java b/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/utils/nanoScheduler/NanoSchedulerIntegrationTest.java index 8466115ca..2fc925e11 100644 --- a/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/utils/nanoScheduler/NanoSchedulerIntegrationTest.java +++ b/protected/gatk-tools-protected/src/test/java/org/broadinstitute/gatk/utils/nanoScheduler/NanoSchedulerIntegrationTest.java @@ -78,7 +78,7 @@ public class NanoSchedulerIntegrationTest extends WalkerTest { WalkerTestSpec spec = new WalkerTestSpec( buildCommandLine( "-T UnifiedGenotyper -R " + b37KGReference, - "--no_cmdline_in_header -G", + "--no_cmdline_in_header -G none", //"--dbsnp " + b37dbSNP132, "-I " + privateTestDir + "NA12878.HiSeq.b37.chr20.10_11mb.bam", "-L 20:10,000,000-10,100,000", diff --git a/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/tools/walkers/annotator/interfaces/AnnotationInterfaceManager.java b/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/tools/walkers/annotator/interfaces/AnnotationInterfaceManager.java index 8f137ccf9..37b570c87 100644 --- a/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/tools/walkers/annotator/interfaces/AnnotationInterfaceManager.java +++ b/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/tools/walkers/annotator/interfaces/AnnotationInterfaceManager.java @@ -32,6 +32,9 @@ import org.broadinstitute.gatk.utils.exceptions.UserException; import java.util.*; public class AnnotationInterfaceManager { + private static final String NULL_ANNOTATION_NAME = "none"; + private static final String NULL_ANNOTATION_GROUP_NAME = "none"; + private static PluginManager infoFieldAnnotationPluginManager = new PluginManager(InfoFieldAnnotation.class); private static PluginManager genotypeAnnotationPluginManager = new PluginManager(GenotypeAnnotation.class); private static PluginManager annotationTypePluginManager = new PluginManager(AnnotationType.class); @@ -53,7 +56,7 @@ public class AnnotationInterfaceManager { for ( Class c : annotationTypePluginManager.getInterfaces() ) classMap.put(c.getSimpleName(), c); - if ( annotationGroupsToUse.size() != 1 || !"none".equals(annotationGroupsToUse.get(0)) ) { + if ( annotationGroupsToUse.size() != 1 || !NULL_ANNOTATION_GROUP_NAME.equals(annotationGroupsToUse.get(0)) ) { for ( String group : annotationGroupsToUse ) { Class interfaceClass = classMap.get(group); if ( interfaceClass == null ) @@ -64,15 +67,17 @@ public class AnnotationInterfaceManager { } // validate the specific classes provided - for ( String annotation : annotationsToUse ) { - Class annotationClass = classMap.get(annotation); - if ( annotationClass == null ) - annotationClass = classMap.get(annotation + "Annotation"); - if ( annotationClass == null ) { - if (DeprecatedToolChecks.isDeprecatedAnnotation(annotation) ) { - throw new UserException.DeprecatedAnnotation(annotation, DeprecatedToolChecks.getAnnotationDeprecationInfo(annotation)); - } else { - throw new UserException.BadArgumentValue("annotation", "Annotation " + annotation + " was not found; please check that you have specified the annotation name correctly"); + if ( annotationsToUse.size() != 1 || !NULL_ANNOTATION_NAME.equals(annotationsToUse.get(0)) ) { + for (String annotation : annotationsToUse) { + Class annotationClass = classMap.get(annotation); + if (annotationClass == null) + annotationClass = classMap.get(annotation + "Annotation"); + if (annotationClass == null) { + if (DeprecatedToolChecks.isDeprecatedAnnotation(annotation)) { + throw new UserException.DeprecatedAnnotation(annotation, DeprecatedToolChecks.getAnnotationDeprecationInfo(annotation)); + } else { + throw new UserException.BadArgumentValue("annotation", "Annotation " + annotation + " was not found; please check that you have specified the annotation name correctly"); + } } } } @@ -105,7 +110,7 @@ public class AnnotationInterfaceManager { } }); - if ( annotationGroupsToUse.size() != 1 || !"none".equals(annotationGroupsToUse.get(0)) ) { + if ( annotationGroupsToUse.size() != 1 || !NULL_ANNOTATION_GROUP_NAME.equals(annotationGroupsToUse.get(0)) ) { for ( String group : annotationGroupsToUse ) { Class interfaceClass = classMap.get(group); if ( interfaceClass == null ) @@ -116,12 +121,14 @@ public class AnnotationInterfaceManager { } // get the specific classes provided - for ( String annotation : annotationsToUse ) { - Class annotationClass = classMap.get(annotation); - if ( annotationClass == null ) - annotationClass = classMap.get(annotation + "Annotation"); - if ( annotationClass != null ) - classes.add(annotationClass); + if ( annotationsToUse.size() != 1 || !NULL_ANNOTATION_NAME.equals(annotationsToUse.get(0)) ) { + for (String annotation : annotationsToUse) { + Class annotationClass = classMap.get(annotation); + if (annotationClass == null) + annotationClass = classMap.get(annotation + "Annotation"); + if (annotationClass != null) + classes.add(annotationClass); + } } // note that technically an annotation can work on both the INFO and FORMAT fields diff --git a/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentDefinitions.java b/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentDefinitions.java index 45c104b0a..8bc17d78d 100644 --- a/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentDefinitions.java +++ b/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentDefinitions.java @@ -174,8 +174,9 @@ public class ArgumentDefinitions implements Iterable { static DefinitionMatcher VerifiableDefinitionMatcher = new DefinitionMatcher() { public boolean matches( ArgumentDefinition definition, Object key ) { - // We can perform some sort of validation for anything that isn't a flag. - return !definition.isFlag; + // We can perform some sort of validation for anything that isn't a flag or enum. + // Because enums can have a default value, it might be valid to specify an enum argument with no value + return !definition.isFlag && !definition.argumentType.isEnum(); } }; } diff --git a/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentMatch.java b/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentMatch.java index 9064930c0..885e02d66 100644 --- a/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentMatch.java +++ b/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentMatch.java @@ -187,7 +187,7 @@ public class ArgumentMatch implements Iterable { * @return True if there's another token waiting in the wings. False otherwise. */ public boolean hasNext() { - return nextToken != null; + return nextSite != null; } /** @@ -195,7 +195,7 @@ public class ArgumentMatch implements Iterable { * @return The next ArgumentMatch in the series. Should never be null. */ public ArgumentMatch next() { - if( nextSite == null || nextToken == null ) + if( nextSite == null ) throw new IllegalStateException( "No more ArgumentMatches are available" ); ArgumentMatch match = new ArgumentMatch( label, definition, nextSite, nextToken, tags ); @@ -221,10 +221,8 @@ public class ArgumentMatch implements Iterable { nextSite = siteIterator.next(); if( sites.get(nextSite) != null ) { tokenIterator = sites.get(nextSite).iterator(); - if( tokenIterator.hasNext() ) { - nextToken = tokenIterator.next(); - break; - } + nextToken = tokenIterator.hasNext() ? tokenIterator.next() : null; + break; } } } @@ -280,6 +278,8 @@ public class ArgumentMatch implements Iterable { for ( final List siteValue : sites.values() ) { if ( siteValue != null ) values.addAll(siteValue); + else + values.add(null); } return values; } diff --git a/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentTypeDescriptor.java b/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentTypeDescriptor.java index b900ad085..5bfc5166b 100644 --- a/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentTypeDescriptor.java +++ b/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ArgumentTypeDescriptor.java @@ -736,8 +736,10 @@ class SimpleArgumentTypeDescriptor extends ArgumentTypeDescriptor { } else if (type.equals(File.class)) { result = value == null ? null : value.asFile(); } else { + if (value == null) + throw new MissingArgumentValueException(createDefaultArgumentDefinition(source)); Constructor ctor = type.getConstructor(String.class); - result = ctor.newInstance(value == null ? null : value.asString()); + result = ctor.newInstance(value.asString()); } } catch (UserException e) { throw e; diff --git a/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ParsingEngine.java b/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ParsingEngine.java index 6cc9ae97c..6244b8652 100644 --- a/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ParsingEngine.java +++ b/public/gatk-tools-public/src/main/java/org/broadinstitute/gatk/utils/commandline/ParsingEngine.java @@ -317,7 +317,7 @@ public class ParsingEngine { // 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()) + if(verifiableMatch.values().size() == 0 && !verifiableArgument.isFlag && !argumentSource.createsTypeDefault()) invalidValues.add(new Pair(verifiableArgument,null)); } diff --git a/public/gatk-tools-public/src/test/java/org/broadinstitute/gatk/utils/commandline/ParsingEngineUnitTest.java b/public/gatk-tools-public/src/test/java/org/broadinstitute/gatk/utils/commandline/ParsingEngineUnitTest.java index 35c6ac17b..b12d9506b 100644 --- a/public/gatk-tools-public/src/test/java/org/broadinstitute/gatk/utils/commandline/ParsingEngineUnitTest.java +++ b/public/gatk-tools-public/src/test/java/org/broadinstitute/gatk/utils/commandline/ParsingEngineUnitTest.java @@ -40,6 +40,8 @@ import java.io.File; import java.io.IOException; import java.util.List; import java.util.EnumSet; +import java.util.Set; + /** * Test suite for the parsing engine. */ @@ -135,7 +137,7 @@ public class ParsingEngineUnitTest extends BaseTest { Assert.assertEquals(argProvider.foo, 5, "Argument is not correctly initialized"); } - @Test(expectedExceptions=MissingArgumentValueException.class) + @Test(expectedExceptions=InvalidArgumentValueException.class) public void primitiveArgumentNoValueTest() { final String[] commandLine = new String[] {"--foo"}; @@ -948,6 +950,7 @@ public class ParsingEngineUnitTest extends BaseTest { } } + @SuppressWarnings("unused") private class NumericRangeArgProvider { @Argument(fullName = "intWithHardMinAndMax", minValue = 5, maxValue = 10) public int intWithHardMinAndMax; @@ -1089,4 +1092,50 @@ public class ParsingEngineUnitTest extends BaseTest { NumericRangeArgProvider argProvider = new NumericRangeArgProvider(); parsingEngine.loadArgumentsIntoObject(argProvider); } + + @SuppressWarnings("unused") + private class VariedTypeArgProvider { + @Argument(fullName = "intVal", required=false) + private int anInt; + + @Argument(fullName = "stringVal", required=false) + private String aString; + + @Argument(fullName = "enumVal", required=false) + private TestEnum anEnum; + + @Argument(fullName = "fileVal", required=false) + private File aFile; + + @Argument(fullName = "stringSet", required=false) + private Set someStrings; + + @Argument(fullName = "intervalVal", required=false) + private IntervalBinding anInterval; + } + + @DataProvider(name = "MissingArgumentValueDataProvider") + public Object[][] missingArgumentDataProvider() { + return new Object[][]{ + { new String[]{"--intVal"} }, + { new String[]{"--stringVal"} }, + { new String[]{"--enumVal"} }, + { new String[]{"--fileVal"} }, + { new String[]{"--stringSet"} }, + { new String[]{"--stringSet", "aha", "--stringSet"} }, + { new String[]{"--intervalVal"} } + }; + } + + @Test(dataProvider = "MissingArgumentValueDataProvider", + expectedExceptions = {InvalidArgumentValueException.class, MissingArgumentValueException.class}) + public void testMissingArguments( final String[] commandLine ) { + parsingEngine.addArgumentSource( VariedTypeArgProvider.class ); + parsingEngine.parse( commandLine ); + parsingEngine.validate(); + + VariedTypeArgProvider argProvider = new VariedTypeArgProvider(); + parsingEngine.loadArgumentsIntoObject(argProvider); + } + }