[Gradle] Improve KotlinSourceSetTreeDependsOnMismatch heuristic
Introduce "White Crow" heuristic that will report incorrect dependency edge when there is only one source set with different Source Set Tree. Otherwise, report all source sets from all Source Set Tree groups to user to choose which of them are incorrect. And cover with more tests. ^KT-47144 Verification Pending
This commit is contained in:
Anton Lakotka
committed by
Space Team
Space Team
parent
681305c8e9
commit
a4705050b1
+25
-6
@@ -331,19 +331,38 @@ object KotlinToolingDiagnostics {
|
||||
)
|
||||
}
|
||||
|
||||
object KotlinSourceSetTreeDependsOnMismatch : ToolingDiagnosticFactory(ERROR) {
|
||||
object KotlinSourceSetTreeDependsOnMismatch : ToolingDiagnosticFactory(WARNING) {
|
||||
operator fun invoke(dependeeName: String, dependencyName: String) = build(
|
||||
"""
|
||||
KotlinSourceSets '${dependeeName}' can't depend on '${dependencyName}'
|
||||
as they belong to different KotlinSourceSet trees. Please remove this dependency.
|
||||
Kotlin Source Set '$dependeeName' can't depend on '$dependencyName' as they are from different Source Set Trees.
|
||||
Please remove this dependency edge.
|
||||
""".trimIndent()
|
||||
)
|
||||
|
||||
operator fun invoke(dependeeAName: String, dependeeBName: String, dependencyName: String) = build(
|
||||
operator fun invoke(dependents: Map<String, List<String>>, dependencyName: String) = build(
|
||||
"""
|
||||
KotlinSourceSets '${dependeeAName}' and '${dependeeBName}' can't both depend on '${dependencyName}'
|
||||
as they belong to different KotlinSourceSet trees. Please remove one of the dependencies.
|
||||
Following Kotlin Source Set groups can't depend on '$dependencyName' together as they belong to different Kotlin Source Set Trees.
|
||||
${renderSourceSetGroups(dependents).indentLines(16)}
|
||||
Please keep dependsOn edges only from one group and remove the others.
|
||||
""".trimIndent()
|
||||
)
|
||||
|
||||
private fun renderSourceSetGroups(sourceSetGroups: Map<String, List<String>>) = buildString {
|
||||
for ((sourceSetTreeName, sourceSets) in sourceSetGroups) {
|
||||
appendLine("Source Sets from '$sourceSetTreeName' Tree")
|
||||
appendLine(sourceSets.joinToString("\n") { " * '$it'" })
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private fun String.indentLines(nSpaces: Int = 4, skipFirstLine: Boolean = true): String {
|
||||
val spaces = String(CharArray(nSpaces) { ' ' })
|
||||
|
||||
return lines()
|
||||
.withIndex()
|
||||
.joinToString(separator = "\n") { (index, line) ->
|
||||
if (skipFirstLine && index == 0) return@joinToString line
|
||||
if (line.isNotBlank()) "$spaces$line" else line
|
||||
}
|
||||
}
|
||||
+79
-29
@@ -11,6 +11,7 @@ import org.jetbrains.kotlin.gradle.plugin.diagnostics.KotlinGradleProjectChecker
|
||||
import org.jetbrains.kotlin.gradle.plugin.diagnostics.KotlinGradleProjectCheckerContext
|
||||
import org.jetbrains.kotlin.gradle.plugin.diagnostics.KotlinToolingDiagnostics.KotlinSourceSetTreeDependsOnMismatch
|
||||
import org.jetbrains.kotlin.gradle.plugin.diagnostics.KotlinToolingDiagnosticsCollector
|
||||
import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinMetadataTarget
|
||||
import org.jetbrains.kotlin.gradle.plugin.mpp.targetHierarchy.orNull
|
||||
import org.jetbrains.kotlin.gradle.plugin.sources.android.androidSourceSetInfoOrNull
|
||||
import org.jetbrains.kotlin.gradle.plugin.sources.awaitPlatformCompilations
|
||||
@@ -20,14 +21,21 @@ internal object KotlinSourceSetTreeDependsOnMismatchChecker : KotlinGradleProjec
|
||||
override suspend fun KotlinGradleProjectCheckerContext.runChecks(collector: KotlinToolingDiagnosticsCollector) {
|
||||
val sourceSets = this.multiplatformExtension
|
||||
?.awaitSourceSets()
|
||||
// Ignoring Android source sets
|
||||
// Android source sets are excluded from verification as they can cause unexpected results
|
||||
// As well as in [UnusedSourceSetsChecker]
|
||||
?.filter { it.androidSourceSetInfoOrNull == null }
|
||||
?: return
|
||||
|
||||
// A "good" source set is part of only single Source Set Tree
|
||||
val goodSourceSets = mutableMapOf<KotlinSourceSet, SourceSetTree?>()
|
||||
// A "bad" source set is part of >=2 Source Set Trees
|
||||
val badSourceSets = mutableMapOf<KotlinSourceSet, List<SourceSetTree?>>()
|
||||
val badSourceSets = mutableMapOf<KotlinSourceSet, Set<SourceSetTree?>>()
|
||||
// A "leaf" source set is a source set with known Source Set Tree by default
|
||||
val leafSourceSets = multiplatformExtension
|
||||
.awaitTargets()
|
||||
.filter { it !is KotlinMetadataTarget }
|
||||
.flatMap { target -> target.compilations.map { it.defaultSourceSet to SourceSetTree.orNull(it) } }
|
||||
.toMap()
|
||||
|
||||
val reverseSourceSetDependencies = mutableMapOf<KotlinSourceSet, MutableSet<KotlinSourceSet>>()
|
||||
fun KotlinSourceSet.addReverseDependencyTo(that: KotlinSourceSet) =
|
||||
@@ -37,7 +45,7 @@ internal object KotlinSourceSetTreeDependsOnMismatchChecker : KotlinGradleProjec
|
||||
sourceSet.dependsOn.forEach { it.addReverseDependencyTo(sourceSet) }
|
||||
|
||||
val platformCompilations = sourceSet.internal.awaitPlatformCompilations()
|
||||
val distinctSourceSetTrees = platformCompilations.map { SourceSetTree.orNull(it) }.distinct()
|
||||
val distinctSourceSetTrees = platformCompilations.map { SourceSetTree.orNull(it) }.toSet()
|
||||
|
||||
val totalDistinctSourceSetTrees = distinctSourceSetTrees.size
|
||||
|
||||
@@ -51,40 +59,82 @@ internal object KotlinSourceSetTreeDependsOnMismatchChecker : KotlinGradleProjec
|
||||
}
|
||||
}
|
||||
|
||||
for ((badSourceSet, sourceSetTrees) in badSourceSets) {
|
||||
for ((badSourceSet, _) in badSourceSets) {
|
||||
val dependents = reverseSourceSetDependencies[badSourceSet].orEmpty()
|
||||
|
||||
// check if any of its dependents is also a bad source set then skip it
|
||||
// check if any of [badSourceSet] dependents is also a bad source set then skip it
|
||||
// For example if nativeTest depends on nativeMain,
|
||||
// then transitively commonMain would have incorrect source set tree as well, but it is not a root cause.
|
||||
// NB: user can still add commonTest to commonMain dependency directly but this case will not be reported
|
||||
// until "dependent source sets relations is fixed
|
||||
// until underlying dependent source sets relations are fixed
|
||||
if (dependents.any { it in badSourceSets }) continue
|
||||
|
||||
// Heuristic: pick two source sets with different SourceSetTree
|
||||
// Theoretically there could be a lot of invalid pairs,
|
||||
// but it should be enough to report only 1 of them as it should cover the majority of cases.
|
||||
// NB: it is safe to do destruction on two elements as per definition of "bad" source set.
|
||||
val (sourceSetTreeA, sourceSetTreeB) = sourceSetTrees
|
||||
val sourceSetA = dependents.firstOrNull { it in goodSourceSets && goodSourceSets[it] == sourceSetTreeA }
|
||||
val sourceSetB = dependents.firstOrNull { it in goodSourceSets && goodSourceSets[it] == sourceSetTreeB }
|
||||
|
||||
if (sourceSetA == null || sourceSetB == null) {
|
||||
val goodSourceSet = sourceSetA ?: sourceSetB
|
||||
if (goodSourceSet == null) {
|
||||
project.logger.warn("Can't identify why kotlin source set '${badSourceSet.name}' has incorrect dependsOn relation")
|
||||
} else {
|
||||
collector.report(
|
||||
project,
|
||||
KotlinSourceSetTreeDependsOnMismatch(goodSourceSet.name, badSourceSet.name)
|
||||
)
|
||||
}
|
||||
} else {
|
||||
collector.report(
|
||||
project,
|
||||
KotlinSourceSetTreeDependsOnMismatch(sourceSetA.name, sourceSetB.name, badSourceSet.name)
|
||||
)
|
||||
// If [badSourceSet] is also a leaf source set then all its dependents edges are incorrect
|
||||
// Therefore report everything that depend on the leaf source set (i.e. iosX64Test -> iosX64Main)
|
||||
// NB: Cyclic diagnostics such as iosX64Main -> commonMain -> iosX64Main is handled in [AbstractKotlinSourceSet::dependsOn]
|
||||
if (badSourceSet in leafSourceSets) {
|
||||
dependents.forEach { collector.report(project, KotlinSourceSetTreeDependsOnMismatch(it.name, badSourceSet.name)) }
|
||||
continue
|
||||
}
|
||||
|
||||
// Heuristic "White Crow": If among dependents there is only one source set with different Source Set Tree then report it
|
||||
// i.e.
|
||||
// commonMain
|
||||
// |
|
||||
// +-----------+------------+
|
||||
// | | |
|
||||
// (jvmMain) (nativeMain) (nativeTest!?)
|
||||
// jvmMain and nativeMain make a group of 'main' Source Set Tree,
|
||||
// but nativeTest is the only one from group of 'test' Source Set Tree
|
||||
// therefore dependency from nativeTest to commonMain is incorrect
|
||||
// A bad scenario for this heuristic is when in Bamboo Source Set Structure (source set with single dependent)
|
||||
// user adds two or more depends on edges from other Source Set Tree.
|
||||
// i.e.
|
||||
// commonMain
|
||||
// |
|
||||
// +-----------+---------------+
|
||||
// | | |
|
||||
// (nativeMain) (appleTest!?) (nativeTest!?)
|
||||
// In this case dependency edge from nativeMain will be considered incorrect
|
||||
val dependentsBySourceSetTree = dependents.groupBy {
|
||||
when (it) {
|
||||
in goodSourceSets -> goodSourceSets[it]
|
||||
in leafSourceSets -> leafSourceSets[it]
|
||||
else -> null
|
||||
}
|
||||
}
|
||||
if (reportSingleSourceSetWithDifferentSourceSetTree(collector, badSourceSet, dependentsBySourceSetTree)) continue
|
||||
|
||||
// If there are more than one Source Sets with different Source Set Trees then we can't
|
||||
// identify which group is incorrect, therefore we should report all of them
|
||||
reportAllIncorrectSourceSetEdges(collector, badSourceSet, dependentsBySourceSetTree)
|
||||
}
|
||||
}
|
||||
|
||||
private fun KotlinGradleProjectCheckerContext.reportSingleSourceSetWithDifferentSourceSetTree(
|
||||
collector: KotlinToolingDiagnosticsCollector,
|
||||
badSourceSet: KotlinSourceSet,
|
||||
dependentsBySourceSetTree: Map<SourceSetTree?, List<KotlinSourceSet>>
|
||||
): Boolean {
|
||||
val singleDependee = dependentsBySourceSetTree
|
||||
.values
|
||||
.singleOrNull { it.size == 1 }
|
||||
?.single()
|
||||
?: return false
|
||||
|
||||
collector.report(project, KotlinSourceSetTreeDependsOnMismatch(singleDependee.name, badSourceSet.name))
|
||||
return true
|
||||
}
|
||||
|
||||
private fun KotlinGradleProjectCheckerContext.reportAllIncorrectSourceSetEdges(
|
||||
collector: KotlinToolingDiagnosticsCollector,
|
||||
badSourceSet: KotlinSourceSet,
|
||||
dependentsBySourceSetTree: Map<SourceSetTree?, List<KotlinSourceSet>>,
|
||||
) {
|
||||
val dependentsGroup = dependentsBySourceSetTree
|
||||
.mapKeys { it.key?.name ?: "null" }
|
||||
.mapValues { it.value.map(KotlinSourceSet::getName) }
|
||||
|
||||
collector.report(project, KotlinSourceSetTreeDependsOnMismatch(dependentsGroup, badSourceSet.name))
|
||||
}
|
||||
}
|
||||
+97
-37
@@ -18,11 +18,10 @@ class KotlinSourceSetTreeDependsOnMismatchTest {
|
||||
val project = buildProjectWithMPP {
|
||||
kotlin {
|
||||
targetHierarchy.default()
|
||||
androidTarget()
|
||||
project.androidApplication { compileSdk = 32 }
|
||||
iosX64(); iosArm64(); iosSimulatorArm64()
|
||||
watchosArm64(); watchosX64(); watchosSimulatorArm64()
|
||||
macosX64(); macosArm64()
|
||||
linuxX64(); linuxArm64()
|
||||
|
||||
configure()
|
||||
}
|
||||
@@ -31,24 +30,13 @@ class KotlinSourceSetTreeDependsOnMismatchTest {
|
||||
return project.kotlinToolingDiagnosticsCollector.getDiagnosticsForProject(project).toList()
|
||||
}
|
||||
|
||||
private fun checkBadSourceSetDependency(
|
||||
correctDependent: String,
|
||||
incorrectDependent: String,
|
||||
dependency: String
|
||||
private fun checkSingleBadSourceSetDependency(
|
||||
dependent: String,
|
||||
dependency: String,
|
||||
expectedDiagnostic: ToolingDiagnostic = KotlinSourceSetTreeDependsOnMismatch(dependent, dependency)
|
||||
) = checkDiagnostics {
|
||||
sourceSets.getByName(incorrectDependent).dependsOn(sourceSets.getByName(dependency))
|
||||
}.assertContainsDiagnostic(
|
||||
KotlinSourceSetTreeDependsOnMismatch(correctDependent, incorrectDependent, dependency)
|
||||
)
|
||||
|
||||
private fun checkBadSourceSetDependency(
|
||||
incorrectDependent: String,
|
||||
dependency: String
|
||||
) = checkDiagnostics {
|
||||
sourceSets.getByName(incorrectDependent).dependsOn(sourceSets.getByName(dependency))
|
||||
}.assertContainsDiagnostic(
|
||||
KotlinSourceSetTreeDependsOnMismatch(incorrectDependent, dependency)
|
||||
)
|
||||
sourceSets.getByName(dependent).dependsOn(sourceSets.getByName(dependency))
|
||||
}.assertContainsDiagnostic(expectedDiagnostic)
|
||||
|
||||
@Test
|
||||
fun `no diagnostics should be reported for correctly configured project`() {
|
||||
@@ -58,36 +46,108 @@ class KotlinSourceSetTreeDependsOnMismatchTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `commonTest cant depend on commonMain`() = checkBadSourceSetDependency(
|
||||
correctDependent = "nativeMain",
|
||||
incorrectDependent = "commonTest",
|
||||
dependency = "commonMain"
|
||||
fun `commonTest cant depend on commonMain`() = checkSingleBadSourceSetDependency(
|
||||
dependent = "commonTest",
|
||||
dependency = "commonMain",
|
||||
// Since for given test project commonMain have only one dependent (nativeMain) it is
|
||||
// impossible to figure out wrong dependency via "white crow" heuristic
|
||||
expectedDiagnostic = KotlinSourceSetTreeDependsOnMismatch(
|
||||
dependents = mapOf(
|
||||
"test" to listOf("commonTest"),
|
||||
"main" to listOf("nativeMain"),
|
||||
),
|
||||
dependencyName = "commonMain"
|
||||
)
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `commonMain cant depend on commonTest`() = checkBadSourceSetDependency(
|
||||
correctDependent = "nativeTest",
|
||||
incorrectDependent = "commonMain",
|
||||
dependency = "commonTest"
|
||||
fun `commonMain cant depend on commonTest`() = checkSingleBadSourceSetDependency(
|
||||
dependent = "commonMain",
|
||||
dependency = "commonTest",
|
||||
expectedDiagnostic = KotlinSourceSetTreeDependsOnMismatch(
|
||||
dependents = mapOf(
|
||||
"main" to listOf("commonMain"),
|
||||
"test" to listOf("nativeTest"),
|
||||
),
|
||||
dependencyName = "commonTest"
|
||||
)
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `appleTest cant depend on appleMain`() = checkBadSourceSetDependency(
|
||||
correctDependent = "iosMain",
|
||||
incorrectDependent = "appleTest",
|
||||
fun `appleTest cant depend on appleMain`() = checkSingleBadSourceSetDependency(
|
||||
dependent = "appleTest",
|
||||
dependency = "appleMain"
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `iosX64Test cant depend on iosX64Main`() = checkBadSourceSetDependency(
|
||||
incorrectDependent = "iosX64Test",
|
||||
dependency = "iosX64Main",
|
||||
fun `appleMain cant depend on appleTest`() = checkSingleBadSourceSetDependency(
|
||||
dependent = "appleMain",
|
||||
dependency = "appleTest"
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `iosX64Test cant depend on commonMain`() = checkBadSourceSetDependency(
|
||||
correctDependent = "nativeMain",
|
||||
incorrectDependent = "iosX64Test",
|
||||
dependency = "commonMain",
|
||||
fun `iosTest cant depend on iosMain`() = checkSingleBadSourceSetDependency(
|
||||
dependent = "iosTest",
|
||||
dependency = "iosMain"
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `iosMain cant depend on iosTest`() = checkSingleBadSourceSetDependency(
|
||||
dependent = "iosMain",
|
||||
dependency = "iosTest"
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `iosX64Test cant depend on iosX64Main`() = checkSingleBadSourceSetDependency(
|
||||
dependent = "iosX64Test",
|
||||
dependency = "iosX64Main"
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `iosX64Main cant depend on iosX64Test`() = checkSingleBadSourceSetDependency(
|
||||
dependent = "iosX64Main",
|
||||
dependency = "iosX64Test"
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `iosX64Test and iosArm64Test cant depend on iosMain`() = checkDiagnostics {
|
||||
sourceSets.getByName("iosX64Test").dependsOn(sourceSets.getByName("iosMain"))
|
||||
sourceSets.getByName("iosArm64Test").dependsOn(sourceSets.getByName("iosMain"))
|
||||
}.assertContainsDiagnostic(
|
||||
KotlinSourceSetTreeDependsOnMismatch(
|
||||
dependents = mapOf(
|
||||
"main" to listOf("iosArm64Main", "iosSimulatorArm64Main", "iosX64Main"),
|
||||
"test" to listOf("iosArm64Test", "iosX64Test")
|
||||
),
|
||||
dependencyName = "iosMain"
|
||||
)
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `test that only lowest source set edges are reported`() = checkDiagnostics {
|
||||
sourceSets.getByName("iosX64Test").dependsOn(sourceSets.getByName("iosMain"))
|
||||
sourceSets.getByName("iosArm64Test").dependsOn(sourceSets.getByName("nativeMain"))
|
||||
sourceSets.getByName("iosSimulatorArm64Main").dependsOn(sourceSets.getByName("commonMain"))
|
||||
}.assertDiagnostics(
|
||||
// Expected only one diagnostic since "nativeMain" and "commonMain" is a "bad source sets" because
|
||||
// iosMain depends on all of them transitively and thus coloring them as "bad source sets" as well.
|
||||
// Thus, only the lowest "bad source set" should be reported
|
||||
KotlinSourceSetTreeDependsOnMismatch(dependeeName = "iosX64Test", dependencyName = "iosMain")
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `test that few incorrect source set dependencies can be reported`() = checkDiagnostics {
|
||||
sourceSets.getByName("iosX64Test").dependsOn(sourceSets.getByName("iosMain"))
|
||||
sourceSets.getByName("macosX64Test").dependsOn(sourceSets.getByName("macosMain"))
|
||||
sourceSets.getByName("macosArm64Test").dependsOn(sourceSets.getByName("macosMain"))
|
||||
}.assertDiagnostics(
|
||||
KotlinSourceSetTreeDependsOnMismatch(dependeeName = "iosX64Test", dependencyName = "iosMain"),
|
||||
KotlinSourceSetTreeDependsOnMismatch(
|
||||
dependents = mapOf(
|
||||
"main" to listOf("macosArm64Main", "macosX64Main"),
|
||||
"test" to listOf("macosArm64Test", "macosX64Test")
|
||||
),
|
||||
dependencyName = "macosMain"
|
||||
),
|
||||
)
|
||||
}
|
||||
+23
-1
@@ -80,6 +80,8 @@ internal fun Project.assertContainsDiagnostic(diagnostic: ToolingDiagnostic) {
|
||||
kotlinToolingDiagnosticsCollector.getDiagnosticsForProject(this).assertContainsDiagnostic(diagnostic)
|
||||
}
|
||||
|
||||
private fun Any.withIndent() = this.toString().prependIndent(" ")
|
||||
|
||||
internal fun Collection<ToolingDiagnostic>.assertContainsDiagnostic(factory: ToolingDiagnosticFactory) {
|
||||
if (!any { it.id == factory.id }) failDiagnosticNotFound("diagnostic with id ${factory.id} ", this)
|
||||
}
|
||||
@@ -89,10 +91,30 @@ internal fun Collection<ToolingDiagnostic>.assertContainsDiagnostic(diagnostic:
|
||||
}
|
||||
|
||||
private fun failDiagnosticNotFound(diagnosticDescription: String, notFoundInCollection: Collection<ToolingDiagnostic>) {
|
||||
fun Any.withIndent() = this.toString().prependIndent(" ")
|
||||
fail("Missing ${diagnosticDescription}in:\n${notFoundInCollection.render().withIndent()}")
|
||||
}
|
||||
|
||||
internal fun Collection<ToolingDiagnostic>.assertDiagnostics(vararg diagnostics: ToolingDiagnostic) {
|
||||
val expectedDiagnostics = diagnostics.toSet()
|
||||
val actualDiagnostic = this.toSet()
|
||||
if (expectedDiagnostics == actualDiagnostic) return
|
||||
|
||||
val missingDiagnostics = this - expectedDiagnostics
|
||||
val unexpectedDiagnostics = expectedDiagnostics - this
|
||||
|
||||
val errorMessage = buildString {
|
||||
if (missingDiagnostics.isNotEmpty()) {
|
||||
appendLine(missingDiagnostics.joinToString(prefix = "Missing diagnostic\n", separator = "\n") { it.withIndent() })
|
||||
}
|
||||
if (unexpectedDiagnostics.isNotEmpty()) {
|
||||
appendLine(unexpectedDiagnostics.joinToString(prefix = "Unexpected diagnostic\n", separator = "\n") { it.withIndent() })
|
||||
}
|
||||
appendLine("in: \n${expectedDiagnostics.render().withIndent()}")
|
||||
}
|
||||
|
||||
fail(errorMessage)
|
||||
}
|
||||
|
||||
internal fun Project.assertNoDiagnostics(id: String) {
|
||||
kotlinToolingDiagnosticsCollector.getDiagnosticsForProject(this).assertNoDiagnostics(id)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user