GATKDoclet cleanup and documentation

-- Fixed bug in the way ArgumentCollections were handled that lead to failure in handling the dbsnp argument collection.

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;
-        }
+        } else if ( option.equals("-test") )
-        return 0;
+            return 1;
+        else
+            return 0;
     }
 
     public boolean showHiddenFeatures() {
         return showHiddenFeatures;
     }
 
+    public static boolean testOnly() {
+        return testOnly;
+    }
+
+    private static final List<Class<?>> testOnlyKeepers = Arrays.asList(
+            DocumentationTest.class, CommandLineGATK.class, UserException.class);
     public Set<GATKDocWorkUnit> workUnits() {
         TreeSet<GATKDocWorkUnit> m = new TreeSet<GATKDocWorkUnit>();
 
@@ -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");
 

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;
+    /** The Class we are documenting */
-    Set<GATKDocWorkUnit> all;
+    private GATKDocWorkUnit toProcess;
-    RootDoc rootDoc;
+
+    /** The set of all classes we are documenting, for cross-referencing */
+    private Set<GATKDocWorkUnit> 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<String, Object> root = new HashMap<String, Object>();
@@ -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<String, Object> 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<String, Object> root) {
+        List<Map<String, Object>> extraDocsData = new ArrayList<Map<String, Object>>();
+
+        // 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<String, Object>(){{
+                        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<String, Object> root) {
         ParsingEngine parsingEngine = createStandardGATKParsingEngine();
 
-        // attempt to instantiate the class
+        Map<String, List<Map<String, Object>>> args = createArgumentMap();
-        Object instance = makeInstanceIfPossible(toProcess.clazz);
-
-        Map<String, List<Map<String, Object>>> args = new HashMap<String, List<Map<String, Object>>>();
         root.put("arguments", args);
-        args.put("all", new ArrayList<Map<String, Object>>());
-        args.put("required", new ArrayList<Map<String, Object>>());
-        args.put("optional", new ArrayList<Map<String, Object>>());
-        args.put("advanced", new ArrayList<Map<String, Object>>());
-        args.put("hidden", new ArrayList<Map<String, Object>>());
-        args.put("depreciated", new ArrayList<Map<String, Object>>());
         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());
+                FieldDoc fieldDoc = getFieldDoc(toProcess.classDoc, argumentSource.field.getName());
-                Map<String, Object> argBindings = docForArgument(fieldDoc, argumentSource, argDef); // todo -- why can you have multiple ones?
+                Map<String, Object> argBindings = docForArgument(fieldDoc, argumentSource, argDef);
                 if ( ! argumentSource.isHidden() || getDoclet().showHiddenFeatures() ) {
-                    logger.debug(String.format("Processing %s", argumentSource));
+                    final String kind = docKindOfArg(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";
 
-                    // get the value of the field
+                    final Object value = argumentValue(toProcess.clazz, argumentSource);
-                    if ( instance != null ) {
+                    if ( value != null )
-                        Object value = getFieldValue(toProcess.clazz, instance, fieldDoc.name());
+                        argBindings.put("defaultValue", prettyPrintValueString(value));
-
-                        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));
-                    }
 
                     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<String, List<Map<String, Object>>> createArgumentMap() {
+        Map<String, List<Map<String, Object>>> args = new HashMap<String, List<Map<String, Object>>>();
+        args.put("all", new ArrayList<Map<String, Object>>());
+        args.put("required", new ArrayList<Map<String, Object>>());
+        args.put("optional", new ArrayList<Map<String, Object>>());
+        args.put("advanced", new ArrayList<Map<String, Object>>());
+        args.put("hidden", new ArrayList<Map<String, Object>>());
+        args.put("depreciated", new ArrayList<Map<String, Object>>());
+        return args;
+    }
+
+
+    /**
+     * Sorts the individual argument list in unsorted according to CompareArgumentsByName
+     * @param unsorted
+     * @return
+     */
     private List<Map<String, Object>> sortArguments(List<Map<String, Object>> unsorted) {
         Collections.sort(unsorted, new CompareArgumentsByName());
         return unsorted;
     }
 
+    /**
+     * Sort arguments by case-insensitive comparison ignoring the -- and - prefixes
+     */
     private class CompareArgumentsByName implements Comparator<Map<String, Object>> {
         public int compare(Map<String, Object> x, Map<String, Object> 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);
+     * Utility function that finds the value of fieldName in any fields of ArgumentCollection fields in
-        if ( field != null ) {
+     * instance of class c.
-            Object value = JVMUtils.getFieldValue(field, instance);
+     *
-            //System.out.printf("Fetched value of field %s in class %s: %s%n", fieldName, c, value);
+     * @param instance the object to query for the field value
-            return value;
+     * @param fieldName the name of the field we are looking for in instance
-        } else {
+     * @return The value assigned to field in the ArgumentCollection, otherwise null
-            return findFieldValueInArgumentCollections(c, instance, fieldName);
+     */
-        }
+    private Object getFieldValue(Object instance, String fieldName) {
-    }
+        //
-
+        // subtle note.  If you have a field named X that is an ArgumentCollection that
-    private Object findFieldValueInArgumentCollections(Class c, Object instance, String fieldName) {
+        // contains a field X as well, you need only consider fields in the argumentCollection, not
-        for ( Field field : JVMUtils.getAllFields(c) ) {
+        // 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<String, Object> root) {
-        List<Map<String, Object>> extraDocsData = new ArrayList<Map<String, Object>>();
 
-        // add in all of the explicitly related items
+    /**
-        for ( final Class extraDocClass : toProcess.annotation.extraDocs() ) {
+     * Create an instance of the GATK parsing engine, for argument processing with GATKDoclet
-            final GATKDocWorkUnit otherUnit = GATKDoclet.findWorkUnitForClass(extraDocClass, all);
+     * @return
-            if ( otherUnit == null )
+     */
-                throw new ReviewedStingException("Requested extraDocs for class without any documentation: " + extraDocClass);
+    private ParsingEngine createStandardGATKParsingEngine() {
-            extraDocsData.add(
-                    new HashMap<String, Object>(){{
-                        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() {
         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);
     }