From 4d1fd17a97aa49e48c6163b84ff3ff15725bea66 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Fri, 19 Aug 2011 13:13:41 -0400 Subject: [PATCH] GATKDoclet cleanup and documentation -- Fixed bug in the way ArgumentCollections were handled that lead to failure in handling the dbsnp argument collection. --- .../sting/utils/help/GATKDoclet.java | 26 +- .../help/GenericDocumentationHandler.java | 263 +++++++++++------- 2 files changed, 189 insertions(+), 100 deletions(-) diff --git a/public/java/src/org/broadinstitute/sting/utils/help/GATKDoclet.java b/public/java/src/org/broadinstitute/sting/utils/help/GATKDoclet.java index 5755d2b37..de6ad359e 100644 --- a/public/java/src/org/broadinstitute/sting/utils/help/GATKDoclet.java +++ b/public/java/src/org/broadinstitute/sting/utils/help/GATKDoclet.java @@ -34,7 +34,10 @@ import org.apache.commons.io.FileUtils; import org.apache.log4j.Level; import org.apache.log4j.Logger; import org.broad.tribble.FeatureCodec; +import org.broadinstitute.sting.gatk.CommandLineGATK; +import org.broadinstitute.sting.gatk.walkers.qc.DocumentationTest; import org.broadinstitute.sting.utils.exceptions.ReviewedStingException; +import org.broadinstitute.sting.utils.exceptions.UserException; import java.io.*; import java.util.*; @@ -48,6 +51,7 @@ public class GATKDoclet { final protected static Logger logger = Logger.getLogger(GATKDoclet.class); protected static String buildTimestamp = null, absoluteVersion = null; protected static boolean showHiddenFeatures = false; + protected static boolean testOnly = false; RootDoc rootDoc; @@ -75,6 +79,8 @@ public class GATKDoclet { absoluteVersion = options[1]; if (options[0].equals("-include-hidden")) showHiddenFeatures = true; + if (options[0].equals("-test")) + testOnly = true; } GATKDoclet doclet = new GATKDoclet(); @@ -88,16 +94,26 @@ public class GATKDoclet { * @return Number of potential parameters; 0 if not supported. */ public static int optionLength(String option) { - if(option.equals("-build-timestamp") || option.equals("-absolute-version") || option.equals("-include-hidden")) { + if(option.equals("-build-timestamp") || + option.equals("-absolute-version") || + option.equals("-include-hidden")) { return 2; - } - return 0; + } else if ( option.equals("-test") ) + return 1; + else + return 0; } public boolean showHiddenFeatures() { return showHiddenFeatures; } + public static boolean testOnly() { + return testOnly; + } + + private static final List> testOnlyKeepers = Arrays.asList( + DocumentationTest.class, CommandLineGATK.class, UserException.class); public Set workUnits() { TreeSet m = new TreeSet(); @@ -105,6 +121,10 @@ public class GATKDoclet { //logger.debug("Considering " + doc); Class clazz = getClassForClassDoc(doc); + // don't add anything that's not DocumentationTest if we are in test mode + if ( clazz != null && testOnly && ! testOnlyKeepers.contains(clazz) ) + continue; + //if ( clazz != null && clazz.getName().equals("org.broadinstitute.sting.gatk.walkers.annotator.AlleleBalance")) // logger.debug("foo"); diff --git a/public/java/src/org/broadinstitute/sting/utils/help/GenericDocumentationHandler.java b/public/java/src/org/broadinstitute/sting/utils/help/GenericDocumentationHandler.java index d7add9af0..08e430c8a 100644 --- a/public/java/src/org/broadinstitute/sting/utils/help/GenericDocumentationHandler.java +++ b/public/java/src/org/broadinstitute/sting/utils/help/GenericDocumentationHandler.java @@ -24,6 +24,7 @@ package org.broadinstitute.sting.utils.help; +import com.google.java.contract.Ensures; import com.google.java.contract.Requires; import com.sun.javadoc.ClassDoc; import com.sun.javadoc.FieldDoc; @@ -31,8 +32,10 @@ import com.sun.javadoc.RootDoc; import com.sun.javadoc.Tag; import org.apache.log4j.Logger; import org.broad.tribble.Feature; +import org.broad.tribble.bed.FullBEDFeature; import org.broadinstitute.sting.commandline.*; import org.broadinstitute.sting.gatk.CommandLineGATK; +import org.broadinstitute.sting.gatk.arguments.DbsnpArgumentCollection; import org.broadinstitute.sting.gatk.refdata.tracks.FeatureManager; import org.broadinstitute.sting.utils.Utils; import org.broadinstitute.sting.utils.classloader.JVMUtils; @@ -49,14 +52,18 @@ import java.util.*; */ public class GenericDocumentationHandler extends DocumentedGATKFeatureHandler { private static Logger logger = Logger.getLogger(GenericDocumentationHandler.class); - GATKDocWorkUnit toProcess; - ClassDoc classdoc; - Set all; - RootDoc rootDoc; + + /** The Class we are documenting */ + private GATKDocWorkUnit toProcess; + + /** The set of all classes we are documenting, for cross-referencing */ + private Set all; + + /** The JavaDoc root */ + private RootDoc rootDoc; @Override public boolean includeInDocs(ClassDoc doc) { -// return true; try { Class type = HelpUtils.getClassForDoc(doc); return JVMUtils.isConcrete(type); @@ -76,7 +83,6 @@ public class GenericDocumentationHandler extends DocumentedGATKFeatureHandler { this.rootDoc = rootDoc; this.toProcess = toProcessArg; this.all = allArg; - this.classdoc = toProcess.classDoc; //System.out.printf("%s class %s%n", toProcess.group, toProcess.classDoc); Map root = new HashMap(); @@ -88,71 +94,76 @@ public class GenericDocumentationHandler extends DocumentedGATKFeatureHandler { toProcess.setHandlerContent((String)root.get("summary"), root); } + /** + * Add high-level summary information about toProcess to root, such as its + * name, summary, description, version, etc. + * + * @param root + */ protected void addHighLevelBindings(Map root) { - root.put("name", classdoc.name()); + root.put("name", toProcess.classDoc.name()); // Extract overrides from the doc tags. StringBuilder summaryBuilder = new StringBuilder(); - for(Tag tag: classdoc.firstSentenceTags()) + for(Tag tag: toProcess.classDoc.firstSentenceTags()) summaryBuilder.append(tag.text()); root.put("summary", summaryBuilder.toString()); - root.put("description", classdoc.commentText().substring(summaryBuilder.toString().length())); + root.put("description", toProcess.classDoc.commentText().substring(summaryBuilder.toString().length())); root.put("timestamp", toProcess.buildTimestamp); root.put("version", toProcess.absoluteVersion); - for(Tag tag: classdoc.tags()) { + for(Tag tag: toProcess.classDoc.tags()) { root.put(tag.name(), tag.text()); } } + /** + * Add bindings describing related GATK capabilites to toProcess + * @param root + */ + protected void addRelatedBindings(Map root) { + List> extraDocsData = new ArrayList>(); + + // add in all of the explicitly related items + for ( final Class extraDocClass : toProcess.annotation.extraDocs() ) { + final GATKDocWorkUnit otherUnit = GATKDoclet.findWorkUnitForClass(extraDocClass, all); + if ( otherUnit == null ) + throw new ReviewedStingException("Requested extraDocs for class without any documentation: " + extraDocClass); + extraDocsData.add( + new HashMap(){{ + put("filename", otherUnit.filename); + put("name", otherUnit.name);}}); + + } + root.put("extradocs", extraDocsData); + } + + /** + * Add information about all of the arguments available to toProcess to root + * + * @param root + */ protected void addArgumentBindings(Map root) { ParsingEngine parsingEngine = createStandardGATKParsingEngine(); - // attempt to instantiate the class - Object instance = makeInstanceIfPossible(toProcess.clazz); - - Map>> args = new HashMap>>(); + Map>> args = createArgumentMap(); root.put("arguments", args); - args.put("all", new ArrayList>()); - args.put("required", new ArrayList>()); - args.put("optional", new ArrayList>()); - args.put("advanced", new ArrayList>()); - args.put("hidden", new ArrayList>()); - args.put("depreciated", new ArrayList>()); try { - for ( ArgumentSource argumentSource : parsingEngine.extractArgumentSources(HelpUtils.getClassForDoc(classdoc)) ) { + // loop over all of the arguments according to the parsing engine + for ( final ArgumentSource argumentSource : parsingEngine.extractArgumentSources(HelpUtils.getClassForDoc(toProcess.classDoc)) ) { + // todo -- why can you have multiple ones? ArgumentDefinition argDef = argumentSource.createArgumentDefinitions().get(0); - FieldDoc fieldDoc = getFieldDoc(classdoc, argumentSource.field.getName()); - Map argBindings = docForArgument(fieldDoc, argumentSource, argDef); // todo -- why can you have multiple ones? + FieldDoc fieldDoc = getFieldDoc(toProcess.classDoc, argumentSource.field.getName()); + Map argBindings = docForArgument(fieldDoc, argumentSource, argDef); if ( ! argumentSource.isHidden() || getDoclet().showHiddenFeatures() ) { - logger.debug(String.format("Processing %s", argumentSource)); - String kind = "optional"; - if ( argumentSource.isRequired() ) kind = "required"; - else if ( argumentSource.isAdvanced() ) kind = "advanced"; - else if ( argumentSource.isHidden() ) kind = "hidden"; - else if ( argumentSource.isDeprecated() ) kind = "depreciated"; + final String kind = docKindOfArg(argumentSource); - // get the value of the field - if ( instance != null ) { - Object value = getFieldValue(toProcess.clazz, instance, fieldDoc.name()); - - if ( value == null && argumentSource.createsTypeDefault() ) { - // handle the case where there's an implicit default - try { - value = argumentSource.typeDefaultDocString(); - } catch (ReviewedStingException e) { - ; // failed to create type default, don't worry about it - } - } - - if ( value != null ) - argBindings.put("defaultValue", prettyPrintValueString(value)); - } + final Object value = argumentValue(toProcess.clazz, argumentSource); + if ( value != null ) + argBindings.put("defaultValue", prettyPrintValueString(value)); args.get(kind).add(argBindings); args.get("all").add(argBindings); - } else { - logger.debug(String.format("Skipping hidden feature %s", argumentSource)); } } @@ -165,11 +176,78 @@ public class GenericDocumentationHandler extends DocumentedGATKFeatureHandler { } } + /** + * Return the argument kind (required, advanced, hidden, etc) of this argumentSource + * @param argumentSource + * @return + */ + @Requires("argumentSource != null") + @Ensures("result != null") + private String docKindOfArg(ArgumentSource argumentSource) { + if ( argumentSource.isRequired() ) return "required"; + else if ( argumentSource.isAdvanced() ) return "advanced"; + else if ( argumentSource.isHidden() ) return "hidden"; + else if ( argumentSource.isDeprecated() ) return "depreciated"; + else return "optional"; + } + + /** + * Attempts to determine the value of argumentSource in an instantiated version of c + * @param c + * @param argumentSource + * @return value of argumentSource, or null if this isn't possible + */ + @Requires({"c != null", "argumentSource != null"}) + private Object argumentValue(Class c, ArgumentSource argumentSource) { + // get the value of the field + // attempt to instantiate the class + final Object instance = makeInstanceIfPossible(toProcess.clazz); + if ( instance != null ) { + final Object value = getFieldValue(instance, argumentSource.field.getName()); + if ( value != null ) + return value; + + if ( argumentSource.createsTypeDefault() ) { + try { // handle the case where there's an implicit default + return argumentSource.typeDefaultDocString(); + } catch (ReviewedStingException e) { + ; // failed to create type default, don't worry about it + } + } + } + + return null; + } + + /** + * Create the argument map for holding class arguments + * @return + */ + private Map>> createArgumentMap() { + Map>> args = new HashMap>>(); + args.put("all", new ArrayList>()); + args.put("required", new ArrayList>()); + args.put("optional", new ArrayList>()); + args.put("advanced", new ArrayList>()); + args.put("hidden", new ArrayList>()); + args.put("depreciated", new ArrayList>()); + return args; + } + + + /** + * Sorts the individual argument list in unsorted according to CompareArgumentsByName + * @param unsorted + * @return + */ private List> sortArguments(List> unsorted) { Collections.sort(unsorted, new CompareArgumentsByName()); return unsorted; } + /** + * Sort arguments by case-insensitive comparison ignoring the -- and - prefixes + */ private class CompareArgumentsByName implements Comparator> { public int compare(Map x, Map y) { return elt(x).compareTo(elt(y)); @@ -186,25 +264,32 @@ public class GenericDocumentationHandler extends DocumentedGATKFeatureHandler { } } - private Object getFieldValue(Class c, Object instance, String fieldName) { - Field field = JVMUtils.findField(c, fieldName); - if ( field != null ) { - Object value = JVMUtils.getFieldValue(field, instance); - //System.out.printf("Fetched value of field %s in class %s: %s%n", fieldName, c, value); - return value; - } else { - return findFieldValueInArgumentCollections(c, instance, fieldName); - } - } - - private Object findFieldValueInArgumentCollections(Class c, Object instance, String fieldName) { - for ( Field field : JVMUtils.getAllFields(c) ) { + /** + * Utility function that finds the value of fieldName in any fields of ArgumentCollection fields in + * instance of class c. + * + * @param instance the object to query for the field value + * @param fieldName the name of the field we are looking for in instance + * @return The value assigned to field in the ArgumentCollection, otherwise null + */ + private Object getFieldValue(Object instance, String fieldName) { + // + // subtle note. If you have a field named X that is an ArgumentCollection that + // contains a field X as well, you need only consider fields in the argumentCollection, not + // matching the argument itself. + // + // @ArgumentCollection + // protected DbsnpArgumentCollection dbsnp = new DbsnpArgumentCollection(); + // + for ( Field field : JVMUtils.getAllFields(instance.getClass()) ) { if ( field.isAnnotationPresent(ArgumentCollection.class) ) { //System.out.printf("Searching for %s in argument collection field %s%n", fieldName, field); Object fieldValue = JVMUtils.getFieldValue(field, instance); - Object value = getFieldValue(fieldValue.getClass(), fieldValue, fieldName); + Object value = getFieldValue(fieldValue, fieldName); if ( value != null ) return value; + } else if ( field.getName().equals(fieldName) ) { + return JVMUtils.getFieldValue(field, instance); } } @@ -212,6 +297,8 @@ public class GenericDocumentationHandler extends DocumentedGATKFeatureHandler { } /** + * Pretty prints value + * * Assumes value != null * @param value * @return @@ -246,6 +333,11 @@ public class GenericDocumentationHandler extends DocumentedGATKFeatureHandler { return value.toString(); } + /** + * Attempt to instantiate class c, if possible. Returns null if this proves impossible. + * @param c + * @return + */ private Object makeInstanceIfPossible(Class c) { Object instance = null; try { @@ -265,47 +357,16 @@ public class GenericDocumentationHandler extends DocumentedGATKFeatureHandler { // this last one is super dangerous, but some of these methods catch ClassNotFoundExceptions // and rethrow then as RuntimeExceptions catch (RuntimeException e) {} -// finally { -// if ( instance == null ) -// logger.warn(String.format("Unable to create instance of class %s => %s", c, instance)); -// } return instance; } - protected void addRelatedBindings(Map root) { - List> extraDocsData = new ArrayList>(); - // add in all of the explicitly related items - for ( final Class extraDocClass : toProcess.annotation.extraDocs() ) { - final GATKDocWorkUnit otherUnit = GATKDoclet.findWorkUnitForClass(extraDocClass, all); - if ( otherUnit == null ) - throw new ReviewedStingException("Requested extraDocs for class without any documentation: " + extraDocClass); - extraDocsData.add( - new HashMap(){{ - put("filename", otherUnit.filename); - put("name", otherUnit.name);}}); - - } - root.put("extradocs", extraDocsData); - } - - private static final String classRelationship(Class me, Class other) { - if ( other.equals(me) ) - // no circular references - return null; - else if ( other.isAssignableFrom(me) ) - // toProcess is a superclass of other.clazz - return "superclass"; - else if ( me.isAssignableFrom(other) ) - // toProcess inherits from other.clazz - return "subclass"; - else - return null; - - } - - protected ParsingEngine createStandardGATKParsingEngine() { + /** + * Create an instance of the GATK parsing engine, for argument processing with GATKDoclet + * @return + */ + private ParsingEngine createStandardGATKParsingEngine() { CommandLineProgram clp = new CommandLineGATK(); try { CommandLineProgram.start(clp, new String[]{}, true); @@ -315,6 +376,14 @@ public class GenericDocumentationHandler extends DocumentedGATKFeatureHandler { } } + /** + * Gets the javadocs associated with field name in classDoc. Throws a + * runtime exception if this proves impossible. + * + * @param classDoc + * @param name + * @return + */ private FieldDoc getFieldDoc(ClassDoc classDoc, String name) { return getFieldDoc(classDoc, name, true); }