From 681305c8e945959db5bf1474c059505fcf564d6b Mon Sep 17 00:00:00 2001 From: Anton Lakotka Date: Fri, 2 Jun 2023 12:22:26 +0200 Subject: [PATCH] [Gradle] Add KotlinSourceSetTreeDependsOnMismatch diagnostic In Kotlin Multiplatform it is not possible to have source set dependencies between two different SourceSet Trees. For example commonTest can't dependOn commonMain as SourceSet dependency instead binaries of commonMain should be included as dependency to commonTest. Which is implemented through different mechanisms ^KT-47144 Verification Pending --- .../diagnostics/KotlinGradleProjectChecker.kt | 3 +- .../diagnostics/KotlinToolingDiagnostics.kt | 16 ++++ ...inSourceSetTreeDependsOnMismatchChecker.kt | 90 ++++++++++++++++++ ...otlinSourceSetTreeDependsOnMismatchTest.kt | 93 +++++++++++++++++++ 4 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/checkers/KotlinSourceSetTreeDependsOnMismatchChecker.kt create mode 100644 libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/diagnosticsTests/KotlinSourceSetTreeDependsOnMismatchTest.kt diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinGradleProjectChecker.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinGradleProjectChecker.kt index 6d5141e083f..7d81be013ea 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinGradleProjectChecker.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinGradleProjectChecker.kt @@ -106,7 +106,8 @@ internal interface KotlinGradleProjectChecker { DisabledNativeTargetsChecker, JsEnvironmentChecker, PreHmppDependenciesUsageChecker, - ExperimentalK2UsageChecker + ExperimentalK2UsageChecker, + KotlinSourceSetTreeDependsOnMismatchChecker, ) } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinToolingDiagnostics.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinToolingDiagnostics.kt index 22c76dd8d07..685fe63529e 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinToolingDiagnostics.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/KotlinToolingDiagnostics.kt @@ -330,4 +330,20 @@ object KotlinToolingDiagnostics { """.trimIndent() ) } + + object KotlinSourceSetTreeDependsOnMismatch : ToolingDiagnosticFactory(ERROR) { + 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. + """.trimIndent() + ) + + operator fun invoke(dependeeAName: String, dependeeBName: 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. + """.trimIndent() + ) + } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/checkers/KotlinSourceSetTreeDependsOnMismatchChecker.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/checkers/KotlinSourceSetTreeDependsOnMismatchChecker.kt new file mode 100644 index 00000000000..33d2e1ab008 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/diagnostics/checkers/KotlinSourceSetTreeDependsOnMismatchChecker.kt @@ -0,0 +1,90 @@ +/* + * 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.gradle.plugin.diagnostics.checkers + +import org.jetbrains.kotlin.gradle.plugin.KotlinSourceSet +import org.jetbrains.kotlin.gradle.plugin.KotlinTargetHierarchy.SourceSetTree +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.targetHierarchy.orNull +import org.jetbrains.kotlin.gradle.plugin.sources.android.androidSourceSetInfoOrNull +import org.jetbrains.kotlin.gradle.plugin.sources.awaitPlatformCompilations +import org.jetbrains.kotlin.gradle.plugin.sources.internal + +internal object KotlinSourceSetTreeDependsOnMismatchChecker : KotlinGradleProjectChecker { + override suspend fun KotlinGradleProjectCheckerContext.runChecks(collector: KotlinToolingDiagnosticsCollector) { + val sourceSets = this.multiplatformExtension + ?.awaitSourceSets() + // Ignoring Android source sets + ?.filter { it.androidSourceSetInfoOrNull == null } + ?: return + + // A "good" source set is part of only single Source Set Tree + val goodSourceSets = mutableMapOf() + // A "bad" source set is part of >=2 Source Set Trees + val badSourceSets = mutableMapOf>() + + val reverseSourceSetDependencies = mutableMapOf>() + fun KotlinSourceSet.addReverseDependencyTo(that: KotlinSourceSet) = + reverseSourceSetDependencies.getOrPut(this) { mutableSetOf() }.add(that) + + for (sourceSet in sourceSets) { + sourceSet.dependsOn.forEach { it.addReverseDependencyTo(sourceSet) } + + val platformCompilations = sourceSet.internal.awaitPlatformCompilations() + val distinctSourceSetTrees = platformCompilations.map { SourceSetTree.orNull(it) }.distinct() + + val totalDistinctSourceSetTrees = distinctSourceSetTrees.size + + @Suppress("KotlinConstantConditions") + when { + totalDistinctSourceSetTrees > 1 -> badSourceSets[sourceSet] = distinctSourceSetTrees + totalDistinctSourceSetTrees == 1 -> goodSourceSets[sourceSet] = distinctSourceSetTrees.single() + // case when source set has no platform compilation and thus its totalDistinctSourceSetTrees == 0 + // is covered by [UnusedSourceSetsChecker] + totalDistinctSourceSetTrees == 0 -> continue + } + } + + for ((badSourceSet, sourceSetTrees) in badSourceSets) { + val dependents = reverseSourceSetDependencies[badSourceSet].orEmpty() + + // check if any of its 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 + 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) + ) + } + } + } +} \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/diagnosticsTests/KotlinSourceSetTreeDependsOnMismatchTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/diagnosticsTests/KotlinSourceSetTreeDependsOnMismatchTest.kt new file mode 100644 index 00000000000..dff5230d0c5 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/diagnosticsTests/KotlinSourceSetTreeDependsOnMismatchTest.kt @@ -0,0 +1,93 @@ +/* + * 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. + */ +@file:Suppress("FunctionName") +package org.jetbrains.kotlin.gradle.unitTests.diagnosticsTests + +import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension +import org.jetbrains.kotlin.gradle.plugin.diagnostics.KotlinToolingDiagnostics.KotlinSourceSetTreeDependsOnMismatch +import org.jetbrains.kotlin.gradle.plugin.diagnostics.ToolingDiagnostic +import org.jetbrains.kotlin.gradle.plugin.diagnostics.kotlinToolingDiagnosticsCollector +import org.jetbrains.kotlin.gradle.util.* +import org.jetbrains.kotlin.gradle.util.assertContainsDiagnostic +import kotlin.test.* + +class KotlinSourceSetTreeDependsOnMismatchTest { + private fun checkDiagnostics(configure: KotlinMultiplatformExtension.() -> Unit): List { + val project = buildProjectWithMPP { + kotlin { + targetHierarchy.default() + project.androidApplication { compileSdk = 32 } + iosX64(); iosArm64(); iosSimulatorArm64() + watchosArm64(); watchosX64(); watchosSimulatorArm64() + macosX64(); macosArm64() + linuxX64(); linuxArm64() + + configure() + } + } + project.evaluate() + return project.kotlinToolingDiagnosticsCollector.getDiagnosticsForProject(project).toList() + } + + private fun checkBadSourceSetDependency( + correctDependent: String, + incorrectDependent: String, + dependency: String + ) = 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) + ) + + @Test + fun `no diagnostics should be reported for correctly configured project`() { + checkDiagnostics { + // no extra configurations + }.assertNoDiagnostics(KotlinSourceSetTreeDependsOnMismatch.id) + } + + @Test + fun `commonTest cant depend on commonMain`() = checkBadSourceSetDependency( + correctDependent = "nativeMain", + incorrectDependent = "commonTest", + dependency = "commonMain" + ) + + @Test + fun `commonMain cant depend on commonTest`() = checkBadSourceSetDependency( + correctDependent = "nativeTest", + incorrectDependent = "commonMain", + dependency = "commonTest" + ) + + @Test + fun `appleTest cant depend on appleMain`() = checkBadSourceSetDependency( + correctDependent = "iosMain", + incorrectDependent = "appleTest", + dependency = "appleMain" + ) + + @Test + fun `iosX64Test cant depend on iosX64Main`() = checkBadSourceSetDependency( + incorrectDependent = "iosX64Test", + dependency = "iosX64Main", + ) + + @Test + fun `iosX64Test cant depend on commonMain`() = checkBadSourceSetDependency( + correctDependent = "nativeMain", + incorrectDependent = "iosX64Test", + dependency = "commonMain", + ) +} \ No newline at end of file