From 50fc9d3692fca7420cfb007f154c69e2d7c6d67e Mon Sep 17 00:00:00 2001 From: gcx11 Date: Tue, 12 May 2020 22:46:44 +0200 Subject: [PATCH] KT-10974 - code review changes --- .../formatter/KotlinCodeStyleSettings.java | 14 +++-- .../idea/core/formatter/KotlinPackageEntry.kt | 16 ----- .../perf/AbstractPerformanceImportTest.kt | 1 - .../messages/KotlinBundle.properties | 3 +- .../formatter/BaseKotlinImportLayoutPanel.kt | 17 +++--- .../kotlin/idea/util/ImportPathComparator.kt | 58 ++++++++++++++----- 6 files changed, 64 insertions(+), 45 deletions(-) diff --git a/idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinCodeStyleSettings.java b/idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinCodeStyleSettings.java index 44677564896..4fe43b83da3 100644 --- a/idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinCodeStyleSettings.java +++ b/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) { diff --git a/idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinPackageEntry.kt b/idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinPackageEntry.kt index 7b4e8e70634..65b8f0fd54d 100644 --- a/idea/formatter/src/org/jetbrains/kotlin/idea/core/formatter/KotlinPackageEntry.kt +++ b/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 { diff --git a/idea/performanceTests/test/org/jetbrains/kotlin/idea/perf/AbstractPerformanceImportTest.kt b/idea/performanceTests/test/org/jetbrains/kotlin/idea/perf/AbstractPerformanceImportTest.kt index bc12e2d54c0..c83de055439 100644 --- a/idea/performanceTests/test/org/jetbrains/kotlin/idea/perf/AbstractPerformanceImportTest.kt +++ b/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 diff --git a/idea/resources-en/messages/KotlinBundle.properties b/idea/resources-en/messages/KotlinBundle.properties index c7d64d2ae36..427e4b5b94a 100644 --- a/idea/resources-en/messages/KotlinBundle.properties +++ b/idea/resources-en/messages/KotlinBundle.properties @@ -2212,4 +2212,5 @@ inspection.logger.initialized.with.foreign.class.display.name=Logger initialized 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 \ No newline at end of file +choose.logger.factory.class=Choose logger factory class +codestyle.layout.import.aliases.separately=Import aliases separately \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/formatter/BaseKotlinImportLayoutPanel.kt b/idea/src/org/jetbrains/kotlin/idea/formatter/BaseKotlinImportLayoutPanel.kt index 17131a78253..7c6e76401ce 100644 --- a/idea/src/org/jetbrains/kotlin/idea/formatter/BaseKotlinImportLayoutPanel.kt +++ b/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"), - ApplicationBundle.message("button.remove"), + "Add", + "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"), - ApplicationBundle.message("button.remove"), - ApplicationBundle.message("button.move.up"), - ApplicationBundle.message("button.move.down") + "Add Package", + "Remove", + "Up", + "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) diff --git a/idea/src/org/jetbrains/kotlin/idea/util/ImportPathComparator.kt b/idea/src/org/jetbrains/kotlin/idea/util/ImportPathComparator.kt index 79b8b0fceba..aba9f16feea 100644 --- a/idea/src/org/jetbrains/kotlin/idea/util/ImportPathComparator.kt +++ b/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( - private val packageTable: KotlinPackageEntryTable -) : Comparator { - private val comparator = compareBy( - { import -> bestMatchIndex(import) }, - { import -> import.toString() } - ) +internal class ImportPathComparator(private val packageTable: KotlinPackageEntryTable) : Comparator { - 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 { - var bestIndex: Int? = null + return compareValuesBy( + 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 } -} \ No newline at end of file +} + +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) +}