CLI: fix non-XML rendering of argument parse error, simplify code

If the compiler is invoked through JPS, an instance of
XmlMessageRenderer is used and the output should be a valid XML.
Previously, we reported the argument parse error (and the note to try
"-help") before calling messageRenderer.renderPreamble, which resulted
in invalid XML. The same was happening for the usage printed on "-help".
Both of these problems are fixed

 #KT-14848 Fixed

compiler/cli/src/org/jetbrains/kotlin/cli/common/CLITool.kt

@@ -42,21 +42,8 @@ abstract class CLITool<A : CommonToolArguments> {
     ): ExitCode {
         K2JVMCompiler.resetInitStartTime()
 
-        val parseArgumentsCollector = PrintingMessageCollector(errStream, messageRenderer, false)
+        val arguments = createArguments()
-        val arguments = try {
+        parseCommandLineArguments(args, arguments)
-            parseArguments(parseArgumentsCollector, args) ?: return ExitCode.INTERNAL_ERROR
-        }
-        catch (e: IllegalArgumentException) {
-            parseArgumentsCollector.report(CompilerMessageSeverity.ERROR, e.message!!, null)
-            parseArgumentsCollector.report(CompilerMessageSeverity.INFO, "Use -help for more information", null)
-            return ExitCode.COMPILATION_ERROR
-        }
-
-        if (arguments.help || arguments.extraHelp) {
-            Usage.print(errStream, this, arguments)
-            return ExitCode.OK
-        }
-
         val collector = PrintingMessageCollector(errStream, messageRenderer, arguments.verbose)
 
         try {
@@ -65,6 +52,19 @@ abstract class CLITool<A : CommonToolArguments> {
             }
 
             errStream.print(messageRenderer.renderPreamble())
+
+            val errorMessage = validateArguments(arguments.errors)
+            if (errorMessage != null) {
+                collector.report(CompilerMessageSeverity.ERROR, errorMessage, null)
+                collector.report(CompilerMessageSeverity.INFO, "Use -help for more information", null)
+                return ExitCode.COMPILATION_ERROR
+            }
+
+            if (arguments.help || arguments.extraHelp) {
+                errStream.print(messageRenderer.renderUsage(Usage.render(this, arguments)))
+                return ExitCode.OK
+            }
+
             return exec(collector, services, arguments)
         }
         finally {
@@ -93,19 +93,6 @@ abstract class CLITool<A : CommonToolArguments> {
     // Used in kotlin-maven-plugin (KotlinCompileMojoBase)
     protected abstract fun execImpl(messageCollector: MessageCollector, services: Services, arguments: A): ExitCode
 
-    private fun parseArguments(messageCollector: MessageCollector, args: Array<out String>): A? {
-        return try {
-            createArguments().also { parseArguments(args, it) }
-        }
-        catch (e: IllegalArgumentException) {
-            throw e
-        }
-        catch (t: Throwable) {
-            messageCollector.report(CompilerMessageSeverity.EXCEPTION, OutputMessageUtil.renderException(t), null)
-            null
-        }
-    }
-
     abstract fun createArguments(): A
 
     // Used in kotlin-maven-plugin (KotlinCompileMojoBase) and in kotlin-gradle-plugin (KotlinJvmOptionsImpl, KotlinJsOptionsImpl)

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

@@ -18,46 +18,43 @@ package org.jetbrains.kotlin.cli.common;
 
 import kotlin.text.StringsKt;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 import org.jetbrains.kotlin.cli.common.arguments.Argument;
 import org.jetbrains.kotlin.cli.common.arguments.CommonToolArguments;
 import org.jetbrains.kotlin.cli.common.arguments.ParseCommandLineArgumentsKt;
 
-import java.io.PrintStream;
 import java.lang.reflect.Field;
 
-class Usage {
+public class Usage {
     // The magic number 29 corresponds to the similar padding width in javac and scalac command line compilers
     private static final int OPTION_NAME_PADDING_WIDTH = 29;
 
-    public static <A extends CommonToolArguments> void print(
+    @NotNull
-            @NotNull PrintStream target, @NotNull CLITool<A> compiler, @NotNull A arguments
+    public static <A extends CommonToolArguments> String render(@NotNull CLITool<A> tool, @NotNull A arguments) {
-    ) {
+        StringBuilder sb = new StringBuilder();
-        target.println("Usage: " + compiler.executableScriptFileName() + " <options> <source files>");
+        appendln(sb, "Usage: " + tool.executableScriptFileName() + " <options> <source files>");
-        target.println("where " + (arguments.extraHelp ? "advanced" : "possible") + " options include:");
+        appendln(sb, "where " + (arguments.extraHelp ? "advanced" : "possible") + " options include:");
         for (Class<?> clazz = arguments.getClass(); clazz != null; clazz = clazz.getSuperclass()) {
             for (Field field : clazz.getDeclaredFields()) {
-                String usage = fieldUsage(field, arguments.extraHelp);
+                fieldUsage(sb, field, arguments.extraHelp);
-                if (usage != null) {
-                    target.println(usage);
-                }
             }
         }
 
         if (arguments.extraHelp) {
-            target.println();
+            appendln(sb, "");
-            target.println("Advanced options are non-standard and may be changed or removed without any notice.");
+            appendln(sb, "Advanced options are non-standard and may be changed or removed without any notice.");
         }
+
+        return sb.toString();
     }
 
-    @Nullable
+    private static void fieldUsage(@NotNull StringBuilder sb, @NotNull Field field, boolean extraHelp) {
-    private static String fieldUsage(@NotNull Field field, boolean extraHelp) {
         Argument argument = field.getAnnotation(Argument.class);
-        if (argument == null) return null;
+        if (argument == null) return;
 
-        if (extraHelp != ParseCommandLineArgumentsKt.isAdvanced(argument)) return null;
+        if (extraHelp != ParseCommandLineArgumentsKt.isAdvanced(argument)) return;
 
-        StringBuilder sb = new StringBuilder("  ");
+        int startLength = sb.length();
+        sb.append("  ");
         sb.append(argument.value());
 
         if (!argument.shortName().isEmpty()) {
@@ -71,17 +68,21 @@ class Usage {
             sb.append(argument.valueDescription());
         }
 
-        int width = OPTION_NAME_PADDING_WIDTH - 1;
+        int margin = startLength + OPTION_NAME_PADDING_WIDTH - 1;
-        if (sb.length() >= width + 5) { // Break the line if it's too long
+        if (sb.length() >= margin + 5) { // Break the line if it's too long
             sb.append("\n");
-            width += sb.length();
+            margin += sb.length() - startLength;
         }
-        while (sb.length() < width) {
+        while (sb.length() < margin) {
             sb.append(" ");
         }
 
         sb.append(" ");
-        sb.append(argument.description().replace("\n", "\n" + StringsKt.repeat(" ", OPTION_NAME_PADDING_WIDTH)));
+        appendln(sb, argument.description().replace("\n", "\n" + StringsKt.repeat(" ", OPTION_NAME_PADDING_WIDTH)));
-        return sb.toString();
+    }
+
+    private static void appendln(@NotNull StringBuilder sb, @NotNull String string) {
+        sb.append(string);
+        StringsKt.appendln(sb);
     }
 }

compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/MessageRenderer.java

@@ -56,5 +56,7 @@ public interface MessageRenderer {
 
     String render(@NotNull CompilerMessageSeverity severity, @NotNull String message, @Nullable CompilerMessageLocation location);
 
+    String renderUsage(@NotNull String usage);
+
     String renderConclusion();
 }

compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/PlainTextMessageRenderer.java

@@ -157,6 +157,11 @@ public abstract class PlainTextMessageRenderer implements MessageRenderer {
     @Nullable
     protected abstract String getPath(@NotNull CompilerMessageLocation location);
 
+    @Override
+    public String renderUsage(@NotNull String usage) {
+        return usage;
+    }
+
     @Override
     public String renderConclusion() {
         return "";

compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/XmlMessageRenderer.java

@@ -48,6 +48,11 @@ public class XmlMessageRenderer implements MessageRenderer {
         return StringUtil.escapeXml(str);
     }
 
+    @Override
+    public String renderUsage(@NotNull String usage) {
+        return render(CompilerMessageSeverity.STRONG_WARNING, usage, null);
+    }
+
     @Override
     public String renderConclusion() {
         return "</MESSAGES>";

jps-plugin/jps-tests/jps-tests.iml

@@ -19,5 +19,6 @@
     <orderEntry type="library" scope="TEST" name="idea-full" level="project" />
     <orderEntry type="module" module-name="incremental-compilation-impl" scope="TEST" />
     <orderEntry type="library" name="kotlin-reflect" level="project" />
+    <orderEntry type="module" module-name="cli" scope="TEST" />
   </component>
 </module>

jps-plugin/jps-tests/test/org/jetbrains/kotlin/jps/build/KotlinJpsBuildTest.kt

@@ -51,6 +51,9 @@ import org.jetbrains.jps.model.java.JpsJavaSdkType
 import org.jetbrains.jps.model.library.JpsOrderRootType
 import org.jetbrains.jps.model.module.JpsModule
 import org.jetbrains.jps.util.JpsPathUtil
+import org.jetbrains.kotlin.cli.common.Usage
+import org.jetbrains.kotlin.cli.common.arguments.K2JVMCompilerArguments
+import org.jetbrains.kotlin.cli.jvm.K2JVMCompiler
 import org.jetbrains.kotlin.codegen.AsmUtil
 import org.jetbrains.kotlin.codegen.JvmCodegenUtil
 import org.jetbrains.kotlin.config.KotlinCompilerVersion.TEST_IS_PRE_RELEASE_SYSTEM_PROPERTY
@@ -787,6 +790,26 @@ class KotlinJpsBuildTest : AbstractKotlinJpsBuildTestCase() {
         )
     }
 
+    fun testHelp() {
+        initProject()
+
+        val result = buildAllModules()
+        result.assertSuccessful()
+        val warning = result.getMessages(BuildMessage.Kind.WARNING).single()
+
+        Assert.assertEquals(Usage.render(K2JVMCompiler(), K2JVMCompilerArguments()), warning.messageText)
+    }
+
+    fun testWrongArgument() {
+        initProject()
+
+        val result = buildAllModules()
+        result.assertFailed()
+        val errors = result.getMessages(BuildMessage.Kind.ERROR).joinToString("\n\n") { it.messageText }
+
+        Assert.assertEquals("Invalid argument: -abcdefghij-invalid-argument", errors)
+    }
+
     fun testCodeInKotlinPackage() {
         initProject(JVM_MOCK_RUNTIME)
 

jps-plugin/testData/general/Help/kotlinProject.iml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="jdk" jdkName="IDEA_JDK" jdkType="JavaSDK" />
+    <orderEntry type="sourceFolder" forTests="false" />
+    <orderEntry type="module" module-name="kotlinProject" />
+  </component>
+</module>

jps-plugin/testData/general/Help/kotlinProject.ipr

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project version="4">
+  <component name="CompilerConfiguration">
+    <option name="DEFAULT_COMPILER" value="Javac" />
+  </component>
+  <component name="KotlinCompilerSettings">
+    <option name="additionalArguments" value="-help" />
+  </component>
+  <component name="ProjectModuleManager">
+    <modules>
+      <module fileurl="file://$PROJECT_DIR$/kotlinProject.iml" filepath="$PROJECT_DIR$/kotlinProject.iml" />
+    </modules>
+  </component>
+  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_6" assert-keyword="true" jdk-15="true" project-jdk-name="IDEA_JDK" project-jdk-type="JavaSDK">
+    <output url="file://$PROJECT_DIR$/out" />
+  </component>
+</project>

jps-plugin/testData/general/WrongArgument/kotlinProject.iml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<module type="JAVA_MODULE" version="4">
+  <component name="NewModuleRootManager" inherit-compiler-output="true">
+    <exclude-output />
+    <content url="file://$MODULE_DIR$">
+      <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+    </content>
+    <orderEntry type="jdk" jdkName="IDEA_JDK" jdkType="JavaSDK" />
+    <orderEntry type="sourceFolder" forTests="false" />
+    <orderEntry type="module" module-name="kotlinProject" />
+  </component>
+</module>

jps-plugin/testData/general/WrongArgument/kotlinProject.ipr

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project version="4">
+  <component name="CompilerConfiguration">
+    <option name="DEFAULT_COMPILER" value="Javac" />
+  </component>
+  <component name="KotlinCompilerSettings">
+    <option name="additionalArguments" value="-abcdefghij-invalid-argument" />
+  </component>
+  <component name="ProjectModuleManager">
+    <modules>
+      <module fileurl="file://$PROJECT_DIR$/kotlinProject.iml" filepath="$PROJECT_DIR$/kotlinProject.iml" />
+    </modules>
+  </component>
+  <component name="ProjectRootManager" version="2" languageLevel="JDK_1_6" assert-keyword="true" jdk-15="true" project-jdk-name="IDEA_JDK" project-jdk-type="JavaSDK">
+    <output url="file://$PROJECT_DIR$/out" />
+  </component>
+</project>