Kotlin Facet: Validate "additional arguments"

Validate consistency of "additional arguments" with respect to settings specified in other UI controls

idea/idea-jps-common/src/org/jetbrains/kotlin/config/CompilerSettings.kt

@@ -32,6 +32,8 @@ class CompilerSettings {
 }
 
 val CompilerSettings.additionalArgumentsAsList: List<String>
-    get() = StringUtil.splitHonorQuotes(additionalArguments, ' ').map {
+    get() = splitArgumentString(additionalArguments)
-        if (it.startsWith('"')) StringUtil.unescapeChar(StringUtil.unquoteString(it), '"') else it
+
-    }
+fun splitArgumentString(arguments: String) = StringUtil.splitHonorQuotes(arguments, ' ').map {
+    if (it.startsWith('"')) StringUtil.unescapeChar(StringUtil.unquoteString(it), '"') else it
+}

idea/src/org/jetbrains/kotlin/idea/compiler/configuration/KotlinCompilerConfigurableTab.java

@@ -388,8 +388,12 @@ public class KotlinCompilerConfigurableTab implements SearchableConfigurable, Co
         return getLanguageVersionOrDefault((String) apiVersionComboBox.getSelectedItem());
     }
 
-    @Override
+    public void applyTo(
-    public void apply() throws ConfigurationException {
+            CommonCompilerArguments commonCompilerArguments,
+            K2JVMCompilerArguments k2jvmCompilerArguments,
+            K2JSCompilerArguments k2jsCompilerArguments,
+            CompilerSettings compilerSettings
+    ) throws ConfigurationException {
         if (isProjectSettings) {
             boolean shouldInvalidateCaches =
                     commonCompilerArguments.languageVersion != getSelectedLanguageVersion() ||
@@ -450,6 +454,11 @@ public class KotlinCompilerConfigurableTab implements SearchableConfigurable, Co
         BuildManager.getInstance().clearState(project);
     }
 
+    @Override
+    public void apply() throws ConfigurationException {
+        applyTo(commonCompilerArguments, k2jvmCompilerArguments, k2jsCompilerArguments, compilerSettings);
+    }
+
     @Override
     public void reset() {
         generateNoWarningsCheckBox.setSelected(commonCompilerArguments.suppressWarnings);

idea/src/org/jetbrains/kotlin/idea/facet/KotlinFacetEditorGeneralTab.kt

@@ -25,21 +25,20 @@ import com.intellij.ui.DocumentAdapter
 import com.intellij.ui.HoverHyperlinkLabel
 import com.intellij.util.ui.FormBuilder
 import com.intellij.util.ui.ThreeStateCheckBox
-import com.intellij.util.ui.UIUtil
 import org.jetbrains.kotlin.cli.common.arguments.*
+import org.jetbrains.kotlin.cli.common.parser.com.sampullara.cli.Argument
 import org.jetbrains.kotlin.config.*
 import org.jetbrains.kotlin.idea.compiler.configuration.*
+import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull
 import java.awt.BorderLayout
-import javax.swing.JComboBox
+import javax.swing.*
-import javax.swing.JComponent
-import javax.swing.JPanel
 import javax.swing.border.EmptyBorder
 import javax.swing.event.DocumentEvent
 
 class KotlinFacetEditorGeneralTab(
         private val configuration: KotlinFacetConfiguration,
         private val editorContext: FacetEditorContext,
-        validatorsManager: FacetValidatorsManager
+        private val validatorsManager: FacetValidatorsManager
 ) : FacetEditorTab() {
     class EditorComponent(
             private val project: Project,
@@ -164,20 +163,56 @@ class KotlinFacetEditorGeneralTab(
         }
     }
 
-    inner class CoroutineContradictionValidator : FacetEditorValidator() {
+    inner class ArgumentConsistencyValidator : FacetEditorValidator() {
         override fun check(): ValidationResult {
-            val selectedOption = editor.compilerConfigurable.coroutineSupportComboBox.selectedItem as? LanguageFeature.State
+            val platform = editor.targetPlatformComboBox.selectedItem as TargetPlatformKind<*>? ?: return ValidationResult.OK
-                                 ?: return ValidationResult.OK
+            val primaryArguments = platform.createCompilerArguments().apply {
-            val parsedArguments = configuration.settings.compilerArguments?.javaClass?.newInstance()
+                editor.compilerConfigurable.applyTo(
-                                  ?: return ValidationResult.OK
+                        this,
-            parseArguments(
+                        this as? K2JVMCompilerArguments ?: K2JVMCompilerArguments(),
-                    editor.compilerConfigurable.additionalArgsOptionsField.text.split(Regex("\\s")).toTypedArray(),
+                        this as? K2JSCompilerArguments ?: K2JSCompilerArguments(),
-                    parsedArguments,
+                        CompilerSettings()
-                    true
+                )
-            )
+            }
-            val parsedOption = CoroutineSupport.byCompilerArgumentsOrNull(parsedArguments) ?: return ValidationResult.OK
+            val argumentClass = primaryArguments.javaClass
-            if (parsedOption != selectedOption) {
+            val additionalArguments = argumentClass.newInstance().apply {
-                return ValidationResult("Coroutine support setting specified as an additional argument differs from the one in \"Coroutines\" field", null)
+                try {
+                    parseArguments(splitArgumentString(editor.compilerConfigurable.additionalArgsOptionsField.text).toTypedArray(), this)
+                }
+                catch(e: IllegalArgumentException) {
+                    return ValidationResult(e.message)
+                }
+            }
+            val emptyArguments = argumentClass.newInstance()
+            val fieldNamesToCheck = when (platform) {
+                is TargetPlatformKind.Jvm -> jvmUIExposedFields
+                is TargetPlatformKind.JavaScript -> jsUIExposedFields
+                else -> commonUIExposedFields
+            }
+            val fieldsToCheck = collectFieldsToCopy(argumentClass, false).filter { it.name in fieldNamesToCheck }
+            val overridingArguments = ArrayList<String>()
+            val redundantArguments = ArrayList<String>()
+            for (field in fieldsToCheck) {
+                val additionalValue = field[additionalArguments]
+                if (additionalValue != field[emptyArguments]) {
+                    val argumentInfo = field.annotations.firstIsInstanceOrNull<Argument>() ?: continue
+                    val addTo = if (additionalValue != field[primaryArguments]) overridingArguments else redundantArguments
+                    addTo += "<strong>" + argumentInfo.value + "</strong>"
+                }
+            }
+            if (overridingArguments.isNotEmpty() || redundantArguments.isNotEmpty()) {
+                val message = buildString {
+                    if (overridingArguments.isNotEmpty()) {
+                        append("Following arguments override facet settings: ${overridingArguments.joinToString()}")
+                    }
+                    if (redundantArguments.isNotEmpty()) {
+                        if (isNotEmpty()) {
+                            append("<br/>")
+                        }
+                        append("Following arguments are redundant: ${redundantArguments.joinToString()}")
+                    }
+                }
+                return ValidationResult(message)
             }
 
             return ValidationResult.OK
@@ -188,7 +223,7 @@ class KotlinFacetEditorGeneralTab(
 
     private val libraryValidator: FrameworkLibraryValidator
     private val versionValidator = VersionValidator()
-    private val coroutineValidator = CoroutineContradictionValidator()
+    private val coroutineValidator = ArgumentConsistencyValidator()
 
     init {
         libraryValidator = FrameworkLibraryValidatorWithDynamicDescription(
@@ -201,38 +236,48 @@ class KotlinFacetEditorGeneralTab(
         validatorsManager.registerValidator(versionValidator)
         validatorsManager.registerValidator(coroutineValidator)
 
-        editor.compilerConfigurable.languageVersionComboBox.addActionListener {
+        with(editor.compilerConfigurable) {
-            validatorsManager.validate()
+            generateNoWarningsCheckBox.validateOnChange()
+            additionalArgsOptionsField.textField.validateOnChange()
+            generateSourceMapsCheckBox.validateOnChange()
+            outputPrefixFile.textField.validateOnChange()
+            outputPostfixFile.textField.validateOnChange()
+            outputDirectory.textField.validateOnChange()
+            copyRuntimeFilesCheckBox.validateOnChange()
+            moduleKindComboBox.validateOnChange()
+            languageVersionComboBox.validateOnChange()
+            apiVersionComboBox.validateOnChange()
+            coroutineSupportComboBox.validateOnChange()
         }
-
+        editor.targetPlatformComboBox.validateOnChange()
-        editor.compilerConfigurable.apiVersionComboBox.addActionListener {
-            validatorsManager.validate()
-        }
-
-        editor.targetPlatformComboBox.addActionListener {
-            validatorsManager.validate()
-        }
-
-        editor.compilerConfigurable.coroutineSupportComboBox.addActionListener {
-            validatorsManager.validate()
-        }
-
-        editor.compilerConfigurable.additionalArgsOptionsField.textField.document.addDocumentListener(
-                object : DocumentAdapter() {
-                    override fun textChanged(e: DocumentEvent) {
-                        val text = e.document.getText(0, e.document.length)
-                        if (text.contains("-Xcoroutine")) {
-                            validatorsManager.validate()
-                        }
-                    }
-                }
-        )
 
         editor.updateCompilerConfigurable()
 
         reset()
     }
 
+    private fun JTextField.validateOnChange() {
+        document.addDocumentListener(
+                object : DocumentAdapter() {
+                    override fun textChanged(e: DocumentEvent) {
+                        doValidate()
+                    }
+                }
+        )
+    }
+
+    private fun AbstractButton.validateOnChange() {
+        addChangeListener { doValidate() }
+    }
+
+    private fun JComboBox<*>.validateOnChange() {
+        addActionListener { doValidate() }
+    }
+
+    private fun doValidate() {
+        validatorsManager.validate()
+    }
+
     override fun isModified(): Boolean {
         if (editor.useProjectSettingsCheckBox.isSelected != configuration.settings.useProjectSettings) return true
         if (editor.targetPlatformComboBox.selectedItem != configuration.settings.targetPlatformKind) return true

idea/src/org/jetbrains/kotlin/idea/facet/facetUtils.kt

@@ -133,21 +133,26 @@ fun KotlinFacet.configureFacet(
 
 // "Primary" fields are written to argument beans directly and thus not presented in the "additional arguments" string
 // Update these lists when facet/project settings UI changes
-private val commonPrimaryFields = listOf("languageVersion",
+val commonUIExposedFields = listOf("languageVersion",
-                                         "apiVersion",
+                                   "apiVersion",
-                                         "suppressWarnings",
+                                   "suppressWarnings",
-                                         "coroutinesEnable",
+                                   "coroutinesEnable",
-                                         "coroutinesWarn",
+                                   "coroutinesWarn",
-                                         "coroutinesError",
+                                   "coroutinesError")
-                                         "pluginClasspaths",
+private val commonUIHiddenFields = listOf("pluginClasspaths",
-                                         "pluginOptions")
+                                          "pluginOptions")
-private val jvmPrimaryFields = commonPrimaryFields +
+private val commonPrimaryFields = commonUIExposedFields + commonUIHiddenFields
-                               listOf("jvmTarget")
+
-private val jsPrimaryFields = commonPrimaryFields +
+private val jvmSpecificUIExposedFields = listOf("jvmTarget")
-                              listOf("sourceMap",
+val jvmUIExposedFields = commonUIExposedFields + jvmSpecificUIExposedFields
-                                     "outputPrefix",
+private val jvmPrimaryFields = commonPrimaryFields + jvmSpecificUIExposedFields
-                                     "outputPostfix",
+
-                                     "moduleKind")
+private val jsSpecificUIExposedFields = listOf("sourceMap",
+                                               "outputPrefix",
+                                               "outputPostfix",
+                                               "moduleKind")
+val jsUIExposedFields = commonUIExposedFields + jsSpecificUIExposedFields
+private val jsPrimaryFields = commonPrimaryFields + jsSpecificUIExposedFields
 
 private val CommonCompilerArguments.primaryFields: List<String>
     get() = when (this) {