From f259003b7bd8e035131c7f90fee9d9b121f955ab Mon Sep 17 00:00:00 2001 From: Oleg Yukhnevich Date: Wed, 3 Jul 2024 16:52:26 +0300 Subject: [PATCH 1/2] Correctly handle case when we have `internal expect` but `public actual` declarations --- .../DefaultDocumentableMerger.kt | 15 +++- .../kotlin/expectActuals/ExpectActualsTest.kt | 83 +++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) diff --git a/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/transformers/documentation/DefaultDocumentableMerger.kt b/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/transformers/documentation/DefaultDocumentableMerger.kt index 2ea9af2f5e..b934819f48 100644 --- a/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/transformers/documentation/DefaultDocumentableMerger.kt +++ b/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/transformers/documentation/DefaultDocumentableMerger.kt @@ -106,9 +106,20 @@ public class DefaultDocumentableMerger(context: DokkaContext) : DocumentableMerg merged as List } + fun processClashingElements(elements: List) = + elements.map { it to it.sourceSets } + .groupBy { it.first.dri } + .values + .flatMap(::mergeClashingElements) fun analyzeExpectActual(sameDriElements: List): List { val (expects, actuals) = sameDriElements.partition { it.expectPresentInSet != null } + // It's possible that there are no `expect` declarations, but there are `actual` declarations, + // e.g. in case `expect` is `internal` or filtered previously for some other reason. + // In this case we process `actual` declarations as just clashing + if (expects.isEmpty()) { + return processClashingElements(actuals) + } val groupedByOwnExpectWithActualSourceSetIds = expects.map { expect -> val actualsForGivenExpect = actuals.filter { actual -> dependencyInfo[actual.sourceSets.single()] @@ -122,12 +133,10 @@ public class DefaultDocumentableMerger(context: DokkaContext) : DocumentableMerg return reducedToOneDocumentableWithActualSourceSetIds.let(::mergeClashingElements) } - return elements.partition { (it as? WithIsExpectActual)?.isExpectActual ?: false }.let { (expectActuals, notExpectActuals) -> - notExpectActuals.map { it to it.sourceSets } - .groupBy { it.first.dri }.values.flatMap(::mergeClashingElements) + + processClashingElements(notExpectActuals) + expectActuals.groupBy { it.dri }.values.flatMap(::analyzeExpectActual) } } diff --git a/dokka-subprojects/plugin-base/src/test/kotlin/expectActuals/ExpectActualsTest.kt b/dokka-subprojects/plugin-base/src/test/kotlin/expectActuals/ExpectActualsTest.kt index 0dcc7b4884..4a9087ea2c 100644 --- a/dokka-subprojects/plugin-base/src/test/kotlin/expectActuals/ExpectActualsTest.kt +++ b/dokka-subprojects/plugin-base/src/test/kotlin/expectActuals/ExpectActualsTest.kt @@ -305,4 +305,87 @@ class ExpectActualsTest : BaseAbstractTest() { } } } + + @Test + fun `public actual function should be present when expect is internal`() = testInline( + """ + /src/common/test.kt + internal expect fun function(): String? + + /src/jvm/test.kt + public actual fun function(): String? = null + + /src/native/test.kt + public actual fun function(): String? = null + """.trimMargin(), + multiplatformConfiguration + ) { + documentablesTransformationStage = { module -> + val functions = module.packages.single().functions.filter { it.name == "function" } + // no `expect` is present, just 2 `actual` functions with the same name + assertEquals(2, functions.size) + functions.forEach { function -> + assertTrue(function.isExpectActual) + } + assertEquals( + setOf("jvm", "native"), + functions.map { it.sourceSets.single().sourceSetID.sourceSetName }.toSet() + ) + } + } + + @Test + fun `public actual function should be present when expect is internal and other actual is internal`() = testInline( + """ + /src/common/test.kt + internal expect fun function(): String? + + /src/jvm/test.kt + public actual fun function(): String? = null + + /src/native/test.kt + internal actual fun function(): String? = null + """.trimMargin(), + multiplatformConfiguration + ) { + documentablesTransformationStage = { module -> + val functions = module.packages.single().functions.filter { it.name == "function" } + assertEquals(1, functions.size) + functions.forEach { function -> + assertTrue(function.isExpectActual) + } + assertEquals( + setOf("jvm"), + functions.map { it.sourceSets.single().sourceSetID.sourceSetName }.toSet() + ) + } + } + + @Test + fun `public actual classe should be present when expect is internal`() = testInline( + """ + /src/common/test.kt + internal expect class Class + + /src/jvm/test.kt + public actual class Class + + /src/native/test.kt + public actual class Class + """.trimMargin(), + multiplatformConfiguration + ) { + documentablesTransformationStage = { module -> + val classlikes = module.packages.single().classlikes.filter { it.name == "Class" } + // no `expect` is present, just 2 `actual` functions with the same name + assertEquals(2, classlikes.size) + classlikes.forEach { classlike -> + assertTrue(classlike.isExpectActual) + } + assertEquals( + setOf("jvm", "native"), + classlikes.map { it.sourceSets.single().sourceSetID.sourceSetName }.toSet() + ) + } + } } From ada38df095713b96714bdf22bc7ece51aac2efe9 Mon Sep 17 00:00:00 2001 From: Oleg Yukhnevich Date: Fri, 5 Jul 2024 11:45:28 +0300 Subject: [PATCH 2/2] Just merge declarations and not handle them as clashing --- .../DefaultDocumentableMerger.kt | 20 +++++------- .../kotlin/expectActuals/ExpectActualsTest.kt | 32 +++++++------------ 2 files changed, 20 insertions(+), 32 deletions(-) diff --git a/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/transformers/documentation/DefaultDocumentableMerger.kt b/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/transformers/documentation/DefaultDocumentableMerger.kt index b934819f48..9de2c2b1a4 100644 --- a/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/transformers/documentation/DefaultDocumentableMerger.kt +++ b/dokka-subprojects/core/src/main/kotlin/org/jetbrains/dokka/transformers/documentation/DefaultDocumentableMerger.kt @@ -106,21 +106,15 @@ public class DefaultDocumentableMerger(context: DokkaContext) : DocumentableMerg merged as List } - fun processClashingElements(elements: List) = - elements.map { it to it.sourceSets } - .groupBy { it.first.dri } - .values - .flatMap(::mergeClashingElements) fun analyzeExpectActual(sameDriElements: List): List { val (expects, actuals) = sameDriElements.partition { it.expectPresentInSet != null } // It's possible that there are no `expect` declarations, but there are `actual` declarations, // e.g. in case `expect` is `internal` or filtered previously for some other reason. - // In this case we process `actual` declarations as just clashing - if (expects.isEmpty()) { - return processClashingElements(actuals) - } - val groupedByOwnExpectWithActualSourceSetIds = expects.map { expect -> + // In this case we just merge `actual` declarations without `expect` + val groupedActualsWithSourceSets = if (expects.isEmpty()) { + listOf(actuals to actuals.flatMap { it.sourceSets }.toSet()) + } else expects.map { expect -> val actualsForGivenExpect = actuals.filter { actual -> dependencyInfo[actual.sourceSets.single()] ?.contains(expect.expectPresentInSet!!) @@ -129,14 +123,16 @@ public class DefaultDocumentableMerger(context: DokkaContext) : DocumentableMerg (listOf(expect) + actualsForGivenExpect) to actualsForGivenExpect.flatMap { it.sourceSets }.toSet() } val reducedToOneDocumentableWithActualSourceSetIds = - groupedByOwnExpectWithActualSourceSetIds.map { it.first.reduce(reducer) to it.second } + groupedActualsWithSourceSets.map { it.first.reduce(reducer) to it.second } return reducedToOneDocumentableWithActualSourceSetIds.let(::mergeClashingElements) } + return elements.partition { (it as? WithIsExpectActual)?.isExpectActual ?: false }.let { (expectActuals, notExpectActuals) -> - processClashingElements(notExpectActuals) + + notExpectActuals.map { it to it.sourceSets } + .groupBy { it.first.dri }.values.flatMap(::mergeClashingElements) + expectActuals.groupBy { it.dri }.values.flatMap(::analyzeExpectActual) } } diff --git a/dokka-subprojects/plugin-base/src/test/kotlin/expectActuals/ExpectActualsTest.kt b/dokka-subprojects/plugin-base/src/test/kotlin/expectActuals/ExpectActualsTest.kt index 4a9087ea2c..e7f535aa49 100644 --- a/dokka-subprojects/plugin-base/src/test/kotlin/expectActuals/ExpectActualsTest.kt +++ b/dokka-subprojects/plugin-base/src/test/kotlin/expectActuals/ExpectActualsTest.kt @@ -321,15 +321,12 @@ class ExpectActualsTest : BaseAbstractTest() { multiplatformConfiguration ) { documentablesTransformationStage = { module -> - val functions = module.packages.single().functions.filter { it.name == "function" } - // no `expect` is present, just 2 `actual` functions with the same name - assertEquals(2, functions.size) - functions.forEach { function -> - assertTrue(function.isExpectActual) - } + val function = module.packages.single().functions.single { it.name == "function" } + assertTrue(function.isExpectActual) + // no `common` is present assertEquals( setOf("jvm", "native"), - functions.map { it.sourceSets.single().sourceSetID.sourceSetName }.toSet() + function.sourceSets.map { it.sourceSetID.sourceSetName }.toSet() ) } } @@ -349,14 +346,12 @@ class ExpectActualsTest : BaseAbstractTest() { multiplatformConfiguration ) { documentablesTransformationStage = { module -> - val functions = module.packages.single().functions.filter { it.name == "function" } - assertEquals(1, functions.size) - functions.forEach { function -> - assertTrue(function.isExpectActual) - } + val function = module.packages.single().functions.single { it.name == "function" } + assertTrue(function.isExpectActual) + // `common` - internal, `native` - internal, so only `jvm` is present assertEquals( setOf("jvm"), - functions.map { it.sourceSets.single().sourceSetID.sourceSetName }.toSet() + function.sourceSets.map { it.sourceSetID.sourceSetName }.toSet() ) } } @@ -376,15 +371,12 @@ class ExpectActualsTest : BaseAbstractTest() { multiplatformConfiguration ) { documentablesTransformationStage = { module -> - val classlikes = module.packages.single().classlikes.filter { it.name == "Class" } - // no `expect` is present, just 2 `actual` functions with the same name - assertEquals(2, classlikes.size) - classlikes.forEach { classlike -> - assertTrue(classlike.isExpectActual) - } + val classlike = module.packages.single().classlikes.single { it.name == "Class" } + assertTrue(classlike.isExpectActual) + // no `common` is present assertEquals( setOf("jvm", "native"), - classlikes.map { it.sourceSets.single().sourceSetID.sourceSetName }.toSet() + classlike.sourceSets.map { it.sourceSetID.sourceSetName }.toSet() ) } }