changeset 37752:4243173b58db

8155061: javadoc incorrectly sorted items in All Classes list and Index files Reviewed-by: jjg
author ksrini
date Fri, 29 Apr 2016 16:06:52 -0700
parents 77e7bb904a13
children dbc87c966861
files langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/IndexBuilder.java langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java langtools/test/jdk/javadoc/doclet/testOrdering/TestOrdering.java
diffstat 3 files changed, 115 insertions(+), 29 deletions(-) [+]
line wrap: on
line diff
--- a/langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/IndexBuilder.java	Fri Apr 29 19:53:19 2016 -0700
+++ b/langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/IndexBuilder.java	Fri Apr 29 16:06:52 2016 -0700
@@ -73,6 +73,7 @@
 
     private final Configuration configuration;
     private final Utils utils;
+    private final Comparator<Element> comparator;
 
     /**
      * Constructor. Build the index map.
@@ -106,6 +107,9 @@
         this.classesOnly = classesOnly;
         this.javafx = configuration.javafx;
         this.indexmap = new TreeMap<>();
+        comparator = classesOnly
+                ? utils.makeAllClassesComparator()
+                : utils.makeIndexUseComparator();
         buildIndexMap(configuration.root);
     }
 
@@ -175,7 +179,7 @@
                           Character.toUpperCase(name.charAt(0));
                 Character unicode = ch;
                 SortedSet<Element> list = indexmap.computeIfAbsent(unicode,
-                        c -> new TreeSet<>(utils.makeIndexUseComparator()));
+                        c -> new TreeSet<>(comparator));
                 list.add(element);
             }
         }
--- a/langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java	Fri Apr 29 19:53:19 2016 -0700
+++ b/langtools/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java	Fri Apr 29 16:06:52 2016 -0700
@@ -1628,6 +1628,7 @@
 
     /**
      * Comparator for ModuleElements, simply compares the fully qualified names
+     * @return a Comparator
      */
     public Comparator<Element> makeModuleComparator() {
         return new Utils.ElementComparator<Element>() {
@@ -1639,7 +1640,28 @@
     }
 
     /**
-     * Comparator for PackageElements, simply compares the fully qualified names
+     * Returns a Comparator for all classes, compares the simple names of
+     * TypeElement, if equal then the fully qualified names.
+     *
+     * @return Comparator
+     */
+    public Comparator<Element> makeAllClassesComparator() {
+        return new Utils.ElementComparator<Element>() {
+            @Override
+            public int compare(Element e1, Element e2) {
+                int result =  compareNames(e1, e2);
+                if (result == 0)
+                    result =  compareFullyQualifiedNames(e1, e2);
+
+                return result;
+            }
+        };
+    }
+
+    /**
+     * Returns a Comparator for packages, by comparing the fully qualified names.
+     *
+     * @return a Comparator
      */
     public Comparator<Element> makePackageComparator() {
         return new Utils.ElementComparator<Element>() {
@@ -1650,6 +1672,10 @@
         };
     }
 
+    /**
+     * Returns a Comparator for SerialFieldTree.
+     * @return a Comparator
+     */
     public Comparator<SerialFieldTree> makeSerialFieldTreeComparator() {
         return (SerialFieldTree o1, SerialFieldTree o2) -> {
             String s1 = o1.getName().toString();
@@ -1659,18 +1685,19 @@
     }
 
     /**
-     * Comparator for General Purpose
-     * @return a ElementComparatorForClassUse
+     * Returns a general purpose comparator.
+     * @return a Comparator
      */
     public Comparator<Element> makeGeneralPurposeComparator() {
         return makeClassUseComparator();
     }
 
     /**
-     * A Comparator for Overrides and Implements use used on ExecutableElements
-     * compares the name first, then compares the SimpleName of the enclosing
-     * class and the FullyQualifiedName of the enclosing class.
-     * @return
+     * Returns a Comparator for overrides and implements,
+     * used primarily on methods, compares the name first,
+     * then compares the simple names of the enclosing
+     * TypeElement and the fully qualified name of the enclosing TypeElement.
+     * @return a Comparator
      */
     public Comparator<Element> makeOverrideUseComparator() {
         return new Utils.ElementComparator<Element>() {
@@ -1696,21 +1723,24 @@
     }
 
     /**
-     * A comparator for index file presentations, and are sorted as follows:
+     * Returns a Comparator for index file presentations, and are sorted as follows.
+     *  If comparing packages then simply compare the qualified names, otherwise
      *  1. sort on simple names of entities
      *  2. if equal, then compare the ElementKind ex: Package, Interface etc.
      *  3a. if equal and if the type is of ExecutableElement(Constructor, Methods),
      *      a case insensitive comparison of parameter the type signatures
      *  3b. if equal, case sensitive comparison of the type signatures
      *  4. finally, if equal, compare the FQNs of the entities
+     * Iff comparing packages then simply sort on qualified names.
      * @return a comparator for index file use
      */
     public Comparator<Element> makeIndexUseComparator() {
         return new Utils.ElementComparator<Element>() {
             /**
-             * Compare two given elements, first sort on names, then on the kinds,
-             * then on the parameters only if the type is an instance of ExecutableElement,
-             * the parameters are compared and finally the fully qualified names.
+             * Compare two given elements, if comparing two packages, return the
+             * comparison of FullyQualifiedName, first sort on names, then on the
+             * kinds, then on the parameters only if the type is an ExecutableElement,
+             * the parameters are compared and finally the qualified names.
              *
              * @param e1 - an element.
              * @param e2 - an element.
@@ -1719,10 +1749,7 @@
              */
             @Override
             public int compare(Element e1, Element e2) {
-                int result = compareElementTypeKinds(e1, e2);
-                if (result != 0) {
-                    return result;
-                }
+                int result = 0;
                 if (isPackage(e1) && isPackage(e2)) {
                     return compareFullyQualifiedNames(e1, e2);
                 }
@@ -1730,6 +1757,10 @@
                 if (result != 0) {
                     return result;
                 }
+                result = compareElementTypeKinds(e1, e2);
+                if (result != 0) {
+                    return result;
+                }
                 if (hasParameters(e1)) {
                     List<? extends VariableElement> parameters1 = ((ExecutableElement)e1).getParameters();
                     List<? extends VariableElement> parameters2 = ((ExecutableElement)e2).getParameters();
--- a/langtools/test/jdk/javadoc/doclet/testOrdering/TestOrdering.java	Fri Apr 29 19:53:19 2016 -0700
+++ b/langtools/test/jdk/javadoc/doclet/testOrdering/TestOrdering.java	Fri Apr 29 16:06:52 2016 -0700
@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8039410 8042601 8042829 8049393 8050031
+ * @bug 8039410 8042601 8042829 8049393 8050031 8155061
  * @summary test to determine if members are ordered correctly
  * @author ksrini
  * @library ../lib/
@@ -72,6 +72,8 @@
         checkOrder("pkg1/class-use/UsedClass.html", expectedInnerClassContructors);
         checkOrder("pkg1/ImplementsOrdering.html", expectedImplementsOrdering);
         checkOrder("pkg1/OverrideOrdering.html", expectedOverrideOrdering);
+        checkOrder("allclasses-noframe.html", expectedAllClasses);
+        checkOrder("allclasses-frame.html", expectedAllClasses);
     }
 
     enum ListOrder { NONE, REVERSE, SHUFFLE };
@@ -179,6 +181,22 @@
     };
     String[] composeTestVectors() {
         List<String> testList = new ArrayList<>();
+
+        testList.addAll(Arrays.asList(expectedPackageOrdering));
+
+        for (String x : expectedMethodOrdering) {
+            testList.add(x);
+            for (int i = 0; i < MAX_PACKAGES; i++) {
+                String wpkg = "add" + i;
+                testList.add(wpkg + "/" + x);
+                String dpkg = wpkg;
+                for (int j = 1; j < MAX_SUBPACKAGES_DEPTH; j++) {
+                    dpkg = dpkg + "/" + "add";
+                    testList.add(dpkg + "/" + x);
+                }
+            }
+        }
+
         for (String x : expectedEnumOrdering) {
             testList.add(x.replace("REPLACE_ME", "&lt;Unnamed&gt;"));
             for (int i = 0; i < MAX_PACKAGES; i++) {
@@ -194,20 +212,9 @@
 
         testList.addAll(Arrays.asList(expectedFieldOrdering));
 
-        for (String x : expectedMethodOrdering) {
-            testList.add(x);
-            for (int i = 0; i < MAX_PACKAGES; i++) {
-                String wpkg = "add" + i;
-                testList.add(wpkg + "/" + x);
-                String dpkg = wpkg;
-                for (int j = 1; j < MAX_SUBPACKAGES_DEPTH; j++) {
-                    dpkg = dpkg + "/" + "add";
-                    testList.add(dpkg + "/" + x);
-                }
-            }
-        }
         return testList.toArray(new String[testList.size()]);
     }
+
     void checkExecutableMemberOrdering(String usePage) {
         String contents = readFile(usePage);
         // check constructors
@@ -309,6 +316,22 @@
         return in.replace("/", ".");
     }
 
+    final String expectedAllClasses[] = {
+        "pkg1/A.html\" title=\"class in pkg1",
+        "pkg1/A.C.html\" title=\"class in pkg1",
+        "pkg1/B.html\" title=\"class in pkg1",
+        "pkg1/B.A.html\" title=\"class in pkg1",
+        "pkg1/C1.html\" title=\"class in pkg1",
+        "pkg1/C2.html\" title=\"class in pkg1",
+        "pkg1/C3.html\" title=\"class in pkg1",
+        "pkg1/C4.html\" title=\"class in pkg1",
+        "pkg1/ImplementsOrdering.html\" title=\"interface in pkg1",
+        "pkg1/MethodOrder.html\" title=\"class in pkg1",
+        "pkg1/OverrideOrdering.html\" title=\"class in pkg1",
+        "pkg1/UsedClass.html\" title=\"class in pkg1"
+
+    };
+
     final String expectedInnerClassContructors[] = {
         "../../pkg1/A.html#A-pkg1.UsedClass-",
         "../../pkg1/B.A.html#A-pkg1.UsedClass-",
@@ -342,12 +365,33 @@
         "../../pkg1/MethodOrder.html#m-java.util.Collection-",
         "../../pkg1/MethodOrder.html#m-java.util.List-"
     };
+
     final String expectedClassUseWithTypeParams[] = {
         "../../pkg1/MethodOrder.html#tpm-pkg1.UsedClass-",
         "../../pkg1/MethodOrder.html#tpm-pkg1.UsedClass-pkg1.UsedClass-",
         "../../pkg1/MethodOrder.html#tpm-pkg1.UsedClass-pkg1.UsedClass:A-",
         "../../pkg1/MethodOrder.html#tpm-pkg1.UsedClass-java.lang.String-"
     };
+
+    final String expectedPackageOrdering[] = {
+        "\"add0/package-summary.html\">add0</a> - package add0",
+        "\"add0/add/package-summary.html\">add0.add</a> - package add0.add",
+        "\"add0/add/add/package-summary.html\">add0.add.add</a> - package add0.add.add",
+        "\"add0/add/add/add/package-summary.html\">add0.add.add.add</a> - package add0.add.add.add",
+        "\"add1/package-summary.html\">add1</a> - package add1",
+        "\"add1/add/package-summary.html\">add1.add</a> - package add1.add",
+        "\"add1/add/add/package-summary.html\">add1.add.add</a> - package add1.add.add",
+        "\"add1/add/add/add/package-summary.html\">add1.add.add.add</a> - package add1.add.add.add",
+        "\"add2/package-summary.html\">add2</a> - package add2",
+        "\"add2/add/package-summary.html\">add2.add</a> - package add2.add",
+        "\"add2/add/add/package-summary.html\">add2.add.add</a> - package add2.add.add",
+        "\"add2/add/add/add/package-summary.html\">add2.add.add.add</a> - package add2.add.add.add",
+        "\"add3/package-summary.html\">add3</a> - package add3",
+        "\"add3/add/package-summary.html\">add3.add</a> - package add3.add",
+        "\"add3/add/add/package-summary.html\">add3.add.add</a> - package add3.add.add",
+        "\"add3/add/add/add/package-summary.html\">add3.add.add.add</a> - package add3.add.add.add"
+    };
+
     final String expectedMethodOrdering[] = {
         "Add.html#add--",
         "Add.html#add-double-",
@@ -361,10 +405,12 @@
         "Add.html#add-java.lang.Double-",
         "Add.html#add-java.lang.Integer-"
     };
+
     final String expectedEnumOrdering[] = {
         "Add.add.html\" title=\"enum in REPLACE_ME\"",
         "Add.ADD.html\" title=\"enum in REPLACE_ME\""
     };
+
     final String expectedFieldOrdering[] = {
         "Add.html#addadd\"",
         "add0/add/add/add/Add.html#addadd\"",
@@ -418,10 +464,12 @@
         "add3/add/Add.html#ADDADD\"",
         "add3/Add.html#ADDADD\""
     };
+
     final String expectedPackageTreeOrdering[] = {
         "<a href=\"../../add0/add/Add.add.html\" title=\"enum in add0.add\">",
         "<a href=\"../../add0/add/Add.ADD.html\" title=\"enum in add0.add\">"
     };
+
     final String expectedOverviewOrdering[] = {
         "<a href=\"Add.add.html\" title=\"enum in &lt;Unnamed&gt;\">",
         "<a href=\"add0/Add.add.html\" title=\"enum in add0\">",
@@ -458,6 +506,7 @@
         "<a href=\"add3/add/add/Add.ADD.html\" title=\"enum in add3.add.add\">",
         "<a href=\"add3/add/add/add/Add.ADD.html\" title=\"enum in add3.add.add.add\">",
     };
+
     final static String expectedOverviewFrameOrdering[] = {
         "<a href=\"package-frame.html\" target=\"packageFrame\">&lt;unnamed package&gt;</a>",
         "<a href=\"add0/package-frame.html\" target=\"packageFrame\">add0</a>",
@@ -477,11 +526,13 @@
         "<a href=\"add3/add/add/package-frame.html\" target=\"packageFrame\">add3.add.add</a>",
         "<a href=\"add3/add/add/add/package-frame.html\" target=\"packageFrame\">add3.add.add.add</a></li>"
     };
+
     final static String expectedImplementsOrdering[] = {
         "<dd><code>close</code>&nbsp;in interface&nbsp;<code>java.lang.AutoCloseable</code></dd>",
         "<dd><code>close</code>&nbsp;in interface&nbsp;<code>java.nio.channels.Channel</code></dd>",
         "<dd><code>close</code>&nbsp;in interface&nbsp;<code>java.io.Closeable</code></dd>"
     };
+
     final static String expectedOverrideOrdering[] = {
         "<dd><code>iterator</code>&nbsp;in interface&nbsp;<code>java.util.Collection&lt;",
         "<dd><code>iterator</code>&nbsp;in interface&nbsp;<code>java.lang.Iterable&lt;"