From bbcc20ce715d55fd2be8e0e88cea9651960510cc Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Thu, 28 Apr 2022 21:45:07 +0300 Subject: [PATCH 1/2] Make javadoc pages generation deterministic --- plugins/javadoc/api/javadoc.api | 1 - .../dokka/javadoc/pages/JavadocPageNodes.kt | 10 +- .../dokka/javadoc/pages/htmlPreprocessors.kt | 27 +++- .../JavadocContentToTemplateMapTranslator.kt | 2 +- .../dokka/javadoc/JavadocDeprecatedTest.kt | 16 ++ .../dokka/javadoc/JavadocIndexTest.kt | 140 +++++++++++------- 6 files changed, 136 insertions(+), 60 deletions(-) diff --git a/plugins/javadoc/api/javadoc.api b/plugins/javadoc/api/javadoc.api index 0eabce5ca4..70a69a7921 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 6c7691cde7..cafc6c3e51 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 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 { 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 a6eda405cf..45fa3f2e82 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 e2331f5816..f301baf6c0 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() { @@ -67,6 +68,21 @@ internal class JavadocDeprecatedTest : AbstractJavadocTemplateMapTest() { } } + @Test + fun `should be sorted by position`() { + testDeprecatedPageTemplateMaps { templateMap -> + @Suppress("UNCHECKED_CAST") + val contents = (templateMap["sections"] as List).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 -> 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 76c345b0e3..575c289fcb 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 genericQuery = """ + /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(genericQuery) { 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(genericQuery) { indexPages -> assertEquals(if (hasAdditionalFunction()) 41 else 40, indexPages.sumBy { it.elements.size }) } } @Test fun `templateMap for class index`() { - testIndexPagesTemplateMaps { templateMaps -> + testIndexPagesTemplateMaps(genericQuery) { templateMaps -> @Suppress("UNCHECKED_CAST") val element = (templateMaps[2]["elements"] as List)[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(genericQuery) { templateMaps -> @Suppress("UNCHECKED_CAST") val element = (templateMaps[0]["elements"] as List).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(genericQuery) { templateMaps -> @Suppress("UNCHECKED_CAST") val element = (templateMaps[0]["elements"] as List).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) {} + fun funName(param: String) {} + fun funName(param: Map) {} + 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) -> 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) -> Unit) { testTemplateMapInline(query) { operation(allPagesOfType()) } } - private fun testIndexPagesTemplateMaps(operation: (List) -> Unit) = + private fun testIndexPagesTemplateMaps(query: String, operation: (List) -> Unit) = testTemplateMapInline(query) { operation(allPagesOfType().map { it.templateMap }) } From b6c3d6516ee11497db7f529d62b02ec0b38894fc Mon Sep 17 00:00:00 2001 From: Ignat Beresnev Date: Thu, 5 May 2022 13:54:05 +0300 Subject: [PATCH 2/2] Rename `genericQuery` to avoid confusion with Generics --- .../org/jetbrains/dokka/javadoc/JavadocIndexTest.kt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 575c289fcb..1fbc21ea3d 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 @@ -9,7 +9,7 @@ import kotlin.test.assertNotNull internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() { - private val genericQuery = """ + private val commonTestQuery = """ /src/source0.kt package package0 /** @@ -59,7 +59,7 @@ internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() { @Test fun `generates correct number of index pages`() { - testIndexPages(genericQuery) { indexPages -> + testIndexPages(commonTestQuery) { indexPages -> assertEquals(12, indexPages.size) } } @@ -70,14 +70,14 @@ internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() { fun hasAdditionalFunction() = AnnotationTarget.ANNOTATION_CLASS::class.java.methods.any { it.name == "describeConstable" } - testIndexPages(genericQuery) { indexPages -> + testIndexPages(commonTestQuery) { indexPages -> assertEquals(if (hasAdditionalFunction()) 41 else 40, indexPages.sumBy { it.elements.size }) } } @Test fun `templateMap for class index`() { - testIndexPagesTemplateMaps(genericQuery) { templateMaps -> + testIndexPagesTemplateMaps(commonTestQuery) { templateMaps -> @Suppress("UNCHECKED_CAST") val element = (templateMaps[2]["elements"] as List)[1] assertEquals("../package0/ClassA.html", element["address"]) @@ -91,7 +91,7 @@ internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() { @Test fun `templateMap for enum entry index`() { - testIndexPagesTemplateMaps(genericQuery) { templateMaps -> + testIndexPagesTemplateMaps(commonTestQuery) { templateMaps -> @Suppress("UNCHECKED_CAST") val element = (templateMaps[0]["elements"] as List).last() assertEquals("../package1/ClassCEnum.html#A", element["address"]) @@ -105,7 +105,7 @@ internal class JavadocIndexTest : AbstractJavadocTemplateMapTest() { @Test fun `templateMap for function index`() { - testIndexPagesTemplateMaps(genericQuery) { templateMaps -> + testIndexPagesTemplateMaps(commonTestQuery) { templateMaps -> @Suppress("UNCHECKED_CAST") val element = (templateMaps[0]["elements"] as List).first() assertEquals("../package0/ClassA.html#a()", element["address"])