Refactor

Kotlin · Feb 24, 2023 · 7ee0138 · 7ee0138
1 parent e884773
commit 7ee0138
Show file tree

Hide file tree

Showing 9 changed files with 78 additions and 83 deletions.
diff --git a/core/src/main/kotlin/model/Documentable.kt b/core/src/main/kotlin/model/Documentable.kt
@@ -519,9 +519,4 @@ interface DocumentableSource {
     val path: String
 }
 
-data class TypeConstructorWithKind(val typeConstructor: TypeConstructor, val kind: ClassKind)
-
-// Annotations might have constructors to substitute reflection invocations
-// and for internal/compiler purposes, but they are not expected to be documented
-// and instantiated directly under normal circumstances, so constructors should not be rendered.
-fun List<Documentable>.shouldDocumentConstructors() = !this.any { it is DAnnotation }
+data class TypeConstructorWithKind(val typeConstructor: TypeConstructor, val kind: ClassKind)
diff --git a/core/src/main/kotlin/pages/contentNodeProperties.kt b/core/src/main/kotlin/pages/contentNodeProperties.kt
@@ -8,7 +8,6 @@ class SimpleAttr(val extraKey: String, val extraValue: String) : ExtraProperty<C
 
 }
 
-const val TOGGLEABLE_CONTENT_TYPE_ATTR = "data-togglable"
 enum class BasicTabbedContentType : TabbedContentType {
     TYPE, CONSTRUCTOR, FUNCTION, PROPERTY, ENTRY, EXTENSION_PROPERTY, EXTENSION_FUNCTION
 }

diff --git a/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt b/plugins/base/src/main/kotlin/renderers/html/HtmlRenderer.kt
@@ -26,6 +26,7 @@ import org.jetbrains.dokka.utilities.htmlEscape
 import org.jetbrains.kotlin.utils.addIfNotNull
 
 internal const val TEMPLATE_REPLACEMENT: String = "###"
