8311264: JavaDoc index comparator is not transitive

Reviewed-by: jjg

Author: hns

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Comparators.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -238,17 +238,8 @@ public Comparator<Element> makeIndexElementComparator() {
                  */
                 @Override
                 public int compare(Element e1, Element e2) {
-                    int result;
                     // first, compare names as appropriate
-                    if ((utils.isModule(e1) || utils.isPackage(e1)) && (utils.isModule(e2) || utils.isPackage(e2))) {
-                        result = compareFullyQualifiedNames(e1, e2);
-                    } else if (utils.isModule(e1) || utils.isPackage(e1)) {
-                        result = utils.compareStrings(utils.getFullyQualifiedName(e1), utils.getSimpleName(e2));
-                    } else if (utils.isModule(e2) || utils.isPackage(e2)) {
-                        result = utils.compareStrings(utils.getSimpleName(e1), utils.getFullyQualifiedName(e2));
-                    } else {
-                        result = compareNames(e1, e2);
-                    }
+                    int result = utils.compareStrings(getIndexElementKey(e1), getIndexElementKey(e2));
                     if (result != 0) {
                         return result;
                     }
@@ -274,6 +265,20 @@ public int compare(Element e1, Element e2) {
         return indexUseComparator;
     }
 
+    /**
+     * {@return the element's primary key for use in the index comparator}
+     * This method can be used by other comparators which need to produce results
+     * that are consistent with the index comparator.
+     *
+     * @param element an element
+     */
+    public String getIndexElementKey(Element element) {
+        return switch (element.getKind()) {
+            case MODULE, PACKAGE -> utils.getFullyQualifiedName(element);
+            default -> utils.getSimpleName(element);
+        };
+    }
+
     private Comparator<TypeMirror> typeMirrorClassUseComparator = null;
 
     /**

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/IndexBuilder.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -119,7 +119,7 @@ public IndexBuilder(BaseConfiguration configuration,
         itemsByFirstChar = new TreeMap<>();
         itemsByCategory = new EnumMap<>(IndexItem.Category.class);
 
-        mainComparator = makeIndexComparator(classesOnly);
+        mainComparator = classesOnly ? makeClassComparator() : makeIndexComparator();
     }
 
     /**
@@ -310,6 +310,13 @@ private static Character keyCharacter(String s) {
         return '*';
     }
 
+    /**
+     * Returns a comparator for the all-classes list.
+     * @return a comparator for class element items
+     */
+    private Comparator<IndexItem> makeClassComparator() {
+        return Comparator.comparing(IndexItem::getElement, utils.comparators.makeAllClassesComparator());
+    }
 
     /**
      * Returns a comparator for the {@code IndexItem}s in the index page.
@@ -318,15 +325,17 @@ private static Character keyCharacter(String s) {
      *
      * @return a comparator for index page items
      */
-    private Comparator<IndexItem> makeIndexComparator(boolean classesOnly) {
-        Comparator<Element> elementComparator = classesOnly
-                ? utils.comparators.makeAllClassesComparator()
-                : utils.comparators.makeIndexElementComparator();
-
-        Comparator<IndexItem> labelComparator =
-                (ii1, ii2) -> utils.compareStrings(ii1.getLabel(), ii2.getLabel());
+    private Comparator<IndexItem> makeIndexComparator() {
+        // We create comparators specific to element and search tag items, and a
+        // base comparator used to compare between the two kinds of items.
+        // In order to produce consistent results, it is important that the base comparator
+        // uses the same primary sort keys as both the element and search tag comparators
+        // (see JDK-8311264).
+        Comparator<Element> elementComparator = utils.comparators.makeIndexElementComparator();
+        Comparator<IndexItem> baseComparator =
+                (ii1, ii2) -> utils.compareStrings(getIndexItemKey(ii1), getIndexItemKey(ii2));
         Comparator<IndexItem> searchTagComparator =
-                labelComparator
+                baseComparator
                         .thenComparing(IndexItem::getHolder)
                         .thenComparing(IndexItem::getDescription)
                         .thenComparing(IndexItem::getUrl);
@@ -350,9 +359,9 @@ private Comparator<IndexItem> makeIndexComparator(boolean classesOnly) {
                 return d;
             }
 
-            // If one is an element item, compare labels; if equal, put element item last
+            // If one is an element item, compare item keys; if equal, put element item last
             if (ii1.isElementItem() || ii2.isElementItem()) {
-                int d = labelComparator.compare(ii1, ii2);
+                int d = baseComparator.compare(ii1, ii2);
                 return d != 0 ? d : ii1.isElementItem() ? 1 : -1;
             }
 
@@ -361,6 +370,14 @@ private Comparator<IndexItem> makeIndexComparator(boolean classesOnly) {
         };
     }
 
+    private String getIndexItemKey(IndexItem ii) {
+        // For element items return the key used by the element comparator;
+        // for search tag items return the item's label.
+        return ii.isElementItem()
+                ? utils.comparators.getIndexElementKey(ii.getElement())
+                : ii.getLabel();
+    }
+
     /**
      * Returns a Comparator for IndexItems in the types category of the search index.
      * Items are compared by short name, falling back to the main comparator if names are equal.

test/langtools/jdk/javadoc/doclet/testIndex/TestIndex.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,6 +24,7 @@
 /*
  * @test
  * @bug      4852280 4517115 4973608 4994589 8026567 8071982 8196202 8234746
+ *           8311264
  * @summary  Perform tests on index.html file.
  *           Also test that index-all.html has the appropriate output.
  *           Test for unnamed package in index.
@@ -50,22 +51,44 @@ public void test() {
         checkExit(Exit.OK);
 
         //Test index-all.html
-        checkOutput("index-all.html", true,
+        checkOrder("index-all.html",
+                """
+                    <a href="pkg/AnnotationType.html" class="type-name-link" title="annotation inter\
+                    face in pkg">AnnotationType</a> - Annotation Interface in <a href="pkg/package-s\
+                    ummary.html">pkg</a>""",
+                """
+                    <a href="pkg/C.html#c()" class="member-name-link">c()</a> - Method in class pkg.\
+                    <a href="pkg/C.html" title="class in pkg">C</a>""",
+                """
+                    <a href="pkg/C.html#c-heading" class="search-tag-link">C</a> - Search tag in cla\
+                    ss pkg.C""",
                 """
                     <a href="pkg/C.html" class="type-name-link" title="class in pkg">C</a> - Class i\
                     n <a href="pkg/package-summary.html">pkg</a>""",
                 """
-                    <a href="pkg/Interface.html" class="type-name-link" title="interface in pkg">Int\
-                    erface</a> - Interface in <a href="pkg/package-summary.html">pkg</a>""",
+                    <a href="pkg/C.html#%3Cinit%3E()" class="member-name-link">C()</a> - Constructor\
+                     for class pkg.<a href="pkg/C.html" title="class in pkg">C</a>""",
                 """
-                    <a href="pkg/AnnotationType.html" class="type-name-link" title="annotation inter\
-                    face in pkg">AnnotationType</a> - Annotation Interface in <a href="pkg/package-s\
-                    ummary.html">pkg</a>""",
+                    <a href="pkg/C.html#%3Cinit%3E(int)" class="member-name-link">C(int)</a> - Const\
+                    ructor for class pkg.<a href="pkg/C.html" title="class in pkg">C</a>""",
+                """
+                    <a href="pkg/C.html#c_()" class="member-name-link">c_()</a> - Method in class pk\
+                    g.<a href="pkg/C.html" title="class in pkg">C</a>""",
+                """
+                    <a href="pkg/C.html#c/d" class="search-tag-link">c/d</a> - Search tag in class p\
+                    kg.C""",
+                """
+                    <a href="pkg/C.html#c-d-heading" class="search-tag-link">C/D</a> - Search tag in\
+                     class pkg.C""",
                 """
                     <a href="pkg/Coin.html" class="type-name-link" title="enum class in pkg">Coin</a\
                     > - Enum Class in <a href="pkg/package-summary.html">pkg</a>""",
                 """
-                    Class in <a href="package-summary.html">Unnamed Package</a>""",
+                    <dt><a href="pkg/Coin.html#Enum" class="search-tag-link">Enum</a> - Search tag i\
+                    n enum class pkg.Coin</dt>""",
+                """
+                    <a href="pkg/Interface.html" class="type-name-link" title="interface in pkg">Int\
+                    erface</a> - Interface in <a href="pkg/package-summary.html">pkg</a>""",
                 """
                     <dl class="index">
                     <dt><a href="pkg/C.html#Java" class="member-name-link">Java</a> - Static variabl\
@@ -76,6 +99,6 @@ in class pkg.<a href="pkg/C.html" title="class in pkg">C</a></dt>
                     <dd>&nbsp;</dd>
                     </dl>""",
                 """
-                    <dt><a href="pkg/Coin.html#Enum" class="search-tag-link">Enum</a> - Search tag in enum class pkg.Coin</dt>""");
+                    Class in <a href="package-summary.html">Unnamed Package</a>""");
     }
 }

test/langtools/jdk/javadoc/doclet/testIndex/pkg/C.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,11 +23,45 @@
 
 package pkg;
 
+/**
+ * A class to test sorting of index items.
+ *
+ * <h2>C</h2>
+ *
+ * Section "C" should appear right before language elements with the same name.
+ *
+ * <h3>C/D</h3>
+ *
+ * Section "C/D" should appear after items named "C" in the index.
+ *
+ * {@index c/d should appear before the section above}
+ */
 public class C {
 
-    //Test that Java appears before JDK in the index.  The fact
-    //that 'D' is uppercase and 'a' is lowercase should make no difference
-    //in ordering.
+    /**
+     * Empty constructor.
+     */
+    public C() {}
+
+    /**
+     * Constructor with a parameter.
+     * @param i an int
+     */
+    public C(int i) {}
+
+    /**
+     * Lower case "c" method should appear before upper case "C" elements and sections in index.
+     */
+    public void c() {}
+
+    /**
+     * Should appear after all items named "c" or "C".
+     */
+    public void c_() {}
+
+    // Test that Java appears before JDK in the index.  The fact
+    // that 'D' is uppercase and 'a' is lowercase should make no difference
+    // in ordering.
     public static final String JDK = "1.5";
     public static final String Java = "1.5";