From 364274ef90f9ea59f5a8cf82fa17070daf416df9 Mon Sep 17 00:00:00 2001 From: Andrey Uskov Date: Thu, 11 Apr 2019 16:09:44 +0300 Subject: [PATCH] Fix setting production-on-test flags in MPP projects #KT-29854 Fixed --- ...KotlinAndroidGradleMPPModuleDataService.kt | 35 +++++++++----- ...inAndroidGradleMPPModuleDataService.kt.181 | 35 +++++++++----- ...nAndroidGradleMPPModuleDataService.kt.as32 | 35 +++++++++----- .../idea/KotlinJavaMPPSourceSetDataService.kt | 16 ++++++- .../KotlinMPPGradleProjectResolver.kt | 11 +++-- .../src/org/jetbrains/kotlin/idea/utils.kt | 48 +++++++++++++++++-- .../NewMultiplatformProjectImportingTest.kt | 13 +++++ .../kotlin/gradle/projectStructureDSL.kt | 21 +++++++- .../productionOnTestFlag/build.gradle | 16 +++++++ .../javaModule/build.gradle | 6 +++ .../mppModule/build.gradle | 7 +++ .../productionOnTestFlag/settings.gradle | 2 + 12 files changed, 200 insertions(+), 45 deletions(-) create mode 100644 idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/build.gradle create mode 100644 idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/javaModule/build.gradle create mode 100644 idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/mppModule/build.gradle create mode 100644 idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/settings.gradle diff --git a/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt b/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt index 4f2dbc97508..f00566512f1 100644 --- a/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt +++ b/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt @@ -38,6 +38,7 @@ import org.jetbrains.kotlin.utils.addIfNotNull import org.jetbrains.plugins.gradle.model.data.GradleSourceSetData import java.io.File import java.io.IOException +import org.jetbrains.kotlin.idea.configuration.kotlinSourceSet class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService() { override fun getTargetDataKey() = ProjectKeys.MODULE @@ -95,11 +96,17 @@ class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService? ?: continue + val sourceSetData = sourceSetNode.data as? ModuleData ?: continue val sourceSetModule = modelsProvider.findIdeModule(sourceSetData) ?: continue - addModuleDependencyIfNeeded(rootModel, sourceSetModule, activeSourceSetInfo.isTestModule) + addModuleDependencyIfNeeded( + rootModel, + sourceSetModule, + activeSourceSetInfo.isTestModule, + sourceSetNode.kotlinSourceSet?.isTestModule ?: false + ) } } } @@ -169,7 +176,8 @@ class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService>() + + val gradleSourceSetDataNodes = SmartList>() .apply { addIfNotNull( (if (isAndroidModule) relevantNodes.firstByPlatformOrNull(KotlinPlatform.ANDROID) else null) @@ -181,14 +189,19 @@ class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService - addModuleDependencyIfNeeded(rootModel, transitiveDependee, testScope) - } + + val testKotlinModules = + gradleSourceSetDataNodes.filter { it.kotlinSourceSet?.isTestModule ?: false }.mapNotNull { modelsProvider.findIdeModule(it.data) } + .toSet() + + gradleSourceSetDataNodes.forEach { node -> + val module = modelsProvider.findIdeModule(node.data) ?: return + addModuleDependencyIfNeeded(rootModel, module, testScope, node.kotlinSourceSet?.isTestModule ?: false) + val dependeeRootModel = modelsProvider.getModifiableRootModel(module) + dependeeRootModel.getModuleDependencies(testScope).forEach { transitiveDependee -> + addModuleDependencyIfNeeded(rootModel, transitiveDependee, testScope, testKotlinModules.contains(transitiveDependee)) } + } } private fun getAndroidModuleModel(moduleNode: DataNode) = diff --git a/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt.181 b/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt.181 index d2231c8586d..d6e21dd0529 100644 --- a/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt.181 +++ b/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt.181 @@ -38,6 +38,7 @@ import org.jetbrains.kotlin.utils.addIfNotNull import org.jetbrains.plugins.gradle.model.data.GradleSourceSetData import java.io.File import java.io.IOException +import org.jetbrains.kotlin.idea.configuration.kotlinSourceSet class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService() { override fun getTargetDataKey() = ProjectKeys.MODULE @@ -95,11 +96,17 @@ class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService? ?: continue + val sourceSetData = sourceSetNode.data as? ModuleData ?: continue val sourceSetModule = modelsProvider.findIdeModule(sourceSetData) ?: continue - addModuleDependencyIfNeeded(rootModel, sourceSetModule, activeSourceSetInfo.isTestModule) + addModuleDependencyIfNeeded( + rootModel, + sourceSetModule, + activeSourceSetInfo.isTestModule, + sourceSetNode.kotlinSourceSet?.isTestModule ?: false + ) } } } @@ -169,7 +176,8 @@ class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService>() + + val gradleSourceSetDataNodes = SmartList>() .apply { addIfNotNull( (if (isAndroidModule) relevantNodes.firstByPlatformOrNull(KotlinPlatform.ANDROID) else null) @@ -181,14 +189,19 @@ class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService - addModuleDependencyIfNeeded(rootModel, transitiveDependee, testScope) - } + + val testKotlinModules = + gradleSourceSetDataNodes.filter { it.kotlinSourceSet?.isTestModule ?: false }.mapNotNull { modelsProvider.findIdeModule(it.data) } + .toSet() + + gradleSourceSetDataNodes.forEach { node -> + val module = modelsProvider.findIdeModule(node.data) ?: return + addModuleDependencyIfNeeded(rootModel, module, testScope, node.kotlinSourceSet?.isTestModule ?: false) + val dependeeRootModel = modelsProvider.getModifiableRootModel(module) + dependeeRootModel.getModuleDependencies(testScope).forEach { transitiveDependee -> + addModuleDependencyIfNeeded(rootModel, transitiveDependee, testScope, testKotlinModules.contains(transitiveDependee)) } + } } private fun getAndroidModuleModel(moduleNode: DataNode) = diff --git a/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt.as32 b/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt.as32 index 4f2dbc97508..f00566512f1 100644 --- a/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt.as32 +++ b/idea/idea-android/src/org/jetbrains/kotlin/android/configure/KotlinAndroidGradleMPPModuleDataService.kt.as32 @@ -38,6 +38,7 @@ import org.jetbrains.kotlin.utils.addIfNotNull import org.jetbrains.plugins.gradle.model.data.GradleSourceSetData import java.io.File import java.io.IOException +import org.jetbrains.kotlin.idea.configuration.kotlinSourceSet class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService() { override fun getTargetDataKey() = ProjectKeys.MODULE @@ -95,11 +96,17 @@ class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService? ?: continue + val sourceSetData = sourceSetNode.data as? ModuleData ?: continue val sourceSetModule = modelsProvider.findIdeModule(sourceSetData) ?: continue - addModuleDependencyIfNeeded(rootModel, sourceSetModule, activeSourceSetInfo.isTestModule) + addModuleDependencyIfNeeded( + rootModel, + sourceSetModule, + activeSourceSetInfo.isTestModule, + sourceSetNode.kotlinSourceSet?.isTestModule ?: false + ) } } } @@ -169,7 +176,8 @@ class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService>() + + val gradleSourceSetDataNodes = SmartList>() .apply { addIfNotNull( (if (isAndroidModule) relevantNodes.firstByPlatformOrNull(KotlinPlatform.ANDROID) else null) @@ -181,14 +189,19 @@ class KotlinAndroidGradleMPPModuleDataService : AbstractProjectDataService - addModuleDependencyIfNeeded(rootModel, transitiveDependee, testScope) - } + + val testKotlinModules = + gradleSourceSetDataNodes.filter { it.kotlinSourceSet?.isTestModule ?: false }.mapNotNull { modelsProvider.findIdeModule(it.data) } + .toSet() + + gradleSourceSetDataNodes.forEach { node -> + val module = modelsProvider.findIdeModule(node.data) ?: return + addModuleDependencyIfNeeded(rootModel, module, testScope, node.kotlinSourceSet?.isTestModule ?: false) + val dependeeRootModel = modelsProvider.getModifiableRootModel(module) + dependeeRootModel.getModuleDependencies(testScope).forEach { transitiveDependee -> + addModuleDependencyIfNeeded(rootModel, transitiveDependee, testScope, testKotlinModules.contains(transitiveDependee)) } + } } private fun getAndroidModuleModel(moduleNode: DataNode) = diff --git a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/KotlinJavaMPPSourceSetDataService.kt b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/KotlinJavaMPPSourceSetDataService.kt index 925ed248cb6..9d46cf950d8 100644 --- a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/KotlinJavaMPPSourceSetDataService.kt +++ b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/KotlinJavaMPPSourceSetDataService.kt @@ -28,6 +28,8 @@ class KotlinJavaMPPSourceSetDataService : AbstractProjectDataService - addModuleDependencyIfNeeded(rootModel, transitiveDependee, isTestSourceSet) + addModuleDependencyIfNeeded( + rootModel, + transitiveDependee, + isTestSourceSet, + testKotlinModules.contains(transitiveDependee) + ) } } rootModel.removeOrderEntry(libraryEntry) diff --git a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinMPPGradleProjectResolver.kt b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinMPPGradleProjectResolver.kt index 3a85fd44a2a..d363924b3a4 100644 --- a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinMPPGradleProjectResolver.kt +++ b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/configuration/KotlinMPPGradleProjectResolver.kt @@ -31,7 +31,6 @@ import org.jetbrains.kotlin.cli.common.arguments.parseCommandLineArguments import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.gradle.* import org.jetbrains.kotlin.idea.configuration.GradlePropertiesFileFacade.Companion.KOTLIN_NOT_IMPORTED_COMMON_SOURCE_SETS_SETTING -import org.jetbrains.kotlin.idea.configuration.KotlinMPPGradleProjectResolver.Companion.isNotSupported import org.jetbrains.kotlin.idea.platform.IdePlatformKindTooling import org.jetbrains.plugins.gradle.model.* import org.jetbrains.plugins.gradle.model.data.BuildScriptClasspathData @@ -361,7 +360,7 @@ open class KotlinMPPGradleProjectResolver : AbstractProjectResolverExtension() { for (sourceSet in compilation.sourceSets) { if (sourceSet.fullName() == compilation.fullName()) continue val targetDataNode = getSiblingKotlinModuleData(sourceSet, gradleModule, ideModule, resolverCtx) ?: continue - addDependency(dataNode, targetDataNode) + addDependency(dataNode, targetDataNode, sourceSet.isTestModule) } } } @@ -421,7 +420,7 @@ open class KotlinMPPGradleProjectResolver : AbstractProjectResolverExtension() { if (sourceSet.platform == KotlinPlatform.ANDROID) continue for (dependeeSourceSet in dependeeSourceSets) { val toDataNode = getSiblingKotlinModuleData(dependeeSourceSet, gradleModule, ideModule, resolverCtx) ?: continue - addDependency(fromDataNode, toDataNode) + addDependency(fromDataNode, toDataNode, dependeeSourceSet.isTestModule) } if (processedModuleIds.add(getKotlinModuleId(gradleModule, sourceSet, resolverCtx))) { val mergedDependencies = LinkedHashSet().apply { @@ -454,12 +453,13 @@ open class KotlinMPPGradleProjectResolver : AbstractProjectResolverExtension() { .filterNotNull() } - private fun addDependency(fromModule: DataNode<*>, toModule: DataNode<*>) { + private fun addDependency(fromModule: DataNode<*>, toModule: DataNode<*>, dependOnTestModule: Boolean) { val fromData = fromModule.data as? ModuleData ?: return val toData = toModule.data as? ModuleData ?: return val moduleDependencyData = ModuleDependencyData(fromData, toData).also { it.scope = DependencyScope.COMPILE it.isExported = false + it.isProductionOnTestDependency = dependOnTestModule } fromModule.createChild(ProjectKeys.MODULE_DEPENDENCY, moduleDependencyData) } @@ -567,6 +567,9 @@ open class KotlinMPPGradleProjectResolver : AbstractProjectResolverExtension() { sourceSet.name = compilation.fullName() sourceSet.targetCompatibility = compilationData.targetCompatibility sourceSet.dependencies += compilation.dependencies + //TODO after applying patch to IDEA core uncomment the following line: + // sourceSet.isTest = compilation.sourceSets.filter { isTestModule }.isNotEmpty() + // It will allow to get rid of hacks with guessing module type in DataServices and obtain properly set productionOnTest flags val sourcesWithTypes = SmartList>() if (effectiveClassesDir != null) { sourcesWithTypes += compilation.sourceType to DefaultExternalSourceDirectorySet().also { dirSet -> diff --git a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/utils.kt b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/utils.kt index 6e40bbc74b4..00e2c78036b 100644 --- a/idea/idea-gradle/src/org/jetbrains/kotlin/idea/utils.kt +++ b/idea/idea-gradle/src/org/jetbrains/kotlin/idea/utils.kt @@ -8,20 +8,58 @@ package org.jetbrains.kotlin.idea import com.intellij.openapi.module.Module import com.intellij.openapi.roots.DependencyScope import com.intellij.openapi.roots.ModifiableRootModel +import com.intellij.openapi.roots.impl.ModuleOrderEntryImpl +import com.intellij.openapi.diagnostic.Logger + +/** + * Returns the dependency scope which has flags specific to the both provided scopes + */ +fun getScopeContainingBoth(scope1: DependencyScope, scope2: DependencyScope?): DependencyScope { + if (scope2 == null) { + return scope1 + } + + val compile = scope1.isForProductionCompile || scope2.isForProductionCompile + val runtime = scope1.isForProductionRuntime || scope2.isForProductionRuntime + val testCompile = scope1.isForTestCompile || scope2.isForTestCompile + val testRuntime = scope1.isForTestRuntime || scope2.isForTestRuntime + + //we suppose that scope1 or scope 2 is at least Test + val result = if (compile || runtime) DependencyScope.COMPILE else DependencyScope.TEST + // validate that result is valid. We could not express several cases (e.g. RUNTIME + TEST), we will log this case + if (result.isForProductionCompile != compile || + result.isForProductionRuntime != runtime || + result.isForTestCompile != testCompile || + result.isForTestRuntime != testRuntime + ) { + Logger.getInstance(KotlinJavaMPPSourceSetDataService::class.java) + .warn("Could not express cross-module dependency with flags Compile=$compile Runtime=$runtime TestCompile=$testCompile TestRuntime=$testRuntime") + } + return result +} fun addModuleDependencyIfNeeded( rootModel: ModifiableRootModel, dependeeModule: Module, - testScope: Boolean + testScope: Boolean, + dependOnTest: Boolean ) { - val dependencyScope = if (testScope) DependencyScope.TEST else DependencyScope.COMPILE val existingEntry = rootModel.findModuleOrderEntry(dependeeModule) + val existingDependOnTest = (existingEntry as? ModuleOrderEntryImpl)?.isProductionOnTestDependency ?: false + + val requiredScope = getScopeContainingBoth(if (testScope) DependencyScope.TEST else DependencyScope.COMPILE, existingEntry?.scope) if (existingEntry != null) { val existingScope = existingEntry.scope - if (existingScope == DependencyScope.COMPILE || existingScope == dependencyScope) return - if (dependencyScope == DependencyScope.COMPILE) { + + + if (requiredScope != existingScope || (dependOnTest && !existingDependOnTest)) { rootModel.removeOrderEntry(existingEntry) + } else { + return } } - rootModel.addModuleOrderEntry(dependeeModule).also { it.scope = dependencyScope } + rootModel.addModuleOrderEntry(dependeeModule).also { + it.scope = requiredScope + (it as? ModuleOrderEntryImpl)?.isProductionOnTestDependency = dependOnTest || existingDependOnTest + } } \ No newline at end of file diff --git a/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/NewMultiplatformProjectImportingTest.kt b/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/NewMultiplatformProjectImportingTest.kt index c8c53b914a3..2c16b82b8a7 100644 --- a/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/NewMultiplatformProjectImportingTest.kt +++ b/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/NewMultiplatformProjectImportingTest.kt @@ -536,6 +536,19 @@ class NewMultiplatformProjectImportingTest : MultiplePluginVersionGradleImportin } } + @Test + fun testProductionOnTestFlag() { + configureByFiles() + importProject(true) + + checkProjectStructure(false, false, false ) { + module("project.javaModule.test") { + moduleDependency("project.mppModule.jvmTest", DependencyScope.TEST, true) + moduleDependency("project.mppModule.jvmMain", DependencyScope.TEST, false) + } + } + } + private fun checkProjectStructure( exhaustiveModuleList: Boolean = true, exhaustiveSourceSourceRootList: Boolean = true, diff --git a/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/projectStructureDSL.kt b/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/projectStructureDSL.kt index 33a9b40a57a..0be5cf74f63 100644 --- a/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/projectStructureDSL.kt +++ b/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/projectStructureDSL.kt @@ -10,6 +10,7 @@ import com.intellij.openapi.module.ModuleManager import com.intellij.openapi.project.Project import com.intellij.openapi.project.rootManager import com.intellij.openapi.roots.* +import com.intellij.openapi.roots.impl.ModuleOrderEntryImpl import com.intellij.openapi.util.io.FileUtil import org.jetbrains.jps.model.module.JpsModuleSourceRootType import org.jetbrains.jps.util.JpsPathUtil @@ -147,7 +148,7 @@ class ModuleInfo( expectedDependencyNames += libraryEntry.presentableName } - fun moduleDependency(moduleName: String, scope: DependencyScope) { + fun moduleDependency(moduleName: String, scope: DependencyScope, productionOnTest: Boolean? = null) { val moduleEntry = rootModel.orderEntries.filterIsInstance().singleOrNull { it.moduleName == moduleName } if (moduleEntry == null) { val allModules = rootModel.orderEntries.filterIsInstance().map { it.moduleName }.joinToString(", ") @@ -155,6 +156,7 @@ class ModuleInfo( return } checkDependencyScope(moduleEntry, scope) + checkProductionOnTest(moduleEntry, productionOnTest) expectedDependencyNames += moduleEntry.presentableName } @@ -241,6 +243,23 @@ class ModuleInfo( ) } } + + private fun checkProductionOnTest(library: ExportableOrderEntry, productionOnTest: Boolean?) { + if (productionOnTest == null) return + val actualFlag = (library as? ModuleOrderEntryImpl)?.isProductionOnTestDependency + if (actualFlag == null) { + projectInfo.messageCollector.report( + "Module '${module.name}': Dependency '${library.presentableName}' has no productionOnTest property" + ) + } else { + if (actualFlag != productionOnTest) { + projectInfo.messageCollector.report( + "Module '${module.name}': Dependency '${library.presentableName}': expected productionOnTest '$productionOnTest' but found '$actualFlag'" + ) + } + } + + } } fun checkProjectStructure( diff --git a/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/build.gradle b/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/build.gradle new file mode 100644 index 00000000000..842170d06ce --- /dev/null +++ b/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/build.gradle @@ -0,0 +1,16 @@ +subprojects { + buildscript { + repositories { + mavenLocal() + mavenCentral() + } + dependencies { + classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:1.3.21" + } + } + + repositories { + mavenLocal() + mavenCentral() + } +} diff --git a/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/javaModule/build.gradle b/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/javaModule/build.gradle new file mode 100644 index 00000000000..a58b2b14715 --- /dev/null +++ b/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/javaModule/build.gradle @@ -0,0 +1,6 @@ +apply plugin: 'kotlin' + + +dependencies { + testCompile project(':mppModule') +} diff --git a/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/mppModule/build.gradle b/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/mppModule/build.gradle new file mode 100644 index 00000000000..6296ee19a8a --- /dev/null +++ b/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/mppModule/build.gradle @@ -0,0 +1,7 @@ +apply plugin: 'kotlin-multiplatform' + +kotlin { + jvm() + +} +println("mppModule") \ No newline at end of file diff --git a/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/settings.gradle b/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/settings.gradle new file mode 100644 index 00000000000..30c76d69ad9 --- /dev/null +++ b/idea/testData/gradle/newMultiplatformImport/productionOnTestFlag/settings.gradle @@ -0,0 +1,2 @@ +include 'javaModule' +include 'mppModule'