Perform shorten references under modal dialog

To prevent freezes and show progress to user shorten references has to be performed in a background thread

^KT-42170 Fixed

idea/idea-analysis/src/org/jetbrains/kotlin/idea/codeInsight/shorten/delayedRequestsWaitingSet.kt

@@ -16,17 +16,20 @@
 
 package org.jetbrains.kotlin.idea.codeInsight.shorten
 
-import com.intellij.openapi.application.ApplicationManager
+import com.intellij.openapi.application.runReadAction
 import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.util.Key
 import com.intellij.psi.*
+import com.intellij.psi.codeStyle.CodeStyleManager
 import org.jetbrains.kotlin.asJava.unwrapped
 import org.jetbrains.kotlin.idea.caches.resolve.unsafeResolveToDescriptor
 import org.jetbrains.kotlin.idea.caches.resolve.util.getJavaMemberDescriptor
 import org.jetbrains.kotlin.idea.core.ShortenReferences
 import org.jetbrains.kotlin.idea.core.ShortenReferences.Options
 import org.jetbrains.kotlin.idea.util.ImportInsertHelper
+import org.jetbrains.kotlin.idea.util.application.assertIsDispatchThread
+import org.jetbrains.kotlin.idea.util.application.runWriteAction
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.createSmartPointer
 import org.jetbrains.kotlin.psi.psiUtil.forEachDescendantOfType
@@ -73,14 +76,14 @@ private fun Project.getOrCreateRefactoringRequests(): MutableSet<DelayedRefactor
 }
 
 fun KtElement.addToShorteningWaitSet(options: Options = Options.DEFAULT) {
-    assert(ApplicationManager.getApplication()!!.isWriteAccessAllowed) { "Write access needed" }
+    assertIsDispatchThread()
     val project = project
     val elementPointer = SmartPointerManager.getInstance(project).createSmartPsiElementPointer(this)
     project.getOrCreateRefactoringRequests().add(ShorteningRequest(elementPointer, options))
 }
 
 fun addDelayedImportRequest(elementToImport: PsiElement, file: KtFile) {
-    assert(ApplicationManager.getApplication()!!.isWriteAccessAllowed) { "Write access needed" }
+    assertIsDispatchThread()
     file.project.getOrCreateRefactoringRequests() += ImportRequest(elementToImport.createSmartPointer(), file.createSmartPointer())
 }
 
@@ -98,18 +101,29 @@ fun performDelayedRefactoringRequests(project: Project) {
             }
         }
 
-        val elementToOptions = shorteningRequests.mapNotNull { req -> req.pointer.element?.let { it to req.options } }.toMap()
+        val elementToOptions = shorteningRequests.mapNotNull { req ->
+            runReadAction { req.pointer.element }?.let { it to req.options }
+        }.toMap()
         val elements = elementToOptions.keys
+        val files = runReadAction { elements.map(KtElement::getContainingKtFile).toSet() }
         //TODO: this is not correct because it should not shorten deep into the elements!
         ShortenReferences { elementToOptions[it] ?: Options.DEFAULT }.process(elements)
 
