Move Member out of Companion: Improve conflict checking

Also do not show conflict dialog under write action

idea/src/org/jetbrains/kotlin/idea/intentions/MoveMemberOutOfCompanionObjectIntention.kt

@@ -24,19 +24,28 @@ import com.intellij.refactoring.util.RefactoringUIUtil
 import com.intellij.util.containers.MultiMap
 import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor
 import org.jetbrains.kotlin.descriptors.ClassDescriptor
+import org.jetbrains.kotlin.idea.caches.resolve.analyzeFully
 import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny
 import org.jetbrains.kotlin.idea.codeInsight.DescriptorToSourceUtilsIde
 import org.jetbrains.kotlin.idea.refactoring.checkConflictsInteractively
+import org.jetbrains.kotlin.idea.refactoring.getUsageContext
 import org.jetbrains.kotlin.idea.refactoring.move.moveDeclarations.*
 import org.jetbrains.kotlin.idea.runSynchronouslyWithProgress
+import org.jetbrains.kotlin.idea.util.application.runReadAction
 import org.jetbrains.kotlin.idea.util.application.runWriteAction
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.containingClassOrObject
-import org.jetbrains.kotlin.psi.psiUtil.isAncestor
+import org.jetbrains.kotlin.psi.psiUtil.parents
+import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
+import org.jetbrains.kotlin.resolve.descriptorUtil.isSubclassOf
+import org.jetbrains.kotlin.resolve.scopes.receivers.ImplicitClassReceiver
 import org.jetbrains.kotlin.util.findCallableMemberBySignature
 
 class MoveMemberOutOfCompanionObjectIntention : SelfTargetingRangeIntention<KtNamedDeclaration>(KtNamedDeclaration::class.java,
                                                                                                 "Move out of companion object") {
+    override fun startInWriteAction() = false
+
     override fun applicabilityRange(element: KtNamedDeclaration): TextRange? {
         if (element !is KtNamedFunction && element !is KtProperty && element !is KtClassOrObject) return null
         val container = element.containingClassOrObject
@@ -62,30 +71,52 @@ class MoveMemberOutOfCompanionObjectIntention : SelfTargetingRangeIntention<KtNa
                                                             listOf(element),
                                                             KotlinMoveTargetForExistingElement(targetClass),
                                                             MoveDeclarationsDelegate.NestedClass())
-            MoveKotlinDeclarationsProcessor(moveDescriptor).run()
+            runWriteAction {
-            deleteCompanionIfEmpty()
+                MoveKotlinDeclarationsProcessor(moveDescriptor).run()
+                deleteCompanionIfEmpty()
+            }
             return
         }
 
