Fix reversed ordering of enum entries (#2469)

* Fix reversed ordering of enum entries

Fixes #2466

* Add navigation sideMenu unit tests

* Make enum entries in tests non alphabetical

3 files changed, 241 insertions, 49 deletions

diff --git a/plugins/base/src/main/kotlin/renderers/html/htmlPreprocessors.kt b/plugins/base/src/main/kotlin/renderers/html/htmlPreprocessors.kt
index 3297d09f..4527baa7 100644
--- a/plugins/base/src/main/kotlin/renderers/html/htmlPreprocessors.kt
+++ b/plugins/base/src/main/kotlin/renderers/html/htmlPreprocessors.kt
@@ -26,15 +26,21 @@ abstract class NavigationDataProvider {
             children = page.navigableChildren()
         )
 
-    private fun ContentPage.navigableChildren(): List<NavigationNode> =
-        when {
-            this !is ClasslikePageNode ->
-                children.filterIsInstance<ContentPage>().map { visit(it) }
-            documentables.any { it is DEnum } ->
-                children.filter { it is WithDocumentables && it.documentables.any { it is DEnumEntry } }
-                    .map { visit(it as ContentPage) }
-            else -> emptyList()
-        }.sortedBy { it.name.toLowerCase() }
+    private fun ContentPage.navigableChildren(): List<NavigationNode> {
+        return if (this !is ClasslikePageNode) {
+            children
+                .filterIsInstance<ContentPage>()
+                .map { visit(it) }
+                .sortedBy { it.name.toLowerCase() }
+        } else if (documentables.any { it is DEnum }) {
+            // no sorting for enum entries, should be the same as in source code
+            children
+                .filter { child -> child is WithDocumentables && child.documentables.any { it is DEnumEntry } }
+                .map { visit(it as ContentPage) }
+        } else {
+            emptyList()
+        }
+    }
 
     /**
      * Parenthesis is applied in 1 case:
diff --git a/plugins/base/src/main/kotlin/transformers/documentables/DocumentableVisibilityFilterTransformer.kt b/plugins/base/src/main/kotlin/transformers/documentables/DocumentableVisibilityFilterTransformer.kt
index 4b9da03b..4ca3d861 100644
--- a/plugins/base/src/main/kotlin/transformers/documentables/DocumentableVisibilityFilterTransformer.kt
+++ b/plugins/base/src/main/kotlin/transformers/documentables/DocumentableVisibilityFilterTransformer.kt
@@ -216,7 +216,7 @@ class DocumentableVisibilityFilterTransformer(val context: DokkaContext) : PreMe
         }
 
         private fun filterEnumEntries(entries: List<DEnumEntry>, filteredPlatforms: Set<DokkaSourceSet>): Pair<Boolean, List<DEnumEntry>> =
-            entries.foldRight(Pair(false, emptyList())) { entry, acc ->
+            entries.fold(Pair(false, emptyList())) { acc, entry ->
                 val intersection = filteredPlatforms.intersect(entry.sourceSets)
                 if (intersection.isEmpty()) Pair(true, acc.second)
                 else {
diff --git a/plugins/base/src/test/kotlin/enums/EnumsTest.kt b/plugins/base/src/test/kotlin/enums/EnumsTest.kt
index bfdfa73b..778125e2 100644
--- a/plugins/base/src/test/kotlin/enums/EnumsTest.kt
+++ b/plugins/base/src/test/kotlin/enums/EnumsTest.kt
@@ -5,13 +5,18 @@ import org.jetbrains.dokka.pages.*
 import org.jetbrains.dokka.base.testApi.testRunner.BaseAbstractTest
 import org.jetbrains.dokka.model.*
 import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstance
+import org.jsoup.Jsoup
+import org.jsoup.nodes.Element
 import org.junit.jupiter.api.Assertions.*
 import org.junit.jupiter.api.Test
+import signatures.renderedContent
+import utils.TestOutputWriter
+import utils.TestOutputWriterPlugin
 
 class EnumsTest : BaseAbstractTest() {
 
     @Test
-    fun basicEnum() {
+    fun `should preserve enum source ordering for documentables`() {
         val configuration = dokkaConfiguration {
             sourceSets {
                 sourceSet {
@@ -20,29 +25,112 @@ class EnumsTest : BaseAbstractTest() {
             }
         }
 
+        val writerPlugin = TestOutputWriterPlugin()
+
         testInline(
             """
             |/src/main/kotlin/basic/Test.kt
-            |package enums
+            |package testpackage
             |
-            |enum class Test {
-            |   E1,
-            |   E2
+            |enum class TestEnum {
+            |   ZERO,
+            |   ONE,
+            |   TWO,
+            |   THREE,
+            |   FOUR,
+            |   FIVE,
+            |   SIX,
+            |   SEVEN,
+            |   EIGHT,
+            |   NINE
             |}
         """.trimMargin(),
-            configuration
+            configuration,
+            pluginOverrides = listOf(writerPlugin)
+        ) {
+            documentablesTransformationStage = { module ->
+                val testPackage = module.packages[0]
+                assertEquals("testpackage", testPackage.name)
+
+                val testEnum = testPackage.classlikes[0] as DEnum
+                assertEquals("TestEnum", testEnum.name)
+
+                val enumEntries = testEnum.entries
+                assertEquals(10, enumEntries.count())
+
+                assertEquals("ZERO", enumEntries[0].name)
+                assertEquals("ONE", enumEntries[1].name)
+                assertEquals("TWO", enumEntries[2].name)
+                assertEquals("THREE", enumEntries[3].name)
+                assertEquals("FOUR", enumEntries[4].name)
+                assertEquals("FIVE", enumEntries[5].name)
+                assertEquals("SIX", enumEntries[6].name)
+                assertEquals("SEVEN", enumEntries[7].name)
+                assertEquals("EIGHT", enumEntries[8].name)
+                assertEquals("NINE", enumEntries[9].name)
+            }
+        }
+    }
+
+    @Test
+    fun `should preserve enum source ordering for generated pages`() {
+        val configuration = dokkaConfiguration {
+            sourceSets {
+                sourceSet {
+                    sourceRoots = listOf("src/")
+                }
+            }
+        }
+
+        val writerPlugin = TestOutputWriterPlugin()
+
+        testInline(
+            """
+            |/src/main/kotlin/basic/Test.kt
+            |package testpackage
+            |
+            |enum class TestEnum {
+            |   ZERO,
+            |   ONE,
+            |   TWO,
+            |   THREE,
+            |   FOUR,
+            |   FIVE,
+            |   SIX,
+            |   SEVEN,
+            |   EIGHT,
+            |   NINE
+            |}
+        """.trimMargin(),
+            configuration,
+            pluginOverrides = listOf(writerPlugin)
         ) {
-            pagesGenerationStage = {
-                val map = it.getClasslikeToMemberMap()
-                val test = map.filterKeys { it.name == "Test" }.values.firstOrNull()
-                assertTrue(test != null) { "Test not found" }
-                assertTrue(test!!.any { it.name == "E1" } && test.any { it.name == "E2" }) { "Enum entries missing in parent" }
+            pagesGenerationStage = { rootPage ->
+                val packagePage = rootPage.children[0]
+                assertEquals("testpackage", packagePage.name)
+
+                val testEnumNode = packagePage.children[0]
+                assertEquals("TestEnum", testEnumNode.name)
+
+                val enumEntries = testEnumNode.children
+                assertEquals(10, enumEntries.size)
+
+                assertEquals("ZERO", enumEntries[0].name)
+                assertEquals("ONE", enumEntries[1].name)
+                assertEquals("TWO", enumEntries[2].name)
+                assertEquals("THREE", enumEntries[3].name)
+                assertEquals("FOUR", enumEntries[4].name)
+                assertEquals("FIVE", enumEntries[5].name)
+                assertEquals("SIX", enumEntries[6].name)
+                assertEquals("SEVEN", enumEntries[7].name)
+                assertEquals("EIGHT", enumEntries[8].name)
+                assertEquals("NINE", enumEntries[9].name)
             }
         }
     }
 
     @Test
-    fun enumWithCompanion() {
+    fun `should preserve enum source ordering for rendered entries`() {
         val configuration = dokkaConfiguration {
             sourceSets {
                 sourceSet {
@@ -51,12 +139,122 @@ class EnumsTest : BaseAbstractTest() {
             }
         }
 
+        val writerPlugin = TestOutputWriterPlugin()
+
         testInline(
             """
             |/src/main/kotlin/basic/Test.kt
-            |package enums
+            |package testpackage
             |
-            |enum class Test {
+            |enum class TestEnum {
+            |   ZERO,
+            |   ONE,
+            |   TWO,
+            |   THREE,
+            |   FOUR,
+            |   FIVE,
+            |   SIX,
+            |   SEVEN,
+            |   EIGHT,
+            |   NINE
+            |}
+        """.trimMargin(),
+            configuration,
+            pluginOverrides = listOf(writerPlugin)
+        ) {
+            renderingStage = { _, _ ->
+                val enumEntriesOnPage = writerPlugin.writer.renderedContent("root/testpackage/-test-enum/index.html")
+                    .select("div.table[data-togglable=Entries]")
+                    .select("div.table-row")
+                    .select("div.keyValue")
+                    .select("div.title")
+                    .select("a")
+
+                val enumEntries = enumEntriesOnPage.map { it.text() }
+                assertEquals(10, enumEntries.size)
+
+                assertEquals("ZERO", enumEntries[0])
+                assertEquals("ONE", enumEntries[1])
+                assertEquals("TWO", enumEntries[2])
+                assertEquals("THREE", enumEntries[3])
+                assertEquals("FOUR", enumEntries[4])
+                assertEquals("FIVE", enumEntries[5])
+                assertEquals("SIX", enumEntries[6])
+                assertEquals("SEVEN", enumEntries[7])
+                assertEquals("EIGHT", enumEntries[8])
+                assertEquals("NINE", enumEntries[9])
+            }
+        }
+    }
+
+    @Test
+    fun `should preserve enum source ordering for navigation menu`() {
+        val configuration = dokkaConfiguration {
+            sourceSets {
+                sourceSet {
+                    sourceRoots = listOf("src/")
+                }
+            }
+        }
+
+        val writerPlugin = TestOutputWriterPlugin()
+
+        testInline(
+            """
+            |/src/main/kotlin/basic/Test.kt
+            |package testpackage
+            |
+            |enum class TestEnum {
+            |   ZERO,
+            |   ONE,
+            |   TWO,
+            |   THREE,
+            |   FOUR,
+            |   FIVE,
+            |   SIX,
+            |   SEVEN,
+            |   EIGHT,
+            |   NINE
+            |}
+        """.trimMargin(),
+            configuration,
+            pluginOverrides = listOf(writerPlugin)
+        ) {
+            renderingStage = { _, _ ->
+                val sideMenu = writerPlugin.writer.navigationHtml().select("div.sideMenuPart")
+
+                assertEquals("ZERO", sideMenu.select("#root-nav-submenu-0-0-0").text())
+                assertEquals("ONE", sideMenu.select("#root-nav-submenu-0-0-1").text())
+                assertEquals("TWO", sideMenu.select("#root-nav-submenu-0-0-2").text())
+                assertEquals("THREE", sideMenu.select("#root-nav-submenu-0-0-3").text())
+                assertEquals("FOUR", sideMenu.select("#root-nav-submenu-0-0-4").text())
+                assertEquals("FIVE", sideMenu.select("#root-nav-submenu-0-0-5").text())
+                assertEquals("SIX", sideMenu.select("#root-nav-submenu-0-0-6").text())
+                assertEquals("SEVEN", sideMenu.select("#root-nav-submenu-0-0-7").text())
+                assertEquals("EIGHT", sideMenu.select("#root-nav-submenu-0-0-8").text())
+                assertEquals("NINE", sideMenu.select("#root-nav-submenu-0-0-9").text())
+            }
+        }
+    }
+
+    fun TestOutputWriter.navigationHtml(): Element = contents.getValue("navigation.html").let { Jsoup.parse(it) }
+
+    @Test
+    fun `should handle companion object within enum`() {
+        val configuration = dokkaConfiguration {
+            sourceSets {
+                sourceSet {
+                    sourceRoots = listOf("src/")
+                }
+            }
+        }
+
+        testInline(
+            """
+            |/src/main/kotlin/basic/Test.kt
+            |package testpackage
+            |
+            |enum class TestEnum {
             |   E1,
             |   E2;
             |   companion object {}
@@ -71,18 +269,10 @@ class EnumsTest : BaseAbstractTest() {
                         assertTrue(c.isNotEmpty(), "Classlikes list cannot be empty")
 
                         val enum = c.first() as DEnum
-                        assertEquals(enum.name, "Test")
-                        assertEquals(enum.entries.count(), 2)
                         assertNotNull(enum.companion)
                     }
                 }
             }
-            pagesGenerationStage = { module ->
-                val map = module.getClasslikeToMemberMap()
-                val test = map.filterKeys { it.name == "Test" }.values.firstOrNull()
-                assertNotNull(test, "Test not found")
-                assertTrue(test!!.any { it.name == "E1" } && test.any { it.name == "E2" }) { "Enum entries missing in parent" }
-            }
         }
     }
 
@@ -98,11 +288,11 @@ class EnumsTest : BaseAbstractTest() {
 
         testInline(
             """
-            |/src/main/kotlin/basic/Test.kt
-            |package enums
+            |/src/main/kotlin/basic/TestEnum.kt
+            |package testpackage
             |
             |
-            |enum class Test(name: String, index: Int, excluded: Boolean) {
+            |enum class TestEnum(name: String, index: Int, excluded: Boolean) {
             |   E1("e1", 1, true),
             |   E2("e2", 2, false);
             |}
@@ -125,7 +315,7 @@ class EnumsTest : BaseAbstractTest() {
             pagesGenerationStage = { module ->
                 val entryPage = module.dfs { it.name == "E1" } as ClasslikePageNode
                 val signaturePart = (entryPage.content.dfs {
-                    it is ContentGroup && it.dci.toString() == "[enums/Test.E1///PointingToDeclaration/{\"org.jetbrains.dokka.links.EnumEntryDRIExtra\":{\"key\":\"org.jetbrains.dokka.links.EnumEntryDRIExtra\"}}][Symbol]"
+                    it is ContentGroup && it.dci.toString() == "[testpackage/TestEnum.E1///PointingToDeclaration/{\"org.jetbrains.dokka.links.EnumEntryDRIExtra\":{\"key\":\"org.jetbrains.dokka.links.EnumEntryDRIExtra\"}}][Symbol]"
                 } as ContentGroup)
                 assertEquals("(\"e1\", 1, true)", signaturePart.constructorSignature())
             }
@@ -144,15 +334,15 @@ class EnumsTest : BaseAbstractTest() {
 
         testInline(
             """
-            |/src/main/kotlin/basic/Test.kt
-            |package enums
+            |/src/main/kotlin/basic/TestEnum.kt
+            |package testpackage
             |
             |
             |interface Sample {
             |    fun toBeImplemented(): String
             |}
             |
-            |enum class Test: Sample {
+            |enum class TestEnum: Sample {
             |    E1 {
             |        override fun toBeImplemented(): String = "e1"
             |    }
@@ -176,7 +366,7 @@ class EnumsTest : BaseAbstractTest() {
     }
 
     @Test
-    fun enumWithAnnotationsOnEntries(){
+    fun enumWithAnnotationsOnEntries() {
         val configuration = dokkaConfiguration {
             sourceSets {
                 sourceSet {
@@ -187,10 +377,10 @@ class EnumsTest : BaseAbstractTest() {
 
         testInline(
             """
-            |/src/main/kotlin/basic/Test.kt
-            |package enums
+            |/src/main/kotlin/basic/TestEnum.kt
+            |package testpackage
             |
-            |enum class Test {
+            |enum class TestEnum {
             |    /**
             |       Sample docs for E1
             |    **/
@@ -201,8 +391,8 @@ class EnumsTest : BaseAbstractTest() {
             configuration
         ) {
             pagesTransformationStage = { m ->
-                val entryNode = m.children.first { it.name == "enums" }.children.first { it.name == "Test" }.children.firstIsInstance<ClasslikePageNode>()
-                val signature = (entryNode.content as ContentGroup).dfs { it is ContentGroup && it.dci.toString() == "[enums/Test.E1///PointingToDeclaration/{\"org.jetbrains.dokka.links.EnumEntryDRIExtra\":{\"key\":\"org.jetbrains.dokka.links.EnumEntryDRIExtra\"}}][Cover]" } as ContentGroup
+                val entryNode = m.children.first { it.name == "testpackage" }.children.first { it.name == "TestEnum" }.children.firstIsInstance<ClasslikePageNode>()
+                val signature = (entryNode.content as ContentGroup).dfs { it is ContentGroup && it.dci.toString() == "[testpackage/TestEnum.E1///PointingToDeclaration/{\"org.jetbrains.dokka.links.EnumEntryDRIExtra\":{\"key\":\"org.jetbrains.dokka.links.EnumEntryDRIExtra\"}}][Cover]" } as ContentGroup
 
                 signature.assertNode {
                     header(1) { +"E1" }
@@ -226,10 +416,6 @@ class EnumsTest : BaseAbstractTest() {
         }
     }
 
-
-    fun RootPageNode.getClasslikeToMemberMap() =
-        this.parentMap.filterValues { it is ClasslikePageNode }.entries.groupBy({ it.value }) { it.key }
-
     private fun ContentGroup.constructorSignature(): String =
         (children.single() as ContentGroup).children.drop(1).joinToString(separator = "") { (it as ContentText).text }
 }