[LL] Sort modules from the same KMP project in symbol providers
Sort dependency modules topologically if they belong to the same KMP project, preserving their relative positions. Sorting all modules and changing positions of unrelated modules can be harmful: in the case of a classpath hell, IDE results can become different from runtime behavior. Sorting in place can help to avoid this problem, because conflicting declarations shouldn't be allowed in different source sets of the same multiplatform project. KTIJ-27569
This commit is contained in:
Pavel Kirpichenkov
committed by
Space Team
Space Team
parent
16b1caff7e
commit
ce3c05500e
+115
@@ -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<KtModule>) {
|
||||
private val groupsByModules = mutableMapOf<KtModule, KmpGroup>()
|
||||
private val originalPositions = mutableMapOf<KtModule, Int>()
|
||||
|
||||
private fun sort(): List<KtModule> {
|
||||
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<KtModule> {
|
||||
val sortedModules = arrayOfNulls<KtModule>(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<KtModule, KmpGroup>,
|
||||
): 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<KtModule>()
|
||||
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<KtModule>): List<KtModule> {
|
||||
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
|
||||
}
|
||||
}
|
||||
}
|
||||
+3
-1
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
-33
@@ -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(<!ARGUMENT_TYPE_MISMATCH!>handlerBase<!>)
|
||||
handlerBase.<!UNRESOLVED_REFERENCE!>foo<!>()
|
||||
}
|
||||
@@ -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<Any>(), topologicalSort(emptyList<Any>()) { emptyList<Any>() })
|
||||
}
|
||||
|
||||
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<String>) {
|
||||
val graph = parseFromString(description)
|
||||
assertEquals("Incorrect order of sorted nodes", expected, sortedNodes(graph))
|
||||
}
|
||||
|
||||
private fun <T> sortedNodes(graph: Graph<T>): List<T> {
|
||||
return topologicalSort(graph.nodes) { graph.edges[this].orEmpty() }
|
||||
}
|
||||
}
|
||||
|
||||
private class Graph<T>(
|
||||
val nodes: Set<T>,
|
||||
val edges: Map<T, List<T>>
|
||||
)
|
||||
|
||||
/**
|
||||
* Expected format: line with the list of nodes, line with the list of edges.
|
||||
* [node[;...]]
|
||||
* [node > node[;...]]
|
||||
*/
|
||||
private fun parseFromString(description: String): Graph<String> {
|
||||
val (nodeStrings, edgeStrings) = description.lines()
|
||||
val nodes = nodeStrings.split(";").map(String::trim).toSet()
|
||||
val edges = buildMap<String, MutableList<String>> {
|
||||
edgeStrings.takeIf { it.isNotBlank() }?.split(";")?.forEach { edgeDescription ->
|
||||
val (from, to) = edgeDescription.split(">").map(String::trim)
|
||||
getOrPut(from) { mutableListOf() }.add(to)
|
||||
}
|
||||
}
|
||||
|
||||
return Graph(nodes, edges)
|
||||
}
|
||||
Reference in New Issue
Block a user