-        val externalRefs = project.runSynchronouslyWithProgress("Searching for ${element.name}", true) {
+        val targetClassDescriptor = runReadAction { targetClass.resolveToDescriptor() as ClassDescriptor }
-            ReferencesSearch.search(element).filter {
+
-                val refElement = it.element ?: return@filter false
+        val conflicts = MultiMap<PsiElement, String>()
-                !targetClass.isAncestor(refElement) || companionObject.isAncestor(refElement)
+
+        val refsRequiringClassInstance = project.runSynchronouslyWithProgress("Searching for ${element.name}", true) {
+            runReadAction {
+                ReferencesSearch
+                        .search(element)
+                        .mapNotNull { it.element }
+                        .filter {
+                            if (it !is KtElement) return@filter true
+                            val resolvedCall = it.getResolvedCall(it.analyzeFully()) ?: return@filter false
+                            val dispatchReceiver = resolvedCall.dispatchReceiver ?: return@filter false
+                            if (dispatchReceiver !is ImplicitClassReceiver) return@filter true
+                            it.parents
+                                    .filterIsInstance<KtClassOrObject>()
+                                    .none {
+                                        val classDescriptor = it.resolveToDescriptorIfAny() as? ClassDescriptor
+                                        if (classDescriptor != null && classDescriptor.isSubclassOf(targetClassDescriptor)) return@none true
+                                        if (it.isTopLevel() || it is KtObjectDeclaration || (it is KtClass && !it.isInner())) return@filter true
+                                        false
+                                    }
+                        }
             }
         } ?: return
 
-        val conflicts = MultiMap<PsiElement, String>()
+        for (refElement in refsRequiringClassInstance) {
-        for (ref in externalRefs) {
+            val context = refElement.getUsageContext()
-            val refElement = ref.element ?: continue
+            val message = "'${refElement.text}' in ${RefactoringUIUtil.getDescription(context, false)} will require class instance"
-            conflicts.putValue(refElement, "Class instance required: ${refElement.text}")
+            conflicts.putValue(refElement, message)
         }
 
-        val targetClassDescriptor = targetClass.resolveToDescriptor() as ClassDescriptor
+        runReadAction {
-        val callableDescriptor = element.resolveToDescriptor() as CallableMemberDescriptor
+            val callableDescriptor = element.resolveToDescriptor() as CallableMemberDescriptor
-        targetClassDescriptor.findCallableMemberBySignature(callableDescriptor)?.let {
+            targetClassDescriptor.findCallableMemberBySignature(callableDescriptor)?.let {
-            DescriptorToSourceUtilsIde.getAnyDeclaration(project, it)
+                DescriptorToSourceUtilsIde.getAnyDeclaration(project, it)
-        }?.let {
+            }?.let {
-            conflicts.putValue(it, "Class '${targetClass.name}' already contains ${RefactoringUIUtil.getDescription(it, false)}")
+                conflicts.putValue(it, "Class '${targetClass.name}' already contains ${RefactoringUIUtil.getDescription(it, false)}")
+            }
         }
 
         project.checkConflictsInteractively(conflicts) {

idea/testData/intentions/moveOutOfCompanion/companionAsImplicitReceiver.kt

@@ -0,0 +1,46 @@
+// WITH_RUNTIME
+// SHOULD_FAIL_WITH: 'FOO' in property bar2 will require class instance, 'FOO' in property bar4 will require class instance, 'FOO' in property bar6 will require class instance, 'FOO' in variable bar7 will require class instance, 'FOO' in variable bar8 will require class instance
+package test
+
+open class A {
+    companion object {
+        val <caret>FOO = 1
+    }
+}
+
+class B : A() {
+    val bar1 = FOO
+
+    class X {
+        val bar2 = FOO
+    }
+
+    class W : A() {
+        val bar3 = FOO
+    }
+
+    object Y {
+        val bar4 = FOO
+    }
+
+    inner class Z {
+        val bar5 = FOO
+    }
+
+    companion object {
+        val bar6 = FOO
+    }
+}
+
+class C {
+    inner class D : A() {
+        val bar9 = FOO
+    }
+}
+
+fun foo() {
+    val bar7 = A.FOO
+    with(A) {
+        val bar8 = FOO
+    }
+}

idea/testData/intentions/moveOutOfCompanion/moveFunctionWithExternalRefs.kt

@@ -1,5 +1,5 @@
 // WITH_RUNTIME
-// SHOULD_FAIL_WITH: Class instance required: foo, Class instance required: foo, Class instance required: foo, Class instance required: foo
+// SHOULD_FAIL_WITH: 'foo' in class B will require class instance, 'foo' in function test() will require class instance, 'foo' in function test() will require class instance, 'foo' in lambda <anonymous>() on A.Companion will require class instance
 class A {
     companion object {
         class B {

idea/testData/intentions/moveOutOfCompanion/movePropertyWithExternalRefs.kt

@@ -1,5 +1,5 @@
 // WITH_RUNTIME
-// SHOULD_FAIL_WITH: Class instance required: foo, Class instance required: foo, Class instance required: foo, Class instance required: foo
+// SHOULD_FAIL_WITH: 'foo' in class B will require class instance, 'foo' in function test() will require class instance, 'foo' in function test() will require class instance, 'foo' in lambda <anonymous>() on A.Companion will require class instance
 class A {
     companion object {
         class B {

idea/tests/org/jetbrains/kotlin/idea/intentions/AbstractIntentionTest.kt

@@ -169,7 +169,7 @@ abstract class AbstractIntentionTest : KotlinLightCodeInsightFixtureTestCase() {
             TestCase.assertEquals("Expected test to fail.", "", shouldFailString)
         }
         catch (e: BaseRefactoringProcessor.ConflictsInTestsException) {
-            TestCase.assertEquals("Failure message mismatch.", shouldFailString, StringUtil.join(e.messages, ", "))
+            TestCase.assertEquals("Failure message mismatch.", shouldFailString, StringUtil.join(e.messages.sorted(), ", "))
         }
         catch (e: CommonRefactoringUtil.RefactoringErrorHintException) {
             TestCase.assertEquals("Failure message mismatch.", shouldFailString, e.message?.replace('\n', ' '))

idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java

@@ -11178,6 +11178,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
             KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/intentions/moveOutOfCompanion"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), TargetBackend.ANY, true);
         }
 
+        @TestMetadata("companionAsImplicitReceiver.kt")
+        public void testCompanionAsImplicitReceiver() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/moveOutOfCompanion/companionAsImplicitReceiver.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("moveAndDropCompanion.kt")
         public void testMoveAndDropCompanion() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/moveOutOfCompanion/moveAndDropCompanion.kt");