From 4325632b3ec56beba2a009b7f5eeca08c70a289e Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Thu, 2 Feb 2017 20:42:07 +0300 Subject: [PATCH] Kotlin Facet: Always parse argument string to proper compiler arguments bean #KT-16137 Fixed #KT-16157 Fixed #KT-16206 Fixed --- .idea/artifacts/KotlinPlugin.xml | 1 + .../cli/common/arguments/argumentUtils.kt | 20 +- .../kotlin/config/CompilerSettings.kt | 2 +- .../kotlin/config/KotlinFacetSettings.kt | 2 + idea/idea.iml | 1 + .../idea/facet/KotlinFacetEditorGeneralTab.kt | 4 +- .../jetbrains/kotlin/idea/facet/facetUtils.kt | 96 ++++++-- .../gradle/GradleFacetImportTest.kt | 220 ++++++++++++++++++ .../kotlin/jps/JpsKotlinCompilerSettings.kt | 3 +- 9 files changed, 314 insertions(+), 35 deletions(-) create mode 100644 idea/tests/org/jetbrains/kotlin/idea/codeInsight/gradle/GradleFacetImportTest.kt diff --git a/.idea/artifacts/KotlinPlugin.xml b/.idea/artifacts/KotlinPlugin.xml index 31b46acd7b9..ceb9ba2a2b2 100644 --- a/.idea/artifacts/KotlinPlugin.xml +++ b/.idea/artifacts/KotlinPlugin.xml @@ -25,6 +25,7 @@ + diff --git a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/argumentUtils.kt b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/argumentUtils.kt index 1ec96cf8aa5..33a87c78d47 100644 --- a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/argumentUtils.kt +++ b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/argumentUtils.kt @@ -37,16 +37,22 @@ import java.util.* } } -fun copyBean(bean: T) = copyFields(bean, bean.javaClass.newInstance(), true) +fun copyBean(bean: T) = copyFields(bean, bean.javaClass.newInstance(), true, collectFieldsToCopy(bean.javaClass, false)) fun mergeBeans(from: From, to: To): To { // TODO: rewrite when updated version of com.intellij.util.xmlb is available on TeamCity - return copyFields(from, XmlSerializerUtil.createCopy(to), false) + return copyFields(from, XmlSerializerUtil.createCopy(to), false, collectFieldsToCopy(from.javaClass, false)) } -private fun copyFields(from: From, to: To, deepCopyWhenNeeded: Boolean = false): To { - val fromFields = collectFieldsToCopy(from.javaClass) - for (fromField in fromFields) { +fun copyInheritedFields(from: From, to: To) = copyFields(from, to, true, collectFieldsToCopy(from.javaClass, true)) + +fun copyFieldsSatisfying(from: From, to: To, predicate: (Field) -> Boolean) = + copyFields(from, to, true, collectFieldsToCopy(from.javaClass, false).filter(predicate)) + +private fun copyFields(from: From, to: To, deepCopyWhenNeeded: Boolean, fieldsToCopy: List): To { + if (from == to) return to + + for (fromField in fieldsToCopy) { val toField = to.javaClass.getField(fromField.name) val fromValue = fromField.get(from) toField.set(to, if (deepCopyWhenNeeded) fromValue?.copyValueIfNeeded() else fromValue) @@ -83,10 +89,10 @@ private fun Any.copyValueIfNeeded(): Any { } } -private fun collectFieldsToCopy(clazz: Class<*>): List { +private fun collectFieldsToCopy(clazz: Class<*>, inheritedOnly: Boolean): List { val fromFields = ArrayList() - var currentClass: Class<*>? = clazz + var currentClass: Class<*>? = if (inheritedOnly) clazz.superclass else clazz while (currentClass != null) { for (field in currentClass.declaredFields) { val modifiers = field.modifiers diff --git a/idea/idea-jps-common/src/org/jetbrains/kotlin/config/CompilerSettings.kt b/idea/idea-jps-common/src/org/jetbrains/kotlin/config/CompilerSettings.kt index 8ca191aaf90..89af21eee09 100644 --- a/idea/idea-jps-common/src/org/jetbrains/kotlin/config/CompilerSettings.kt +++ b/idea/idea-jps-common/src/org/jetbrains/kotlin/config/CompilerSettings.kt @@ -24,7 +24,7 @@ class CompilerSettings { @JvmField var outputDirectoryForJsLibraryFiles: String = DEFAULT_OUTPUT_DIRECTORY companion object { - private val DEFAULT_ADDITIONAL_ARGUMENTS = "-version" + val DEFAULT_ADDITIONAL_ARGUMENTS = "-version" private val DEFAULT_OUTPUT_DIRECTORY = "lib" } } diff --git a/idea/idea-jps-common/src/org/jetbrains/kotlin/config/KotlinFacetSettings.kt b/idea/idea-jps-common/src/org/jetbrains/kotlin/config/KotlinFacetSettings.kt index e71fd230cdc..c77ffee8e09 100644 --- a/idea/idea-jps-common/src/org/jetbrains/kotlin/config/KotlinFacetSettings.kt +++ b/idea/idea-jps-common/src/org/jetbrains/kotlin/config/KotlinFacetSettings.kt @@ -23,6 +23,7 @@ import com.intellij.util.xmlb.annotations.Property import com.intellij.util.xmlb.annotations.Transient import org.jetbrains.kotlin.cli.common.arguments.CommonCompilerArguments import org.jetbrains.kotlin.cli.common.arguments.K2JSCompilerArguments +import org.jetbrains.kotlin.cli.common.arguments.K2JVMCompilerArguments import org.jetbrains.kotlin.utils.DescriptionAware sealed class TargetPlatformKind( @@ -96,6 +97,7 @@ class KotlinCompilerInfo { _commonCompilerArguments = value as? CommonCompilerArguments.DummyImpl } var k2jsCompilerArguments: K2JSCompilerArguments? = null + var k2jvmCompilerArguments: K2JVMCompilerArguments? = null var compilerSettings: CompilerSettings? = null @get:Transient var coroutineSupport: CoroutineSupport diff --git a/idea/idea.iml b/idea/idea.iml index f2e18e49954..1a666ff6ae5 100644 --- a/idea/idea.iml +++ b/idea/idea.iml @@ -61,5 +61,6 @@ + \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/facet/KotlinFacetEditorGeneralTab.kt b/idea/src/org/jetbrains/kotlin/idea/facet/KotlinFacetEditorGeneralTab.kt index 9334cae6f1c..35dba3196f2 100644 --- a/idea/src/org/jetbrains/kotlin/idea/facet/KotlinFacetEditorGeneralTab.kt +++ b/idea/src/org/jetbrains/kotlin/idea/facet/KotlinFacetEditorGeneralTab.kt @@ -25,6 +25,7 @@ import com.intellij.util.ui.ThreeStateCheckBox import com.intellij.util.ui.UIUtil import org.jetbrains.kotlin.cli.common.arguments.CommonCompilerArguments import org.jetbrains.kotlin.cli.common.arguments.K2JSCompilerArguments +import org.jetbrains.kotlin.cli.common.arguments.K2JVMCompilerArguments import org.jetbrains.kotlin.config.CompilerSettings import org.jetbrains.kotlin.config.KotlinCompilerInfo import org.jetbrains.kotlin.config.LanguageVersion @@ -49,6 +50,7 @@ class KotlinFacetEditorGeneralTab( .apply { commonCompilerArguments = object : CommonCompilerArguments() {} k2jsCompilerArguments = K2JSCompilerArguments() + k2jvmCompilerArguments = K2JVMCompilerArguments() compilerSettings = CompilerSettings() } val compilerConfigurable = with(compilerInfo) { @@ -58,7 +60,7 @@ class KotlinFacetEditorGeneralTab( k2jsCompilerArguments, compilerSettings, null, - null, + k2jvmCompilerArguments, false, isMultiEditor ) diff --git a/idea/src/org/jetbrains/kotlin/idea/facet/facetUtils.kt b/idea/src/org/jetbrains/kotlin/idea/facet/facetUtils.kt index 8f398f7c909..21cf66d62d9 100644 --- a/idea/src/org/jetbrains/kotlin/idea/facet/facetUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/facet/facetUtils.kt @@ -25,11 +25,11 @@ import com.intellij.openapi.roots.ModuleRootManager import com.intellij.openapi.roots.ModuleRootModel import com.intellij.openapi.roots.OrderRootType import com.intellij.util.text.VersionComparatorUtil -import org.jetbrains.kotlin.cli.common.arguments.K2JVMCompilerArguments -import org.jetbrains.kotlin.cli.common.arguments.copyBean -import org.jetbrains.kotlin.cli.common.arguments.parseArguments +import org.jetbrains.kotlin.cli.common.arguments.* +import org.jetbrains.kotlin.compilerRunner.ArgumentUtils import org.jetbrains.kotlin.config.* import org.jetbrains.kotlin.idea.compiler.configuration.Kotlin2JsCompilerArgumentsHolder +import org.jetbrains.kotlin.idea.compiler.configuration.Kotlin2JvmCompilerArgumentsHolder import org.jetbrains.kotlin.idea.compiler.configuration.KotlinCommonCompilerArgumentsHolder import org.jetbrains.kotlin.idea.compiler.configuration.KotlinCompilerSettings import org.jetbrains.kotlin.idea.framework.JSLibraryStdPresentationProvider @@ -139,6 +139,10 @@ fun KotlinFacetSettings.initializeIfNeeded(module: Module, rootModel: ModuleRoot if (k2jsCompilerArguments == null) { k2jsCompilerArguments = copyBean(Kotlin2JsCompilerArgumentsHolder.getInstance(project).settings) } + + if (k2jvmCompilerArguments == null) { + k2jvmCompilerArguments = copyBean(Kotlin2JvmCompilerArgumentsHolder.getInstance(project).settings) + } } } @@ -181,38 +185,80 @@ fun KotlinFacet.configureFacet( } } +// Update these lists when facet/project settings UI changes +private val commonExposedFields = listOf("languageVersion", + "apiVersion", + "suppressWarnings", + "coroutinesEnable", + "coroutinesWarn", + "coroutinesError") +private val jvmExposedFields = commonExposedFields + + listOf("jvmTarget") +private val jsExposedFields = commonExposedFields + + listOf("sourceMap", + "outputPrefix", + "outputPostfix", + "moduleKind") + +private val CommonCompilerArguments.exposedFields: List + get() = when (this) { + is K2JVMCompilerArguments -> jvmExposedFields + is K2JSCompilerArguments -> jsExposedFields + else -> commonExposedFields + } + fun parseCompilerArgumentsToFacet(arguments: List, kotlinFacet: KotlinFacet) { val argumentArray = arguments.toTypedArray() + with(kotlinFacet.configuration.settings) { // todo: merge common arguments with platform-specific ones in facet settings - compilerInfo.commonCompilerArguments!!.let { commonCompilerArguments -> - val oldCoroutineSupport = CoroutineSupport.byCompilerArguments(commonCompilerArguments) - commonCompilerArguments.coroutinesEnable = false - commonCompilerArguments.coroutinesWarn = false - commonCompilerArguments.coroutinesError = false - parseArguments(argumentArray, commonCompilerArguments, ignoreInvalidArguments = true) - if (!commonCompilerArguments.coroutinesEnable && !commonCompilerArguments.coroutinesWarn && !commonCompilerArguments.coroutinesError) { - compilerInfo.coroutineSupport = oldCoroutineSupport - } + val commonCompilerArguments = compilerInfo.commonCompilerArguments!! + val compilerArguments = when (versionInfo.targetPlatformKind) { + is TargetPlatformKind.Jvm -> compilerInfo.k2jvmCompilerArguments + is TargetPlatformKind.JavaScript -> compilerInfo.k2jsCompilerArguments + else -> commonCompilerArguments + }!! - versionInfo.apiLevel = LanguageVersion.fromVersionString(commonCompilerArguments.apiVersion) - versionInfo.languageLevel = LanguageVersion.fromVersionString(commonCompilerArguments.languageVersion) + val oldCoroutineSupport = CoroutineSupport.byCompilerArguments(commonCompilerArguments) + commonCompilerArguments.coroutinesEnable = false + commonCompilerArguments.coroutinesWarn = false + commonCompilerArguments.coroutinesError = false + + parseArguments(argumentArray, compilerArguments, true) + + if (!compilerArguments.coroutinesEnable && !compilerArguments.coroutinesWarn && !compilerArguments.coroutinesError) { + compilerInfo.coroutineSupport = oldCoroutineSupport } - when (versionInfo.targetPlatformKind) { - is TargetPlatformKind.Jvm -> { - val jvmTarget = K2JVMCompilerArguments().apply { parseArguments(argumentArray, this, ignoreInvalidArguments = true) }.jvmTarget - if (jvmTarget != null) { - versionInfo.targetPlatformKind = TargetPlatformKind.Jvm.JVM_PLATFORMS.firstOrNull { - VersionComparatorUtil.compare(it.version.description, jvmTarget) >= 0 - } ?: TargetPlatformKind.Jvm.JVM_PLATFORMS.last() - } + versionInfo.apiLevel = LanguageVersion.fromVersionString(compilerArguments.apiVersion) + versionInfo.languageLevel = LanguageVersion.fromVersionString(compilerArguments.languageVersion) + + if (versionInfo.targetPlatformKind is TargetPlatformKind.Jvm) { + val jvmTarget = compilerInfo.k2jvmCompilerArguments!!.jvmTarget + if (jvmTarget != null) { + versionInfo.targetPlatformKind = TargetPlatformKind.Jvm.JVM_PLATFORMS.firstOrNull { + VersionComparatorUtil.compare(it.version.description, jvmTarget) >= 0 + } ?: TargetPlatformKind.Jvm.JVM_PLATFORMS.last() } - is TargetPlatformKind.JavaScript -> parseArguments(argumentArray, compilerInfo.k2jsCompilerArguments!!, ignoreInvalidArguments = true) - else -> {} } - compilerInfo.compilerSettings!!.additionalArguments = compilerInfo.commonCompilerArguments!!.freeArgs.joinToString(separator = " ") + // Retain only fields exposed in facet configuration editor. + // The rest is combined into string and stored in CompilerSettings.additionalArguments + + val exposedFields = compilerArguments.exposedFields + + val additionalArgumentsString = with(compilerArguments.javaClass.newInstance()) { + copyFieldsSatisfying(compilerArguments, this) { it.name !in exposedFields } + ArgumentUtils.convertArgumentsToStringList(this).joinToString(separator = " ") + } + compilerInfo.compilerSettings!!.additionalArguments = + if (additionalArgumentsString.isNotEmpty()) additionalArgumentsString else CompilerSettings.DEFAULT_ADDITIONAL_ARGUMENTS + + with(compilerArguments.javaClass.newInstance()) { + copyFieldsSatisfying(this, compilerArguments) { it.name !in exposedFields } + } + + copyInheritedFields(compilerArguments, commonCompilerArguments) } } \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/gradle/GradleFacetImportTest.kt b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/gradle/GradleFacetImportTest.kt new file mode 100644 index 00000000000..48b4021cf53 --- /dev/null +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/gradle/GradleFacetImportTest.kt @@ -0,0 +1,220 @@ +/* + * Copyright 2010-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.jetbrains.kotlin.idea.codeInsight.gradle + +import org.jetbrains.kotlin.config.CoroutineSupport +import org.jetbrains.kotlin.config.JvmTarget +import org.jetbrains.kotlin.config.KotlinFacetSettings +import org.jetbrains.kotlin.config.TargetPlatformKind +import org.jetbrains.kotlin.idea.facet.KotlinFacet +import org.junit.Assert +import org.junit.Test + +class GradleFacetImportTest : GradleImportingTestCase() { + private val facetSettings: KotlinFacetSettings + get() = KotlinFacet.get(getModule("project_main"))!!.configuration.settings + + @Test + fun testJvmImport() { + createProjectSubFile("build.gradle", """ + group 'Again' + version '1.0-SNAPSHOT' + + buildscript { + repositories { + mavenCentral() + maven { + url 'http://dl.bintray.com/kotlin/kotlin-eap-1.1' + } + } + + dependencies { + classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.1.0-beta-38") + } + } + + apply plugin: 'kotlin' + + dependencies { + compile "org.jetbrains.kotlin:kotlin-stdlib:1.1.0-beta-38" + } + + compileKotlin { + kotlinOptions.jvmTarget = "1.7" + kotlinOptions.freeCompilerArgs = ["-Xsingle-module", "-Xdump-declarations-to", "tmp"] + } + """) + importProject() + + with (facetSettings) { + Assert.assertEquals("1.1", versionInfo.languageLevel!!.versionString) + Assert.assertEquals("1.1", versionInfo.apiLevel!!.versionString) + Assert.assertEquals(TargetPlatformKind.Jvm[JvmTarget.JVM_1_8], versionInfo.targetPlatformKind) + Assert.assertEquals("1.7", compilerInfo.k2jvmCompilerArguments!!.jvmTarget) + Assert.assertEquals("-no-stdlib -no-reflect -module-name project_main -Xdump-declarations-to tmp -Xsingle-module -Xadd-compiler-builtins", + compilerInfo.compilerSettings!!.additionalArguments) + } + } + + @Test + fun testCoroutineImportByOptions() { + createProjectSubFile("build.gradle", """ + group 'Again' + version '1.0-SNAPSHOT' + + buildscript { + repositories { + mavenCentral() + maven { + url 'http://dl.bintray.com/kotlin/kotlin-eap-1.1' + } + } + + dependencies { + classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.1.0-beta-38") + } + } + + apply plugin: 'kotlin' + + dependencies { + compile "org.jetbrains.kotlin:kotlin-stdlib:1.1.0-beta-38" + } + + kotlin { + experimental { + coroutines 'enable' + } + } + """) + importProject() + + with (facetSettings) { + Assert.assertEquals(CoroutineSupport.ENABLED, compilerInfo.coroutineSupport) + } + } + + @Test + fun testCoroutineImportByProperties() { + createProjectSubFile("gradle.properties", "kotlin.coroutines=enable") + createProjectSubFile("build.gradle", """ + group 'Again' + version '1.0-SNAPSHOT' + + buildscript { + repositories { + mavenCentral() + maven { + url 'http://dl.bintray.com/kotlin/kotlin-eap-1.1' + } + } + + dependencies { + classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.1.0-beta-38") + } + } + + apply plugin: 'kotlin' + + dependencies { + compile "org.jetbrains.kotlin:kotlin-stdlib:1.1.0-beta-38" + } + """) + importProject() + + with (facetSettings) { + Assert.assertEquals(CoroutineSupport.ENABLED, compilerInfo.coroutineSupport) + } + } + + // TODO: Uncomment the test below when 1.1-RC is available (see KT-16174) + /*@Test + fun testJsImport() { + createProjectSubFile("build.gradle", """ + group 'Again' + version '1.0-SNAPSHOT' + + buildscript { + repositories { + mavenCentral() + maven { + url 'http://dl.bintray.com/kotlin/kotlin-eap-1.1' + } + } + + dependencies { + classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.1.0-rc") + } + } + + apply plugin: 'kotlin2js' + + dependencies { + compile "org.jetbrains.kotlin:kotlin-stdlib-js:1.1.0-rc" + } + + compileKotlin2Js { + kotlinOptions.sourceMap = true + kotlinOptions.freeCompilerArgs = ["-module-kind", "plain"] + } + """) + importProject() + + with (facetSettings) { + Assert.assertEquals("1.1", versionInfo.languageLevel!!.versionString) + Assert.assertEquals("1.1", versionInfo.apiLevel!!.versionString) + Assert.assertEquals(TargetPlatformKind.JavaScript, versionInfo.targetPlatformKind) + Assert.assertEquals(true, compilerInfo.k2jsCompilerArguments!!.sourceMap) + Assert.assertEquals("-source-map -module-kind plain -target v5 -main call", + compilerInfo.compilerSettings!!.additionalArguments) + } + }*/ + + @Test + fun testCommonImport() { + createProjectSubFile("build.gradle", """ + group 'Again' + version '1.0-SNAPSHOT' + + buildscript { + repositories { + mavenCentral() + maven { + url 'http://dl.bintray.com/kotlin/kotlin-eap-1.1' + } + } + + dependencies { + classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:1.1.0-beta-38") + } + } + + apply plugin: 'kotlin' + + dependencies { + compile "org.jetbrains.kotlin:kotlin-stdlib-common:1.1.0-beta-38" + } + """) + importProject() + + with (facetSettings) { + Assert.assertEquals("1.1", versionInfo.languageLevel!!.versionString) + Assert.assertEquals("1.1", versionInfo.apiLevel!!.versionString) + Assert.assertEquals(TargetPlatformKind.Common, versionInfo.targetPlatformKind) + } + } +} diff --git a/jps-plugin/src/org/jetbrains/kotlin/jps/JpsKotlinCompilerSettings.kt b/jps-plugin/src/org/jetbrains/kotlin/jps/JpsKotlinCompilerSettings.kt index 99d006e70af..b7632daa9a8 100644 --- a/jps-plugin/src/org/jetbrains/kotlin/jps/JpsKotlinCompilerSettings.kt +++ b/jps-plugin/src/org/jetbrains/kotlin/jps/JpsKotlinCompilerSettings.kt @@ -87,7 +87,8 @@ class JpsKotlinCompilerSettings : JpsElementBase() { val facetSettings = module.kotlinFacetExtension?.settings ?: return defaultArguments if (facetSettings.useProjectSettings) return defaultArguments val targetPlatform = facetSettings.versionInfo.targetPlatformKind as? TargetPlatformKind.Jvm ?: return defaultArguments - return copyBean(defaultArguments).apply { + val arguments = facetSettings.compilerInfo.k2jvmCompilerArguments ?: defaultArguments + return copyBean(arguments).apply { jvmTarget = targetPlatform.version.description } }