Improve code around AllTypesPageNode construction after PR review

Kotlin · Nov 2, 2023 · 7ae7926 · 7ae7926
1 parent 0598517
commit 7ae7926
Show file tree

Hide file tree

Showing 4 changed files with 103 additions and 23 deletions.
diff --git a/core/api/core.api b/core/api/core.api
@@ -3481,8 +3481,8 @@ public abstract interface class org/jetbrains/dokka/model/properties/WithExtraPr
 
 public final class org/jetbrains/dokka/pages/AllTypesPageNode : org/jetbrains/dokka/pages/ContentPage {
 	public static final field Companion Lorg/jetbrains/dokka/pages/AllTypesPageNode$Companion;
-	public fun <init> (Ljava/lang/String;Lorg/jetbrains/dokka/pages/ContentNode;Ljava/util/List;Ljava/util/List;)V
-	public synthetic fun <init> (Ljava/lang/String;Lorg/jetbrains/dokka/pages/ContentNode;Ljava/util/List;Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public fun <init> (Lorg/jetbrains/dokka/pages/ContentNode;Ljava/util/List;)V
+	public synthetic fun <init> (Lorg/jetbrains/dokka/pages/ContentNode;Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public fun getChildren ()Ljava/util/List;
 	public fun getContent ()Lorg/jetbrains/dokka/pages/ContentNode;
 	public fun getDocumentable ()Lorg/jetbrains/dokka/model/Documentable;

diff --git a/core/src/main/kotlin/pages/PageNodes.kt b/core/src/main/kotlin/pages/PageNodes.kt
@@ -102,12 +102,12 @@ public class ModulePageNode(
 }
 
 public class AllTypesPageNode(
-    override val name: String,
     override val content: ContentNode,
-    override val children: List<PageNode>,
     override val embeddedResources: List<String> = listOf()
 ) : ContentPage {
     override val dri: Set<DRI> = setOf(DRI)
+    override val name: String = "All Types"
+    override val children: List<PageNode> get() = emptyList()
 
     override fun modified(name: String, children: List<PageNode>): AllTypesPageNode =
         modified(name = name, content = this.content, dri = dri, children = children)
@@ -120,7 +120,7 @@ public class AllTypesPageNode(
         children: List<PageNode>
     ): AllTypesPageNode =
         if (name == this.name && content === this.content && embeddedResources === this.embeddedResources && children shallowEq this.children) this
-        else AllTypesPageNode(name, content, children, embeddedResources)
+        else AllTypesPageNode(content, embeddedResources)
 
     public companion object {
         public val DRI: DRI = DRI(packageName = ".alltypes")

diff --git a/plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt b/plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt
@@ -49,21 +49,13 @@ public open class DefaultPageCreator(
         configuration?.separateInheritedMembers ?: DokkaBaseConfiguration.separateInheritedMembersDefault
 
     public open fun pageForModule(m: DModule): ModulePageNode {
-        val hasTypes = m.packages.any {
-            it.classlikes.isNotEmpty() || it.typealiases.isNotEmpty()
-        }
         val packagePages = m.packages.map(::pageForPackage)
         return ModulePageNode(
             name = m.name.ifEmpty { "<root>" },
             content = contentForModule(m),
             documentables = listOf(m),
             children = when {
-                hasTypes && shouldDisplayAllTypesPage() -> packagePages + AllTypesPageNode(
-                    name = "All Types",
-                    content = contentForAllTypes(m),
-                    children = emptyList()
-                )
-
+                m.needAllTypesPage -> packagePages + AllTypesPageNode(content = contentForAllTypes(m))
                 else -> packagePages
             }
         )
@@ -281,18 +273,15 @@ public open class DefaultPageCreator(
             ) { pkg ->
                 val comment = pkg.sourceSets.mapNotNull { sourceSet ->
                     pkg.descriptions[sourceSet]?.let { sourceSet to it }
-                }.distinctBy { it.second }.singleOrNull()
+                }.distinctBy { firstParagraphBrief(it.second.root) }.singleOrNull()
 
                 link(pkg.name, pkg.dri)
                 comment?.let { (sourceSet, description) ->
                     createBriefComment(pkg, sourceSet, description)
                 }
             }
 
-            val hasTypes = m.packages.any {
-                it.classlikes.isNotEmpty() || it.typealiases.isNotEmpty()
-            }
-            if (hasTypes && shouldDisplayAllTypesPage()) {
+            if (m.needAllTypesPage) {
                 header(2, "Index", kind = ContentKind.Cover)
                 link("All Types", AllTypesPageNode.DRI)
             }
@@ -326,8 +315,9 @@ public open class DefaultPageCreator(
             //  2. if not, try to take common data
             //  3. if not, try to take JVM data (as this is most likely to be the best variant)
             //  4. if not, just take any data
-            fun <T> List<Pair<DokkaSourceSet, T>>.selectBestVariant(): Pair<DokkaSourceSet, T>? {
-                val uniqueElements = distinctBy { it.second }
+            fun <T, K> List<Pair<DokkaSourceSet, T>>.selectBestVariant(selector: (T) -> K): Pair<DokkaSourceSet, T>? {
+                if (isEmpty()) return null
+                val uniqueElements = distinctBy { selector(it.second) }
                 return uniqueElements.singleOrNull()
                     ?: uniqueElements.firstOrNull { it.first.analysisPlatform == Platform.common }
                     ?: uniqueElements.firstOrNull { it.first.analysisPlatform == Platform.jvm }
@@ -336,12 +326,12 @@ public open class DefaultPageCreator(
 
             val comment = typelike.sourceSets.mapNotNull { sourceSet ->
                 typelike.descriptions[sourceSet]?.let { sourceSet to it }
-            }.selectBestVariant()
+            }.selectBestVariant { firstParagraphBrief(it.root) }
 
             val customTags = typelike.customTags.values.mapNotNull { sourceSetTag ->
                 typelike.sourceSets.mapNotNull { sourceSet ->
                     sourceSetTag[sourceSet]?.let { sourceSet to it }
-                }.selectBestVariant()
+                }.selectBestVariant { it }
             }
 
             link(typelike.qualifiedName(), typelike.dri)
@@ -821,6 +811,14 @@ public open class DefaultPageCreator(
         internal const val SHOULD_DISPLAY_ALL_TYPES_PAGE_SYS_PROP = "dokka.shouldDisplayAllTypesPage"
         private fun shouldDisplayAllTypesPage() =
             System.getProperty(SHOULD_DISPLAY_ALL_TYPES_PAGE_SYS_PROP) in listOf("true", "1")
+
+        private val DModule.needAllTypesPage: Boolean
+            get() {
+                val hasTypes = packages.any {
+                    it.classlikes.isNotEmpty() || it.typealiases.isNotEmpty()
+                }
+                return hasTypes && shouldDisplayAllTypesPage()
+            }
     }
 }
 

diff --git a/plugins/base/src/test/kotlin/content/AllTypesPageTest.kt b/plugins/base/src/test/kotlin/content/AllTypesPageTest.kt
@@ -156,4 +156,86 @@ class AllTypesPageTest : BaseAbstractTest() {
             }
         }
     }
+
+    @Test
+    fun `all types description should be taken from most relevant sourceSet`() = withAllTypesPage {
+        testInline(
+            """
+            |/src/common/test.kt
+            |package test
+            |/**
+            | * Common
+            | */
+            |expect class FromCommon
+            |expect class FromJvm
+            |expect class FromNative
+            |/src/jvm/test.kt
+            |package test
+            |/**
+            | * JVM
+            | */
+            |actual class FromCommon
+            |/**
+            | * JVM
+            | */
+            |actual class FromJvm
+            |actual class FromNative
+            |/src/native/test.kt
+            |package test
+            |/**
+            | * Native
+            | */
+            |actual class FromCommon
+            |actual class FromJvm
+            |/**
+            | * Native
+            | */
+            |actual class FromNative
+            """.trimIndent(),
+            dokkaConfiguration {
+                sourceSets {
+                    val commonId = sourceSet {
+                        sourceRoots = listOf("src/common/")
+                        analysisPlatform = "common"
+                        name = "common"
+                    }.value.sourceSetID
+                    sourceSet {
+                        sourceRoots = listOf("src/jvm/")
+                        analysisPlatform = "jvm"
+                        name = "jvm"
+                        dependentSourceSets = setOf(commonId)
+                    }
+                    sourceSet {
+                        sourceRoots = listOf("src/native/")
+                        analysisPlatform = "native"
+                        name = "native"
+                        dependentSourceSets = setOf(commonId)
+                    }
+                }
+            }
+        ) {
+            pagesTransformationStage = { rootPage ->
+                assertNotNull(rootPage.allTypesPageNode()).content.assertNode {
+                    group {
+                        header { +"root" } // module name
+                    }
+                    header { +"All Types" }
+                    table {
+                        group {
+                            link { +"test.FromCommon" }
+                            group { group { +"Common" } }
+                        }
+                        group {
+                            link { +"test.FromJvm" }
+                            group { group { +"JVM" } }
+                        }
+                        group {
+                            link { +"test.FromNative" }
+                            group { group { +"Native" } }
+                        }
+                    }
+                }
+            }
+        }
+    }
 }