From d825428718c15f20bb672d4e09778dd82a15a36e Mon Sep 17 00:00:00 2001 From: Ilya Kirillov Date: Thu, 13 Feb 2020 16:51:40 +0300 Subject: [PATCH] New J2K : move resolve resolve out of EDT in shorten references processing relates to #KT-31812 --- .../kotlin/idea/util/ActionRunningMode.kt | 26 +++++ .../kotlin/idea/util/ImportInsertHelper.kt | 1 + .../kotlin/idea/core/ShortenReferences.kt | 108 +++++++++++------- .../kotlin/idea/codeInliner/CodeInliner.kt | 2 +- .../ScopeFunctionConversionInspection.kt | 7 +- ...bsoleteExperimentalCoroutinesInspection.kt | 3 +- .../idea/util/ImportInsertHelperImpl.kt | 21 ++-- .../processings/ShortenReferenceProcessing.kt | 17 ++- 8 files changed, 126 insertions(+), 59 deletions(-) create mode 100644 idea/ide-common/src/org/jetbrains/kotlin/idea/util/ActionRunningMode.kt diff --git a/idea/ide-common/src/org/jetbrains/kotlin/idea/util/ActionRunningMode.kt b/idea/ide-common/src/org/jetbrains/kotlin/idea/util/ActionRunningMode.kt new file mode 100644 index 00000000000..d759bc3e1ca --- /dev/null +++ b/idea/ide-common/src/org/jetbrains/kotlin/idea/util/ActionRunningMode.kt @@ -0,0 +1,26 @@ +/* + * Copyright 2010-2020 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.idea.util + +import com.intellij.openapi.application.ApplicationManager + +enum class ActionRunningMode { + RUN_IN_CURRENT_THREAD, + RUN_IN_EDT +} + +inline fun ActionRunningMode.runAction(crossinline action: () -> T): T = when (this) { + ActionRunningMode.RUN_IN_CURRENT_THREAD -> { + action() + } + ActionRunningMode.RUN_IN_EDT -> { + var result: T? = null + ApplicationManager.getApplication().invokeAndWait { + result = ApplicationManager.getApplication().runWriteAction { action() } + } + result!! + } +} \ No newline at end of file diff --git a/idea/ide-common/src/org/jetbrains/kotlin/idea/util/ImportInsertHelper.kt b/idea/ide-common/src/org/jetbrains/kotlin/idea/util/ImportInsertHelper.kt index a6eefab4777..95ba30b4504 100644 --- a/idea/ide-common/src/org/jetbrains/kotlin/idea/util/ImportInsertHelper.kt +++ b/idea/ide-common/src/org/jetbrains/kotlin/idea/util/ImportInsertHelper.kt @@ -25,6 +25,7 @@ abstract class ImportInsertHelper { abstract fun importDescriptor( file: KtFile, descriptor: DeclarationDescriptor, + actionRunningMode: ActionRunningMode = ActionRunningMode.RUN_IN_CURRENT_THREAD, forceAllUnderImport: Boolean = false ): ImportDescriptorResult diff --git a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/ShortenReferences.kt b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/ShortenReferences.kt index 77375d2b190..5a249ad15e5 100644 --- a/idea/idea-core/src/org/jetbrains/kotlin/idea/core/ShortenReferences.kt +++ b/idea/idea-core/src/org/jetbrains/kotlin/idea/core/ShortenReferences.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.idea.core +import com.intellij.openapi.application.runReadAction import com.intellij.openapi.util.TextRange import com.intellij.psi.PsiDocumentManager import com.intellij.psi.PsiElement @@ -19,7 +20,6 @@ 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.util.* -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.* @@ -95,19 +95,31 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT } @JvmOverloads - fun process(element: KtElement, elementFilter: (PsiElement) -> FilterResult = { FilterResult.PROCESS }): KtElement { - return process(listOf(element), elementFilter).single() + fun process( + element: KtElement, + elementFilter: (PsiElement) -> FilterResult = { FilterResult.PROCESS }, + actionRunningMode: ActionRunningMode = ActionRunningMode.RUN_IN_CURRENT_THREAD + ): KtElement { + return process(listOf(element), elementFilter, actionRunningMode).single() } @JvmOverloads - fun process(file: KtFile, startOffset: Int, endOffset: Int, additionalFilter: (PsiElement) -> FilterResult = { FilterResult.PROCESS }) { - val documentManager = PsiDocumentManager.getInstance(file.project) - val document = file.viewProvider.document!! - check(documentManager.isCommitted(document)) { "Document should be committed to shorten references in range" } - - val rangeMarker = document.createRangeMarker(startOffset, endOffset) - rangeMarker.isGreedyToLeft = true - rangeMarker.isGreedyToRight = true + fun process( + file: KtFile, + startOffset: Int, + endOffset: Int, + additionalFilter: (PsiElement) -> FilterResult = { FilterResult.PROCESS }, + actionRunningMode: ActionRunningMode = ActionRunningMode.RUN_IN_CURRENT_THREAD + ) { + val rangeMarker = runReadAction { + val documentManager = PsiDocumentManager.getInstance(file.project) + val document = file.viewProvider.document!! + check(documentManager.isCommitted(document)) { "Document should be committed to shorten references in range" } + document.createRangeMarker(startOffset, endOffset).apply { + isGreedyToLeft = true + isGreedyToRight = true + } + } val rangeFilter = { element: PsiElement -> if (rangeMarker.isValid) { @@ -136,11 +148,12 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT } } try { - process(listOf(file)) { element -> + val filter = { element: PsiElement -> minOf(rangeFilter(element), additionalFilter(element)) } + process(listOf(file), filter, actionRunningMode) } finally { - rangeMarker.dispose() + runReadAction { rangeMarker.dispose() } } } @@ -153,16 +166,18 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT @JvmOverloads fun process( elements: Iterable, - elementFilter: (PsiElement) -> FilterResult = { FilterResult.PROCESS } + elementFilter: (PsiElement) -> FilterResult = { FilterResult.PROCESS }, + actionRunningMode: ActionRunningMode = ActionRunningMode.RUN_IN_CURRENT_THREAD ): Collection { - return elements.groupBy(KtElement::getContainingKtFile) - .flatMap { shortenReferencesInFile(it.key, it.value, elementFilter) } + return runReadAction { elements.groupBy(KtElement::getContainingKtFile) } + .flatMap { shortenReferencesInFile(it.key, it.value, elementFilter, actionRunningMode) } } private fun shortenReferencesInFile( file: KtFile, elements: List, - elementFilter: (PsiElement) -> FilterResult + elementFilter: (PsiElement) -> FilterResult, + actionRunningMode: ActionRunningMode = ActionRunningMode.RUN_IN_CURRENT_THREAD ): Collection { //TODO: that's not correct since we have options! val elementsToUse = dropNestedElements(elements) @@ -182,39 +197,45 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT while (true) { // 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> = listOf( - ShortenTypesProcessor(file, elementFilter, failedToImportDescriptors), - ShortenThisExpressionsProcessor(file, elementFilter, failedToImportDescriptors), - ShortenQualifiedExpressionsProcessor(file, elementFilter, failedToImportDescriptors), - RemoveExplicitCompanionObjectReferenceProcessor(file, companionElementFilter, failedToImportDescriptors) - ) + val processors: List> = runReadAction { + listOf( + ShortenTypesProcessor(file, elementFilter, failedToImportDescriptors), + ShortenThisExpressionsProcessor(file, elementFilter, failedToImportDescriptors), + ShortenQualifiedExpressionsProcessor(file, elementFilter, failedToImportDescriptors), + RemoveExplicitCompanionObjectReferenceProcessor(file, companionElementFilter, failedToImportDescriptors) + ) + } // step 1: collect qualified elements to analyze (no resolve at this step) val visitors = processors.map { it.collectElementsVisitor } - for (visitor in visitors) { - for (element in elementsToUse) { - visitor.options = options(element) - element.accept(visitor) + runReadAction { + for (visitor in visitors) { + for (element in elementsToUse) { + 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 = allowResolveInDispatchThread { - file.getResolutionFacade().analyze(allElementsToAnalyze, BodyResolveMode.PARTIAL_WITH_CFA) - } - processors.forEach { it.analyzeCollectedElements(bindingContext) } + // 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 = allowResolveInDispatchThread { + file.getResolutionFacade().analyze(allElementsToAnalyze, BodyResolveMode.PARTIAL_WITH_CFA) + } + + processors.forEach { it.analyzeCollectedElements(bindingContext) } + } // step 3: shorten elements that can be shortened right now - runWriteAction { + actionRunningMode.runAction { processors.forEach { it.shortenElements(elementSetToUpdate = elementsToUse, options = options) } } - - // step 4: try to import descriptors needed to shorten other elements - val descriptorsToImport = processors.flatMap { it.getDescriptorsToImport() }.toSet() var anyChange = false - runWriteAction { + // step 4: try to import descriptors needed to shorten other elements + val descriptorsToImport = runReadAction { + processors.flatMap { it.getDescriptorsToImport() }.toSet() + } + actionRunningMode.runAction { for (descriptor in descriptorsToImport) { assert(descriptor !in failedToImportDescriptors) @@ -226,7 +247,10 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT failedToImportDescriptors.add(descriptor) } } - if (!anyChange) processors.forEach { it.removeRootPrefixes() } + + if (!anyChange) { + processors.forEach { it.removeRootPrefixes() } + } } if (!anyChange) break @@ -235,9 +259,9 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT return elementsToUse } - private fun dropNestedElements(elements: List): LinkedHashSet { + private fun dropNestedElements(elements: List): LinkedHashSet = runReadAction { val elementSet = elements.toSet() - return elementSet.filterTo(LinkedHashSet(elementSet.size)) { element -> + elementSet.filterTo(LinkedHashSet(elementSet.size)) { element -> element.parents.none { it in elementSet } } } diff --git a/idea/src/org/jetbrains/kotlin/idea/codeInliner/CodeInliner.kt b/idea/src/org/jetbrains/kotlin/idea/codeInliner/CodeInliner.kt index 59b238e4e9f..f5237a9f796 100644 --- a/idea/src/org/jetbrains/kotlin/idea/codeInliner/CodeInliner.kt +++ b/idea/src/org/jetbrains/kotlin/idea/codeInliner/CodeInliner.kt @@ -441,7 +441,7 @@ class CodeInliner( } val newElements = elements.map { - ShortenReferences { ShortenReferences.Options(removeThis = true) }.process(it, shortenFilter) + ShortenReferences { ShortenReferences.Options(removeThis = true) }.process(it, elementFilter = shortenFilter) } newElements.forEach { element -> diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/ScopeFunctionConversionInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/ScopeFunctionConversionInspection.kt index 30630e24d59..e0a6dbc6c19 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/ScopeFunctionConversionInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/ScopeFunctionConversionInspection.kt @@ -238,12 +238,14 @@ class ConvertScopeFunctionToParameter(counterpartName: String) : ConvertScopeFun } override fun postprocessLambda(lambda: KtLambdaArgument) { - ShortenReferences { ShortenReferences.Options(removeThisLabels = true) }.process(lambda) { element -> + val filter = { element: PsiElement -> if (element is KtThisExpression && element.getLabelName() != null) ShortenReferences.FilterResult.PROCESS else ShortenReferences.FilterResult.GO_INSIDE } + + ShortenReferences{ ShortenReferences.Options(removeThisLabels = true) }.process(lambda, filter) } private fun needUniqueNameForParameter( @@ -342,7 +344,7 @@ class ConvertScopeFunctionToReceiver(counterpartName: String) : ConvertScopeFunc } override fun postprocessLambda(lambda: KtLambdaArgument) { - ShortenReferences { ShortenReferences.Options(removeThis = true, removeThisLabels = true) }.process(lambda) { element -> + val filter = { element: PsiElement -> if (element is KtThisExpression && element.getLabelName() != null) ShortenReferences.FilterResult.PROCESS else if (element is KtQualifiedExpression && element.receiverExpression is KtThisExpression) @@ -350,6 +352,7 @@ class ConvertScopeFunctionToReceiver(counterpartName: String) : ConvertScopeFunc else ShortenReferences.FilterResult.GO_INSIDE } + ShortenReferences { ShortenReferences.Options(removeThis = true, removeThisLabels = true) }.process(lambda, filter) } } diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteExperimentalCoroutinesInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteExperimentalCoroutinesInspection.kt index d29f64b3dde..089c7a0c27e 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteExperimentalCoroutinesInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/migration/ObsoleteExperimentalCoroutinesInspection.kt @@ -256,7 +256,8 @@ private class ObsoleteExtensionFunctionUsage( .map { it.resolveToDescriptorIfAny() } .find { it != null && it.importableFqName?.asString() == fqName } ?: return - ImportInsertHelper.getInstance(element.project).importDescriptor(element.containingKtFile, importFun, false) + ImportInsertHelper.getInstance(element.project) + .importDescriptor(element.containingKtFile, importFun, forceAllUnderImport = false) } } } diff --git a/idea/src/org/jetbrains/kotlin/idea/util/ImportInsertHelperImpl.kt b/idea/src/org/jetbrains/kotlin/idea/util/ImportInsertHelperImpl.kt index d41ce55363b..d5e21cdb910 100644 --- a/idea/src/org/jetbrains/kotlin/idea/util/ImportInsertHelperImpl.kt +++ b/idea/src/org/jetbrains/kotlin/idea/util/ImportInsertHelperImpl.kt @@ -82,8 +82,13 @@ class ImportInsertHelperImpl(private val project: Project) : ImportInsertHelper( } } - override fun importDescriptor(file: KtFile, descriptor: DeclarationDescriptor, forceAllUnderImport: Boolean): ImportDescriptorResult { - val importer = Importer(file) + override fun importDescriptor( + file: KtFile, + descriptor: DeclarationDescriptor, + actionRunningMode: ActionRunningMode, + forceAllUnderImport: Boolean + ): ImportDescriptorResult { + val importer = Importer(file, actionRunningMode) return if (forceAllUnderImport) { importer.importDescriptorWithStarImport(descriptor) } else { @@ -92,7 +97,8 @@ class ImportInsertHelperImpl(private val project: Project) : ImportInsertHelper( } private inner class Importer( - private val file: KtFile + private val file: KtFile, + private val actionRunningMode: ActionRunningMode ) { private val resolutionFacade = file.getResolutionFacade() @@ -260,10 +266,9 @@ class ImportInsertHelperImpl(private val project: Project) : ImportInsertHelper( val addedImport = addImport(parentFqName, true) if (!isAlreadyImported(targetDescriptor, resolutionFacade.getFileResolutionScope(file), targetFqName)) { - addedImport.delete() + actionRunningMode.runAction { addedImport.delete() } return ImportDescriptorResult.FAIL } - dropRedundantExplicitImports(parentFqName) val conflicts = futureCheckMap @@ -326,7 +331,7 @@ class ImportInsertHelperImpl(private val project: Project) : ImportInsertHelper( if (targets.any { it is PackageViewDescriptor }) continue // do not drop import of package val classDescriptor = targets.filterIsInstance().firstOrNull() importsToCheck.addIfNotNull(classDescriptor?.importableFqName) - import.delete() + actionRunningMode.runAction { import.delete() } } if (importsToCheck.isNotEmpty()) { @@ -380,7 +385,9 @@ class ImportInsertHelperImpl(private val project: Project) : ImportInsertHelper( private fun KtReferenceExpression.resolveTargets(): Collection = this.getImportableTargets(resolutionFacade.analyze(this, BodyResolveMode.PARTIAL)) - private fun addImport(fqName: FqName, allUnder: Boolean): KtImportDirective = addImport(project, file, fqName, allUnder) + private fun addImport(fqName: FqName, allUnder: Boolean): KtImportDirective = actionRunningMode.runAction { + addImport(project, file, fqName, allUnder) + } } companion object { diff --git a/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/processings/ShortenReferenceProcessing.kt b/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/processings/ShortenReferenceProcessing.kt index a7fe7ffc9de..02a218214dd 100644 --- a/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/processings/ShortenReferenceProcessing.kt +++ b/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/processings/ShortenReferenceProcessing.kt @@ -5,15 +5,14 @@ package org.jetbrains.kotlin.nj2k.postProcessing.processings +import com.intellij.openapi.command.CommandProcessor import com.intellij.openapi.editor.RangeMarker import com.intellij.psi.PsiElement import org.jetbrains.kotlin.idea.core.ShortenReferences +import org.jetbrains.kotlin.idea.util.ActionRunningMode import org.jetbrains.kotlin.nj2k.JKImportStorage import org.jetbrains.kotlin.nj2k.NewJ2kConverterContext import org.jetbrains.kotlin.nj2k.postProcessing.FileBasedPostProcessing -import org.jetbrains.kotlin.nj2k.postProcessing.GeneralPostProcessing -import org.jetbrains.kotlin.nj2k.postProcessing.PostProcessingOptions -import org.jetbrains.kotlin.nj2k.postProcessing.runUndoTransparentActionInEdt import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.psi.KtQualifiedExpression @@ -29,13 +28,19 @@ class ShortenReferenceProcessing : FileBasedPostProcessing() { } override fun runProcessing(file: KtFile, allFiles: List, rangeMarker: RangeMarker?, converterContext: NewJ2kConverterContext) { - runUndoTransparentActionInEdt(inWriteAction = false) { + CommandProcessor.getInstance().runUndoTransparentAction { if (rangeMarker != null) { if (rangeMarker.isValid) { - ShortenReferences.DEFAULT.process(file, rangeMarker.startOffset, rangeMarker.endOffset, filter) + ShortenReferences.DEFAULT.process( + file, + rangeMarker.startOffset, + rangeMarker.endOffset, + filter, + actionRunningMode = ActionRunningMode.RUN_IN_EDT + ) } } else { - ShortenReferences.DEFAULT.process(file, filter) + ShortenReferences.DEFAULT.process(file, filter, actionRunningMode = ActionRunningMode.RUN_IN_EDT) } } }