Make javadoc pages generation deterministic (#2479)

* Make javadoc pages generation deterministic

* Rename `genericQuery` to avoid confusion with Generics

6 files changed, 136 insertions, 60 deletions

diff --git a/plugins/javadoc/api/javadoc.api b/plugins/javadoc/api/javadoc.api
index 0eabce5c..70a69a79 100644
--- a/plugins/javadoc/api/javadoc.api
+++ b/plugins/javadoc/api/javadoc.api
@@ -119,7 +119,6 @@ public final class org/jetbrains/dokka/javadoc/pages/DeprecatedPageSection : jav
 	public final fun getCaption ()Ljava/lang/String;
 	public final fun getHeader ()Ljava/lang/String;
 	public final fun getId ()Ljava/lang/String;
-	public final fun getPriority ()I
 	public static fun valueOf (Ljava/lang/String;)Lorg/jetbrains/dokka/javadoc/pages/DeprecatedPageSection;
 	public static fun values ()[Lorg/jetbrains/dokka/javadoc/pages/DeprecatedPageSection;
 }
diff --git a/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/pages/JavadocPageNodes.kt b/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/pages/JavadocPageNodes.kt
index 6c7691cd..cafc6c3e 100644
--- a/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/pages/JavadocPageNodes.kt
+++ b/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/pages/JavadocPageNodes.kt
@@ -317,17 +317,19 @@ class DeprecatedNode(val name: String, val address: DRI, val description: List<C
     override fun hashCode(): Int = address.hashCode()
 }
 
-enum class DeprecatedPageSection(val id: String, val caption: String, val header: String, val priority: Int = 100) {
-    DeprecatedForRemoval("forRemoval", "For Removal", "Element", priority = 90),
+enum class DeprecatedPageSection(val id: String, val caption: String, val header: String) {
     DeprecatedModules("module", "Modules", "Module"),
     DeprecatedInterfaces("interface", "Interfaces", "Interface"),
     DeprecatedClasses("class", "Classes", "Class"),
-    DeprecatedEnums("enum", "Enums", "Enum"),
     DeprecatedExceptions("exception", "Exceptions", "Exceptions"),
     DeprecatedFields("field", "Fields", "Field"),
     DeprecatedMethods("method", "Methods", "Method"),
     DeprecatedConstructors("constructor", "Constructors", "Constructor"),
-    DeprecatedEnumConstants("enum.constant", "Enum Constants", "Enum Constant")
+    DeprecatedEnums("enum", "Enums", "Enum"),
+    DeprecatedEnumConstants("enum.constant", "Enum Constants", "Enum Constant"),
+    DeprecatedForRemoval("forRemoval", "For Removal", "Element");
+
+    internal fun getPosition() = ordinal
 }
 
 class IndexPage(
diff --git a/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/pages/htmlPreprocessors.kt b/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/pages/htmlPreprocessors.kt
index 057b9e85..4438e7e9 100644
--- a/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/pages/htmlPreprocessors.kt
+++ b/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/pages/htmlPreprocessors.kt
@@ -90,9 +90,30 @@ object IndexGenerator : PageTransformer {
         }
         val keys = elements.keys.sortedBy { it }
         val sortedElements = elements.entries.sortedBy { (a, _) -> a }
-        return input.modified(children = input.children + sortedElements.mapIndexed { i, (_, set) ->
-            IndexPage(i + 1, set.sortedBy { it.getId().toLowerCase() }, keys, input.sourceSets())
-        })
+
+        val indexNodeComparator = compareBy<NavigableJavadocNode> { it.getId().toLowerCase() }
+            .thenBy { it.getFullComparatorKey() }
+
+        val indexPages = sortedElements.mapIndexed { idx, (_, set) ->
+            IndexPage(
+                id = idx + 1,
+                elements = set.sortedWith(indexNodeComparator),
+                keys = keys,
+                sourceSet = input.sourceSets()
+            )
+        }
+        return input.modified(children = input.children + indexPages)
+    }
+
+    private fun NavigableJavadocNode.getFullComparatorKey(): String {
+        return getDRI().let { dri ->
+            val packageName = dri.packageName.orEmpty()
+            val className = dri.classNames.orEmpty()
+            val callableName = dri.callable?.name.orEmpty()
+            val parameters = dri.callable?.signature().orEmpty()
+
+            "$packageName/$className/$callableName/$parameters"
+        }
     }
 }
 
diff --git a/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/renderer/JavadocContentToTemplateMapTranslator.kt b/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/renderer/JavadocContentToTemplateMapTranslator.kt
index a6eda405..45fa3f2e 100644
--- a/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/renderer/JavadocContentToTemplateMapTranslator.kt
+++ b/plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/renderer/JavadocContentToTemplateMapTranslator.kt
@@ -81,7 +81,7 @@ internal class JavadocContentToTemplateMapTranslator(
                 "id" to node.name,
                 "title" to "Deprecated",
                 "kind" to "deprecated",
-                "sections" to node.elements.toList().sortedBy { (section, _) -> section.priority }
+                "sections" to node.elements.toList().sortedBy { (section, _) -> section.getPosition() }
                     .map { (s, e) -> templateMapForDeprecatedPageSection(s, e) }
             )
 
diff --git a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocDeprecatedTest.kt b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocDeprecatedTest.kt
index e2331f58..f301baf6 100644
--- a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocDeprecatedTest.kt
+++ b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocDeprecatedTest.kt
@@ -4,6 +4,7 @@ import org.jetbrains.dokka.javadoc.pages.DeprecatedPage
 import org.jetbrains.dokka.javadoc.renderer.TemplateMap
 import org.junit.jupiter.api.Assertions
 import org.junit.jupiter.api.Test
+import kotlin.test.assertEquals
 
 internal class JavadocDeprecatedTest : AbstractJavadocTemplateMapTest() {
 
@@ -68,6 +69,21 @@ internal class JavadocDeprecatedTest : AbstractJavadocTemplateMapTest() {
     }
 
     @Test
+    fun `should be sorted by position`() {
+        testDeprecatedPageTemplateMaps { templateMap ->
+            @Suppress("UNCHECKED_CAST")
+            val contents = (templateMap["sections"] as List<TemplateMap>).map { it["caption"] }
+
+            // maybe some other ordering is required by the javadoc spec
+            // but it has to be deterministic
+            val expected = "Classes, Exceptions, Methods, Constructors, Enums, For Removal"
+            val actual = contents.joinToString(separator = ", ")
+
+            assertEquals(expected, actual)
+        }
+    }
+
+    @Test
     fun `provides correct information for deprecated element`() {
         testDeprecatedPageTemplateMaps { templateMap ->
             val map = templateMap.section("Enums")
diff --git a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocIndexTest.kt b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocIndexTest.kt
index 76c345b0..1fbc21ea 100644
--- a/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocIndexTest.kt
+++ b/plugins/javadoc/src/test/kotlin/org/jetbrains/dokka/javadoc/JavadocIndexTest.kt
@@ -2,14 +2,64 @@ package org.jetbrains.dokka.javadoc
 
 import org.jetbrains.dokka.javadoc.pages.IndexPage
 import org.jetbrains.dokka.javadoc.renderer.TemplateMap
+import org.jetbrains.dokka.links.DRI
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Test
+import kotlin.test.assertNotNull
 
 internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() {
 
+    private val commonTestQuery = """
+            /src/source0.kt
+            package package0
+            /** 
+            * Documentation for ClassA 
+            */
+            class ClassA {
+                fun a() {}
+                fun b() {}
+                fun c() {}
+            }
+            
+            /src/source1.kt
+            package package1
+            /**
+            * Documentation for ClassB
+            */
+            class ClassB {
+                fun d() {}
+                fun e() {}
+                fun f() {}
+            }
+            
+            /src/source2.kt
+            package package1
+            /**
+            * Documentation for ClassB
+            */
+            class ClassC {
+                fun g() {}
+                fun h() {}
+                fun j() {}
+                
+                class InnerClass {
+                    fun k() {}
+                }
+            }
+
+            /src/source3.kt
+            package package1
+            /**
+            * Documentation for ClassCEnum
+            */
+            enum class ClassCEnum {
+                A, D, E
+            }
+        """.trimIndent()
+
     @Test
     fun `generates correct number of index pages`() {
-        testIndexPages { indexPages ->
+        testIndexPages(commonTestQuery) { indexPages ->
             assertEquals(12, indexPages.size)
         }
     }
@@ -20,14 +70,14 @@ internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() {
         fun hasAdditionalFunction() =
             AnnotationTarget.ANNOTATION_CLASS::class.java.methods.any { it.name == "describeConstable" }
 
-        testIndexPages { indexPages ->
+        testIndexPages(commonTestQuery) { indexPages ->
             assertEquals(if (hasAdditionalFunction()) 41 else 40, indexPages.sumBy { it.elements.size })
         }
     }
 
     @Test
     fun `templateMap for class index`() {
-        testIndexPagesTemplateMaps { templateMaps ->
+        testIndexPagesTemplateMaps(commonTestQuery) { templateMaps ->
             @Suppress("UNCHECKED_CAST")
             val element = (templateMaps[2]["elements"] as List<TemplateMap>)[1]
             assertEquals("../package0/ClassA.html", element["address"])
@@ -41,7 +91,7 @@ internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() {
 
     @Test
     fun `templateMap for enum entry index`() {
-        testIndexPagesTemplateMaps { templateMaps ->
+        testIndexPagesTemplateMaps(commonTestQuery) { templateMaps ->
             @Suppress("UNCHECKED_CAST")
             val element = (templateMaps[0]["elements"] as List<TemplateMap>).last()
             assertEquals("../package1/ClassCEnum.html#A", element["address"])
@@ -55,7 +105,7 @@ internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() {
 
     @Test
     fun `templateMap for function index`() {
-        testIndexPagesTemplateMaps { templateMaps ->
+        testIndexPagesTemplateMaps(commonTestQuery) { templateMaps ->
             @Suppress("UNCHECKED_CAST")
             val element = (templateMaps[0]["elements"] as List<TemplateMap>).first()
             assertEquals("../package0/ClassA.html#a()", element["address"])
@@ -67,61 +117,49 @@ internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() {
         }
     }
 
-    private val query = """
-            /src/source0.kt
-            package package0
-            /** 
-            * Documentation for ClassA 
-            */
-            class ClassA {
-                fun a() {}
-                fun b() {}
-                fun c() {}
-            }
-            
-            /src/source1.kt
-            package package1
-            /**
-            * Documentation for ClassB
-            */
-            class ClassB {
-                fun d() {}
-                fun e() {}
-                fun f() {}
-            }
-            
-            /src/source2.kt
-            package package1
-            /**
-            * Documentation for ClassB
-            */
-            class ClassC {
-                fun g() {}
-                fun h() {}
-                fun j() {}
-                
-                class InnerClass {
-                    fun k() {}
-                }
+    @Test
+    fun `should sort overloaded functions deterministically`() {
+        val query = """
+            /src/overloaded.kt
+            package overloaded
+           
+            class Clazz {
+                fun funName(param: List<String>) {}
+                fun funName(param: String) {}
+                fun funName(param: Map<String>) {}
+                fun funName(param: Int) {}
             }
+        """.trimIndent()
 
-            /src/source3.kt
-            package package1
-            /**
-            * Documentation for ClassCEnum
-            */
-            enum class ClassCEnum {
-                A, D, E
+        testIndexPages(query) { allPages ->
+            val indexPage = allPages.find { it.elements.any { el -> el.getId() == "funName" } }
+            assertNotNull(indexPage) { "Index page with functions not found" }
+
+            val indexElementDRIs = indexPage.elements.map { it.getDRI() }
+            assertEquals(4, indexElementDRIs.size)
+            indexElementDRIs.forEach {
+                assertEquals("overloaded", it.packageName)
+                assertEquals("Clazz", it.classNames)
+                assertEquals("funName", it.callable!!.name)
+                assertEquals(1, it.callable!!.params.size)
             }
-        """.trimIndent()
 
-    private fun testIndexPages(operation: (List<IndexPage>) -> Unit) {
+            assertEquals("[kotlin.String]", indexElementDRIs[0].getParam(0))
+            assertEquals("kotlin.Int", indexElementDRIs[1].getParam(0))
+            assertEquals("kotlin.String", indexElementDRIs[2].getParam(0))
+            assertEquals("kotlin.collections.List[kotlin.String]", indexElementDRIs[3].getParam(0))
+        }
+    }
+
+    private fun DRI.getParam(index: Int) = this.callable!!.params[index].toString()
+
+    private fun testIndexPages(query: String, operation: (List<IndexPage>) -> Unit) {
         testTemplateMapInline(query) {
             operation(allPagesOfType())
         }
     }
 
-    private fun testIndexPagesTemplateMaps(operation: (List<TemplateMap>) -> Unit) =
+    private fun testIndexPagesTemplateMaps(query: String, operation: (List<TemplateMap>) -> Unit) =
         testTemplateMapInline(query) {
             operation(allPagesOfType<IndexPage>().map { it.templateMap })
         }