From 0e34d0b01a57cdd88731d477568a786852754e45 Mon Sep 17 00:00:00 2001 From: Anton Lakotka Date: Fri, 14 Jul 2023 13:50:06 +0200 Subject: [PATCH] [Gradle] Deprecate multiple same targets Having multiple jvm, js and other targets in the same project is now deprecated and will be forbidden in the future. Users should migrate away from that by using custom compilations or different projects. ^KT-59316 Verification Pending --- .../dsl/KotlinMultiplatformExtension.kt | 33 ++++- .../diagnostics/KotlinToolingDiagnostics.kt | 25 ++++ .../KotlinTargetAlreadyDeclaredTest.kt | 125 ++++++++++++++++++ .../kotlin/gradle/util/diagnosticUtils.kt | 14 +- .../targetsDisambiguation.txt | 92 ++++++++++++- 5 files changed, 281 insertions(+), 8 deletions(-) create mode 100644 libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/diagnosticsTests/KotlinTargetAlreadyDeclaredTest.kt diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/KotlinMultiplatformExtension.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/KotlinMultiplatformExtension.kt index 5508b378298..f6195563c5a 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/KotlinMultiplatformExtension.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/dsl/KotlinMultiplatformExtension.kt @@ -5,14 +5,20 @@ package org.jetbrains.kotlin.gradle.dsl -import org.gradle.api.* +import org.gradle.api.Action +import org.gradle.api.InvalidUserCodeException +import org.gradle.api.NamedDomainObjectCollection +import org.gradle.api.Project import org.jetbrains.kotlin.gradle.ExperimentalKotlinGradlePluginApi import org.jetbrains.kotlin.gradle.InternalKotlinGradlePluginApi import org.jetbrains.kotlin.gradle.plugin.* import org.jetbrains.kotlin.gradle.plugin.KotlinPluginLifecycle.Stage.AfterFinaliseDsl import org.jetbrains.kotlin.gradle.plugin.PropertiesProvider.Companion.kotlinPropertiesProvider +import org.jetbrains.kotlin.gradle.plugin.diagnostics.KotlinToolingDiagnostics.KotlinTargetAlreadyDeclared +import org.jetbrains.kotlin.gradle.plugin.diagnostics.kotlinToolingDiagnosticsCollector import org.jetbrains.kotlin.gradle.plugin.hierarchy.KotlinHierarchyDslImpl import org.jetbrains.kotlin.gradle.plugin.mpp.* +import org.jetbrains.kotlin.gradle.targets.js.ir.KotlinJsIrTargetPreset import javax.inject.Inject @Suppress("DEPRECATION") @@ -194,7 +200,7 @@ internal abstract class DefaultTargetsFromPresetExtension @Inject constructor( } } -internal fun KotlinTarget.isProducedFromPreset(kotlinTargetPreset: KotlinTargetPreset<*>): Boolean = +private fun KotlinTarget.isProducedFromPreset(kotlinTargetPreset: KotlinTargetPreset<*>): Boolean = preset == kotlinTargetPreset internal fun KotlinTargetsContainerWithPresets.configureOrCreate( @@ -211,6 +217,9 @@ internal fun KotlinTargetsContainerWithPresets.configureOrCre } existingTarget == null -> { + if (this is KotlinMultiplatformExtension) { + project.reportIfTargetOfTheSameTypeAlreadyCreated(targets, targetPreset, targetName) + } val newTarget = targetPreset.createTarget(targetName) targets.add(newTarget) configure(newTarget) @@ -226,4 +235,22 @@ internal fun KotlinTargetsContainerWithPresets.configureOrCre ) } } -} \ No newline at end of file +} + +private fun Project.reportIfTargetOfTheSameTypeAlreadyCreated( + targets: NamedDomainObjectCollection, + preset: KotlinTargetPreset<*>, + targetName: String, +) { + val existingTargets = targets.matching { it.preset?.name == preset.name } + val targetDslFunctionName = when(preset) { + is KotlinJsIrTargetPreset -> "js" + is KotlinJsTargetPreset -> "js" + is KotlinAndroidTargetPreset -> "androidTarget" + else -> preset.name + } + if (existingTargets.isNotEmpty()) { + kotlinToolingDiagnosticsCollector.report(this, KotlinTargetAlreadyDeclared(targetDslFunctionName, targetName, Throwable())) + } +} + 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 47d08f229ba..725ef497428 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 @@ -546,6 +546,31 @@ object KotlinToolingDiagnostics { throwable = trace ) } + + object KotlinTargetAlreadyDeclared : ToolingDiagnosticFactory(WARNING) { + operator fun invoke(targetDslFunctionName: String, targetName: String, trace: Throwable?) = build( + """ + Kotlin Target '$targetDslFunctionName()' is already declared. + Declaring multiple Kotlin Targets of the same type is deprecated. + + i.e. + kotlin { + $targetDslFunctionName() + $targetDslFunctionName("$targetName") /* <- second '$targetDslFunctionName' target in the project is deprecated */ + } + + Please use different Gradle Projects or create Kotlin Compilations. + For example: + + kotlin { + $targetDslFunctionName() { + val $targetName by compilations.creating + } + } + """.trimIndent(), + throwable = trace + ) + } } private fun String.indentLines(nSpaces: Int = 4, skipFirstLine: Boolean = true): String { diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/diagnosticsTests/KotlinTargetAlreadyDeclaredTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/diagnosticsTests/KotlinTargetAlreadyDeclaredTest.kt new file mode 100644 index 00000000000..19631f77c34 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/diagnosticsTests/KotlinTargetAlreadyDeclaredTest.kt @@ -0,0 +1,125 @@ +/* + * 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 com.android.build.gradle.LibraryPlugin +import org.jetbrains.kotlin.gradle.plugin.diagnostics.KotlinToolingDiagnostics.KotlinTargetAlreadyDeclared +import org.jetbrains.kotlin.gradle.util.* +import kotlin.test.Test + +class KotlinTargetAlreadyDeclaredTest { + + @Test + fun `no diagnostic reported when only single targets declared`() { + val project = buildProjectWithMPP { + plugins.apply(LibraryPlugin::class.java) + androidLibrary { compileSdk = 33 } + kotlin { + jvm() + js() + androidTarget() + linuxX64() + linuxArm64() + } + } + + project.evaluate() + project.assertNoDiagnostics(KotlinTargetAlreadyDeclared) + } + + @Test + fun `no diagnostic reported when custom name is used`() { + val project = buildProjectWithMPP { + kotlin { + jvm("jvm2") + linuxX64("linux") + } + } + + project.evaluate() + project.assertNoDiagnostics(KotlinTargetAlreadyDeclared) + } + + @Test + fun `no diagnostic reported when target declaration dsl is used to get existing target`() { + val project = buildProjectWithMPP { + kotlin { + jvm() // declare + jvm() // get target declared above + } + } + + project.evaluate() + project.assertNoDiagnostics(KotlinTargetAlreadyDeclared) + } + + @Test + fun `diagnostic reported when jvm target declared twice with different names`() { + val project = buildProjectWithMPP { + kotlin { + jvm() + jvm("jvm2") + } + } + + project.evaluate() + project.assertContainsDiagnostic( + KotlinTargetAlreadyDeclared("jvm", "jvm2", null), + ignoreThrowable = true + ) + } + + @Test + fun `diagnostic reported when native target declared twice with different names`() { + val project = buildProjectWithMPP { + kotlin { + linuxArm64() + linuxArm64("linux") + } + } + + project.evaluate() + project.assertContainsDiagnostic( + KotlinTargetAlreadyDeclared("linuxArm64", "linux", null), + ignoreThrowable = true + ) + } + + @Test + fun `diagnostic reported when android target declared twice with different names`() { + val project = buildProjectWithMPP { + plugins.apply(LibraryPlugin::class.java) + androidLibrary { compileSdk = 33 } + kotlin { + androidTarget() + androidTarget("android2") + } + } + + project.evaluate() + project.assertContainsDiagnostic( + KotlinTargetAlreadyDeclared("androidTarget", "android2", null), + ignoreThrowable = true + ) + } + + @Test + fun `diagnostic reported when js target declared twice with different names`() { + val project = buildProjectWithMPP { + kotlin { + js("browser") + js("nodejs") + } + } + + project.evaluate() + project.assertContainsDiagnostic( + KotlinTargetAlreadyDeclared("js", "nodejs", null), + ignoreThrowable = true + ) + } +} \ No newline at end of file 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 0209c8bd1fa..54006bb68aa 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 @@ -23,6 +23,11 @@ internal fun checkDiagnosticsWithMppProject(projectName: String, projectConfigur project.checkDiagnostics(projectName) } +internal fun ToolingDiagnostic.equals(that: ToolingDiagnostic, ignoreThrowable: Boolean) = if (ignoreThrowable) { + this.id == that.id && this.message == that.message && this.severity == that.severity +} else { + this == that +} /** * [compactRendering] == true will omit projects with no diagnostics from the report, as well as @@ -76,8 +81,9 @@ internal fun Project.assertContainsDiagnostic(factory: ToolingDiagnosticFactory) kotlinToolingDiagnosticsCollector.getDiagnosticsForProject(this).assertContainsDiagnostic(factory) } -internal fun Project.assertContainsDiagnostic(diagnostic: ToolingDiagnostic) { - kotlinToolingDiagnosticsCollector.getDiagnosticsForProject(this).assertContainsDiagnostic(diagnostic) +internal fun Project.assertContainsDiagnostic(diagnostic: ToolingDiagnostic, ignoreThrowable: Boolean = false) { + kotlinToolingDiagnosticsCollector.getDiagnosticsForProject(this) + .assertContainsDiagnostic(diagnostic, ignoreThrowable) } private fun Any.withIndent() = this.toString().prependIndent(" ") @@ -86,8 +92,8 @@ internal fun Collection.assertContainsDiagnostic(factory: Too if (!any { it.id == factory.id }) failDiagnosticNotFound("diagnostic with id ${factory.id} ", this) } -internal fun Collection.assertContainsDiagnostic(diagnostic: ToolingDiagnostic) { - if (diagnostic !in this) failDiagnosticNotFound("diagnostic $diagnostic\n", this) +internal fun Collection.assertContainsDiagnostic(diagnostic: ToolingDiagnostic, ignoreThrowable: Boolean = false) { + if (none { it.equals(diagnostic, ignoreThrowable) }) failDiagnosticNotFound("diagnostic $diagnostic\n", this) } private fun failDiagnosticNotFound(diagnosticDescription: String, notFoundInCollection: Collection) { diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/resources/expectedDiagnostics/targetsDisambiguation.txt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/resources/expectedDiagnostics/targetsDisambiguation.txt index 52306f1d3a6..bc73fad7e7c 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/resources/expectedDiagnostics/targetsDisambiguation.txt +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/resources/expectedDiagnostics/targetsDisambiguation.txt @@ -1,6 +1,96 @@ +[KotlinTargetAlreadyDeclared | WARNING] Kotlin Target 'linuxArm64()' is already declared. +Declaring multiple Kotlin Targets of the same type is deprecated. + +i.e. + kotlin { + linuxArm64() + linuxArm64("linuxArm_B") /* <- second 'linuxArm64' target in the project is deprecated */ + } + +Please use different Gradle Projects or create Kotlin Compilations. +For example: + + kotlin { + linuxArm64() { + val linuxArm_B by compilations.creating + } + } +---- +[KotlinTargetAlreadyDeclared | WARNING] Kotlin Target 'jvm()' is already declared. +Declaring multiple Kotlin Targets of the same type is deprecated. + +i.e. + kotlin { + jvm() + jvm("jvm_B") /* <- second 'jvm' target in the project is deprecated */ + } + +Please use different Gradle Projects or create Kotlin Compilations. +For example: + + kotlin { + jvm() { + val jvm_B by compilations.creating + } + } +---- +[KotlinTargetAlreadyDeclared | WARNING] Kotlin Target 'jvm()' is already declared. +Declaring multiple Kotlin Targets of the same type is deprecated. + +i.e. + kotlin { + jvm() + jvm("jvm_C") /* <- second 'jvm' target in the project is deprecated */ + } + +Please use different Gradle Projects or create Kotlin Compilations. +For example: + + kotlin { + jvm() { + val jvm_C by compilations.creating + } + } +---- +[KotlinTargetAlreadyDeclared | WARNING] Kotlin Target 'jvm()' is already declared. +Declaring multiple Kotlin Targets of the same type is deprecated. + +i.e. + kotlin { + jvm() + jvm("jvm_D") /* <- second 'jvm' target in the project is deprecated */ + } + +Please use different Gradle Projects or create Kotlin Compilations. +For example: + + kotlin { + jvm() { + val jvm_D by compilations.creating + } + } +---- +[KotlinTargetAlreadyDeclared | WARNING] Kotlin Target 'js()' is already declared. +Declaring multiple Kotlin Targets of the same type is deprecated. + +i.e. + kotlin { + js() + js("js_B") /* <- second 'js' target in the project is deprecated */ + } + +Please use different Gradle Projects or create Kotlin Compilations. +For example: + + kotlin { + js() { + val js_B by compilations.creating + } + } +---- [TargetsNeedDisambiguation | WARNING] The following targets are not distinguishable: * 'js_A', 'js_B' * 'jvm_C', 'jvm_D' * 'linuxArm_A', 'linuxArm_B' Use an additional attribute to disambiguate them -See https://kotlinlang.org/docs/multiplatform-set-up-targets.html#distinguish-several-targets-for-one-platform for more details +See https://kotlinlang.org/docs/multiplatform-set-up-targets.html#distinguish-several-targets-for-one-platform for more details \ No newline at end of file