Fix fixes based on testing, fix javadoc process arts, use FIELD kind …

…elements only, use canonical class name.

src/main/java/org/broadinstitute/barclay/argparser/CommandLinePluginProvider.java

@@ -1,6 +1,5 @@
 package org.broadinstitute.barclay.argparser;
 
-import java.util.ArrayList;
 import java.util.List;
 
 /**

src/main/java/org/broadinstitute/barclay/help/BashTabCompletionDoclet.java

@@ -3,7 +3,6 @@
 import freemarker.template.Configuration;
 import freemarker.template.Template;
 import freemarker.template.TemplateException;
-import jdk.javadoc.doclet.DocletEnvironment;
 import org.broadinstitute.barclay.argparser.CommandLineProgramProperties;
 
 import javax.lang.model.element.Element;

src/main/java/org/broadinstitute/barclay/help/DefaultDocWorkUnitHandler.java

@@ -1,5 +1,6 @@
 package org.broadinstitute.barclay.help;
 
+import jdk.javadoc.doclet.DocletEnvironment;
 import org.apache.commons.lang3.tuple.Pair;
 
 import org.apache.logging.log4j.LogManager;
@@ -9,9 +10,9 @@
 import org.broadinstitute.barclay.utils.Utils;
 
 import javax.lang.model.element.Element;
+import javax.lang.model.element.ElementKind;
 import java.lang.reflect.*;
 import java.util.*;
-import java.util.stream.Collectors;
 
 
 /**
@@ -64,7 +65,7 @@ public String getSummaryForWorkUnit(final DocWorkUnit workUnit) {
             }
             if (summary == null || summary.isEmpty()) {
                 // If no summary was found from annotations, use the javadoc if there is any
-                summary = JavaLanguageModelScanners.getDocComment(getDoclet().getDocletEnv(), workUnit.getDocElement());
+                summary = JavaLanguageModelScanners.getDocCommentFirstSentence(getDoclet().getDocletEnv(), workUnit.getDocElement());
             }
         }
 
@@ -235,6 +236,9 @@ protected void addHighLevelBindings(final DocWorkUnit workUnit)
         workUnit.setProperty(TemplateProperties.FEATURE_DEPRECATED, workUnit.isDeprecatedFeature());
         workUnit.setProperty(TemplateMapConstants.FEATURE_DEPRECATION_DETAIL, workUnit.getDeprecationDetail());
 
+        //this property is called "description" for work units, but "fulltext" for arguments). ideally
+        // these would be unified, but it would require a lot of downstream changes to templates and test
+        // result expected files
         workUnit.setProperty("description", getDescription(workUnit));
 
         workUnit.setProperty("version", getDoclet().getBuildVersion());
@@ -250,7 +254,7 @@ protected void addHighLevelBindings(final DocWorkUnit workUnit)
     protected void addCustomBindings(final DocWorkUnit currentWorkUnit) {
         final String tagFilterPrefix = getTagPrefix();
         if (tagFilterPrefix != null) {
-            final Map<String, List<String>> parts = JavaLanguageModelScanners.getInlineTags(
+            final Map<String, List<String>> parts = JavaLanguageModelScanners.getUnknownInlineTags(
                     getDoclet().getDocletEnv(),
                     currentWorkUnit.getDocElement());
             // create properties for any custom tags
@@ -394,12 +398,39 @@ protected void processNamedArgument(
     }
 
     private String getDocCommentForField(final DocWorkUnit workUnit, final NamedArgumentDefinition argDef) {
-        final Element fieldElement = JavaLanguageModelScanners.getElementForField(getDoclet().getDocletEnv(), workUnit.getDocElement(), argDef.getUnderlyingField());
+        // first, see if the field (@Argument) we're looking for is declared directly in the work unit's class
+        Element fieldElement = JavaLanguageModelScanners.getElementForField(
+                getDoclet().getDocletEnv(),
+                workUnit.getDocElement(),
+                argDef.getUnderlyingField(),
+                ElementKind.FIELD);
         if (fieldElement == null) {
+            // the field isn't defined directly in the workunit's enclosing class/element, so it must be
+            // defined in a class that is referenced by the workunit's enclosing class. find that class and get
+            // it's type element
+            Class<?> containingClass = argDef.getUnderlyingField().getDeclaringClass();
+            if (containingClass != null) {
+                String className = containingClass.getCanonicalName();
+                // className can be null if the containing class is an anonymous class, in which case we don't
+                // want to traverse the containment hierarchy because we'll just wind up back at the work unit
+                // class, which we don't want, so in order to be compatible with the way the old javadoc used
+                // to work, just bail...
+                if (className != null) {
+                    final Element classElement = getDoclet().getDocletEnv().getElementUtils().getTypeElement(className);
+                    fieldElement = JavaLanguageModelScanners.getElementForField(
+                            getDoclet().getDocletEnv(),
+                            classElement,
+                            argDef.getUnderlyingField(),
+                            ElementKind.FIELD);
+                }
+            }
+        }
+        if (fieldElement != null) {
+            final String comment = JavaLanguageModelScanners.getDocCommentWithoutTags(getDoclet().getDocletEnv(), fieldElement);
+            return comment == null ? "" : comment;
+        } else {
             return "";
         }
-        final String comment = JavaLanguageModelScanners.getDocComment(getDoclet().getDocletEnv(), fieldElement);
-        return comment == null ? "" : comment;
     }
 
     /**
@@ -722,32 +753,77 @@ private List<Map<String, String>> getPossibleValues(final ArgumentDefinition arg
         }
 
         return targetClass.isEnum() ?
-                docForEnumArgument(targetClass) :
+                docForEnumArgument(argDef, targetClass) :
                 Collections.emptyList();
     }
 
     /**
      * Helper routine that provides a FreeMarker map for an enumClass, grabbing the
      * values of the enum and their associated javadoc or {@link CommandLineArgumentParser.ClpEnum} documentation.
      *
+     * @param argDef argument definition for the argument of the enum type
      * @param enumClass an enum Class that may optionally implement {@link CommandLineArgumentParser.ClpEnum}
      * @return a List of maps with keys for "name" and "summary" for each of the class's possible enum constants
+     * @param <T> type param for this enum
+     *
      */
     @SuppressWarnings("unchecked")
