KT-10974 - code review changes

idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinCodeStyleSettings.java

@@ -79,13 +79,15 @@ public class KotlinCodeStyleSettings extends CustomCodeStyleSettings {
         if (!ApplicationManager.getApplication().isUnitTestMode()) {
             PACKAGES_TO_USE_STAR_IMPORTS.addEntry(new KotlinPackageEntry("java.util", false));
             PACKAGES_TO_USE_STAR_IMPORTS.addEntry(new KotlinPackageEntry("kotlinx.android.synthetic", true));
-
-            PACKAGES_IMPORT_LAYOUT.addEntry(KotlinPackageEntry.ALL_OTHER_IMPORTS_ENTRY);
-            PACKAGES_IMPORT_LAYOUT.addEntry(new KotlinPackageEntry("java", true));
-            PACKAGES_IMPORT_LAYOUT.addEntry(new KotlinPackageEntry("javax", true));
-            PACKAGES_IMPORT_LAYOUT.addEntry(new KotlinPackageEntry("kotlin", true));
-            PACKAGES_IMPORT_LAYOUT.addEntry(KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY);
         }
+
+        // Many of test data actually depend on this order of imports,
+        // that is why we put it here even for test mode
+        PACKAGES_IMPORT_LAYOUT.addEntry(KotlinPackageEntry.ALL_OTHER_IMPORTS_ENTRY);
+        PACKAGES_IMPORT_LAYOUT.addEntry(new KotlinPackageEntry("java", true));
+        PACKAGES_IMPORT_LAYOUT.addEntry(new KotlinPackageEntry("javax", true));
+        PACKAGES_IMPORT_LAYOUT.addEntry(new KotlinPackageEntry("kotlin", true));
+        PACKAGES_IMPORT_LAYOUT.addEntry(KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY);
     }
 
     public static KotlinCodeStyleSettings getInstance(Project project) {

idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinPackageEntry.kt

@@ -24,8 +24,6 @@ class KotlinPackageEntry(
     }
 
     fun matchesPackageName(otherPackageName: String): Boolean {
-        if (isSpecial) return true
-
         if (otherPackageName.startsWith(packageName)) {
             if (otherPackageName.length == packageName.length) return true
             if (withSubpackages) {
@@ -35,20 +33,6 @@ class KotlinPackageEntry(
         return false
     }
 
-    fun isBetterMatchForPackageThan(entry: KotlinPackageEntry?, path: ImportPath): Boolean {
-        if (!matchesPackageName(path.pathStr)) return false
-        if (entry == null) return true
-
-        if (this == ALL_OTHER_ALIAS_IMPORTS_ENTRY && path.hasAlias()) return true
-        if (entry == ALL_OTHER_ALIAS_IMPORTS_ENTRY && path.hasAlias()) return false
-
-        if (entry == ALL_OTHER_IMPORTS_ENTRY && this != ALL_OTHER_ALIAS_IMPORTS_ENTRY) return true
-        if (this == ALL_OTHER_IMPORTS_ENTRY && entry != ALL_OTHER_ALIAS_IMPORTS_ENTRY) return false
-        if (entry.withSubpackages != withSubpackages) return !withSubpackages
-
-        return entry.packageName.count { it == '.' } < packageName.count { it == '.' }
-    }
-
     val isSpecial: Boolean get() = this == ALL_OTHER_IMPORTS_ENTRY || this == ALL_OTHER_ALIAS_IMPORTS_ENTRY
 
     override fun toString(): String {

idea/performanceTests/test/org/jetbrains/kotlin/idea/perf/AbstractPerformanceImportTest.kt

@@ -8,7 +8,6 @@ package org.jetbrains.kotlin.idea.perf
 import com.intellij.application.options.CodeStyle
 import com.intellij.openapi.application.runWriteAction
 import com.intellij.psi.PsiDocumentManager
-import com.intellij.psi.codeStyle.PackageEntry
 import com.intellij.testFramework.LightProjectDescriptor
 import junit.framework.TestCase
 import org.jetbrains.kotlin.descriptors.ClassDescriptor

idea/resources-en/messages/KotlinBundle.properties

@@ -2213,3 +2213,4 @@ logger.initialized.with.foreign.class=Logger initialized with foreign class ''{0
 logger.factory.method.name=Logger factory method name
 logger.factory.class.name=Logger factory class name
 choose.logger.factory.class=Choose logger factory class
+codestyle.layout.import.aliases.separately=Import aliases separately

idea/src/org/jetbrains/kotlin/idea/formatter/BaseKotlinImportLayoutPanel.kt

@@ -15,6 +15,7 @@ import com.intellij.ui.components.JBCheckBox
 import com.intellij.ui.table.JBTable
 import com.intellij.util.IconUtil
 import com.intellij.util.ui.JBUI
+import org.jetbrains.kotlin.idea.KotlinBundle
 import org.jetbrains.kotlin.idea.core.formatter.KotlinPackageEntry
 import org.jetbrains.kotlin.idea.core.formatter.KotlinPackageEntryTable
 import org.jetbrains.kotlin.idea.highlighter.KotlinHighlightingColors
@@ -171,8 +172,8 @@ class KotlinStarImportLayoutPanel : BaseKotlinImportLayoutPanel(ApplicationBundl
             .setAddAction { addPackage() }
             .setRemoveAction { removePackage() }
             .setButtonComparator(
-                ApplicationBundle.message("button.add.package"),
+                "Add",
-                ApplicationBundle.message("button.remove"),
+                "Remove"
             ).setPreferredSize(Dimension(-1, 100))
             .createPanel()
 
@@ -182,7 +183,7 @@ class KotlinStarImportLayoutPanel : BaseKotlinImportLayoutPanel(ApplicationBundl
 }
 
 class KotlinImportOrderLayoutPanel : BaseKotlinImportLayoutPanel(ApplicationBundle.message("title.import.layout")) {
-    private val cbImportAliasesSeparately = JBCheckBox("Import aliases separately")
+    private val cbImportAliasesSeparately = JBCheckBox(KotlinBundle.message("codestyle.layout.import.aliases.separately"))
 
     init {
         add(cbImportAliasesSeparately, BorderLayout.NORTH)
@@ -208,17 +209,17 @@ class KotlinImportOrderLayoutPanel : BaseKotlinImportLayoutPanel(ApplicationBund
 
                 entry?.isSpecial == false
             }.setButtonComparator(
-                ApplicationBundle.message("button.add.package"),
+                "Add Package",
-                ApplicationBundle.message("button.remove"),
+                "Remove",
-                ApplicationBundle.message("button.move.up"),
+                "Up",
-                ApplicationBundle.message("button.move.down")
+                "Down"
             ).setPreferredSize(Dimension(-1, 100))
             .createPanel()
 
         add(importLayoutPanel, BorderLayout.CENTER)
         resizeColumns()
 
-        cbImportAliasesSeparately.addItemListener { _ ->
+        cbImportAliasesSeparately.addItemListener {
             if (areImportAliasesEnabled()) {
                 if (KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY !in packageTable.getEntries()) {
                     packageTable.addEntry(KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY)

idea/src/org/jetbrains/kotlin/idea/util/ImportPathComparator.kt

@@ -6,31 +6,63 @@
 package org.jetbrains.kotlin.idea.util
 
 import org.jetbrains.kotlin.idea.core.formatter.KotlinPackageEntry
+import org.jetbrains.kotlin.idea.core.formatter.KotlinPackageEntry.Companion.ALL_OTHER_ALIAS_IMPORTS_ENTRY
+import org.jetbrains.kotlin.idea.core.formatter.KotlinPackageEntry.Companion.ALL_OTHER_IMPORTS_ENTRY
 import org.jetbrains.kotlin.idea.core.formatter.KotlinPackageEntryTable
 import org.jetbrains.kotlin.resolve.ImportPath
 import java.util.Comparator
 
-class ImportPathComparator(
+internal class ImportPathComparator(private val packageTable: KotlinPackageEntryTable) : Comparator<ImportPath> {
-    private val packageTable: KotlinPackageEntryTable
-) : Comparator<ImportPath> {
-    private val comparator = compareBy<ImportPath>(
-        { import -> bestMatchIndex(import) },
-        { import -> import.toString() }
-    )
 
-    override fun compare(import1: ImportPath, import2: ImportPath): Int = comparator.compare(import1, import2)
+    override fun compare(import1: ImportPath, import2: ImportPath): Int {
+        val ignoreAlias = import1.hasAlias() && import2.hasAlias()
 
-    private fun bestMatchIndex(path: ImportPath): Int {
+        return compareValuesBy(
-        var bestIndex: Int? = null
+            import1,
+            import2,
+            { import -> bestEntryMatchIndex(import, ignoreAlias) },
+            { import -> import.toString() }
+        )
+    }
+
+    private fun bestEntryMatchIndex(path: ImportPath, ignoreAlias: Boolean): Int {
         var bestEntryMatch: KotlinPackageEntry? = null
+        var bestIndex: Int = -1
 
         for ((index, entry) in packageTable.getEntries().withIndex()) {
-            if (entry.isBetterMatchForPackageThan(bestEntryMatch, path)) {
+            if (entry.isBetterMatchForPackageThan(bestEntryMatch, path, ignoreAlias)) {
                 bestEntryMatch = entry
                 bestIndex = index
             }
         }
 
-        return bestIndex!!
+        return bestIndex
     }
 }
+
+private fun KotlinPackageEntry.isBetterMatchForPackageThan(entry: KotlinPackageEntry?, path: ImportPath, ignoreAlias: Boolean): Boolean {
+    if (!matchesImportPath(path, ignoreAlias)) return false
+    if (entry == null) return true
+
+    // Any matched package is better than ALL_OTHER_IMPORTS_ENTRY
+    if (this == ALL_OTHER_IMPORTS_ENTRY) return false
+    if (entry == ALL_OTHER_IMPORTS_ENTRY) return true
+
+    if (entry.withSubpackages != withSubpackages) return !withSubpackages
+
+    return entry.packageName.count { it == '.' } < packageName.count { it == '.' }
+}
+
+/**
+ * In current implementation we assume that aliased import can be matched only by
+ * [ALL_OTHER_ALIAS_IMPORTS_ENTRY] which is always present.
+ */
+private fun KotlinPackageEntry.matchesImportPath(importPath: ImportPath, ignoreAlias: Boolean): Boolean {
+    if (!ignoreAlias && importPath.hasAlias()) {
+        return this == ALL_OTHER_ALIAS_IMPORTS_ENTRY
+    }
+
+    if (this == ALL_OTHER_IMPORTS_ENTRY) return true
+
+    return matchesPackageName(importPath.pathStr)
+}