From acf8f5b26b2c6b1a45dbeb9e34c2dee51bdfd226 Mon Sep 17 00:00:00 2001 From: Anton Lakotka Date: Thu, 15 Jun 2023 10:16:12 +0200 Subject: [PATCH] [Gradle] Include android source sets to source set depends on checker Excluding android source sets can actually bring unexpected behaviour when, for example, androidInstrumentedTest get depends on commonMain. In this case "nativeMain" will be reported as checked skipped android source sets, but they are still present in dependsOn edges. Another important aspect is that mpp+android is a majority and their cases should be checked as well ^KT-47144 Verification Pending --- ...inSourceSetTreeDependsOnMismatchChecker.kt | 7 +-- ...otlinSourceSetTreeDependsOnMismatchTest.kt | 52 ++++++++++++------- .../kotlin/gradle/util/diagnosticUtils.kt | 2 +- 3 files changed, 35 insertions(+), 26 deletions(-) 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 index c854b69b62a..5c555c04ce2 100644 --- 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 @@ -19,12 +19,7 @@ import org.jetbrains.kotlin.gradle.plugin.sources.internal internal object KotlinSourceSetTreeDependsOnMismatchChecker : KotlinGradleProjectChecker { override suspend fun KotlinGradleProjectCheckerContext.runChecks(collector: KotlinToolingDiagnosticsCollector) { - val sourceSets = this.multiplatformExtension - ?.awaitSourceSets() - // Android source sets are excluded from verification as they can cause unexpected results - // As well as in [UnusedSourceSetsChecker] - ?.filter { it.androidSourceSetInfoOrNull == null } - ?: return + val sourceSets = this.multiplatformExtension?.awaitSourceSets() ?: return // A "good" source set is part of only single Source Set Tree val goodSourceSets = mutableMapOf() 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 index bca9239f942..59968537bb8 100644 --- 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 @@ -6,6 +6,7 @@ package org.jetbrains.kotlin.gradle.unitTests.diagnosticsTests import org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension +import org.jetbrains.kotlin.gradle.plugin.diagnostics.KotlinToolingDiagnostics.CommonMainWithDependsOnDiagnostic 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 @@ -33,10 +34,10 @@ class KotlinSourceSetTreeDependsOnMismatchTest { private fun checkSingleBadSourceSetDependency( dependent: String, dependency: String, - expectedDiagnostic: ToolingDiagnostic = KotlinSourceSetTreeDependsOnMismatch(dependent, dependency) + vararg expectedDiagnostics: ToolingDiagnostic = arrayOf(KotlinSourceSetTreeDependsOnMismatch(dependent, dependency)) ) = checkDiagnostics { sourceSets.getByName(dependent).dependsOn(sourceSets.getByName(dependency)) - }.assertContainsDiagnostic(expectedDiagnostic) + }.assertDiagnostics(*expectedDiagnostics) @Test fun `no diagnostics should be reported for correctly configured project`() { @@ -48,29 +49,15 @@ class KotlinSourceSetTreeDependsOnMismatchTest { @Test 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" - ) + dependency = "commonMain" ) @Test 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" - ) + KotlinSourceSetTreeDependsOnMismatch("commonMain", "commonTest"), + CommonMainWithDependsOnDiagnostic() ) @Test @@ -123,6 +110,33 @@ class KotlinSourceSetTreeDependsOnMismatchTest { ) ) + @Test + fun `androidInstrumentedTest cant depend on commonMain`() = checkSingleBadSourceSetDependency( + dependent = "androidInstrumentedTest", + dependency = "commonMain" + ) + + @Test + fun `commonMain cant depend on androidInstrumentedTest`() = checkSingleBadSourceSetDependency( + dependent = "androidInstrumentedTest", + dependency = "commonMain" + ) + + @Test + fun `test multiple incorrect source set dependencies`() = checkDiagnostics { + sourceSets.getByName("iosX64Test").dependsOn(sourceSets.getByName("commonMain")) + sourceSets.getByName("androidInstrumentedTest").dependsOn(sourceSets.getByName("commonMain")) + }.assertDiagnostics( + KotlinSourceSetTreeDependsOnMismatch( + dependents = mapOf( + "main" to listOf("androidDebug", "androidMain", "androidRelease", "nativeMain"), + "instrumentedTest" to listOf("androidInstrumentedTest"), + "test" to listOf("iosX64Test") + ), + dependencyName = "commonMain" + ), + ) + @Test fun `test that only lowest source set edges are reported`() = checkDiagnostics { sourceSets.getByName("iosX64Test").dependsOn(sourceSets.getByName("iosMain")) diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/util/diagnosticUtils.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/util/diagnosticUtils.kt index 4bd566ec461..5d95b937a5e 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/util/diagnosticUtils.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/util/diagnosticUtils.kt @@ -109,7 +109,7 @@ internal fun Collection.assertDiagnostics(vararg diagnostics: if (unexpectedDiagnostics.isNotEmpty()) { appendLine(unexpectedDiagnostics.joinToString(prefix = "Unexpected diagnostic\n", separator = "\n") { it.withIndent() }) } - appendLine("in: \n${expectedDiagnostics.render().withIndent()}") + appendLine("in: \n${actualDiagnostic.render().withIndent()}") } fail(errorMessage)