+internal const val TOGGLEABLE_CONTENT_TYPE_ATTR = "data-togglable"
 
 open class HtmlRenderer(
     context: DokkaContext
@@ -47,19 +48,17 @@ open class HtmlRenderer(
 
     override val preprocessors = context.plugin<DokkaBase>().query { htmlPreprocessors }
 
-    private fun <R> TagConsumer<R>.prepareForTemplates() =
-        if (context.configuration.delayTemplateSubstitution || this is ImmediateResolutionTagConsumer) this
-        else ImmediateResolutionTagConsumer(this, context)
-
-    override fun ContentNode.build(
-        builder: FlowContent,
-        pageContext: ContentPage,
-        sourceSetRestriction: Set<DisplaySourceSet>?
-    ) {
-        builder.buildContentNode(this, pageContext, sourceSetRestriction)
+    private fun createTabs(pageContext: ContentPage): List<ContentTab> {
+        return when(pageContext) {
+            is ClasslikePage -> createTabsForClasslikes(pageContext)
+            is PackagePage -> createTabsForPackage(pageContext)
+            else -> throw IllegalArgumentException("Page ${pageContext.name} cannot have tabs")
+        }
     }
 
-    private fun createTabs(documentables: List<Documentable>) =  if (documentables.all { it is DClasslike || it is DEnumEntry}) {
+    private fun createTabsForClasslikes(page: ClasslikePage): List<ContentTab> {
+        val documentables = page.documentables
+        fun List<Documentable>.shouldDocumentConstructors() = !this.any { it is DAnnotation }
         val csEnum = documentables.filterIsInstance<DEnum>()
         val csWithConstructor = documentables.filterIsInstance<WithConstructors>()
         val scopes = documentables.filterIsInstance<WithScope>()
@@ -75,17 +74,17 @@ open class HtmlRenderer(
                 ?.filterIsInstance<Documentable>().orEmpty()
         }
             .distinctBy { it.sourceSets to it.dri } // [Documentable] has expensive equals/hashCode at the moment, see #2620
-        listOfNotNull(
+        return listOfNotNull(
             if(!containsRenderableMembers) null else
-            ContentTab(
-                "Members",
-                listOf(
-                    BasicTabbedContentType.CONSTRUCTOR,
-                    BasicTabbedContentType.TYPE,
-                    BasicTabbedContentType.FUNCTION,
-                    BasicTabbedContentType.PROPERTY
-                )
-            ),
+                ContentTab(
+                    "Members",
+                    listOf(
+                        BasicTabbedContentType.CONSTRUCTOR,
+                        BasicTabbedContentType.TYPE,
+                        BasicTabbedContentType.FUNCTION,
+                        BasicTabbedContentType.PROPERTY
+                    )
+                ),
             if (extensions.isEmpty()) null else ContentTab(
                 "Members & Extensions",
                 listOf(
@@ -104,9 +103,11 @@ open class HtmlRenderer(
                 )
             )
         )
-    } else if (documentables.all { it is DPackage }) {
-        val p = documentables.single() as DPackage
-        listOfNotNull(
+    }
+
+    private fun createTabsForPackage(page: PackagePage): List<ContentTab> {
+        val p = page.documentables.single() as DPackage
+        return listOfNotNull(
             ContentTab(
                 "Types",
                 listOf(
@@ -128,8 +129,11 @@ open class HtmlRenderer(
                 )
             )
         )
-    } else throw IllegalStateException("The tabbed content must have ContentTabsExtra")
+    }
 
+    private fun <R> TagConsumer<R>.prepareForTemplates() =
+        if (context.configuration.delayTemplateSubstitution || this is ImmediateResolutionTagConsumer) this
+        else ImmediateResolutionTagConsumer(this, context)
 
     override fun FlowContent.wrapGroup(
         node: ContentGroup,
@@ -139,18 +143,14 @@ open class HtmlRenderer(
         val additionalClasses = node.style.joinToString(" ") { it.toString().toLowerCase() }
         return when {
             node.hasStyle(ContentStyle.TabbedContent) -> div(additionalClasses) {
-                /**
-                 * This is not the bast implementation.
-                 * The idea behind it is to decrease public API that we are not sure about
-                 */
-                val contentTabs =createTabs(pageContext.documentables())
+                val contentTabs = createTabs(pageContext)
 
                 div(classes = "tabs-section") {
                     attributes["tabs-section"] = "tabs-section"
                     contentTabs.forEachIndexed { index, contentTab ->
                         button(classes = "section-tab") {
                             if (index == 0) attributes["data-active"] = ""
-                            attributes["data-togglable"] = contentTab.tabbedContentTypes.joinToString(",")
+                            attributes[TOGGLEABLE_CONTENT_TYPE_ATTR] = contentTab.tabbedContentTypes.joinToString(",")
                             text(contentTab.text)
                         }
                     }
@@ -268,7 +268,7 @@ open class HtmlRenderer(
         div(divStyles) {
             attributes["data-platform-hinted"] = "data-platform-hinted"
             extra.extraHtmlAttributes().forEach { attributes[it.extraKey] = it.extraValue }
-            extra.extraToggleableContentType()?.let { attributes[TOGGLEABLE_CONTENT_TYPE_ATTR] = it.value.toHtmlAttribute() }
+            extra.extraTabbedContentType()?.let { attributes[TOGGLEABLE_CONTENT_TYPE_ATTR] = it.value.toHtmlAttribute() }
             if (renderTabs) {
                 div("platform-bookmarks-row") {
                     attributes["data-toggle-list"] = "data-toggle-list"
@@ -510,8 +510,7 @@ open class HtmlRenderer(
     ) {
         buildAnchor(contextNode)
         div(classes = "table-row") {
-            contextNode.extra.extraHtmlAttributes().forEach { attributes[it.extraKey] = it.extraValue }
-            contextNode.extra.extraToggleableContentType()?.let { attributes[TOGGLEABLE_CONTENT_TYPE_ATTR] = it.value.toHtmlAttribute() }
+            contextNode.extra.extraTabbedContentType()?.let { attributes[TOGGLEABLE_CONTENT_TYPE_ATTR] = it.value.toHtmlAttribute() }
             addSourceSetFilteringAttributes(contextNode)
             div("main-subrow keyValue " + contextNode.style.joinToString(separator = " ")) {
                 buildRowHeaderLink(toRender, pageContext, sourceSetRestriction, contextNode.anchor)
@@ -616,7 +615,7 @@ open class HtmlRenderer(
             node.style.contains(CommentTable) -> buildDefaultTable(node, pageContext, sourceSetRestriction)
             else -> div(classes = "table") {
                 node.extra.extraHtmlAttributes().forEach { attributes[it.extraKey] = it.extraValue }
-                node.extra.extraToggleableContentType()?.let { attributes[TOGGLEABLE_CONTENT_TYPE_ATTR] = it.value.toHtmlAttribute() }
+                node.extra.extraTabbedContentType()?.let { attributes[TOGGLEABLE_CONTENT_TYPE_ATTR] = it.value.toHtmlAttribute() }
                 node.children.forEach {
                     buildRow(it, pageContext, sourceSetRestriction)
                 }
@@ -660,8 +659,7 @@ open class HtmlRenderer(
     override fun FlowContent.buildHeader(level: Int, node: ContentHeader, content: FlowContent.() -> Unit) {
         val classes = node.style.joinToString { it.toString() }.toLowerCase()
         val contentWithExtraAttributes: FlowContent.() -> Unit = {
-            node.extra.extraHtmlAttributes().forEach { attributes[it.extraKey] = it.extraValue }
-            node.extra.extraToggleableContentType()?.let { attributes[TOGGLEABLE_CONTENT_TYPE_ATTR] = it.value.toHtmlAttribute() }
+            node.extra.extraTabbedContentType()?.let { attributes[TOGGLEABLE_CONTENT_TYPE_ATTR] = it.value.toHtmlAttribute() }
             content()
         }
         when (level) {
@@ -883,7 +881,7 @@ open class HtmlRenderer(
         templater.renderFromTemplate(DokkaTemplateTypes.BASE) {
             val generatedContent =
                 createHTML().div("main-content") {
-                    page.getDocumentableType()?.let { attributes["data-documentable-type"] = it }
+                    page.getDocumentableType()?.let { attributes["data-page-type"] = it }
                     id = "content"
                     (page as? ContentPage)?.let {
                         attributes["pageIds"] = "${context.configuration.moduleName}::${page.pageId}"
@@ -974,7 +972,7 @@ private val PageNode.isNavigable: Boolean
     get() = this !is RendererSpecificPage || strategy != RenderingStrategy.DoNothing
 
 private fun PropertyContainer<ContentNode>.extraHtmlAttributes() = allOfType<SimpleAttr>()
-private fun PropertyContainer<ContentNode>.extraToggleableContentType() = this[TabbedContentTypeExtra]
+private fun PropertyContainer<ContentNode>.extraTabbedContentType() = this[TabbedContentTypeExtra]
 
 private val ContentNode.sourceSetsFilters: String
     get() = sourceSets.sourceSetIDs.joinToString(" ") { it.toString() }

diff --git a/plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt b/plugins/base/src/main/kotlin/translators/documentables/DefaultPageCreator.kt
@@ -253,7 +253,7 @@ open class DefaultPageCreator(
             }
         }
         group(styles = setOf(ContentStyle.TabbedContent), extra = mainExtra) {
-            +contentForPackageScope(p, p.dri, p.sourceSets)
+            +contentForScope(p, p.dri, p.sourceSets)
         }
     }
 
@@ -274,8 +274,8 @@ open class DefaultPageCreator(
         )
     }
 
-    protected open fun contentForPackageScope(
-        s: DPackage,
+    protected open fun contentForScope(
+        s: WithScope,
         dri: DRI,
         sourceSets: Set<DokkaSourceSet>,
     ): ContentGroup {
@@ -470,8 +470,7 @@ open class DefaultPageCreator(
     }
 
     protected open fun DocumentableContentBuilder.contentForBrief(
-        documentable: Documentable,
-        extra: PropertyContainer<ContentNode> = mainExtra
+        documentable: Documentable
     ) {
         documentable.sourceSets.forEach { sourceSet ->
             documentable.documentation[sourceSet]?.let {
@@ -485,7 +484,7 @@ open class DefaultPageCreator(
                 it.firstMemberOfTypeOrNull<Description>() ?: it.firstMemberOfTypeOrNull<Property>()
                     .takeIf { documentable is DProperty }
             }?.let {
-                group(sourceSets = setOf(sourceSet), kind = ContentKind.BriefComment, extra = extra) {
+                group(sourceSets = setOf(sourceSet), kind = ContentKind.BriefComment) {
                     if (documentable.hasSeparatePage) createBriefComment(documentable, sourceSet, it)
                     else comment(it.root)
                 }
@@ -739,3 +738,8 @@ internal inline fun <reified T : NamedTagWrapper> GroupedTags.withTypeNamed(): M
         ?.groupByTo(linkedMapOf()) { it.second.name }
         ?.mapValues { (_, v) -> v.toMap() }
         .orEmpty()
+
+// Annotations might have constructors to substitute reflection invocations
+// and for internal/compiler purposes, but they are not expected to be documented
+// and instantiated directly under normal circumstances, so constructors should not be rendered.
+fun List<Documentable>.shouldDocumentConstructors() = !this.any { it is DAnnotation }
diff --git a/plugins/base/src/main/kotlin/translators/documentables/PageContentBuilder.kt b/plugins/base/src/main/kotlin/translators/documentables/PageContentBuilder.kt
@@ -8,7 +8,6 @@ import org.jetbrains.dokka.links.DRI
 import org.jetbrains.dokka.model.Documentable
 import org.jetbrains.dokka.model.SourceSetDependent
 import org.jetbrains.dokka.model.doc.DocTag
-import org.jetbrains.dokka.model.isExtension
 import org.jetbrains.dokka.model.properties.PropertyContainer
 import org.jetbrains.dokka.model.properties.plus
 import org.jetbrains.dokka.model.toDisplaySourceSets
@@ -260,14 +259,10 @@ open class PageContentBuilder(
                             }
                             .map {
                                 val documentables = it.second
-                                val newExtraWithTab =
-                                    if (documentables.all { it.isExtension() }) mainExtra + TabbedContentTypeExtra(
-                                        BasicTabbedContentType.EXTENSION_PROPERTY
-                                    ) else mainExtra
-                                val newExtra = if (needsAnchors) newExtraWithTab + SymbolAnchorHint(
+                                val newExtra = if (needsAnchors) extra + SymbolAnchorHint(
                                     it.first,
                                     kind
-                                ) else newExtraWithTab
+                                ) else extra
                                 buildGroup(
                                     documentables.map { it.dri }.toSet(),
                                     documentables.flatMap { it.sourceSets }.toSet(),

diff --git a/plugins/base/src/main/resources/dokka/styles/style.css b/plugins/base/src/main/resources/dokka/styles/style.css
@@ -1298,7 +1298,8 @@ div.runnablesample {
     float: right;
 }
 
-// the hack to hide the headers inside tabs for a package page
-.main-content[data-documentable-type="package"] .tabs-section-body > h2  !important {
+// the hack to hide the headers inside tabs for a package page because each tab
+// has only one header, and the header text is the same as the tab name, so no point in showing it
+.main-content[data-page-type="package"] .tabs-section-body > h2  !important {
     display: none;
 }
diff --git a/plugins/base/src/test/kotlin/renderers/html/CoverPageTest.kt b/plugins/base/src/test/kotlin/renderers/html/CoverPageTest.kt
@@ -38,7 +38,7 @@ class CoverPageTest : BaseAbstractTest() {
         ) {
             renderingStage = { _, _ ->
                 val content = writerPlugin.writer.renderedContent("root/example/-result/index.html")
-                val tableInheritors = content.select("div.table").single { it.childrenSize() == 2 }
+                val tableInheritors = content.select("div.table").single { it.previousElementSibling()?.text() == "Inheritors" && it.childrenSize() == 2 }
                 assertEquals(tableInheritors.getElementsContainingOwnText("Failed").singleOrNull()?.tagName(), "a")
                 assertEquals(tableInheritors.getElementsContainingOwnText("Success").singleOrNull()?.tagName(), "a")
             }

diff --git a/plugins/base/src/test/kotlin/transformerBuilders/PageTransformerBuilderTest.kt b/plugins/base/src/test/kotlin/transformerBuilders/PageTransformerBuilderTest.kt
@@ -13,7 +13,7 @@ import org.jsoup.Jsoup
 import org.junit.jupiter.api.Test
 import utils.TestOutputWriterPlugin
 import utils.assertContains
-
+import utils.assertNotNull
 class PageTransformerBuilderTest : BaseAbstractTest() {
 
     class ProxyPlugin(transformer: PageTransformer) : DokkaPlugin() {
@@ -136,7 +136,7 @@ class PageTransformerBuilderTest : BaseAbstractTest() {
     }
 
     @Test
-    fun `kotlin constructors tab should exist even though there is primary constructor only`() {
+    fun `kotlin constructors should exist even though there is primary constructor only`() {
         val configuration = dokkaConfiguration {
             sourceSets {
                 sourceSet {
@@ -160,9 +160,12 @@ class PageTransformerBuilderTest : BaseAbstractTest() {
                     .filterIsInstance<ContentGroup>()
                     .single { it.dci.kind == ContentKind.Main }.children
 
-                val existsConstructorsHeader = content.any { tabContent -> tabContent.dfs {  it is ContentText && (it as? ContentText)?.text == "Constructors"} != null }
+                val contentWithConstructorsHeader = content.find { tabContent -> tabContent.dfs {  it is ContentText && (it as? ContentText)?.text == "Constructors"} != null }
+
+                contentWithConstructorsHeader.assertNotNull("contentWithConstructorsHeader")
 
-                assert(existsConstructorsHeader)
+                contentWithConstructorsHeader?.dfs { it.dci.kind == ContentKind.Constructors && it is ContentGroup }
+                    .assertNotNull("constructor group")
             }
         }
     }

diff --git a/plugins/kotlin-as-java/src/test/kotlin/KotlinAsJavaPluginTest.kt b/plugins/kotlin-as-java/src/test/kotlin/KotlinAsJavaPluginTest.kt
@@ -15,6 +15,7 @@ import org.junit.jupiter.api.Test
 import signatures.*
 import utils.*
 import kotlin.test.assertEquals
+import kotlin.test.assertNotNull
 
 class KotlinAsJavaPluginTest : BaseAbstractTest() {
 
@@ -47,19 +48,18 @@ class KotlinAsJavaPluginTest : BaseAbstractTest() {
             pagesGenerationStage = { root ->
                 val content = (root.children.single().children.first { it.name == "TestKt" } as ContentPage).content
 
-                val functionRows = content.mainContents.filterIsInstance<ContentTable>()
-                    .single().children
-
+                val functionRows = content.findTableWithKind(kind = ContentKind.Functions).children
                 functionRows.assertCount(6)
+
+                val propRows = content.findTableWithKind(kind = ContentKind.Properties).children
+                propRows.assertCount(1)
             }
         }
     }
 
-    private fun ContentNode.findTabWithType(type: TabbedContentType): ContentNode? = dfs { node ->
-        node.children.filterIsInstance<ContentGroup>().any { gr ->
-            gr.extra[TabbedContentTypeExtra]?.value == type
-        }
-    }
+    private fun ContentNode.findTableWithKind(kind: ContentKind): ContentNode = dfs { node ->
+        node is ContentTable && node.dci.kind === kind
+    }.let { assertNotNull(it, "the table with kind $kind") }
 
     @Test
     fun topLevelWithClassTest() {
@@ -89,17 +89,17 @@ class KotlinAsJavaPluginTest : BaseAbstractTest() {
             pagesGenerationStage = { root ->
                 val contentList = root.children
                     .flatMap { it.children<ContentPage>() }
-                    .map { it.content }
 
-                val children = contentList.flatMap { content ->
-                    val functionTab = content.findTabWithType(BasicTabbedContentType.FUNCTION)
-                        ?: throw IllegalStateException("A function tab is not found")
-
-                    functionTab.children[0].children.filterIsInstance<ContentTable>()
-                        .flatMap { it.children }
+                contentList.find {it.name == "Test"}.apply {
+                    assertNotNull(this)
+                    content.findTableWithKind(ContentKind.Functions).children.assertCount(2)
+                    content.findTableWithKind(ContentKind.Properties).children.assertCount(1)
+                }
+                contentList.find {it.name == "TestKt"}.apply {
+                    assertNotNull(this)
+                    content.findTableWithKind(ContentKind.Functions).children.assertCount(2)
+                    content.findTableWithKind(ContentKind.Properties).children.assertCount(1)
                 }
-
-                children.assertCount(4)
             }
         }
     }