KT-10974 - code review changes

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

@@ -5,6 +5,7 @@
 
 package org.jetbrains.kotlin.idea.core.formatter
 
+import com.intellij.openapi.application.ApplicationBundle
 import org.jetbrains.kotlin.resolve.ImportPath
 
 class KotlinPackageEntry(
@@ -15,14 +16,15 @@ class KotlinPackageEntry(
 
     companion object {
         @JvmField
-        val ALL_OTHER_IMPORTS_ENTRY = KotlinPackageEntry("<all other imports>", withSubpackages = true)
+        val ALL_OTHER_IMPORTS_ENTRY =
+            KotlinPackageEntry(ApplicationBundle.message("listbox.import.all.other.imports"), withSubpackages = true)
 
         @JvmField
         val ALL_OTHER_ALIAS_IMPORTS_ENTRY = KotlinPackageEntry("<all other alias imports>", withSubpackages = true)
     }
 
     fun matchesPackageName(otherPackageName: String): Boolean {
-        if (this == ALL_OTHER_IMPORTS_ENTRY || this == ALL_OTHER_ALIAS_IMPORTS_ENTRY) return true
+        if (isSpecial) return true
 
         if (otherPackageName.startsWith(packageName)) {
             if (otherPackageName.length == packageName.length) return true
@@ -47,10 +49,7 @@ class KotlinPackageEntry(
         return entry.packageName.count { it == '.' } < packageName.count { it == '.' }
     }
 
-    val isSpecial: Boolean
+    val isSpecial: Boolean get() = this == ALL_OTHER_IMPORTS_ENTRY || this == ALL_OTHER_ALIAS_IMPORTS_ENTRY
-        get() {
-            return (this == ALL_OTHER_IMPORTS_ENTRY || this == ALL_OTHER_ALIAS_IMPORTS_ENTRY)
-        }
 
     override fun toString(): String {
         return packageName

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

@@ -9,15 +9,10 @@ import com.intellij.openapi.util.InvalidDataException
 import com.intellij.openapi.util.JDOMExternalizable
 import org.jdom.Element
 
-class KotlinPackageEntryTable: JDOMExternalizable, Cloneable {
+class KotlinPackageEntryTable : JDOMExternalizable, Cloneable {
     private val entries = mutableListOf<KotlinPackageEntry>()
 
-    override fun equals(other: Any?): Boolean {
+    val entryCount: Int get() = entries.size
-        if (other !is KotlinPackageEntryTable) return false
-        if (other.entries.size != entries.size) return false
-
-        return entries.zip(other.entries).any { (entry, otherEntry) -> entry != otherEntry }
-    }
 
     public override fun clone(): KotlinPackageEntryTable {
         val clone = KotlinPackageEntryTable()
@@ -25,10 +20,6 @@ class KotlinPackageEntryTable: JDOMExternalizable, Cloneable {
         return clone
     }
 
-    override fun hashCode(): Int {
-        return entries.firstOrNull()?.hashCode() ?: 0
-    }
-
     fun copyFrom(packageTable: KotlinPackageEntryTable) {
         entries.clear()
         entries.addAll(packageTable.entries)
@@ -50,24 +41,12 @@ class KotlinPackageEntryTable: JDOMExternalizable, Cloneable {
         return entries[index]
     }
 
-    fun getEntryCount(): Int {
-        return entries.size
-    }
-
     fun setEntryAt(entry: KotlinPackageEntry, index: Int) {
         entries[index] = entry
     }
 
     operator fun contains(packageName: String): Boolean {
-        for (entry in entries) {
+        return entries.any { !it.isSpecial && it.matchesPackageName(packageName) }
-            if (packageName.startsWith(entry.packageName)) {
-                if (packageName.length == entry.packageName.length) return true
-                if (entry.withSubpackages) {
-                    if (packageName[entry.packageName.length] == '.') return true
-                }
-            }
-        }
-        return false
     }
 
     fun removeEmptyPackages() {
@@ -102,7 +81,7 @@ class KotlinPackageEntryTable: JDOMExternalizable, Cloneable {
         for (entry in entries) {
             val element = Element("package")
             parentNode.addContent(element)
-            val name = if (entry == KotlinPackageEntry.ALL_OTHER_IMPORTS_ENTRY || entry == KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY) "" else entry.packageName
+            val name = if (entry.isSpecial) "" else entry.packageName
             val alias = (entry == KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY)
 
             element.setAttribute("name", name)

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

@@ -26,7 +26,7 @@ import javax.swing.JTable
 import javax.swing.ListSelectionModel
 import javax.swing.table.AbstractTableModel
 
-open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
+open class BaseKotlinImportLayoutPanel(title: String) : JPanel(BorderLayout()) {
     val packageTable = KotlinPackageEntryTable()
     val layoutTable = createTableForPackageEntries(packageTable)
 
@@ -41,7 +41,7 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
     protected fun addPackage() {
         var row = layoutTable.selectedRow + 1
         if (row < 0) {
-            row = packageTable.getEntryCount()
+            row = packageTable.entryCount
         }
         val entry = KotlinPackageEntry("", true)
         packageTable.insertEntryAt(entry, row)
@@ -53,9 +53,7 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
         if (row < 0) return
 
         val entry = packageTable.getEntryAt(row)
-        if (entry == KotlinPackageEntry.ALL_OTHER_IMPORTS_ENTRY || entry == KotlinPackageEntry.ALL_OTHER_IMPORTS_ENTRY) {
+        if (entry.isSpecial) return
-            return
-        }
 
         TableUtil.stopEditing(layoutTable)
         packageTable.removeEntryAt(row)
@@ -63,7 +61,7 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
         val model = layoutTable.model as AbstractTableModel
         model.fireTableRowsDeleted(row, row)
 
-        if (row >= packageTable.getEntryCount()) {
+        if (row >= packageTable.entryCount) {
             row--
         }
 
@@ -89,7 +87,7 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
 
     protected fun movePackageDown() {
         val row = layoutTable.selectedRow
-        if (row >= packageTable.getEntryCount() - 1) return
+        if (row >= packageTable.entryCount - 1) return
 
         TableUtil.stopEditing(layoutTable)
         val entry = packageTable.getEntryAt(row)
@@ -109,7 +107,8 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
         TableUtil.editCellAt(layoutTable, row, 0)
         val editorComp = layoutTable.editorComponent
         if (editorComp != null) {
-            IdeFocusManager.getGlobalInstance().doWhenFocusSettlesDown { IdeFocusManager.getGlobalInstance().requestFocus(editorComp, true) }
+            IdeFocusManager.getGlobalInstance()
+                .doWhenFocusSettlesDown { IdeFocusManager.getGlobalInstance().requestFocus(editorComp, true) }
         }
     }
 
@@ -156,7 +155,7 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
     }
 
     private fun fixColumnWidthToHeader(columnIndex: Int) {
-        with (layoutTable) {
+        with(layoutTable) {
             val column = columnModel.getColumn(columnIndex)
             val width = 15 + tableHeader.getFontMetrics(tableHeader.font).stringWidth(getColumnName(columnIndex))
 
@@ -166,14 +165,14 @@ open class BaseKotlinImportLayoutPanel(title: String): JPanel(BorderLayout()) {
     }
 }
 
-class KotlinStarImportLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundle.message("title.packages.to.use.import.with")) {
+class KotlinStarImportLayoutPanel : BaseKotlinImportLayoutPanel(ApplicationBundle.message("title.packages.to.use.import.with")) {
     init {
         val importLayoutPanel = ToolbarDecorator.createDecorator(layoutTable)
             .setAddAction { addPackage() }
             .setRemoveAction { removePackage() }
             .setButtonComparator(
-                "Add",
+                ApplicationBundle.message("button.add.package"),
-                "Remove"
+                ApplicationBundle.message("button.remove"),
             ).setPreferredSize(Dimension(-1, 100))
             .createPanel()
 
@@ -182,7 +181,7 @@ class KotlinStarImportLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundle
     }
 }
 
-class KotlinImportOrderLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundle.message("title.import.layout")) {
+class KotlinImportOrderLayoutPanel : BaseKotlinImportLayoutPanel(ApplicationBundle.message("title.import.layout")) {
     private val cbImportAliasesSeparately = JBCheckBox("Import aliases separately")
 
     init {
@@ -190,7 +189,7 @@ class KotlinImportOrderLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundl
 
         val importLayoutPanel = ToolbarDecorator.createDecorator(layoutTable)
             .addExtraAction(
-                object: DumbAwareActionButton(ApplicationBundle.message("button.add.package"), IconUtil.getAddPackageIcon()) {
+                object : DumbAwareActionButton(ApplicationBundle.message("button.add.package"), IconUtil.getAddPackageIcon()) {
                     override fun actionPerformed(event: AnActionEvent) {
                         addPackage()
                     }
@@ -205,14 +204,14 @@ class KotlinImportOrderLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundl
             .setMoveDownAction { movePackageDown() }
             .setRemoveActionUpdater {
                 val selectedRow = layoutTable.selectedRow
-                val entry = if (selectedRow in 0 until packageTable.getEntryCount()) packageTable.getEntryAt(selectedRow) else null
+                val entry = if (selectedRow in 0 until packageTable.entryCount) packageTable.getEntryAt(selectedRow) else null
 
-                entry != null && entry != KotlinPackageEntry.ALL_OTHER_IMPORTS_ENTRY && entry != KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY
+                entry?.isSpecial == false
             }.setButtonComparator(
                 ApplicationBundle.message("button.add.package"),
-                "Remove",
+                ApplicationBundle.message("button.remove"),
-                "Up",
+                ApplicationBundle.message("button.move.up"),
-                "Down"
+                ApplicationBundle.message("button.move.down")
             ).setPreferredSize(Dimension(-1, 100))
             .createPanel()
 
@@ -221,15 +220,15 @@ class KotlinImportOrderLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundl
 
         cbImportAliasesSeparately.addItemListener { _ ->
             if (areImportAliasesEnabled()) {
-                if (packageTable.getEntries().none { it == KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY }) {
+                if (KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY !in packageTable.getEntries()) {
                     packageTable.addEntry(KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY)
-                    val row = packageTable.getEntryCount() - 1
+                    val row = packageTable.entryCount - 1
                     val model = layoutTable.model as AbstractTableModel
                     model.fireTableRowsInserted(row, row)
                     layoutTable.setRowSelectionInterval(row, row)
                 }
             } else {
-                val entryIndex = packageTable.getEntries().indexOfFirst { it == KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY }
+                val entryIndex = packageTable.getEntries().indexOf(KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY)
 
                 if (entryIndex != -1) {
                     val currentIndex = layoutTable.selectedRow
@@ -248,7 +247,7 @@ class KotlinImportOrderLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundl
     }
 
     fun recomputeAliasesCheckbox() {
-        cbImportAliasesSeparately.isSelected = packageTable.getEntries().any { it == KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY }
+        cbImportAliasesSeparately.isSelected = KotlinPackageEntry.ALL_OTHER_ALIAS_IMPORTS_ENTRY in packageTable.getEntries()
     }
 
     private fun areImportAliasesEnabled(): Boolean {
@@ -258,6 +257,8 @@ class KotlinImportOrderLayoutPanel: BaseKotlinImportLayoutPanel(ApplicationBundl
 
 fun createTableForPackageEntries(packageTable: KotlinPackageEntryTable): JBTable {
     val names = arrayOf(ApplicationBundle.message("listbox.import.package"), ApplicationBundle.message("listbox.import.with.subpackages"))
+    val packageNameColumnIndex = 0
+    val withSubpackagesColumnIndex = 1
 
     val dataModel = object : AbstractTableModel() {
         override fun getColumnCount(): Int {
@@ -265,7 +266,7 @@ fun createTableForPackageEntries(packageTable: KotlinPackageEntryTable): JBTable
         }
 
         override fun getRowCount(): Int {
-            return packageTable.getEntryCount()
+            return packageTable.entryCount
         }
 
         override fun getValueAt(rowIndex: Int, columnIndex: Int): Any? {
@@ -273,8 +274,8 @@ fun createTableForPackageEntries(packageTable: KotlinPackageEntryTable): JBTable
             if (!isCellEditable(rowIndex, columnIndex)) return null
 
             return when (columnIndex) {
-                0 -> entry.packageName
+                packageNameColumnIndex -> entry.packageName
-                1 -> entry.withSubpackages
+                withSubpackagesColumnIndex -> entry.withSubpackages
                 else -> throw IllegalArgumentException(columnIndex.toString())
             }
         }
@@ -290,8 +291,8 @@ fun createTableForPackageEntries(packageTable: KotlinPackageEntryTable): JBTable
 
         override fun getColumnClass(columnIndex: Int): Class<*> {
             return when (columnIndex) {
-                0 -> String::class.java
+                packageNameColumnIndex -> String::class.java
-                1 -> Boolean::class.javaObjectType
+                withSubpackagesColumnIndex -> Boolean::class.javaObjectType
                 else -> throw IllegalArgumentException(columnIndex.toString())
             }
         }
@@ -300,8 +301,8 @@ fun createTableForPackageEntries(packageTable: KotlinPackageEntryTable): JBTable
             val entry = packageTable.getEntryAt(rowIndex)
 
             val newEntry = when (columnIndex) {
-                0 -> KotlinPackageEntry((value as String).trim(), entry.withSubpackages)
+                packageNameColumnIndex -> KotlinPackageEntry((value as String).trim(), entry.withSubpackages)
-                1 -> KotlinPackageEntry(entry.packageName, value.toString().toBoolean())
+                withSubpackagesColumnIndex -> KotlinPackageEntry(entry.packageName, value.toString().toBoolean())
                 else -> throw IllegalArgumentException(columnIndex.toString())
             }
 

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

@@ -146,11 +146,11 @@ class ImportSettingsPanel(private val commonSettings: CodeStyleSettings) : JPane
         }
 
         private fun isModified(list: KotlinPackageEntryTable, table: KotlinPackageEntryTable): Boolean {
-            if (list.getEntryCount() != table.getEntryCount()) {
+            if (list.entryCount != table.entryCount) {
                 return true
             }
 
-            for (i in 0 until list.getEntryCount()) {
+            for (i in 0 until list.entryCount) {
                 val entry1 = list.getEntryAt(i)
                 val entry2 = table.getEntryAt(i)
                 if (entry1 != entry2) {

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

@@ -13,16 +13,12 @@ import java.util.Comparator
 class ImportPathComparator(
     private val packageTable: KotlinPackageEntryTable
 ) : Comparator<ImportPath> {
-    override fun compare(import1: ImportPath, import2: ImportPath): Int {
+    private val comparator = compareBy<ImportPath>(
-        val index1 = bestMatchIndex(import1)
+        { import -> bestMatchIndex(import) },
-        val index2 = bestMatchIndex(import2)
+        { import -> import.toString() }
+    )
 
-        return when {
+    override fun compare(import1: ImportPath, import2: ImportPath): Int = comparator.compare(import1, import2)
-            index1 == index2 -> import1.toString().compareTo(import2.toString())
-            index1 < index2 -> -1
-            else -> +1
-        }
-    }
 
     private fun bestMatchIndex(path: ImportPath): Int {
         var bestIndex: Int? = null