Improve sorting of compiler messages in GroupingMessageCollector

Ensure that messages reported on a file but with no line information are
reported before diagnostics related to code. This fixes
Java9ModulesIntegrationTest.testSeveralModulesWithTheSameName on some
machines

compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/messages/GroupingMessageCollector.java

@@ -18,16 +18,19 @@ package org.jetbrains.kotlin.cli.common.messages;
 
 import com.google.common.collect.LinkedHashMultimap;
 import com.google.common.collect.Multimap;
+import kotlin.collections.CollectionsKt;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.*;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Objects;
 
 public class GroupingMessageCollector implements MessageCollector {
     private final MessageCollector delegate;
 
-    // File path (nullable) -> message
+    // Note that the key in this map can be null
-    private final Multimap<String, Message> groupedMessages = LinkedHashMultimap.create();
+    private final Multimap<CompilerMessageLocation, Message> groupedMessages = LinkedHashMultimap.create();
 
     public GroupingMessageCollector(@NotNull MessageCollector delegate) {
         this.delegate = delegate;
@@ -48,7 +51,7 @@ public class GroupingMessageCollector implements MessageCollector {
             delegate.report(severity, message, location);
         }
         else {
-            groupedMessages.put(location != null ? location.getPath() : null, new Message(severity, message, location));
+            groupedMessages.put(location, new Message(severity, message, location));
         }
     }
 
@@ -60,8 +63,10 @@ public class GroupingMessageCollector implements MessageCollector {
     public void flush() {
         boolean hasErrors = hasErrors();
 
-        for (String path : sortedKeys()) {
+        List<CompilerMessageLocation> sortedKeys =
-            for (Message message : groupedMessages.get(path)) {
+                CollectionsKt.sortedWith(groupedMessages.keySet(), Comparator.nullsFirst(CompilerMessageLocationComparator.INSTANCE));
+        for (CompilerMessageLocation location : sortedKeys) {
+            for (Message message : groupedMessages.get(location)) {
                 if (!hasErrors || message.severity.isError() || message.severity == CompilerMessageSeverity.STRONG_WARNING) {
                     delegate.report(message.severity, message.message, message.location);
                 }
@@ -71,12 +76,28 @@ public class GroupingMessageCollector implements MessageCollector {
         groupedMessages.clear();
     }
 
-    @NotNull
+    private static class CompilerMessageLocationComparator implements Comparator<CompilerMessageLocation> {
-    private Collection<String> sortedKeys() {
+        public static final CompilerMessageLocationComparator INSTANCE = new CompilerMessageLocationComparator();
-        // ensure that messages with no location i.e. perf, incomplete hierarchy are always reported first
+
-        List<String> sortedKeys = new ArrayList<>(groupedMessages.keySet());
+        // First, output all messages without any location information. Then, only those with the file path.
-        sortedKeys.sort(Comparator.nullsFirst(Comparator.naturalOrder()));
+        // Next, all messages with the file path and the line number. Next, all messages with file path, line number and column number.
-        return sortedKeys;
+        //
+        // Example of the order of compiler messages:
+        //
+        // error: bad classpath
+        // foo.kt: error: bad file
+        // foo.kt:42: error: bad line
+        // foo.kt:42:43: error: bad character
+        @Override
+        public int compare(CompilerMessageLocation o1, CompilerMessageLocation o2) {
+            if (o1.getColumn() == -1 && o2.getColumn() != -1) return -1;
+            if (o1.getColumn() != -1 && o2.getColumn() == -1) return 1;
+
+            if (o1.getLine() == -1 && o2.getLine() != -1) return -1;
+            if (o1.getLine() != -1 && o2.getLine() == -1) return 1;
+
+            return o1.getPath().compareTo(o2.getPath());
+        }
     }
 
     private static class Message {

compiler/testData/javaModules/severalModulesWithTheSameName/main.txt

@@ -1,3 +1,4 @@
+$TMP_DIR$/dependency2.jar: warning: the root is ignored because a module with the same name 'dependency' has been found earlier on the module path at: $TMP_DIR$/dependency1.jar
 compiler/testData/javaModules/severalModulesWithTheSameName/main/usage.kt:2:8: error: unresolved reference: bar
 import bar.Bar
        ^
@@ -7,5 +8,4 @@ compiler/testData/javaModules/severalModulesWithTheSameName/main/usage.kt:6:12:
 compiler/testData/javaModules/severalModulesWithTheSameName/main/usage.kt:6:18: error: unresolved reference: Bar
     val b: Bar = Bar()
                  ^
-$TMP_DIR$/dependency2.jar: warning: the root is ignored because a module with the same name 'dependency' has been found earlier on the module path at: $TMP_DIR$/dependency1.jar
 COMPILATION_ERROR