Shorten references inserts imports for global functions and properties too

idea/idea-analysis/src/org/jetbrains/kotlin/idea/util/ShortenReferences.kt

@@ -40,6 +40,7 @@ import org.jetbrains.kotlin.resolve.calls.tasks.ExplicitReceiverKind
 import org.jetbrains.kotlin.resolve.scopes.receivers.ExpressionReceiver
 import org.jetbrains.kotlin.resolve.scopes.receivers.ThisReceiver
 import org.jetbrains.kotlin.idea.util.ShortenReferences.Options
+import org.jetbrains.kotlin.idea.imports.*
 
 public class ShortenReferences(val options: (JetElement) -> Options = { Options.DEFAULT }) {
     public data class Options(
@@ -61,25 +62,12 @@ public class ShortenReferences(val options: (JetElement) -> Options = { Options.
             val targets = context[BindingContext.REFERENCE_TARGET, this]?.let { listOf(it) }
                           ?: context[BindingContext.AMBIGUOUS_REFERENCE_TARGET, this]
                           ?: listOf()
-            return targets.map { descriptorToImport(it) }.toSet()
-        }
-
-        private fun descriptorToImport(target: DeclarationDescriptor): DeclarationDescriptor {
-            val descriptor = target.getImportableDescriptor()
-            // if there is a class with the same fq-name then prefer to consider it as target (otherwise we won't insert import)
-            if (descriptor is CallableDescriptor) {
-                val container = descriptor.getContainingDeclaration()
-                if (container is PackageFragmentDescriptor) {
-                    val classifier = container.getMemberScope().getClassifier(descriptor.getName())
-                    if (classifier != null) return classifier
-                }
-            }
-            return descriptor
+            return targets.map { it.getImportableDescriptor() }.toSet()
         }
 
         private fun mayImport(descriptor: DeclarationDescriptor, file: JetFile): Boolean {
-            if (descriptor !is ClassDescriptor && descriptor !is PackageViewDescriptor) return false
-            return ImportInsertHelper.getInstance(file.getProject()).mayImportByCodeStyle(descriptor)
+            return descriptor.canBeReferencedViaImport()
+                   && ImportInsertHelper.getInstance(file.getProject()).mayImportByCodeStyle(descriptor)
         }
     }
 

idea/src/org/jetbrains/kotlin/idea/codeInsight/KotlinCopyPasteReferenceProcessor.kt

@@ -257,7 +257,7 @@ public class KotlinCopyPasteReferenceProcessor() : CopyPastePostProcessor<Kotlin
         )
 
         val bindingRequests = ArrayList<BindingRequest>()
-        val descriptorsToImport = ArrayList<CallableDescriptor>()
+        val extensionsToImport = ArrayList<CallableDescriptor>()
         for ((reference, refData) in referencesToRestore) {
             val fqName = FqName(refData.fqName)
 
@@ -266,13 +266,12 @@ public class KotlinCopyPasteReferenceProcessor() : CopyPastePostProcessor<Kotlin
                 bindingRequests.add(BindingRequest(pointer, fqName))
             }
 
-            //TODO: this may sometimes cause insertion of redundant imports (see 3 ignored tests)
-            if (refData.kind == KotlinReferenceData.Kind.NON_EXTENSION_CALLABLE || refData.kind == KotlinReferenceData.Kind.EXTENSION_CALLABLE) {
-                descriptorsToImport.addIfNotNull(findCallableToImport(fqName, file))
+            if (refData.kind == KotlinReferenceData.Kind.EXTENSION_CALLABLE) {
+                extensionsToImport.addIfNotNull(findCallableToImport(fqName, file))
             }
         }
 
-        for (descriptor in descriptorsToImport) {
+        for (descriptor in extensionsToImport) {
             importHelper.importDescriptor(file, descriptor)
         }
         for ((pointer, fqName) in bindingRequests) {

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

@@ -161,9 +161,8 @@ public class ImportInsertHelperImpl(private val project: Project) : ImportInsert
     override fun mayImportByCodeStyle(descriptor: DeclarationDescriptor): Boolean {
         val importable = descriptor.getImportableDescriptor()
         return when (importable) {
-            is ClassDescriptor -> importable.getContainingDeclaration() is PackageFragmentDescriptor // do not import nested classes
             is PackageViewDescriptor -> JetCodeStyleSettings.getInstance(project).IMPORT_PACKAGES
-            else -> true
+            else -> importable.getContainingDeclaration() is PackageFragmentDescriptor // do not import nested classes and non-top-level declarations
         }
     }
 

idea/testData/copyPaste/imports/DeepInnerClasses.expected.kt

@@ -1,7 +1,7 @@
 package to
 
-import a.with
 import a.Outer
+import a.with
 
 fun f(p1: Outer.Nested.NN, p2: Outer.Nested.NI, p3: Outer.Inner.II) {
     Outer.Nested.NN2()

idea/testData/copyPaste/imports/DependencyOnKotlinLibrary.expected.kt

@@ -1,11 +1,11 @@
 package to
 
-import d.g
-import d.c
 import d.ext
 import d.A
 import d.T
 import d.Outer
+import d.c
+import d.g
 import d.O1
 import d.O2
 import d.E

idea/testData/copyPaste/imports/ImportDependency.expected.kt

@@ -1,11 +1,11 @@
 package to
 
-import d.g
-import d.c
 import d.ext
 import d.A
 import d.T
 import d.Outer
+import d.c
+import d.g
 import d.O1
 import d.O2
 import d.E

idea/testData/copyPaste/imports/ImportableEntityInExtensionLiteral.expected.kt

@@ -1,7 +1,7 @@
 package to
 
-import a.f
 import a.ext
+import a.f
 
 fun g() {
     f {

idea/testData/refactoring/move/kotlin/moveFile/internalReferences/after/source/some.kt

@@ -1,5 +1,6 @@
 package source
 
 import target.A
+import target.test
 
-fun test1() = target.test(A())
+fun test1() = test(A())

idea/testData/refactoring/move/kotlin/moveFile/moveFileWithPackageRename/after/c/noImports.kt

@@ -1,10 +1,12 @@
 package c
 
 import b.Test
+import b.test
+import b.TEST
 
 fun bar() {
     val t: Test = Test()
-    b.test()
-    println(b.TEST)
-    b.TEST = ""
+    test()
+    println(TEST)
+    TEST = ""
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToFile/after/_a/noImports.kt

@@ -1,5 +1,7 @@
 package a
 
+import b.test
+
 fun bar() {
-    b.test()
+    test()
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToFile/after/a/main.kt

@@ -1,7 +1,9 @@
 package a
 
+import b.test
+
 class Test {
     fun foo() {
-        b.test()
+        test()
     }
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToFile/after/a/noImports.kt

@@ -1,5 +1,7 @@
 package a
 
+import b.test
+
 fun bar() {
-    b.test()
+    test()
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToFile/after/c/noImports.kt

@@ -1,5 +1,7 @@
 package c
 
+import b.test
+
 fun bar() {
-    b.test()
+    test()
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToFile/after/c/onDemandImport.kt

@@ -1,7 +1,8 @@
 package c
 
 import a.*
+import b.test
 
 fun bar() {
-    b.test()
+    test()
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToPackage/after/_a/noImports.kt

@@ -1,5 +1,7 @@
 package a
 
+import b.test
+
 fun bar() {
-    b.test()
+    test()
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToPackage/after/a/main.kt

@@ -1,7 +1,9 @@
 package a
 
+import b.test
+
 class Test {
     fun foo() {
-        b.test()
+        test()
     }
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToPackage/after/a/noImports.kt

@@ -1,5 +1,7 @@
 package a
 
+import b.test
+
 fun bar() {
-    b.test()
+    test()
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToPackage/after/c/noImports.kt

@@ -1,5 +1,7 @@
 package c
 
+import b.test
+
 fun bar() {
-    b.test()
+    test()
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/moveFunctionToPackage/after/c/onDemandImport.kt

@@ -1,7 +1,8 @@
 package c
 
 import a.*
+import b.test
 
 fun bar() {
-    b.test()
+    test()
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToFile/after/_a/noImports.kt

@@ -1,6 +1,8 @@
 package a
 
+import b.test
+
 fun bar() {
-    b.test = ""
-    println(b.test)
+    test = ""
+    println(test)
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToFile/after/a/main.kt

@@ -1,7 +1,9 @@
 package a
 
+import b.test
+
 class Test {
     fun foo() {
-        b.test()
+        test()
     }
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToFile/after/a/noImports.kt

@@ -1,6 +1,8 @@
 package a
 
+import b.test
+
 fun bar() {
-    b.test = ""
-    println(b.test)
+    test = ""
+    println(test)
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToFile/after/c/noImports.kt

@@ -1,6 +1,8 @@
 package c
 
+import b.test
+
 fun bar() {
-    b.test = ""
-    println(b.test)
+    test = ""
+    println(test)
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToFile/after/c/onDemandImport.kt

@@ -1,8 +1,9 @@
 package c
 
 import a.*
+import b.test
 
 fun bar() {
-    b.test = ""
-    println(b.test)
+    test = ""
+    println(test)
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToPackage/after/_a/noImports.kt

@@ -1,6 +1,8 @@
 package a
 
+import b.test
+
 fun bar() {
-    b.test = ""
-    println(b.test)
+    test = ""
+    println(test)
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToPackage/after/a/main.kt

@@ -1,7 +1,9 @@
 package a
 
+import b.test
+
 class Test {
     fun foo() {
-        b.test()
+        test()
     }
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToPackage/after/a/noImports.kt

@@ -1,6 +1,8 @@
 package a
 
+import b.test
+
 fun bar() {
-    b.test = ""
-    println(b.test)
+    test = ""
+    println(test)
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToPackage/after/c/noImports.kt

@@ -1,6 +1,8 @@
 package c
 
+import b.test
+
 fun bar() {
-    b.test = ""
-    println(b.test)
+    test = ""
+    println(test)
 }

idea/testData/refactoring/move/kotlin/moveTopLevelDeclarations/movePropertyToPackage/after/c/onDemandImport.kt

@@ -1,8 +1,9 @@
 package c
 
 import a.*
+import b.test
 
 fun bar() {
-    b.test = ""
-    println(b.test)
+    test = ""
+    println(test)
 }

idea/testData/shortenRefs/imports/importGlobalCallables.dependency.kt

@@ -0,0 +1,4 @@
+package p
+
+fun globalFun(p: Int) {}
+val globalVal: Int = 1

idea/testData/shortenRefs/imports/importGlobalCallables.kt

@@ -0,0 +1,3 @@
+fun foo() {
+    <selection>p.globalFun(p.globalVal)</selection>
+}

idea/testData/shortenRefs/imports/importGlobalCallables.kt.after

@@ -0,0 +1,6 @@
+import p.globalVal
+import p.globalFun
+
+fun foo() {
+    globalFun(globalVal)
+}

idea/tests/org/jetbrains/kotlin/shortenRefs/ShortenRefsTestGenerated.java

@@ -174,6 +174,12 @@ public class ShortenRefsTestGenerated extends AbstractShortenRefsTest {
             JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/shortenRefs/imports"), Pattern.compile("^([^\\.]+)\\.kt$"), true);
         }
 
+        @TestMetadata("importGlobalCallables.kt")
+        public void testImportGlobalCallables() throws Exception {
+            String fileName = JetTestUtils.navigationMetadata("idea/testData/shortenRefs/imports/importGlobalCallables.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("leaveQualifiedConstructor.kt")
         public void testLeaveQualifiedConstructor() throws Exception {
             String fileName = JetTestUtils.navigationMetadata("idea/testData/shortenRefs/imports/leaveQualifiedConstructor.kt");