-    private <T extends Enum<T>> List<Map<String, String>> docForEnumArgument(final Class<?> enumClass) {
+    private <T extends Enum<T>> List<Map<String, String>> docForEnumArgument(
+            final ArgumentDefinition argDef,
+            final Class<?> enumClass) {
         final List<Map<String, String>> bindings = new ArrayList<>();
+        final T[] enumConstants = (T[]) enumClass.getEnumConstants();
         if (CommandLineArgumentParser.ClpEnum.class.isAssignableFrom(enumClass)) {
-            final T[] enumConstants = (T[]) enumClass.getEnumConstants();
             for ( final T enumConst : enumConstants ) {
                 bindings.add(createPossibleValuesMap(
                         enumConst.name(),
                         ((CommandLineArgumentParser.ClpEnum) enumConst).getHelpDoc()));
             }
+        } else {
+            final String canonicalClassName = enumClass.getCanonicalName();
+            final Element enumClassElement = getDoclet().getDocletEnv().getElementUtils().getTypeElement(canonicalClassName);
+            if (enumClassElement != null ) {
+                for ( final T enumConst : enumConstants ) {
+                    final Field enumConstField = getFieldForEnumConstant(enumClass, enumConst.name());
+                    if (enumConstField != null) {
+                        final Element fieldElement = JavaLanguageModelScanners.getElementForField(
+                                getDoclet().getDocletEnv(),
+                                enumClassElement,
+                                enumConstField,
+                                ElementKind.ENUM_CONSTANT);
+                        if (fieldElement != null) {
+                            final String comment = JavaLanguageModelScanners.getDocCommentWithoutTags(
+                                    getDoclet().getDocletEnv(),
+                                    fieldElement);
+                            bindings.add(createPossibleValuesMap(
+                                    enumConst.name(),
+                                    comment));
+                        } else {
+                            bindings.add(createPossibleValuesMap(
+                                    enumConst.name(),
+                                    "")); // empty string since there is no detail
+                        }
+                    } else {
+                        bindings.add(createPossibleValuesMap(
+                                enumConst.name(),
+                                "No documentation available"));
+                    }
+                }
+            }
         }
 
         return bindings;
     }
 
+    private <T> Field getFieldForEnumConstant(final Class<?> enumClass, final String enumConstantName) {
+        for (final Field enumClassField : enumClass.getFields()) {
+            if (enumClassField.getName().equals(enumConstantName)) {
+                return enumClassField;
+            }
+        }
+        return null;
+    }
+
     private HashMap<String, String> createPossibleValuesMap(final String name, final String summary) {
         return new HashMap<String, String>() {
             static final long serialVersionUID = 0L;

src/main/java/org/broadinstitute/barclay/help/DocWorkUnit.java

@@ -1,6 +1,5 @@
 package org.broadinstitute.barclay.help;
 
-import jdk.javadoc.doclet.DocletEnvironment;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.broadinstitute.barclay.argparser.CommandLineException;
@@ -238,4 +237,47 @@ public CommandLineProgramGroup getCommandLineProgramGroup() {
     public int compareTo(DocWorkUnit other) {
         return this.name.compareTo(other.name);
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof DocWorkUnit)) return false;
+
+        DocWorkUnit that = (DocWorkUnit) o;
+
+        if (!name.equals(that.name)) return false;
+        if (!clazz.equals(that.clazz)) return false;
+        if (!docElement.equals(that.docElement)) return false;
+        if (documentedFeature != null ? !documentedFeature.equals(that.documentedFeature) :
+                that.documentedFeature != null)
+            return false;
+        if (commandLineProperties != null ? !commandLineProperties.equals(that.commandLineProperties) :
+                that.commandLineProperties != null)
+            return false;
+        if (experimentalFeature != null ? !experimentalFeature.equals(that.experimentalFeature) :
+                that.experimentalFeature != null)
+            return false;
+        if (betaFeature != null ? !betaFeature.equals(that.betaFeature) : that.betaFeature != null) return false;
+        if (!propertyMap.equals(that.propertyMap)) return false;
+        if (summary != null ? !summary.equals(that.summary) : that.summary != null) return false;
+        if (groupName != null ? !groupName.equals(that.groupName) : that.groupName != null) return false;
+        return groupSummary != null ? groupSummary.equals(that.groupSummary) : that.groupSummary == null;
+    }
+
+    @Override
+    public int hashCode() {
+        int result = name.hashCode();
+        result = 31 * result + clazz.hashCode();
+        result = 31 * result + docElement.hashCode();
+        result = 31 * result + (documentedFeature != null ? documentedFeature.hashCode() : 0);
+        result = 31 * result + (commandLineProperties != null ? commandLineProperties.hashCode() : 0);
+        result = 31 * result + (experimentalFeature != null ? experimentalFeature.hashCode() : 0);
+        result = 31 * result + (betaFeature != null ? betaFeature.hashCode() : 0);
+        result = 31 * result + propertyMap.hashCode();
+        result = 31 * result + (summary != null ? summary.hashCode() : 0);
+        result = 31 * result + (groupName != null ? groupName.hashCode() : 0);
+        result = 31 * result + (groupSummary != null ? groupSummary.hashCode() : 0);
+        return result;
+    }
+
 }

src/main/java/org/broadinstitute/barclay/help/DocletUtils.java

@@ -1,39 +1,39 @@
 package org.broadinstitute.barclay.help;
 
-import jdk.javadoc.doclet.DocletEnvironment;
-
 import javax.lang.model.element.Element;
-import javax.lang.model.element.ElementKind;
 import javax.lang.model.element.PackageElement;
-import javax.lang.model.element.TypeElement;
-import javax.lang.model.type.DeclaredType;
-import javax.lang.model.type.TypeKind;
-import javax.lang.model.type.TypeMirror;
+import javax.tools.Diagnostic;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
+import jdk.javadoc.doclet.DocletEnvironment;
+import jdk.javadoc.doclet.Reporter;
 
 /**
  * Package protected - Methods in the class must ONLY be used by doclets, since the com.sun.javadoc.* classes
  * are not available on all systems, and we don't want the GATK proper to depend on them.
  */
 public class DocletUtils {
-
-    public static Class<?> getClassForDeclaredElement(final Element docElement, final DocletEnvironment docEnv) {
-         return getClassForClassName(getClassName(docElement, docEnv));
+    public static Class<?> getClassForDeclaredElement(
+            final Element docElement,
+            final DocletEnvironment docEnv,
+            final Reporter reporter) {
+         return getClassForClassName(getClassName(docElement, docEnv), reporter);
     }
 
-    public static Class<?> getClassForClassName(final String className) {
+    public static Class<?> getClassForClassName(final String className, final Reporter reporter) {
         try {
             return Class.forName(className);
         } catch (ClassNotFoundException | NoClassDefFoundError e) {
             // we got an Element for a class we can't find.  Maybe in a library or something
             return null;
+        } catch (IncompatibleClassChangeError e) {
+            reporter.print(Diagnostic.Kind.WARNING, String.format(
+                    "Unexpected class change error for class %s ignored: %s", e.getMessage(), className));
+            return null;
         }
     }
 
     /**
-     * Reconstitute the class name from the given class JavaDoc object.
+     * Reconstitute the class name from the given class Element.
      *
      * @param element the Element for a class.
      * @return The (string) class name of the given class. Maybe null if no class can be found.

src/main/java/org/broadinstitute/barclay/help/HelpDoclet.java

@@ -14,8 +14,8 @@
 
 import jdk.javadoc.doclet.DocletEnvironment;
 import jdk.javadoc.doclet.Doclet;
-
 import jdk.javadoc.doclet.Reporter;
+
 import org.broadinstitute.barclay.argparser.Hidden;
 import org.broadinstitute.barclay.help.scanners.JavaLanguageModelScanners;
 import org.broadinstitute.barclay.utils.JVMUtils;
@@ -131,14 +131,14 @@ public boolean process(String option, List<String> arguments) {
             add(new BarclayDocletOption.SimpleStandardOption(BarclayDocletOptions.WINDOW_TITLE_OPTION) {
                 @Override
                 public boolean process(String option, List<String> arguments) {
-                    return false;
+                    return true;
                 }
             });
 
             add(new BarclayDocletOption.SimpleStandardOption(BarclayDocletOptions.DOC_TITLE_OPTION) {
                 @Override
                 public boolean process(String option, List<String> arguments) {
-                    return false;
+                    return true;
                 }
             });
 
@@ -150,17 +150,17 @@ public boolean process(String option, List<String> arguments) {
                     null) {
                 @Override
                 public boolean process(String option, List<String> arguments) {
-                    return false;
+                    // the gradle javadoc task includes this since its a standard doclet arg
+                    // so we need to tolerate it, but ignore it
+                    return true;
                 }
             });
 
-            // custom Barclay options
-
             add(new BarclayDocletOption(
                     Arrays.asList(BarclayDocletOptions.SETTINGS_DIR_OPTION),
                     "settings dir",
                     1,
-                    Option.Kind.OTHER,
+                    Option.Kind.STANDARD,
                     "<string>") {
                 @Override
                 public boolean process(String option, List<String> arguments) {
@@ -169,6 +169,9 @@ public boolean process(String option, List<String> arguments) {
                     return true;
                 }
             });
+
+            // custom Barclay options
+
             add(new BarclayDocletOption(
                     Arrays.asList(BarclayDocletOptions.BUILD_TIMESTAMP_OPTION),
                     "build timestamp",

src/main/java/org/broadinstitute/barclay/help/WDLDoclet.java

@@ -3,7 +3,6 @@
 import freemarker.template.Configuration;
 import freemarker.template.Template;
 import freemarker.template.TemplateException;
-import jdk.javadoc.doclet.DocletEnvironment;
 import org.broadinstitute.barclay.argparser.CommandLineProgramProperties;
 import org.broadinstitute.barclay.argparser.WorkflowProperties;
 

src/main/java/org/broadinstitute/barclay/help/scanners/CommentScanner.java

@@ -33,7 +33,6 @@ class CommentScanner extends DocTreeScanner<Void, Void> {
     @Override
     public Void scan(final DocTree docTree, final Void unused) {
         Utils.nonNull(docTree, "DocTree");
-
         if (docTree.getKind().equals(DocTree.Kind.DOC_COMMENT)) {
             targetComment = docTree.toString();
             return null;

src/main/java/org/broadinstitute/barclay/help/scanners/CommentScannerWithTagFilter.java

@@ -21,7 +21,7 @@ class CommentScannerWithTagFilter extends DocTreeScanner<Void, Void> {
 
     /**
      * For internal use only. External callers should use
-     * {@link JavaLanguageModelScanners#getDocCommentWithoutTags(DocletEnvironment, DocTree)} getDocCommentWithoutTags}
+     * {@link JavaLanguageModelScanners#getDocCommentWithoutTags(DocletEnvironment, Element)}}
      *
      * @param docEnv the {@link DocletEnvironment}
      * @param docTree the {@link DocTree} for which the javadoc comment should be retrieved
@@ -50,7 +50,7 @@ public Void scan(final DocTree t, final Void unused) {
      * @return the javadoc comment for the {@link Element}, with any embedded tags removed. If no javadoc
      * comment is present, an empty string will be returned.
      */
-     String getComment() {
+     String getCommentWithoutTags() {
         return targetDocCommentParts.stream().collect(Collectors.joining(" "));
     }
 

src/main/java/org/broadinstitute/barclay/help/scanners/DocumentedFeatureScanner.java

@@ -49,7 +49,7 @@ public class DocumentedFeatureScanner extends ElementScanner14<Void, Void> {
     @Override
     public Void scan(final Element e, final Void unused) {
         if (e.asType().getKind().equals(TypeKind.DECLARED)) {
-            final Class<?> clazz = DocletUtils.getClassForDeclaredElement(e, docEnv);
+            final Class<?> clazz = DocletUtils.getClassForDeclaredElement(e, docEnv, reporter);
             final DocumentedFeature documentedFeature = DocletUtils.getDocumentedFeatureForClass(clazz);
             if (documentedFeature != null) {
                 if (documentedFeature.enable() && helpDoclet.includeInDocs(documentedFeature, clazz)) {