+        files.forEach { file ->
+            file.importList?.let { importList ->
+                runWriteAction {
+                    CodeStyleManager.getInstance(file.project).reformat(importList, true)
+                }
+            }
+        }
+
         val importInsertHelper = ImportInsertHelper.getInstance(project)
 
         for ((file, requestsForFile) in importRequests.groupBy { it.filePointer.element }) {
             if (file == null) continue
 
             for (requestForFile in requestsForFile) {
-                val elementToImport = requestForFile.elementToImportPointer.element?.unwrapped ?: continue
+                val elementToImport = runReadAction { requestForFile.elementToImportPointer.element }?.unwrapped ?: continue
                 val descriptorToImport = when (elementToImport) {
                     is KtDeclaration -> elementToImport.unsafeResolveToDescriptor(BodyResolveMode.PARTIAL)
                     is PsiMember -> elementToImport.getJavaMemberDescriptor()

idea/idea-analysis/src/org/jetbrains/kotlin/idea/references/KtSimpleNameReference.kt

@@ -8,7 +8,6 @@ package org.jetbrains.kotlin.idea.references
 import com.intellij.openapi.extensions.Extensions
 import com.intellij.openapi.util.TextRange
 import com.intellij.psi.PsiElement
-import com.intellij.psi.PsiReference
 import com.intellij.psi.util.PsiTreeUtil
 import com.intellij.util.IncorrectOperationException
 import com.intellij.util.SmartList
@@ -21,6 +20,7 @@ import org.jetbrains.kotlin.idea.codeInsight.shorten.addToShorteningWaitSet
 import org.jetbrains.kotlin.idea.core.*
 import org.jetbrains.kotlin.idea.intentions.OperatorToFunctionIntention
 import org.jetbrains.kotlin.idea.refactoring.fqName.getKotlinFqName
+import org.jetbrains.kotlin.idea.util.application.disablePostprocessFormattingInside
 import org.jetbrains.kotlin.lexer.KtToken
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.load.java.descriptors.JavaPropertyDescriptor
@@ -171,15 +171,18 @@ class KtSimpleNameReferenceDescriptorsImpl(
         if (expression !is KtNameReferenceExpression) return expression
         if (expression.parent is KtThisExpression || expression.parent is KtSuperExpression) return expression // TODO: it's a bad design of PSI tree, we should change it
 
-        val newExpression = expression.changeQualifiedName(
+        val newExpression =
-            fqName.quoteIfNeeded().let {
+            expression.project.disablePostprocessFormattingInside {
-                if (shorteningMode == ShorteningMode.NO_SHORTENING)
+                expression.changeQualifiedName(
-                    it
+                    fqName.quoteIfNeeded().let {
-                else
+                        if (shorteningMode == ShorteningMode.NO_SHORTENING)
-                    it.withRootPrefixIfNeeded(expression)
+                            it
-            },
+                        else
-            targetElement
+                            it.withRootPrefixIfNeeded(expression)
-        )
+                    },
+                    targetElement
+                )
+            }
         val newQualifiedElement = newExpression.getQualifiedElementOrCallableRef()
 
         if (shorteningMode == ShorteningMode.NO_SHORTENING) return newExpression

idea/idea-core/resources/messages/KotlinIdeaCoreBundle.properties

@@ -15,3 +15,5 @@ text.loading.kotlin.script.definitions=Loading kotlin script definitions
 text.loading.kotlin.script.configuration=Loading script configuration
 implement.members.handler.title=Implement Members
 override.members.handler.title=Override Members
+shorten.references=Shortening references
+shorten.references.pass.0=Shortening references, pass {0}

idea/idea-core/src/org/jetbrains/kotlin/idea/core/ShortenReferences.kt

@@ -5,22 +5,27 @@
 
 package org.jetbrains.kotlin.idea.core
 
-import com.intellij.openapi.application.runReadAction
+import com.intellij.openapi.diagnostic.Logger
+import com.intellij.openapi.progress.ProgressManager
 import com.intellij.openapi.util.TextRange
 import com.intellij.psi.PsiDocumentManager
 import com.intellij.psi.PsiElement
 import com.intellij.psi.SmartPsiElementPointer
-import com.intellij.psi.impl.source.PostprocessReformattingAspect
 import com.intellij.psi.util.PsiTreeUtil
 import org.jetbrains.kotlin.descriptors.*
 import org.jetbrains.kotlin.idea.analysis.analyzeAsReplacement
 import org.jetbrains.kotlin.idea.caches.resolve.allowResolveInDispatchThread
 import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
+import org.jetbrains.kotlin.idea.core.util.KotlinIdeaCoreBundle
 import org.jetbrains.kotlin.idea.imports.canBeReferencedViaImport
 import org.jetbrains.kotlin.idea.imports.getImportableTargets
 import org.jetbrains.kotlin.idea.references.mainReference
 import org.jetbrains.kotlin.idea.references.resolveToDescriptors
 import org.jetbrains.kotlin.idea.util.*
+import org.jetbrains.kotlin.idea.util.application.disablePostprocessFormattingInside
+import org.jetbrains.kotlin.idea.util.application.isWriteAccessAllowed
+import org.jetbrains.kotlin.idea.util.application.runReadAction
+import org.jetbrains.kotlin.idea.util.application.runWriteAction
 import org.jetbrains.kotlin.incremental.components.NoLookupLocation
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.*
@@ -54,6 +59,8 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT
     }
 
     companion object {
+        private val LOG = Logger.getInstance(ShortenReferences::class.java)
+
         @JvmField
         val DEFAULT = ShortenReferences()
 
@@ -190,7 +197,8 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT
         //TODO: that's not correct since we have options!
         val elementsToUse = dropNestedElements(elements)
 
-        val helper = ImportInsertHelper.getInstance(file.project)
+        val project = file.project
+        val helper = ImportInsertHelper.getInstance(project)
 
         val failedToImportDescriptors = LinkedHashSet<DeclarationDescriptor>()
 
@@ -202,7 +210,9 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT
             }
         }
 
+        var pass = 0
         while (true) {
+            pass++
             // Processors order is important here so that enclosing elements are not shortened before their children are, e.g.
             // test.foo(this@A) -> foo(this)
             val processors: List<ShorteningProcessor<*>> = runReadAction {
@@ -214,24 +224,41 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT
                 )
             }
 
-            // step 1: collect qualified elements to analyze (no resolve at this step)
+            val step1And2 = {
-            val visitors = processors.map { it.collectElementsVisitor }
+                // step 1: collect qualified elements to analyze (no resolve at this step)
-            runReadAction {
+                val visitors = processors.map { it.collectElementsVisitor }
-                for (visitor in visitors) {
+                runReadAction {
-                    for (element in elementsToUse) {
+                    for (visitor in visitors) {
-                        visitor.options = options(element)
+                        for (element in elementsToUse) {
-                        element.accept(visitor)
+                            visitor.options = options(element)
+                            element.accept(visitor)
+                        }
                     }
+
+                    // step 2: analyze collected elements with resolve and decide which can be shortened now and which need descriptors to be imported before shortening
+                    val allElementsToAnalyze = visitors.flatMap { visitor -> visitor.getElementsToAnalyze().map { it.element } }.toSet()
+
+                    val bindingContext =
+                        if (isWriteAccessAllowed()) {
+                            allowResolveInDispatchThread {
+                                file.getResolutionFacade().analyze(allElementsToAnalyze, BodyResolveMode.PARTIAL_WITH_CFA)
+                            }
+                        } else {
+                            file.getResolutionFacade().analyze(allElementsToAnalyze, BodyResolveMode.PARTIAL_WITH_CFA)
+                        }
+
+
+                    processors.forEach { it.analyzeCollectedElements(bindingContext) }
                 }
+            }
 
-
+            if (isWriteAccessAllowed()) {
-                // step 2: analyze collected elements with resolve and decide which can be shortened now and which need descriptors to be imported before shortening
+                LOG.warn("Shorten reference should not be invoked under write action")
-                val allElementsToAnalyze = visitors.flatMap { visitor -> visitor.getElementsToAnalyze().map { it.element } }.toSet()
+                step1And2.invoke()
-                val bindingContext = allowResolveInDispatchThread {
+            } else {
-                    file.getResolutionFacade().analyze(allElementsToAnalyze, BodyResolveMode.PARTIAL_WITH_CFA)
+                ProgressManager.getInstance().runProcessWithProgressSynchronously(
-                }
+                    step1And2, KotlinIdeaCoreBundle.message("shorten.references.pass.0", pass), false, project
-
+                )
-                processors.forEach { it.analyzeCollectedElements(bindingContext) }
             }
 
             // step 3: shorten elements that can be shortened right now
@@ -247,7 +274,10 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT
                 for (descriptor in descriptorsToImport) {
                     assert(descriptor !in failedToImportDescriptors)
 
-                    val result = helper.importDescriptor(file, descriptor)
+                    val result =
+                        runWriteAction {
+                            helper.importDescriptor(file, descriptor)
+                        }
                     if (result != ImportDescriptorResult.ALREADY_IMPORTED) {
                         anyChange = true
                     }
@@ -387,10 +417,9 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT
                 val element = elementPointer.element ?: continue
                 if (!element.isValid) continue
 
-                var newElement: KtElement? = null
                 // we never want any reformatting to happen because sometimes it causes strange effects (see KT-11633)
-                PostprocessReformattingAspect.getInstance(element.project).disablePostprocessFormattingInside {
+                val newElement = element.project.disablePostprocessFormattingInside {
-                    newElement = shortenElement(element, options(element))
+                    runWriteAction { shortenElement(element, options(element)) }
                 }
 
                 if (element in elementSetToUpdate && newElement != element) {
@@ -402,8 +431,10 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT
 
         fun removeRootPrefixes() {
             for (pointer in collectElementsVisitor.getElementsWithRootPrefix()) {
-                val element = pointer.element ?: continue
+                val element = runReadAction { pointer.element } ?: continue
-                shortenElement(element, Options.DEFAULT)
+                runWriteAction {
+                    shortenElement(element, Options.DEFAULT)
+                }
             }
         }
 

idea/idea-frontend-independent/src/org/jetbrains/kotlin/idea/util/ApplicationUtils.kt

@@ -11,6 +11,8 @@ import com.intellij.openapi.components.ComponentManager
 import com.intellij.openapi.progress.impl.CancellationCheck
 import com.intellij.openapi.project.DumbService
 import com.intellij.openapi.project.Project
+import com.intellij.openapi.util.Computable
+import com.intellij.psi.impl.source.PostprocessReformattingAspect
 
 fun <T> runReadAction(action: () -> T): T {
     return ApplicationManager.getApplication().runReadAction<T>(action)
@@ -53,6 +55,15 @@ inline fun invokeLater(crossinline action: () -> Unit) =
     ApplicationManager.getApplication().invokeLater { action() }
 
 inline fun isUnitTestMode(): Boolean = ApplicationManager.getApplication().isUnitTestMode
+inline fun isReadAccessAllowed() = ApplicationManager.getApplication().isReadAccessAllowed
+inline fun isWriteAccessAllowed() = ApplicationManager.getApplication().isWriteAccessAllowed
+inline fun isDispatchThread() = ApplicationManager.getApplication().isDispatchThread
+inline fun assertWriteAccessAllowed() = ApplicationManager.getApplication().assertWriteAccessAllowed()
+inline fun assertReadAccessAllowed() = ApplicationManager.getApplication().assertReadAccessAllowed()
+inline fun assertIsDispatchThread() = ApplicationManager.getApplication().assertIsDispatchThread()
+
+inline fun <reified T : Any> Project.disablePostprocessFormattingInside(action: Computable<T>): T =
+    PostprocessReformattingAspect.getInstance(this).disablePostprocessFormattingInside(action)
 
 inline fun <reified T : Any> ComponentManager.getServiceSafe(): T =
     this.getService(T::class.java) ?: error("Unable to locate service ${T::class.java.name}")
@@ -61,3 +72,4 @@ fun <T> Project.runReadActionInSmartMode(action: () -> T): T {
     if (ApplicationManager.getApplication().isReadAccessAllowed) return action()
     return DumbService.getInstance(this).runReadActionInSmartMode<T>(action)
 }
+

idea/idea-frontend-independent/src/org/jetbrains/kotlin/idea/util/ApplicationUtils.kt.193

@@ -11,6 +11,8 @@ import com.intellij.openapi.components.ComponentManager
 import com.intellij.openapi.project.Project
 import org.jetbrains.annotations.Nls
 import com.intellij.openapi.project.DumbService
+import com.intellij.openapi.util.Computable
+import com.intellij.psi.impl.source.PostprocessReformattingAspect
 
 fun <T> runReadAction(action: () -> T): T {
     return ApplicationManager.getApplication().runReadAction<T>(action)
@@ -44,6 +46,17 @@ inline fun invokeLater(crossinline action: () -> Unit) =
     ApplicationManager.getApplication().invokeLater { action() }
 
 inline fun isUnitTestMode(): Boolean = ApplicationManager.getApplication().isUnitTestMode
+inline fun isReadAccessAllowed() = ApplicationManager.getApplication().isReadAccessAllowed
+inline fun isWriteAccessAllowed() = ApplicationManager.getApplication().isWriteAccessAllowed
+inline fun isDispatchThread() = ApplicationManager.getApplication().isDispatchThread
+inline fun assertWriteAccessAllowed() =
+    assert(isWriteAccessAllowed()) { "Write access is allowed inside write-action only (see com.intellij.openapi.application.Application.runWriteAction())" }
+inline fun assertReadAccessAllowed() =
+    assert(isReadAccessAllowed()) { "Read access is allowed from event dispatch thread or inside read-action only (see com.intellij.openapi.application.Application.runReadAction())" }
+inline fun assertIsDispatchThread() = ApplicationManager.getApplication().assertIsDispatchThread()
+
+inline fun <reified T : Any> Project.disablePostprocessFormattingInside(action: Computable<T>): T =
+    PostprocessReformattingAspect.getInstance(this).disablePostprocessFormattingInside(action)
 
 inline fun <reified T : Any> ComponentManager.getServiceSafe(): T =
     this.getService(T::class.java) ?: error("Unable to locate service ${T::class.java.name}")

idea/idea-frontend-independent/src/org/jetbrains/kotlin/idea/util/ApplicationUtils.kt.as40

@@ -25,6 +25,8 @@ import com.intellij.openapi.progress.ProgressIndicatorProvider
 import com.intellij.openapi.progress.ProgressManager
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.project.DumbService
+import com.intellij.openapi.util.Computable
+import com.intellij.psi.impl.source.PostprocessReformattingAspect
 
 fun <T> runReadAction(action: () -> T): T {
     return ApplicationManager.getApplication().runReadAction<T>(action)
@@ -58,6 +60,17 @@ inline fun invokeLater(crossinline action: () -> Unit) =
     ApplicationManager.getApplication().invokeLater { action() }
 
 inline fun isUnitTestMode(): Boolean = ApplicationManager.getApplication().isUnitTestMode
+inline fun isReadAccessAllowed() = ApplicationManager.getApplication().isReadAccessAllowed
+inline fun isWriteAccessAllowed() = ApplicationManager.getApplication().isWriteAccessAllowed
+inline fun isDispatchThread() = ApplicationManager.getApplication().isDispatchThread
+inline fun assertWriteAccessAllowed() =
+    assert(isWriteAccessAllowed()) { "Write access is allowed inside write-action only (see com.intellij.openapi.application.Application.runWriteAction())" }
+inline fun assertReadAccessAllowed() =
+    assert(isReadAccessAllowed()) { "Read access is allowed from event dispatch thread or inside read-action only (see com.intellij.openapi.application.Application.runReadAction())" }
+inline fun assertIsDispatchThread() = ApplicationManager.getApplication().assertIsDispatchThread()
+
+inline fun <reified T : Any> Project.disablePostprocessFormattingInside(action: Computable<T>): T =
+    PostprocessReformattingAspect.getInstance(this).disablePostprocessFormattingInside(action)
 
 inline fun <reified T : Any> ComponentManager.getServiceSafe(): T =
     this.getService(T::class.java) ?: error("Unable to locate service ${T::class.java.name}")

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

@@ -9,6 +9,7 @@ import com.intellij.codeInsight.CodeInsightSettings
 import com.intellij.codeInsight.editorActions.CopyPastePostProcessor
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.application.ReadAction
+import com.intellij.openapi.command.CommandProcessor
 import com.intellij.openapi.diagnostic.ControlFlowException
 import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.editor.Editor
@@ -43,10 +44,8 @@ import org.jetbrains.kotlin.idea.core.util.start
 import org.jetbrains.kotlin.idea.imports.importableFqName
 import org.jetbrains.kotlin.idea.references.*
 import org.jetbrains.kotlin.idea.util.*
-import org.jetbrains.kotlin.idea.util.application.executeWriteCommand
+import org.jetbrains.kotlin.idea.util.application.*
-import org.jetbrains.kotlin.idea.util.application.invokeLater
+import org.jetbrains.kotlin.idea.util.runReadActionInSmartMode
-import org.jetbrains.kotlin.idea.util.application.isUnitTestMode
-import org.jetbrains.kotlin.idea.util.application.runReadAction
 import org.jetbrains.kotlin.incremental.components.NoLookupLocation
 import org.jetbrains.kotlin.kdoc.psi.api.KDocElement
 import org.jetbrains.kotlin.name.FqName
@@ -376,12 +375,10 @@ class KotlinCopyPasteReferenceProcessor : CopyPastePostProcessor<BasicKotlinRefe
                 showRestoreReferencesDialog(project, referencesPossibleToRestore)
             if (selectedReferencesToRestore.isEmpty()) return@invokeLater
 
-            project.executeWriteCommand(KotlinBundle.message("resolve.pasted.references")) {
+            val imported = TreeSet<String>()
-                val imported = TreeSet<String>()
+            restoreReferences(selectedReferencesToRestore, file, imported)
-                restoreReferences(selectedReferencesToRestore, file, imported)
 
-                reviewAddedImports(project, editor, file, imported)
+            reviewAddedImports(project, editor, file, imported)
-            }
         }
     }
 
@@ -689,8 +686,9 @@ class KotlinCopyPasteReferenceProcessor : CopyPastePostProcessor<BasicKotlinRefe
         file: KtFile,
         imported: MutableSet<String>
     ) {
-        val importHelper = ImportInsertHelper.getInstance(file.project)
+        val project = file.project
-        val smartPointerManager = SmartPointerManager.getInstance(file.project)
+        val importHelper = ImportInsertHelper.getInstance(project)
+        val smartPointerManager = SmartPointerManager.getInstance(project)
 
         data class BindingRequest(
             val pointer: SmartPsiElementPointer<KtSimpleNameExpression>,
@@ -717,18 +715,22 @@ class KotlinCopyPasteReferenceProcessor : CopyPastePostProcessor<BasicKotlinRefe
             }
         }
 
-        for (descriptor in descriptorsToImport) {
+        project.executeWriteCommand(KotlinBundle.message("resolve.pasted.references")) {
-            importHelper.importDescriptor(file, descriptor)
+            for (descriptor in descriptorsToImport) {
-            descriptor.getImportableDescriptor().importableFqName?.let { imported.add(it.asString()) }
+                importHelper.importDescriptor(file, descriptor)
-        }
+                descriptor.getImportableDescriptor().importableFqName?.let { imported.add(it.asString()) }
+            }
 
-        for ((pointer, fqName) in bindingRequests) {
+            for ((pointer, fqName) in bindingRequests) {
-            pointer.element?.mainReference?.let {
+                pointer.element?.mainReference?.let {
-                it.bindToFqName(fqName, KtSimpleNameReference.ShorteningMode.DELAYED_SHORTENING)
+                    it.bindToFqName(fqName, KtSimpleNameReference.ShorteningMode.DELAYED_SHORTENING)
-                imported.add(fqName.asString())
+                    imported.add(fqName.asString())
+                }
             }
         }
-        performDelayedRefactoringRequests(file.project)
+        CommandProcessor.getInstance().runUndoTransparentAction {
+            performDelayedRefactoringRequests(file.project)
+        }
     }
 
     private fun findImportableDescriptors(fqName: FqName, file: KtFile): Collection<DeclarationDescriptor> {