8049393: [javadoc] parameters are not sorted correctly

Reviewed-by: jjg

langtools/src/share/classes/com/sun/tools/doclets/internal/toolkit/util/Util.java

@@ -814,22 +814,21 @@ public class Util {
     }
 
     /**
-     * A comparator for index file presentations,
-     *  1. this sorts first on simple names
+     * A comparator for index file presentations, and are sorted as follows:
+     *  1. sort on simple names of entities
      *  2. if equal, then compare the DocKind ex: Package, Interface etc.
-     *  3a. if equal and if the type is of ExecutableMemberDoc(Constructor, Fields),
-     *      a case insensitive comparison of parameter types
-     *  3b. if equal, a case sensitive comparison of parameter types
+     *  3a. if equal and if the type is of ExecutableMemberDoc(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
      * @return a comparator for index file use
      */
     public static Comparator<Doc> makeComparatorForIndexUse() {
         return new Util.DocComparator<Doc>() {
             /**
-             * Compare two given Doc entities, first sort on name, then on the kinds,
+             * Compare two given Doc entities, first sort on names, then on the kinds,
              * then on the parameters only if the type is an instance of ExecutableMemberDocs,
-             * the parameters are compared ignoring the case first, then a case sensitive comparison,
-             * and finally the fully qualified names.
+             * the parameters are compared and finally the fully qualified names.
              *
              * @param d1 - a Doc element.
              * @param d2 - a Doc element.
@@ -862,17 +861,19 @@ public class Util {
         };
     }
     /**
-     * Comparator for ClassUse presentations, and sorts as follows:
-     * 1. member names
-     * 2. then fully qualified member names
-     * 3. then parameter types if applicable
+     * Comparator for ClassUse presentations, and sorted as follows,
+     * 1. compares simple names of entities
+     * 2. if equal, the fully qualified names of the entities
+     * 3. if equal and if applicable, the string representation of parameter types
+     * 3a. first by using case insensitive comparison
+     * 3b. second by using a case sensitive comparison
      * 4. finally the Doc kinds ie. package, class, interface etc.
      * @return a comparator to sort classes and members for class use
      */
     public static Comparator<Doc> makeComparatorForClassUse() {
         return new Util.DocComparator<Doc>() {
             /**
-             * Compare two given Doc entities, first sort on name, and if
+             * Compares two given Doc entities, first sort on name, and if
              * applicable on the fully qualified name, and if applicable
              * on the parameter types, and finally the DocKind.
              * @param d1 - a Doc element.
@@ -952,23 +953,37 @@ public class Util {
             return getDocKind(d1).compareTo(getDocKind(d2));
         }
         /**
-         * Compares two parameter arrays by comparing each Type of the parameter in the array,
-         * and as many as possible, otherwise compare their lengths.
+         * Compares arrays of parameters as a string representation of their types.
+         *
          * @param ignoreCase specifies case sensitive or insensitive comparison.
          * @param params1 the first parameter array.
          * @param params2 the first parameter array.
-         * @return a negative integer, zero, or a positive integer as the first
-         *         argument is less than, equal to, or greater than the second.
+         * @return a negative integer, zero, or a positive integer as the first argument is less
+         * than, equal to, or greater than the second.
          */
-        protected int compareParameters(boolean ignoreCase, Parameter[] params1, Parameter[] params2) {
-            // try to compare as many as possible
-            for (int i = 0; i < params1.length && i < params2.length; i++) {
-                int result = compareStrings(ignoreCase, params1[i].typeName(), params2[i].typeName());
-                if (result != 0) {
-                    return result;
-                }
+        protected int compareParameters(boolean caseSensitive,
+                                        Parameter[] params1,
+                                        Parameter[] params2) {
+            String s1 = getParametersAsString(params1);
+            String s2 = getParametersAsString(params2);
+            return compareStrings(caseSensitive, s1, s2);
+        }
+        /*
+         * This method returns a string representation solely for comparison purposes.
+         */
+        protected String getParametersAsString(Parameter[] params) {
+            StringBuilder sb = new StringBuilder();
+            for (Parameter param : params) {
+                Type t = param.type();
+                // add parameter type to arrays, as TypeMirror does.
+                String tname = (t.asParameterizedType() != null && t.getElementType() != null)
+                        ? t.getElementType() + t.dimension()
+                        : t.toString();
+                // prefix P for primitive and R for reference types, thus items will
+                // be ordered naturally.
+                sb.append(t.isPrimitive() ? "P" : "R").append("-").append(tname).append("-");
             }
-            return Integer.compare(params1.length, params2.length);
+            return sb.toString();
         }
 
         /**

langtools/test/com/sun/javadoc/testOrdering/TestOrdering.java

@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8039410 8042601 8042829
+ * @bug 8039410 8042601 8042829 8049393
  * @summary test to determine if members are ordered correctly
  * @author ksrini
  * @library ../lib/
@@ -66,6 +66,9 @@ public class TestOrdering extends JavadocTester {
                 "pkg1");
         checkExit(Exit.OK);
         checkClassUseOrdering("pkg1/class-use/UsedClass.html");
+        checkOrder("pkg1/class-use/UsedClass.html", expectedClassUseMethodOrdering);
+        checkOrder("pkg1/class-use/UsedClass.html", expectedClassUseWithTypeParams);
+        checkOrder("pkg1/class-use/UsedClass.html", expectedInnerClassContructors);
     }
 
     enum ListOrder { NONE, REVERSE, SHUFFLE };
@@ -273,10 +276,44 @@ public class TestOrdering extends JavadocTester {
         return in.replace("/", ".");
     }
 
+    final String expectedInnerClassContructors[] = {
+        "../../pkg1/A.html#A-pkg1.UsedClass-",
+        "../../pkg1/B.A.html#A-pkg1.UsedClass-",
+        "../../pkg1/B.html#B-pkg1.UsedClass-",
+        "../../pkg1/A.C.html#C-pkg1.UsedClass-java.lang.Object:A-",
+        "../../pkg1/A.C.html#C-pkg1.UsedClass-java.util.Collection-",
+        "../../pkg1/A.C.html#C-pkg1.UsedClass-java.util.List-"
+    };
+
+    final String expectedClassUseMethodOrdering[] = {
+        "../../pkg1/MethodOrder.html#m--",
+        "../../pkg1/MethodOrder.html#m-byte:A-",
+        "../../pkg1/MethodOrder.html#m-double-",
+        "../../pkg1/MethodOrder.html#m-double-double-",
+        "../../pkg1/MethodOrder.html#m-double-java.lang.Double-",
+        "../../pkg1/MethodOrder.html#m-int-",
+        "../../pkg1/MethodOrder.html#m-int-int-",
+        "../../pkg1/MethodOrder.html#m-int-java.lang.Integer-",
+        "../../pkg1/MethodOrder.html#m-java.lang.Double-",
+        "../../pkg1/MethodOrder.html#m-java.lang.Double-double-",
+        "../../pkg1/MethodOrder.html#m-java.lang.Double-java.lang.Double-",
+        "../../pkg1/MethodOrder.html#m-java.lang.Integer-",
+        "../../pkg1/MethodOrder.html#m-java.lang.Integer-int-",
+        "../../pkg1/MethodOrder.html#m-java.lang.Integer-java.lang.Integer-",
+        "../../pkg1/MethodOrder.html#m-java.lang.Object:A-",
+        "../../pkg1/MethodOrder.html#m-java.util.ArrayList-",
+        "../../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 expectedMethodOrdering[] = {
         "Add.html#add--",
         "Add.html#add-double-",
-        "Add.html#add-java.lang.Double-",
         "Add.html#add-double-byte-",
         "Add.html#add-double-double-",
         "Add.html#add-double-java.lang.Double-",
@@ -284,6 +321,7 @@ public class TestOrdering extends JavadocTester {
         "Add.html#add-float-int-",
         "Add.html#add-int-",
         "Add.html#add-int-float-",
+        "Add.html#add-java.lang.Double-",
         "Add.html#add-java.lang.Integer-"
     };
     final String expectedEnumOrdering[] = {

langtools/test/com/sun/javadoc/testOrdering/pkg1/A.java

@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2014, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package pkg1;
+import java.util.Collection;
+import java.util.List;
+public class A {
+
+    /**
+     * Constructor
+     * @param a class
+     */
+    public A(UsedClass a) {}
+
+    /**
+     * test inner classes
+     */
+    public static class C {
+
+        /**
+         * inner classes constructor
+         * @param u a param
+         * @param array a param
+         */
+        public C(UsedClass u, Object[] array){}
+
+        /**
+         * inner classes constructor
+         * @param u a param
+         * @param collection a param
+         */
+        public C(UsedClass u, Collection collection){}
+
+        /**
+         * inner classes constructor
+         * @param u a param
+         * @param list a param
+         */
+        public C(UsedClass u, List list){}
+    }
+}

langtools/test/com/sun/javadoc/testOrdering/pkg1/B.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2014, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package pkg1;
+public class B {
+
+    /**
+     * constructor
+     * @param a param
+     */
+    public B(UsedClass a) {}
+
+    /**
+     * Inner class
+     */
+    static public class A {
+
+        /**
+         * constructor
+         * @param b param
+         */
+        public A(UsedClass b){}
+    }
+}

langtools/test/com/sun/javadoc/testOrdering/pkg1/MethodOrder.java

@@ -0,0 +1,219 @@
+/*
+ * Copyright (c) 2014, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package pkg1;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+public class MethodOrder {
+    /**
+     * method test for ordering parameters
+     * @return UsedClass something
+     */
+    public UsedClass m(){return null;}
+    /**
+     * method test for ordering parameters
+     * @param i a param
+     * @return UsedClass something
+     */
+    public UsedClass m(int i) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param i1 a param
+     * @param i2 a param
+     * @return something
+     */
+    public UsedClass m(int i1, int i2) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param array a param
+     * @return something
+     */
+    public UsedClass m(byte[] array) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param in a param
+     * @return something
+     */
+    public UsedClass m(Integer in) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param i1 a param
+     * @param i2 a param
+     * @return something
+     */
+    public UsedClass m(Integer i1, Integer i2) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param i1 a param
+     * @param i2 a param
+     * @return something
+     */
+    public UsedClass m(int i1, Integer i2) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param i1 a param
+     * @param i2 a param
+     * @return something
+     */
+    public UsedClass m(Integer i1, int i2) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param d a param
+     * @return something
+     */
+    public UsedClass m(double d) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param i1 a param
+     * @param i2 a param
+     * @return something
+     */
+    public UsedClass m(double i1, double i2) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param in a param
+     * @return something
+     */
+    public UsedClass m(Double in) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param i1 a param
+     * @param i2 a param
+     * @return something
+     */
+    public UsedClass m(Double i1, Double i2) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param i1 a param
+     * @param i2 a param
+     * @return something
+     */
+    public UsedClass m(double i1, Double i2) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param l1 param
+     * @return something
+     */
+    public UsedClass m(long l1) {return null;}
+
+    /**
+     *  method test for ordering parameters
+     * @param l1 param
+     * @param l2 param
+     * @return something
+     */
+    public UsedClass m(long l1, Long l2) {return null;}
+
+    /**
+     *  method test for ordering parameters
+     * @param l1 param
+     * @param l2 param
+     * @return something
+     */
+    public UsedClass m(long l1, long l2) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param array a param
+     * @return something
+     */
+    public UsedClass m(Object[] array);
+
+    /**
+     * method test for ordering parameters
+     * @param arrayarray two dimensional array
+     * @return something
+     */
+    public UsedClass m(Object[][] arrayarray);
+
+    /**
+     * method test for ordering parameters
+     * @param i1 a param
+     * @param i2 a param
+     * @return something
+     */
+    public UsedClass m(Double i1, double i2) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param collection a param
+     * @return something
+     */
+    public UsedClass m(Collection collection) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param list a param
+     * @return something
+     */
+    public UsedClass m(List list) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param collection a param
+     * @return something
+     */
+    public UsedClass m(ArrayList<UsedClass> collection) {return null;}
+
+    /**
+     * method test for ordering parameters
+     * @param u use a type param
+     */
+    public void tpm(UsedClass<?> u) {}
+
+    /**
+     * method test for ordering parameters
+     * @param u1 use a type param
+     * @param u2 use a type param
+     */
+    public void tpm(UsedClass<?> u1, UsedClass<?> u2) {}
+
+    /**
+     * method test for ordering parameters
+     * @param u use a type param
+     * @param array use a type param and an array
+     */
+    public void tpm(UsedClass<?> u, UsedClass<?>[] array) {}
+
+    /**
+     * method test for ordering parameters
+     * @param u use type param with extends
+     * @param a some string
+     */
+    public void tpm(UsedClass<? extends UsedClass> u, String a) {}
+}

langtools/test/com/sun/javadoc/testOrdering/pkg1/UsedClass.java

@@ -25,4 +25,4 @@ package pkg1;
 /**
  * For class-use testing
  */
-public class UsedClass {}
+public class UsedClass<T> {}