diff --git a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/providers/KmpModuleSorter.kt b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/providers/KmpModuleSorter.kt new file mode 100644 index 00000000000..40411fe9ec9 --- /dev/null +++ b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/providers/KmpModuleSorter.kt @@ -0,0 +1,115 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.analysis.low.level.api.fir.providers + +import org.jetbrains.kotlin.analysis.project.structure.KtModule +import org.jetbrains.kotlin.utils.topologicalSort + +/** + * Sorts modules that belong to the same KMP project: (A dependsOn B) -> (A goes before B). + * It's necessary to give 'actual' declarations from platform modules higher priority. + * Preserves positions of other non-KMP modules unchanged to avoid potential side effects. + * In case of an unexpected error returns the modules unchanged. + * + * A case when relative position of a non-KMP module to a KMP one changes behaviour is not supported as it conflicts with the KMP logic. + * E.g., a hypothetical situation, when changing order from KMP1 - NOKMP - KMP2 to KMP2 - NOKMP - KMP1 + * changes resolution (declaration clash between NOKMP and one of the modules where its relative position matters). + * Reasons for the decision: + * - For the KMP & NOKMP case, the problem arises from project misconfiguration (classpath hell). + * For KMP dependencies, the IDE gives false positive errors when a project is correctly configured (expect-actual), which is worse. + * - Dependencies from a single KMP project/library normally imported as a single sequential block anyway + */ +class KmpModuleSorter private constructor(private val modules: List) { + private val groupsByModules = mutableMapOf() + private val originalPositions = mutableMapOf() + + private fun sort(): List { + groupModules() + return sortModules() + } + + private fun groupModules() { + for ((index, module) in modules.withIndex()) { + originalPositions[module] = index + val group = findOrCreateKmpGroup(module, groupsByModules) + group.addModule(module) + groupsByModules.putIfAbsent(module, group) + module.directDependsOnDependencies.forEach { dependency -> groupsByModules.putIfAbsent(dependency, group) } + } + } + + private fun sortModules(): List { + val sortedModules = arrayOfNulls(modules.size) + modules.forEachIndexed { index, module -> + val group = groupsByModules[module] + if (group == null) { + sortedModules[index] = module + } else { + sortedModules[group.getUpdatedIndexOf(module)] = module + } + } + + return sortedModules.toList().filterNotNull() + } + + private fun findOrCreateKmpGroup( + module: KtModule, + groups: MutableMap, + ): KmpGroup { + groups[module]?.let { return it } + module.directDependsOnDependencies.firstNotNullOfOrNull { dependency -> groups[dependency] }?.let { return it } + return KmpGroup() + } + + /** + * Modules, corresponding to source sets of the same KMP project. + */ + private inner class KmpGroup() { + private val modules = mutableListOf() + private val sortedModules by lazy { + topologicalSort(modules) { directDependsOnDependencies }.also { + check(it.size == modules.size) { "The number of sorted modules doesn't match the number of registered modules" } + } + } + + private val oldReplacedModulesBySortedModules by lazy { sortedModules.zip(modules).toMap() } + + fun addModule(module: KtModule) { + modules.add(module) + } + + fun getUpdatedIndexOf(module: KtModule): Int { + check(module in modules) + if (modules.size == 1) return originalPositions[module] + ?: error("Can't find position for module: $module") + + return oldReplacedModulesBySortedModules[module]?.let { originalPositions[it] } + ?: error("Can't find position for module: $module") + } + + // N.B.: evaluating debug text before all modules are registered will corrupt the group + @Suppress("unused") + fun debugText(): String = + oldReplacedModulesBySortedModules.entries.joinToString(separator = "; ", prefix = "[", postfix = "]") { (sorted, replaced) -> + "$sorted -> $replaced (ix -> ix': ${originalPositions[sorted]} -> ${originalPositions[replaced]})" + } + } + + companion object { + fun order(modules: List): List { + if (modules.size < 2) return modules + + val sorter = KmpModuleSorter(modules) + val sortedModules = sorter.sort() + + check(sortedModules.size == modules.size) { + "The resulting number of modules (${sortedModules.size}) doesn't match the number of input modules ($modules.size)" + } + + return sortedModules + } + } +} diff --git a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirAbstractSessionFactory.kt b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirAbstractSessionFactory.kt index 2e0680b70ac..28a077f486a 100644 --- a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirAbstractSessionFactory.kt +++ b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/sessions/LLFirAbstractSessionFactory.kt @@ -617,7 +617,9 @@ internal abstract class LLFirAbstractSessionFactory(protected val project: Proje addAll(module.transitiveDependsOnDependencies) } - val dependencySessions = dependencyModules.mapNotNull(::getOrCreateSessionForDependency) + val orderedDependencyModules = KmpModuleSorter.order(dependencyModules.toList()) + + val dependencySessions = orderedDependencyModules.mapNotNull(::getOrCreateSessionForDependency) return computeFlattenedSymbolProviders(dependencySessions) } diff --git a/compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.ll.kt b/compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.ll.kt deleted file mode 100644 index 18d606b92d9..00000000000 --- a/compiler/testData/diagnostics/tests/multiplatform/hmpp/intermediateActualHasAdditionalSupertypes.ll.kt +++ /dev/null @@ -1,33 +0,0 @@ -// LL_FIR_DIVERGENCE -// False positive reports of ARGUMENT_TYPE_MISMATCH and UNRESOLVED_REFERENCE are due to bug KT-63382. -// LL_FIR_DIVERGENCE -// ISSUE: KT-57369 - -// MODULE: common -// TARGET_PLATFORM: Common -interface CompletionHandler { - fun foo() -} - -expect class CompletionHandlerBase() - -fun invokeOnCompletion(handler: CompletionHandler) {} - -// MODULE: intermediate()()(common) -// TARGET_PLATFORM: Common -// actual has an additional super type -actual class CompletionHandlerBase : CompletionHandler { - override fun foo() {} -} - -fun cancelFutureOnCompletionAlt(handlerBase: CompletionHandlerBase) { - invokeOnCompletion(handlerBase) - handlerBase.foo() -} - -// MODULE: main()()(common, intermediate) -// the order of dependencies is important to reproduce KT-57369 -fun cancelFutureOnCompletion(handlerBase: CompletionHandlerBase) { - invokeOnCompletion(handlerBase) - handlerBase.foo() -} diff --git a/compiler/util/tests/org/jetbrains/kotlin/utils/TopologicalSortTest.kt b/compiler/util/tests/org/jetbrains/kotlin/utils/TopologicalSortTest.kt new file mode 100644 index 00000000000..7bf25d52316 --- /dev/null +++ b/compiler/util/tests/org/jetbrains/kotlin/utils/TopologicalSortTest.kt @@ -0,0 +1,137 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.utils + +import junit.framework.TestCase +import kotlin.test.assertFails + +class TopologicalSortTest : TestCase() { + fun testEmptyGraph() { + assertEquals("", emptyList(), topologicalSort(emptyList()) { emptyList() }) + } + + fun testSingleNode() { + checkGraph( + """ + A + + """.trimIndent(), + listOf("A") + ) + } + + fun testSimpleGraph() { + checkGraph( + """ + A; B; C + A > B; B > C; A > C + """.trimIndent(), + listOf("A", "B", "C") + ) + } + + fun testSimpleGraphShuffledNodes() { + checkGraph( + """ + C; A; B + A > B; B > C; A > C + """.trimIndent(), + listOf("A", "B", "C") + ) + } + + fun testBamboo() { + checkGraph( + """ + A; B; C + C > B; B > A + """.trimIndent(), + listOf("C", "B", "A") + ) + } + + fun testLongerPath() { + checkGraph( + """ + A; B; C; D; E + E > D; E > B; D > C; C > B; C > A; B > A + """.trimIndent(), + listOf("E", "D", "C", "B", "A") + ) + } + + fun testRepeatedEdges() { + checkGraph( + """ + A; B; C + C > B; C > B; B > A; C > A + """.trimIndent(), + listOf("C", "B", "A") + ) + } + + fun testSelfLoopReport() { + val graph = parseFromString( + """ + A + A > A + """.trimIndent() + ) + assertFails { topologicalSort(graph.nodes, dependencies = { graph.edges[this].orEmpty() }) } + } + + fun testDirectLoopReport() { + val graph = parseFromString( + """ + A; B + A > B; B > A + """.trimIndent() + ) + assertFails { topologicalSort(graph.nodes, dependencies = { graph.edges[this].orEmpty() }) } + } + + fun testLongLoopReport() { + val graph = parseFromString( + """ + A; B; C; D + D > C; C > B; C > A; A > D + """.trimIndent() + ) + assertFails { topologicalSort(graph.nodes, dependencies = { graph.edges[this].orEmpty() }) } + } + + private fun checkGraph(description: String, expected: List) { + val graph = parseFromString(description) + assertEquals("Incorrect order of sorted nodes", expected, sortedNodes(graph)) + } + + private fun sortedNodes(graph: Graph): List { + return topologicalSort(graph.nodes) { graph.edges[this].orEmpty() } + } +} + +private class Graph( + val nodes: Set, + val edges: Map> +) + +/** + * Expected format: line with the list of nodes, line with the list of edges. + * [node[;...]] + * [node > node[;...]] + */ +private fun parseFromString(description: String): Graph { + val (nodeStrings, edgeStrings) = description.lines() + val nodes = nodeStrings.split(";").map(String::trim).toSet() + val edges = buildMap> { + edgeStrings.takeIf { it.isNotBlank() }?.split(";")?.forEach { edgeDescription -> + val (from, to) = edgeDescription.split(">").map(String::trim) + getOrPut(from) { mutableListOf() }.add(to) + } + } + + return Graph(nodes, edges) +}