Rewritten resolving conflicting imports on adding new import with star action (KT-18503)

#KT-18503 Fixed

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

@@ -220,26 +220,12 @@ class ImportInsertHelperImpl(private val project: Project) : ImportInsertHelper(
             return true
         }
 
-        private fun addStarImport(target: DeclarationDescriptor): ImportDescriptorResult {
-            val targetFqName = target.importableFqName!!
+        private fun addStarImport(targetDescriptor: DeclarationDescriptor): ImportDescriptorResult {
+            val targetFqName = targetDescriptor.importableFqName!!
             val parentFqName = targetFqName.parent()
 
             val moduleDescriptor = resolutionFacade.moduleDescriptor
-            val imports = file.importDirectives
             val scopeToImport = getMemberScope(parentFqName, moduleDescriptor) ?: return ImportDescriptorResult.FAIL
-            val importedScopes = imports
-                .asSequence()
-                .filter { it.isAllUnder }
-                .mapNotNull {
-                    val importPath = it.importPath
-                    if (importPath != null) {
-                        val fqName = importPath.fqName
-                        getMemberScope(fqName, moduleDescriptor)
-                    } else {
-                        null
-                    }
-                }
-                .toList()
 
             val filePackage = moduleDescriptor.getPackage(file.packageFqName)
 
@@ -249,38 +235,57 @@ class ImportInsertHelperImpl(private val project: Project) : ImportInsertHelper(
                 return !visibility.mustCheckInImports() || Visibilities.isVisibleIgnoringReceiver(descriptor, filePackage)
             }
 
-            val classNamesToImport = scopeToImport
-                    .getDescriptorsFiltered(DescriptorKindFilter.CLASSIFIERS, { true })
-                    .filter(::isVisible)
-                    .map { it.name }
+            val kindFilter = DescriptorKindFilter.ALL.withoutKinds(DescriptorKindFilter.PACKAGES_MASK)
+            val allNamesToImport = scopeToImport.getDescriptorsFiltered(kindFilter).filter(::isVisible).map { it.name }.toSet()
 
-            val topLevelScope = resolutionFacade.getFileResolutionScope(file)
-            val conflictCandidates: List<ClassifierDescriptor> = classNamesToImport
-                    .flatMap {
-                        importedScopes.mapNotNull { scope -> scope.getContributedClassifier(it, NoLookupLocation.FROM_IDE) }
+            fun targetFqNameAndType(ref: KtReferenceExpression): Pair<FqName, Class<out Any>>? {
+                val descriptors = ref.resolveTargets()
+                val fqName: FqName? = descriptors.filter(::isVisible). map { it.importableFqName }.toSet().singleOrNull()
+                return if (fqName != null) {
+                    Pair(fqName, descriptors.elementAt(0).javaClass)
+                } else null
+            }
+
+            val futureCheckMap = HashMap<KtSimpleNameExpression, Pair<FqName, Class<out Any>>>()
+            file.accept(object : KtVisitorVoid() {
+                override fun visitElement(element: PsiElement): Unit = element.acceptChildren(this)
+                override fun visitImportList(importList: KtImportList) {}
+                override fun visitPackageDirective(directive: KtPackageDirective) {}
+                override fun visitSimpleNameExpression(expression: KtSimpleNameExpression) {
+                    val refName = expression.getReferencedNameAsName()
+                    if (allNamesToImport.contains(refName)) {
+                        val target = targetFqNameAndType(expression)
+                        if (target != null) {
+                            futureCheckMap += Pair(expression, target)
+                        }
                     }
-                    .filter { importedClass ->
-                        isVisible(importedClass)
-                            // check that class is really imported
-                            && topLevelScope.findClassifier(importedClass.name, NoLookupLocation.FROM_IDE) == importedClass
-                            // and not yet imported explicitly
-                            && imports.all { it.importPath != ImportPath(importedClass.importableFqName!!, false)  }
-                    }
-            val conflicts = detectNeededImports(conflictCandidates)
+                }
+            })
 
             val addedImport = addImport(parentFqName, true)
 
-            val newTopLevelScope = resolutionFacade.getFileResolutionScope(file)
-            if (!isAlreadyImported(target, newTopLevelScope, targetFqName)) {
+            if (!isAlreadyImported(targetDescriptor, resolutionFacade.getFileResolutionScope(file), targetFqName)) {
                 addedImport.delete()
                 return ImportDescriptorResult.FAIL
             }
 
-            for (conflict in conflicts) {
-                addImport(DescriptorUtils.getFqNameSafe(conflict), false)
+            dropRedundantExplicitImports(parentFqName)
+
+            val conflicts = futureCheckMap
+                .mapNotNull { (expr, fqNameAndType) ->
+                    if (targetFqNameAndType(expr) != fqNameAndType) fqNameAndType.first else null
+                }
+                .toSet()
+
+            fun isNotImported(fqName: FqName): Boolean {
+                return file.importDirectives.none { directive ->
+                    !directive.isAllUnder && directive.alias == null && directive.importedFqName == fqName
+                }
             }
 
-            dropRedundantExplicitImports(parentFqName)
+            for (conflict in conflicts.filter(::isNotImported)) {
+                addImport(conflict, false)
+            }
 
             return ImportDescriptorResult.IMPORT_ADDED
         }

idea/testData/addImport/ConflictingNameAppearsAndHasUsage3.kt

@@ -3,4 +3,4 @@ package p
 
 import dependency.*
 
-val d = Date(1, 2)
+val d = Date("") //import dependency.Date is not necessary

idea/testData/addImport/ConflictingNameAppearsAndHasUsage3.kt.after

@@ -2,7 +2,6 @@
 package p
 
 import dependency.*
-import dependency.Date
 import java.util.*
 
-val d = Date(1, 2)
+val d = Date("") //import dependency.Date is not necessary

idea/testData/addImport/ConflictingNameAppearsAndHasUsage7.dependency.kt

@@ -0,0 +1,5 @@
+package dependency
+
+class Date
+
+fun Date(s: String) = Date()

idea/testData/addImport/ConflictingNameAppearsAndHasUsage7.kt

@@ -0,0 +1,6 @@
+// IMPORT: java.util.Calendar
+package p
+
+import dependency.*
+
+val d = Date()

idea/testData/addImport/ConflictingNameAppearsAndHasUsage7.kt.after

@@ -0,0 +1,8 @@
+// IMPORT: java.util.Calendar
+package p
+
+import dependency.*
+import dependency.Date
+import java.util.*
+
+val d = Date()

idea/testData/addImport/ConflictsExtensions.dependency.kt

@@ -0,0 +1,9 @@
+package conflicts.extensions
+
+fun Byte.inv(): Byte = (255 - this).toByte()
+
+fun foo1() {}
+fun foo2() {}
+fun foo3() {}
+fun foo4() {}
+fun foo5() {}

idea/testData/addImport/ConflictsExtensions.dependency1.kt

@@ -0,0 +1,3 @@
+package conflicts.extensions.deps
+
+fun Byte.inv(): Byte = (255 - this).toByte()

idea/testData/addImport/ConflictsExtensions.kt

@@ -0,0 +1,13 @@
+//IMPORT: conflicts.extensions.foo5
+package p
+
+import conflicts.extensions.foo1
+import conflicts.extensions.foo2
+import conflicts.extensions.foo3
+import conflicts.extensions.foo4
+import conflicts.extensions.deps.*
+
+fun main(args: Array<String>) {
+    val b: Byte = 1
+    b.inv()
+}

idea/testData/addImport/ConflictsExtensions.kt.after

@@ -0,0 +1,11 @@
+//IMPORT: conflicts.extensions.foo5
+package p
+
+import conflicts.extensions.*
+import conflicts.extensions.deps.*
+import conflicts.extensions.deps.inv
+
+fun main(args: Array<String>) {
+    val b: Byte = 1
+    b.inv()
+}

idea/testData/addImport/ConflictsExtensions1.dependency.kt

@@ -0,0 +1,9 @@
+package conflicts.extensions
+
+fun Byte.inv(): Byte = (255 - this).toByte()
+
+fun foo1() {}
+fun foo2() {}
+fun foo3() {}
+fun foo4() {}
+fun foo5() {}

idea/testData/addImport/ConflictsExtensions1.dependency1.kt

@@ -0,0 +1,3 @@
+package conflicts.extensions.deps
+
+fun Byte.inv(max: Int): Byte = (max - this).toByte()

idea/testData/addImport/ConflictsExtensions1.kt

@@ -0,0 +1,13 @@
+//IMPORT: conflicts.extensions.foo5
+package p
+
+import conflicts.extensions.foo1
+import conflicts.extensions.foo2
+import conflicts.extensions.foo3
+import conflicts.extensions.foo4
+import conflicts.extensions.deps.*
+
+fun main(args: Array<String>) {
+    val b: Byte = 1
+    b.inv(255)
+}

idea/testData/addImport/ConflictsExtensions1.kt.after

@@ -0,0 +1,10 @@
+//IMPORT: conflicts.extensions.foo5
+package p
+
+import conflicts.extensions.*
+import conflicts.extensions.deps.*
+
+fun main(args: Array<String>) {
+    val b: Byte = 1
+    b.inv(255)
+}

idea/testData/addImport/DoNotDropConflictingOnStar.dependency.kt

@@ -0,0 +1,8 @@
+package dependency
+
+typealias Date = String
+
+fun util1() = 1
+fun util2() = 2
+fun util3() = 3
+fun util4() = 4

idea/testData/addImport/DoNotDropConflictingOnStar.kt

@@ -0,0 +1,18 @@
+// IMPORT: dependency.util4
+package p
+
+import dependency.util1
+import dependency.util2
+import dependency.util3
+import dependency.Date
+import java.util.*
+
+val aMap: AbstractMap<Int, Int>? = null
+val calendar: Calendar? = null
+
+fun klass(): Date {
+    util1()
+    util2()
+    util3()
+    return Date()
+}

idea/testData/addImport/DoNotDropConflictingOnStar.kt.after

@@ -0,0 +1,16 @@
+// IMPORT: dependency.util4
+package p
+
+import dependency.*
+import dependency.Date
+import java.util.*
+
+val aMap: AbstractMap<Int, Int>? = null
+val calendar: Calendar? = null
+
+fun klass(): Date {
+    util1()
+    util2()
+    util3()
+    return Date()
+}

idea/tests/org/jetbrains/kotlin/addImport/AddImportTestGenerated.java

@@ -89,6 +89,12 @@ public class AddImportTestGenerated extends AbstractAddImportTest {
         runTest("idea/testData/addImport/ConflictingNameAppearsAndHasUsage6.kt");
     }
 
+    @TestMetadata("ConflictingNameAppearsAndHasUsage7.kt")
+    public void testConflictingNameAppearsAndHasUsage7() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/addImport/ConflictingNameAppearsAndHasUsage7.kt");
+        doTest(fileName);
+    }
+
     @TestMetadata("ConflictingNameAppearsButUsageIsQualified.kt")
     public void testConflictingNameAppearsButUsageIsQualified() throws Exception {
         runTest("idea/testData/addImport/ConflictingNameAppearsButUsageIsQualified.kt");
@@ -129,6 +135,24 @@ public class AddImportTestGenerated extends AbstractAddImportTest {
         runTest("idea/testData/addImport/ConflictingNameNoAllUnderImport3.kt");
     }
 
+    @TestMetadata("ConflictsExtensions.kt")
+    public void testConflictsExtensions() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/addImport/ConflictsExtensions.kt");
+        doTest(fileName);
+    }
+
+    @TestMetadata("ConflictsExtensions1.kt")
+    public void testConflictsExtensions1() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/addImport/ConflictsExtensions1.kt");
+        doTest(fileName);
+    }
+
+    @TestMetadata("DoNotDropConflictingOnStar.kt")
+    public void testDoNotDropConflictingOnStar() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("idea/testData/addImport/DoNotDropConflictingOnStar.kt");
+        doTest(fileName);
+    }
+
     @TestMetadata("DropExplicitImports.kt")
     public void testDropExplicitImports() throws Exception {
         runTest("idea/testData/addImport/DropExplicitImports.kt");