CLI: refactor argument validation out of parsing

compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/CommonCompilerArguments.java

@@ -109,16 +109,7 @@ public abstract class CommonCompilerArguments implements Serializable {
 
     public List<String> freeArgs = new SmartList<>();
 
-    public List<String> unknownArgs = new SmartList<>();
+    public ArgumentParseErrors errors = new ArgumentParseErrors();
-
-    public List<String> unknownExtraFlags = new SmartList<>();
-
-    // Names of extra (-X...) arguments which have been passed in an obsolete form ("-Xaaa bbb", instead of "-Xaaa=bbb")
-    public List<String> extraArgumentsPassedInObsoleteForm = new SmartList<>();
-
-    // Non-boolean arguments which have been passed multiple times, possibly with different values.
-    // The key in the map is the name of the argument, the value is the last passed value.
-    public Map<String, String> duplicateArguments = new LinkedHashMap<>();
 
     @NotNull
     public static CommonCompilerArguments createDefaultInstance() {

compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/argumentUtils.kt

@@ -20,16 +20,6 @@ import java.lang.reflect.Field
 import java.lang.reflect.Modifier
 import java.util.*
 
-@JvmOverloads
-fun <A : CommonCompilerArguments> parseArguments(args: Array<String>, arguments: A, ignoreInvalidArguments: Boolean = false) {
-    parseCommandLineArguments(args, arguments)
-
-    val invalidArgument = arguments.unknownArgs.firstOrNull()?.takeUnless { ignoreInvalidArguments }
-    if (invalidArgument != null) {
-        throw IllegalArgumentException("Invalid argument: $invalidArgument")
-    }
-}
-
 fun <T : Any> copyBean(bean: T) = copyFields(bean, bean::class.java.newInstance(), true, collectFieldsToCopy(bean::class.java, false))
 
 fun <From : Any, To : From> mergeBeans(from: From, to: To): To {

compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/arguments/parseCommandLineArguments.kt

@@ -16,7 +16,9 @@
 
 package org.jetbrains.kotlin.cli.common.arguments
 
+import com.intellij.util.SmartList
 import java.lang.reflect.Field
+import java.util.LinkedHashMap
 
 annotation class Argument(
         val value: String,
@@ -32,6 +34,21 @@ val Argument.isAdvanced: Boolean
 private val ADVANCED_ARGUMENT_PREFIX = "-X"
 private val FREE_ARGS_DELIMITER = "--"
 
+class ArgumentParseErrors {
+    val unknownArgs: MutableList<String> = SmartList<String>()
+
+    val unknownExtraFlags: MutableList<String> = SmartList<String>()
+
+    // Names of extra (-X...) arguments which have been passed in an obsolete form ("-Xaaa bbb", instead of "-Xaaa=bbb")
+    val extraArgumentsPassedInObsoleteForm: MutableList<String> = SmartList<String>()
+
+    // Non-boolean arguments which have been passed multiple times, possibly with different values.
+    // The key in the map is the name of the argument, the value is the last passed value.
+    val duplicateArguments: MutableMap<String, String> = LinkedHashMap<String, String>()
+
+    var argumentWithoutValue: String? = null
+}
+
 // Parses arguments in the passed [result] object, or throws an [IllegalArgumentException] with the message to be displayed to the user
 fun <A : CommonCompilerArguments> parseCommandLineArguments(args: Array<String>, result: A) {
     data class ArgumentField(val field: Field, val argument: Argument)
@@ -41,11 +58,12 @@ fun <A : CommonCompilerArguments> parseCommandLineArguments(args: Array<String>,
         if (argument != null) ArgumentField(field, argument) else null
     }
 
+    val errors = result.errors
     val visitedArgs = mutableSetOf<String>()
     var freeArgsStarted = false
 
     var i = 0
-    while (i < args.size) {
+    loop@ while (i < args.size) {
         val arg = args[i++]
 
         if (freeArgsStarted) {
@@ -65,8 +83,8 @@ fun <A : CommonCompilerArguments> parseCommandLineArguments(args: Array<String>,
 
         if (argumentField == null) {
             when {
-                arg.startsWith(ADVANCED_ARGUMENT_PREFIX) -> result.unknownExtraFlags.add(arg)
+                arg.startsWith(ADVANCED_ARGUMENT_PREFIX) -> errors.unknownExtraFlags.add(arg)
-                arg.startsWith("-") -> result.unknownArgs.add(arg)
+                arg.startsWith("-") -> errors.unknownArgs.add(arg)
                 else -> result.freeArgs.add(arg)
             }
             continue
@@ -80,17 +98,20 @@ fun <A : CommonCompilerArguments> parseCommandLineArguments(args: Array<String>,
             }
             else -> {
                 if (i == args.size) {
-                    throw IllegalArgumentException("No value passed for argument $arg")
+                    errors.argumentWithoutValue = arg
+                    break@loop
                 }
-                if (argument.isAdvanced) {
+                else {
-                    result.extraArgumentsPassedInObsoleteForm.add(arg)
+                    if (argument.isAdvanced) {
+                        errors.extraArgumentsPassedInObsoleteForm.add(arg)
+                    }
+                    args[i++]
                 }
-                args[i++]
             }
         }
 
         if (!field.type.isArray && !visitedArgs.add(argument.value) && value is String && field.get(result) != value) {
-            result.duplicateArguments.put(argument.value, value)
+            errors.duplicateArguments.put(argument.value, value)
         }
 
         updateField(field, result, value, argument.delimiter)
@@ -109,3 +130,16 @@ private fun <A : CommonCompilerArguments> updateField(field: Field, result: A, v
         else -> throw IllegalStateException("Unsupported argument type: ${field.type}")
     }
 }
+
+/**
+ * @return error message if arguments are parsed incorrectly, null otherwise
+ */
+fun validateArguments(errors: ArgumentParseErrors): String? {
+    if (errors.argumentWithoutValue != null) {
+        return "No value passed for argument ${errors.argumentWithoutValue}"
+    }
+    if (errors.unknownArgs.isNotEmpty()) {
+        return "Invalid argument: ${errors.unknownArgs.first()}"
+    }
+    return null
+}

compiler/cli/src/org/jetbrains/kotlin/cli/common/CLICompiler.java

@@ -22,8 +22,9 @@ import kotlin.collections.ArraysKt;
 import org.fusesource.jansi.AnsiConsole;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
-import org.jetbrains.kotlin.cli.common.arguments.ArgumentUtilsKt;
+import org.jetbrains.kotlin.cli.common.arguments.ArgumentParseErrors;
 import org.jetbrains.kotlin.cli.common.arguments.CommonCompilerArguments;
+import org.jetbrains.kotlin.cli.common.arguments.ParseCommandLineArgumentsKt;
 import org.jetbrains.kotlin.cli.common.messages.*;
 import org.jetbrains.kotlin.cli.jvm.K2JVMCompiler;
 import org.jetbrains.kotlin.cli.jvm.compiler.CompileEnvironmentException;
@@ -82,7 +83,11 @@ public abstract class CLICompiler<A extends CommonCompilerArguments> {
 
     // Used in kotlin-maven-plugin (KotlinCompileMojoBase) and in kotlin-gradle-plugin (KotlinJvmOptionsImpl, KotlinJsOptionsImpl)
     public void parseArguments(@NotNull String[] args, @NotNull A arguments) {
-        ArgumentUtilsKt.parseArguments(args, arguments);
+        ParseCommandLineArgumentsKt.parseCommandLineArguments(args, arguments);
+        String message = ParseCommandLineArgumentsKt.validateArguments(arguments.errors);
+        if (message != null) {
+            throw new IllegalArgumentException(message);
+        }
     }
 
     @NotNull
@@ -142,7 +147,7 @@ public abstract class CLICompiler<A extends CommonCompilerArguments> {
             messageCollector = new FilteringMessageCollector(messageCollector, Predicate.isEqual(WARNING));
         }
 
-        reportArgumentParseProblems(messageCollector, arguments);
+        reportArgumentParseProblems(messageCollector, arguments.errors);
 
         GroupingMessageCollector groupingCollector = new GroupingMessageCollector(messageCollector);
 
@@ -320,15 +325,15 @@ public abstract class CLICompiler<A extends CommonCompilerArguments> {
             @NotNull CompilerConfiguration configuration, @NotNull A arguments, @NotNull Services services
     );
 
-    private void reportArgumentParseProblems(@NotNull MessageCollector collector, @NotNull A arguments) {
+    private static void reportArgumentParseProblems(@NotNull MessageCollector collector, @NotNull ArgumentParseErrors errors) {
-        for (String flag : arguments.unknownExtraFlags) {
+        for (String flag : errors.getUnknownExtraFlags()) {
             collector.report(STRONG_WARNING, "Flag is not supported by this version of the compiler: " + flag, null);
         }
-        for (String argument : arguments.extraArgumentsPassedInObsoleteForm) {
+        for (String argument : errors.getExtraArgumentsPassedInObsoleteForm()) {
             collector.report(STRONG_WARNING, "Advanced option value is passed in an obsolete form. Please use the '=' character " +
                                              "to specify the value: " + argument + "=...", null);
         }
-        for (Map.Entry<String, String> argument : arguments.duplicateArguments.entrySet()) {
+        for (Map.Entry<String, String> argument : errors.getDuplicateArguments().entrySet()) {
             collector.report(STRONG_WARNING, "Argument " + argument.getKey() + " is passed multiple times. " +
                                              "Only the last value will be used: " + argument.getValue(), null);
         }

idea/idea-maven/src/org/jetbrains/kotlin/idea/maven/KotlinMavenImporter.kt

@@ -36,7 +36,7 @@ import org.jetbrains.jps.model.module.JpsModuleSourceRootType
 import org.jetbrains.kotlin.cli.common.arguments.K2JSCompilerArguments
 import org.jetbrains.kotlin.cli.common.arguments.K2JVMCompilerArguments
 import org.jetbrains.kotlin.cli.common.arguments.K2MetadataCompilerArguments
-import org.jetbrains.kotlin.cli.common.arguments.parseArguments
+import org.jetbrains.kotlin.cli.common.arguments.parseCommandLineArguments
 import org.jetbrains.kotlin.compilerRunner.ArgumentUtils
 import org.jetbrains.kotlin.config.JvmTarget
 import org.jetbrains.kotlin.config.LanguageFeature
@@ -139,7 +139,7 @@ class KotlinMavenImporter : MavenImporter(KOTLIN_PLUGIN_GROUP_ID, KOTLIN_PLUGIN_
         }
 
         val additionalArgs = configuration.getChild("args")?.getChildren("arg")?.mapNotNull { it.text } ?: emptyList()
-        parseArguments(additionalArgs.toTypedArray(), arguments, true)
+        parseCommandLineArguments(additionalArgs.toTypedArray(), arguments)
 
         return ArgumentUtils.convertArgumentsToStringList(arguments)
     }
@@ -269,4 +269,4 @@ class KotlinImporterComponent : PersistentStateComponent<KotlinImporterComponent
 }
 
 private val Module.kotlinImporterComponent: KotlinImporterComponent
-    get() = getComponent(KotlinImporterComponent::class.java) ?: throw IllegalStateException("No maven importer state configured")
+    get() = getComponent(KotlinImporterComponent::class.java) ?: throw IllegalStateException("No maven importer state configured")

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

@@ -177,12 +177,10 @@ class KotlinFacetEditorGeneralTab(
             }
             val argumentClass = primaryArguments.javaClass
             val additionalArguments = argumentClass.newInstance().apply {
-                try {
+                parseCommandLineArguments(
-                    parseArguments(splitArgumentString(editor.compilerConfigurable.additionalArgsOptionsField.text).toTypedArray(), this)
+                        splitArgumentString(editor.compilerConfigurable.additionalArgsOptionsField.text).toTypedArray(), this
-                }
+                )
-                catch(e: IllegalArgumentException) {
+                validateArguments(errors)?.let { message -> return ValidationResult(message) }
-                    return ValidationResult(e.message)
-                }
             }
             val emptyArguments = argumentClass.newInstance()
             val fieldNamesToCheck = when (platform) {

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

@@ -175,10 +175,10 @@ fun parseCompilerArgumentsToFacet(arguments: List<String>, defaultArguments: Lis
         val compilerArguments = this.compilerArguments ?: return
 
         val defaultCompilerArguments = compilerArguments::class.java.newInstance()
-        parseArguments(defaultArguments.toTypedArray(), defaultCompilerArguments, true)
+        parseCommandLineArguments(defaultArguments.toTypedArray(), defaultCompilerArguments)
         defaultCompilerArguments.convertPathsToSystemIndependent()
 
-        parseArguments(argumentArray, compilerArguments, true)
+        parseCommandLineArguments(argumentArray, compilerArguments)
 
         compilerArguments.convertPathsToSystemIndependent()