8155061: javadoc incorrectly sorted items in All Classes list and Index files
Reviewed-by: jjg
--- 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", "<Unnamed>"));
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 <Unnamed>\">",
"<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\"><unnamed package></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> in interface <code>java.lang.AutoCloseable</code></dd>",
"<dd><code>close</code> in interface <code>java.nio.channels.Channel</code></dd>",
"<dd><code>close</code> in interface <code>java.io.Closeable</code></dd>"
};
+
final static String expectedOverrideOrdering[] = {
"<dd><code>iterator</code> in interface <code>java.util.Collection<",
"<dd><code>iterator</code> in interface <code>java.lang.Iterable<"