From 2bcd3da3fb2726fd56a316bb9d78ee4faa5b5b16 Mon Sep 17 00:00:00 2001 From: Oleg Yukhnevich Date: Mon, 11 Dec 2023 15:23:44 +0200 Subject: [PATCH] Refactor separation of members and extensions to be more understandable * cleanup logic a bit * add documentation for unclear things * revert some ABI changes --- .../dokka/pages/contentNodeProperties.kt | 25 ++- .../plugin-base/api/plugin-base.api | 3 +- .../documentables/DefaultPageCreator.kt | 179 +++++++++--------- .../src/test/kotlin/content/ExtensionsTest.kt | 20 +- 4 files changed, 121 insertions(+), 106 deletions(-) diff --git a/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/pages/contentNodeProperties.kt b/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/pages/contentNodeProperties.kt index 64f1957291..f3699ee23c 100644 --- a/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/pages/contentNodeProperties.kt +++ b/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/pages/contentNodeProperties.kt @@ -16,7 +16,30 @@ public class SimpleAttr( } public enum class BasicTabbedContentType : TabbedContentType { - TYPE, CONSTRUCTOR, FUNCTION, PROPERTY, ENTRY, EXTENSION_PROPERTY, EXTENSION_FUNCTION + TYPE, CONSTRUCTOR, + + // property/function here means a different things depending on parent: + // - if parent=package - describes just `top-level` property/function without receiver + // - if parent=classlike - describes `member` property/function, + // it could have receiver (becoming member extension property/function) or not (ordinary member property/function) + // for examples look at docs for `EXTENSION_PROPERTY`, `EXTENSION_FUNCTION` + FUNCTION, PROPERTY, + + ENTRY, + + // property/function here means a different things depending on parent, + // and not just `an extension property/function`: + // example 1: `fun Foo.bar()` - top-level extension function + // - on a page describing `Foo` class `bar` will have type=`EXTENSION_FUNCTION` + // - on a page describing package declarations `bar` will have type=`EXTENSION_FUNCTION` + // example 2: `object Namespace { fun Foo.bar() }` - member extension function + // - on a page describing `Foo` class `bar` will have type=`EXTENSION_FUNCTION` + // - on a page describing `Namespace` object `bar` will have type=`FUNCTION` + // + // These types are needed to separate member functions and extension function on classlike pages. + // The same split rules are also used + // when grouping functions/properties with the same name on pages for classlike and package + EXTENSION_PROPERTY, EXTENSION_FUNCTION } /** diff --git a/dokka-subprojects/plugin-base/api/plugin-base.api b/dokka-subprojects/plugin-base/api/plugin-base.api index d0ff8ffe5e..bdcbdc2718 100644 --- a/dokka-subprojects/plugin-base/api/plugin-base.api +++ b/dokka-subprojects/plugin-base/api/plugin-base.api @@ -1371,7 +1371,8 @@ public class org/jetbrains/dokka/base/translators/documentables/DefaultPageCreat protected fun contentForModule (Lorg/jetbrains/dokka/model/DModule;)Lorg/jetbrains/dokka/pages/ContentGroup; protected fun contentForPackage (Lorg/jetbrains/dokka/model/DPackage;)Lorg/jetbrains/dokka/pages/ContentGroup; protected fun contentForProperty (Lorg/jetbrains/dokka/model/DProperty;)Lorg/jetbrains/dokka/pages/ContentGroup; - protected fun contentForScope (Lorg/jetbrains/dokka/model/WithScope;Lorg/jetbrains/dokka/links/DRI;Ljava/util/Set;)Lorg/jetbrains/dokka/pages/ContentGroup; + protected fun contentForScope (Lorg/jetbrains/dokka/model/WithScope;Lorg/jetbrains/dokka/links/DRI;Ljava/util/Set;Ljava/util/List;)Lorg/jetbrains/dokka/pages/ContentGroup; + public static synthetic fun contentForScope$default (Lorg/jetbrains/dokka/base/translators/documentables/DefaultPageCreator;Lorg/jetbrains/dokka/model/WithScope;Lorg/jetbrains/dokka/links/DRI;Ljava/util/Set;Ljava/util/List;ILjava/lang/Object;)Lorg/jetbrains/dokka/pages/ContentGroup; protected fun contentForScopes (Ljava/util/List;Ljava/util/Set;Ljava/util/List;)Lorg/jetbrains/dokka/pages/ContentGroup; public static synthetic fun contentForScopes$default (Lorg/jetbrains/dokka/base/translators/documentables/DefaultPageCreator;Ljava/util/List;Ljava/util/Set;Ljava/util/List;ILjava/lang/Object;)Lorg/jetbrains/dokka/pages/ContentGroup; protected fun getContentBuilder ()Lorg/jetbrains/dokka/base/translators/documentables/PageContentBuilder; diff --git a/dokka-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/translators/documentables/DefaultPageCreator.kt b/dokka-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/translators/documentables/DefaultPageCreator.kt index 1955c9796a..a8b3639011 100644 --- a/dokka-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/translators/documentables/DefaultPageCreator.kt +++ b/dokka-subprojects/plugin-base/src/main/kotlin/org/jetbrains/dokka/base/translators/documentables/DefaultPageCreator.kt @@ -372,7 +372,14 @@ public open class DefaultPageCreator( } } group(styles = setOf(ContentStyle.TabbedContent), extra = mainExtra) { - +contentForScope(p, p.dri, p.sourceSets) + val (functions, extensionFunctions) = p.functions.partition { it.receiver == null } + val (properties, extensionProperties) = p.properties.partition { it.receiver == null } + +contentForScope( + s = p.copy(functions = functions, properties = properties), + dri = p.dri, + sourceSets = p.sourceSets, + extensions = extensionFunctions + extensionProperties + ) } } } @@ -381,45 +388,22 @@ public open class DefaultPageCreator( scopes: List, sourceSets: Set, extensions: List = emptyList() - ): ContentGroup { - val types = scopes.flatMap { it.classlikes } + scopes.filterIsInstance().flatMap { it.typealiases } - val (extensionProperties, extensionFunctions) = extensions.splitPropsAndFuns() - return contentForScope( - dri = @Suppress("UNCHECKED_CAST") - (scopes as List).dri, - sourceSets = sourceSets, - types = types, - functions = scopes.flatMap { it.functions }, - properties = scopes.flatMap { it.properties }, - extensionFunctions = extensionFunctions, - extensionProperties = extensionProperties - ) - } + ): ContentGroup = contentForScope( + dri = @Suppress("UNCHECKED_CAST") (scopes as List).dri, + sourceSets = sourceSets, + types = scopes.flatMap { it.classlikes } + + scopes.filterIsInstance().flatMap { it.typealiases }, + functions = scopes.flatMap { it.functions }, + properties = scopes.flatMap { it.properties }, + extensions = extensions, + ) protected open fun contentForScope( s: WithScope, dri: DRI, - sourceSets: Set - ): ContentGroup { - val types = listOf( - s.classlikes, - (s as? DPackage)?.typealiases ?: emptyList() - ).flatten() - - // can be used only for packages - val (extensionFunctions, functions) = s.functions.partition { it.receiver != null } - val (extensionProperties, properties) = s.properties.partition { it.receiver != null } - - return contentForScope( - dri = setOf(dri), - sourceSets = sourceSets, - types = types, - functions = functions, - properties = properties, - extensionFunctions = extensionFunctions, - extensionProperties = extensionProperties - ) - } + sourceSets: Set, + extensions: List = emptyList() + ): ContentGroup = contentForScopes(listOf(s), sourceSets, extensions) private fun contentForScope( dri: Set, @@ -427,10 +411,10 @@ public open class DefaultPageCreator( types: List, functions: List, properties: List, - extensionFunctions: List, - extensionProperties: List, + extensions: List, ) = contentBuilder.contentFor(dri, sourceSets) { typesBlock(types) + val (extensionProperties, extensionFunctions) = extensions.splitPropsAndFuns() if (separateInheritedMembers) { val (inheritedFunctions, memberFunctions) = functions.splitInherited() val (inheritedProperties, memberProperties) = properties.splitInherited() @@ -445,33 +429,14 @@ public open class DefaultPageCreator( directExtensionProperties ) = extensionProperties.splitInheritedExtension(dri) - declarationsBlock( - name = "Properties", - isFunctions = false, - declarations = memberProperties, - extensions = directExtensionProperties - ) - declarationsBlock( - name = "Inherited properties", - isFunctions = false, - declarations = inheritedProperties, - extensions = inheritedExtensionProperties - ) - declarationsBlock( - name = "Functions", - isFunctions = true, - declarations = memberFunctions, - extensions = directExtensionFunctions - ) - declarationsBlock( - name = "Inherited functions", - isFunctions = true, - declarations = inheritedFunctions, - extensions = inheritedExtensionFunctions - ) + propertiesBlock("Properties", memberProperties, directExtensionProperties) + propertiesBlock("Inherited properties", inheritedProperties, inheritedExtensionProperties) + + functionsBlock("Functions", memberFunctions, directExtensionFunctions) + functionsBlock("Inherited functions", inheritedFunctions, inheritedExtensionFunctions) } else { - declarationsBlock("Properties", isFunctions = false, properties, extensionProperties) - declarationsBlock("Functions", isFunctions = true, functions, extensionFunctions) + propertiesBlock("Properties", properties, extensionProperties) + functionsBlock("Functions", functions, extensionFunctions) } } @@ -688,7 +653,6 @@ public open class DefaultPageCreator( DivergentElementGroup( name = name, kind = ContentKind.Classlikes, - contentTypeOverride = null, elements = elements ) } @@ -702,29 +666,49 @@ public open class DefaultPageCreator( ) } - private fun DocumentableContentBuilder.declarationsBlock( + private fun DocumentableContentBuilder.functionsBlock( + name: String, + declarations: List, + extensions: List + ) { + functionsOrPropertiesBlock( + name = name, + contentKind = ContentKind.Functions, + contentType = when { + declarations.isEmpty() -> BasicTabbedContentType.EXTENSION_FUNCTION + else -> BasicTabbedContentType.FUNCTION + }, + declarations = declarations, + extensions = extensions + ) + } + + private fun DocumentableContentBuilder.propertiesBlock( name: String, - isFunctions: Boolean, + declarations: List, + extensions: List + ) { + functionsOrPropertiesBlock( + name = name, + contentKind = ContentKind.Properties, + contentType = when { + declarations.isEmpty() -> BasicTabbedContentType.EXTENSION_PROPERTY + else -> BasicTabbedContentType.PROPERTY + }, + declarations = declarations, + extensions = extensions + ) + } + + private fun DocumentableContentBuilder.functionsOrPropertiesBlock( + name: String, + contentKind: ContentKind, + contentType: BasicTabbedContentType, declarations: List, extensions: List ) { if (declarations.isEmpty() && extensions.isEmpty()) return - val contentKind = when { - isFunctions -> ContentKind.Functions - else -> ContentKind.Properties - } - - val declarationContentType = when { - isFunctions -> BasicTabbedContentType.FUNCTION - else -> BasicTabbedContentType.PROPERTY - } - - val extensionContentType = when { - isFunctions -> BasicTabbedContentType.EXTENSION_FUNCTION - else -> BasicTabbedContentType.EXTENSION_PROPERTY - } - // This groupBy should probably use LocationProvider val grouped = declarations.groupBy { NameAndIsExtension(it.name, isExtension = false) @@ -741,10 +725,6 @@ public open class DefaultPageCreator( nameAndIsExtension.isExtension -> ContentKind.Extensions else -> contentKind }, - contentTypeOverride = when { - nameAndIsExtension.isExtension -> extensionContentType - else -> null - }, elements = elements ) } @@ -753,11 +733,7 @@ public open class DefaultPageCreator( name = name, kind = contentKind, extra = mainExtra, - contentType = when { - // if only extensions - declarations.isEmpty() -> extensionContentType - else -> declarationContentType - }, + contentType = contentType, groups = groups ) } @@ -774,7 +750,6 @@ public open class DefaultPageCreator( private class DivergentElementGroup( val name: String?, val kind: ContentKind, - val contentTypeOverride: BasicTabbedContentType?, val elements: List ) @@ -786,8 +761,9 @@ public open class DefaultPageCreator( groups: List, ) { if (groups.isEmpty()) return + + // be careful: extra here will be applied for children by default group(extra = extra + TabbedContentTypeExtra(contentType)) { - // be careful: groupExtra will be applied for children by default header(2, name, kind = kind, extra = extra) { } table(kind, extra = extra, styles = emptySet()) { header { @@ -799,6 +775,21 @@ public open class DefaultPageCreator( val rowKind = group.kind val sortedElements = sortDivergentElementsDeterministically(group.elements) + // This override here is needed to be able to split members and extensions into separate tabs in HTML renderer. + // The idea is that `contentType` is set to the `tab group` itself to `FUNCTION` or `PROPERTY` (above in the code), + // and then for `extensions` we override it - in this case we are able to create 2 tabs in HTML renderer: + // - `Members` - which show ONLY member functions/properties + // - `Members & Extensions` - which show BOTH member functions/properties and extensions for this classlike + val rowContentTypeOverride = when (rowKind) { + ContentKind.Extensions -> when (contentType) { + BasicTabbedContentType.FUNCTION -> BasicTabbedContentType.EXTENSION_FUNCTION + BasicTabbedContentType.PROPERTY -> BasicTabbedContentType.EXTENSION_PROPERTY + else -> null + } + + else -> null + } + row( dri = sortedElements.map { it.dri }.toSet(), sourceSets = sortedElements.flatMap { it.sourceSets }.toSet(), @@ -806,7 +797,7 @@ public open class DefaultPageCreator( styles = emptySet(), extra = extra.addAll( listOfNotNull( - group.contentTypeOverride?.let(::TabbedContentTypeExtra), + rowContentTypeOverride?.let(::TabbedContentTypeExtra), elementName?.let { name -> SymbolAnchorHint(name, kind) } ) ) diff --git a/dokka-subprojects/plugin-base/src/test/kotlin/content/ExtensionsTest.kt b/dokka-subprojects/plugin-base/src/test/kotlin/content/ExtensionsTest.kt index 8157c2c04f..fe6ace5948 100644 --- a/dokka-subprojects/plugin-base/src/test/kotlin/content/ExtensionsTest.kt +++ b/dokka-subprojects/plugin-base/src/test/kotlin/content/ExtensionsTest.kt @@ -37,7 +37,7 @@ class ExtensionsTest : BaseAbstractTest() { group { assertTabGroup("Functions", BasicTabbedContentType.EXTENSION_FUNCTION) { assertTableWithTabs( - "extension" to BasicTabbedContentType.EXTENSION_FUNCTION + "extension" to null ) } } @@ -119,7 +119,7 @@ class ExtensionsTest : BaseAbstractTest() { group { assertTabGroup("Functions", BasicTabbedContentType.EXTENSION_FUNCTION) { assertTableWithTabs( - "topLevelExtension" to BasicTabbedContentType.EXTENSION_FUNCTION + "topLevelExtension" to null ) } } @@ -214,7 +214,7 @@ class ExtensionsTest : BaseAbstractTest() { group { assertTabGroup("Functions", BasicTabbedContentType.EXTENSION_FUNCTION) { assertTableWithTabs( - "topLevelExtension" to BasicTabbedContentType.EXTENSION_FUNCTION + "topLevelExtension" to null ) } } @@ -246,7 +246,7 @@ class ExtensionsTest : BaseAbstractTest() { group { assertTabGroup("Functions", BasicTabbedContentType.EXTENSION_FUNCTION) { assertTableWithTabs( - "extensionFromB" to BasicTabbedContentType.EXTENSION_FUNCTION + "extensionFromB" to null ) } } @@ -322,7 +322,7 @@ class ExtensionsTest : BaseAbstractTest() { assertTabGroup("Types", BasicTabbedContentType.TYPE) { skipAllNotMatching() } assertTabGroup("Functions", BasicTabbedContentType.EXTENSION_FUNCTION) { assertTableWithTabs( - "companionMemberExtensionForA" to BasicTabbedContentType.EXTENSION_FUNCTION, + "companionMemberExtensionForA" to null, ) } } @@ -362,7 +362,7 @@ class ExtensionsTest : BaseAbstractTest() { group { assertTabGroup("Properties", BasicTabbedContentType.EXTENSION_PROPERTY) { assertTableWithTabs( - "extension" to BasicTabbedContentType.EXTENSION_PROPERTY + "extension" to null ) } } @@ -444,7 +444,7 @@ class ExtensionsTest : BaseAbstractTest() { group { assertTabGroup("Properties", BasicTabbedContentType.EXTENSION_PROPERTY) { assertTableWithTabs( - "topLevelExtension" to BasicTabbedContentType.EXTENSION_PROPERTY + "topLevelExtension" to null ) } } @@ -539,7 +539,7 @@ class ExtensionsTest : BaseAbstractTest() { group { assertTabGroup("Properties", BasicTabbedContentType.EXTENSION_PROPERTY) { assertTableWithTabs( - "topLevelExtension" to BasicTabbedContentType.EXTENSION_PROPERTY + "topLevelExtension" to null ) } } @@ -571,7 +571,7 @@ class ExtensionsTest : BaseAbstractTest() { group { assertTabGroup("Properties", BasicTabbedContentType.EXTENSION_PROPERTY) { assertTableWithTabs( - "extensionFromB" to BasicTabbedContentType.EXTENSION_PROPERTY + "extensionFromB" to null ) } } @@ -647,7 +647,7 @@ class ExtensionsTest : BaseAbstractTest() { assertTabGroup("Types", BasicTabbedContentType.TYPE) { skipAllNotMatching() } assertTabGroup("Properties", BasicTabbedContentType.EXTENSION_PROPERTY) { assertTableWithTabs( - "companionMemberExtensionForA" to BasicTabbedContentType.EXTENSION_PROPERTY, + "companionMemberExtensionForA" to null, ) } }