Optimize Imports should remove unused import alias

#KT-17375 Fixed

idea/ide-common/src/org/jetbrains/kotlin/idea/util/OptimizedImportsBuilder.kt

@@ -41,7 +41,6 @@ import org.jetbrains.kotlin.resolve.scopes.ImportingScope
 import org.jetbrains.kotlin.resolve.scopes.utils.findClassifier
 import org.jetbrains.kotlin.resolve.scopes.utils.parentsWithSelf
 import org.jetbrains.kotlin.resolve.scopes.utils.replaceImportingScopes
-import java.util.*
 
 class OptimizedImportsBuilder(
     private val file: KtFile,
@@ -88,16 +87,6 @@ class OptimizedImportsBuilder(
     private val importRules = HashSet<ImportRule>()
 
     fun buildOptimizedImports(): List<ImportPath>? {
-        // TODO: should we drop unused aliases?
-        // keep all non-trivial aliases
-        file.importDirectives
-            .mapNotNull { it.importPath }
-            .filter {
-                val aliasName = it.alias
-                aliasName != null && aliasName != it.fqName.shortName()
-            }
-            .mapTo(importRules) { ImportRule.Add(it) }
-
         while (true) {
             val importRulesBefore = importRules.size
             val result = tryBuildOptimizedImports()
@@ -124,18 +113,20 @@ class OptimizedImportsBuilder(
             .mapTo(importsToGenerate) { it.importPath }
 
         val descriptorsByParentFqName = HashMap<FqName, MutableSet<DeclarationDescriptor>>()
-        for (descriptor in data.descriptorsToImport.keys) {
-            val fqName = descriptor.importableFqName!!
+        for ((descriptor, names) in data.descriptorsToImport) {
+            for (name in names) {
+                val fqName = descriptor.importableFqName!!
+                val alias = if (name != fqName.shortName()) name else null
 
-            val explicitImportPath = ImportPath(fqName, false)
-            if (explicitImportPath in importsToGenerate) continue
+                val explicitImportPath = ImportPath(fqName, false, alias)
+                if (explicitImportPath in importsToGenerate) continue
 
-            val parentFqName = fqName.parent()
-            val starImportPath = ImportPath(parentFqName, true)
-            if (canUseStarImport(descriptor, fqName) && starImportPath.isAllowedByRules()) {
-                descriptorsByParentFqName.getOrPut(parentFqName) { HashSet() }.add(descriptor)
-            } else {
-                importsToGenerate.add(explicitImportPath)
+                val parentFqName = fqName.parent()
+                if (alias == null && canUseStarImport(descriptor, fqName) && ImportPath(parentFqName, true).isAllowedByRules()) {
+                    descriptorsByParentFqName.getOrPut(parentFqName) { LinkedHashSet() }.add(descriptor)
+                } else {
+                    importsToGenerate.add(explicitImportPath)
+                }
             }
         }
 
@@ -205,7 +196,12 @@ class OptimizedImportsBuilder(
                     val newTargets = ref.resolve(newBindingContext)
                     if (!areTargetsEqual(oldTargets, newTargets)) {
                         testLog?.append("Changed resolve of $ref\n")
-                        (oldTargets + newTargets).forEach { lockImportForDescriptor(it) }
+                        (oldTargets + newTargets).forEach {
+                            lockImportForDescriptor(
+                                it,
+                                data.descriptorsToImport.getOrElse(it) { listOf(it.name) }.intersect(names)
+                            )
+                        }
                     }
                 }
             }
@@ -214,18 +210,22 @@ class OptimizedImportsBuilder(
         return sortedImportsToGenerate
     }
 
-    private fun lockImportForDescriptor(descriptor: DeclarationDescriptor) {
+    private fun lockImportForDescriptor(descriptor: DeclarationDescriptor, names: Collection<Name>) {
         val fqName = descriptor.importableFqName ?: return
-        val explicitImportPath = ImportPath(fqName, false)
         val starImportPath = ImportPath(fqName.parent(), true)
         val importPaths = file.importDirectives.map { it.importPath }
-        when {
-            explicitImportPath in importPaths ->
-                importRules.add(ImportRule.Add(explicitImportPath))
-            starImportPath in importPaths ->
-                importRules.add(ImportRule.Add(starImportPath))
-            else -> // there is no import for this descriptor in the original import list, so do not allow to import it by star-import
-                importRules.add(ImportRule.DoNotAdd(starImportPath))
+
+        for (name in names) {
+            val alias = if (name != fqName.shortName()) name else null
+            val explicitImportPath = ImportPath(fqName, false, alias)
+            when {
+                explicitImportPath in importPaths ->
+                    importRules.add(ImportRule.Add(explicitImportPath))
+                alias == null && starImportPath in importPaths ->
+                    importRules.add(ImportRule.Add(starImportPath))
+                else -> // there is no import for this descriptor in the original import list, so do not allow to import it by star-import
+                    importRules.add(ImportRule.DoNotAdd(starImportPath))
+            }
         }
     }
 

idea/src/org/jetbrains/kotlin/idea/imports/KotlinImportOptimizer.kt

@@ -73,13 +73,13 @@ class KotlinImportOptimizer : ImportOptimizer {
 
     private class CollectUsedDescriptorsVisitor(file: KtFile) : KtVisitorVoid() {
         private val currentPackageName = file.packageFqName
-        private val withAlias: Set<FqName> = file.importDirectives
+        private val aliases: Map<FqName, List<Name>> = file.importDirectives
             .asSequence()
-            .filter { it.alias != null && it.aliasName != it.importPath?.fqName?.shortName()?.asString() }
-            .mapNotNull(KtImportDirective::importedFqName)
-            .toSet()
+            .filter { !it.isAllUnder && it.alias != null }
+            .mapNotNull { it.importPath }
+            .groupBy(keySelector = { it.fqName }, valueTransform = { it.importedName as Name })
 
-        private val descriptorsToImport = LinkedHashMap<DeclarationDescriptor, Set<Name>>()
+        private val descriptorsToImport = LinkedHashMap<DeclarationDescriptor, HashSet<Name>>()
         private val abstractRefs = ArrayList<OptimizedImportsBuilder.AbstractReference>()
 
         val data: OptimizedImportsBuilder.InputData
@@ -111,13 +111,15 @@ class KotlinImportOptimizer : ImportOptimizer {
                     val importableFqName = target.importableFqName ?: continue
                     val parentFqName = importableFqName.parent()
                     if (target is PackageViewDescriptor && parentFqName == FqName.ROOT) continue // no need to import top-level packages
-                    if (target !is PackageViewDescriptor && parentFqName == currentPackageName && importableFqName !in withAlias) continue
+
+                    if (target !is PackageViewDescriptor && parentFqName == currentPackageName && (importableFqName !in aliases)) continue
 
                     if (!reference.canBeResolvedViaImport(target, bindingContext)) continue
 
                     if (isAccessibleAsMember(importableDescriptor, element, bindingContext)) continue
 
-                    descriptorsToImport.compute(importableDescriptor) { _, u -> u?.plus(names) ?: names.toHashSet() }
+                    val descriptorNames = (aliases[importableFqName].orEmpty() + importableFqName.shortName()).intersect(names)
+                    descriptorsToImport.getOrPut(importableDescriptor) { LinkedHashSet() } += descriptorNames
                 }
             }
 

idea/testData/editor/optimizeImports/common/ConflictWithAlias.kt

@@ -1,4 +1,5 @@
 import name
 import name as names
 
-val a = name
+val a = name
+val b = names

idea/testData/editor/optimizeImports/common/ConflictWithAlias.kt.after

@@ -2,3 +2,4 @@ import name
 import name as names
 
 val a = name
+val b = names

idea/testData/editor/optimizeImports/common/IteratorFunction.dependency.kt

@@ -1,9 +1,8 @@
 package test1
 
-public class MyClass {
-}
+class MyClass
 
-public fun MyClass.iterator(): Iterator<MyClass> {
+operator fun MyClass.iterator(): Iterator<MyClass> {
      return object: Iterator<MyClass> {
          override fun next(): MyClass {
              throw Exception()

idea/testData/editor/optimizeImports/common/TwoConstructors.kt.after

@@ -1,5 +1,5 @@
 // NAME_COUNT_TO_USE_STAR_IMPORT: 2
-import p1.*
+import p1.A
 import p2.A
 
 fun f() {

idea/testData/editor/optimizeImports/common/TwoConstructors.kt.log

@@ -6,4 +6,3 @@ Additional checking of reference KtSimpleNameReference: A
 Changed resolve of KtSimpleNameReference: A
 Additional checking of reference KtInvokeFunctionReference: A("")
 Additional checking of reference KtInvokeFunctionReference: A(1)
-Trying to build import list again with import rules: +p2.A, +p1.*

idea/testData/editor/optimizeImports/jvm/AlreadyOptimized.kt

@@ -1,20 +1,20 @@
 /**
 Comment 1
-*/
+ */
 package sometest
 
-import java.io as JavaIO
-import java.text.Annotation as TextAnnotation
+import java.io.File as JavaFile
+import java.lang.Runnable as Task
 import java.util.ArrayList
 import java.util.HashSet
 
 /**
 Comment 2
-*/
+ */
 class Action {
-    fun test(hash : HashSet<Int>) {
-        val some : TextAnnotation? = null
-        val test : ArrayList<Int>? = null
-        JavaIO.File(StringBuilder().append("Hello").toString())
+    fun test(hash: HashSet<Int>) {
+        val some: Task? = null
+        val test: ArrayList<Int>? = null
+        JavaFile(StringBuilder().append("Hello").toString())
     }
 }

idea/testData/editor/optimizeImports/jvm/AlreadyOptimized.kt.after

@@ -1,20 +1,20 @@
 /**
 Comment 1
-*/
+ */
 package sometest
 
 import java.util.ArrayList
 import java.util.HashSet
-import java.io as JavaIO
-import java.text.Annotation as TextAnnotation
+import java.io.File as JavaFile
+import java.lang.Runnable as Task
 
 /**
 Comment 2
-*/
+ */
 class Action {
-    fun test(hash : HashSet<Int>) {
-        val some : TextAnnotation? = null
-        val test : ArrayList<Int>? = null
-        JavaIO.File(StringBuilder().append("Hello").toString())
+    fun test(hash: HashSet<Int>) {
+        val some: Task? = null
+        val test: ArrayList<Int>? = null
+        JavaFile(StringBuilder().append("Hello").toString())
     }
 }

idea/testData/editor/optimizeImports/jvm/AlreadyOptimized.kt.log

@@ -0,0 +1,4 @@
+Additional checking of reference Getter: JavaFile
+Additional checking of reference KtSimpleNameReference: JavaFile
+Changed resolve of KtSimpleNameReference: JavaFile
+Additional checking of reference KtInvokeFunctionReference: JavaFile(StringBuilder().append("Hello").toString())

idea/testData/editor/optimizeImports/jvm/WithAliases.kt.after

@@ -1,6 +1,4 @@
 import java.util.ArrayList
-import java.io as JavaIO
-import java.util.ArrayList as SomeThing
 
 class Action {
     fun test() {

idea/testData/quickfix/optimizeImports/importAlias.kt

@@ -0,0 +1,10 @@
+// "Optimize imports" "true"
+
+import A as B
+<caret>import A as T
+
+class A
+
+fun foo() {
+    B()
+}

idea/testData/quickfix/optimizeImports/importAlias.kt.after

@@ -0,0 +1,9 @@
+// "Optimize imports" "true"
+
+import A as B
+
+class A
+
+fun foo() {
+    B()
+}

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveClassWithImportsRetained/after/second/A.kt

@@ -1,6 +1,4 @@
 package second
 
-import third.D as D_
-
 class A
 

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/misc/moveFunctionWithImportsRetained/after/second/A.kt

@@ -1,6 +1,4 @@
 package second
 
-import third.D as D_
-
 class A
 

idea/tests/org/jetbrains/kotlin/idea/quickfix/QuickFixTestGenerated.java

@@ -9144,6 +9144,11 @@ public class QuickFixTestGenerated extends AbstractQuickFixTest {
         public void testFileRuntime() throws Exception {
             runTest("idea/testData/quickfix/optimizeImports/fileRuntime.kt");
         }
+
+        @TestMetadata("importAlias.kt")
+        public void testImportAlias() throws Exception {
+            runTest("idea/testData/quickfix/optimizeImports/importAlias.kt");
+        }
     }
 
     @TestMetadata("idea/testData/quickfix/override")