J2K: use refactorings and runWriteAction more accurately #KT-19371 Fixed

Do not invoke runWriteAction for post-processings
in Convert(Text)JavaCopyPasteProcessor.
Instead each processing can need write action or not,
and J2kPostProcessor takes this into account.

idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertJavaCopyPasteProcessor.kt

@@ -153,12 +153,10 @@ class ConvertJavaCopyPasteProcessor : CopyPastePostProcessor<TextBlockTransferab
 
             val newBounds = insertImports(boundsAfterReplace, referenceData, explicitImports)
 
-            runWriteAction {
-                PsiDocumentManager.getInstance(project).commitAllDocuments()
-                AfterConversionPass(project, J2kPostProcessor(formatCode = true)).run(targetFile, newBounds)
+            PsiDocumentManager.getInstance(project).commitAllDocuments()
+            AfterConversionPass(project, J2kPostProcessor(formatCode = true)).run(targetFile, newBounds)
 
-                conversionPerformed = true
-            }
+            conversionPerformed = true
         }
     }
 

idea/src/org/jetbrains/kotlin/idea/conversion/copy/ConvertTextJavaCopyPasteProcessor.kt

@@ -111,7 +111,7 @@ class ConvertTextJavaCopyPasteProcessor : CopyPastePostProcessor<TextBlockTransf
 
         val convertedText = dataForConversion.convertCodeToKotlin(project).text
 
-        runWriteAction {
+        val newBounds = runWriteAction {
 
             val importsInsertOffset = targetFile.importList?.endOffset ?: 0
             if (targetFile.importDirectives.isEmpty() && importsInsertOffset > 0)
@@ -125,14 +125,13 @@ class ConvertTextJavaCopyPasteProcessor : CopyPastePostProcessor<TextBlockTransf
             val endOffsetAfterCopy = startOffset + convertedText.length
             editor.caretModel.moveToOffset(endOffsetAfterCopy)
 
-            val newBounds = TextRange(startOffset, startOffset + convertedText.length)
-
-            psiDocumentManager.commitAllDocuments()
-
-            AfterConversionPass(project, J2kPostProcessor(formatCode = true)).run(targetFile, newBounds)
-
-            conversionPerformed = true
+            TextRange(startOffset, startOffset + convertedText.length)
         }
+
+        psiDocumentManager.commitAllDocuments()
+        AfterConversionPass(project, J2kPostProcessor(formatCode = true)).run(targetFile, newBounds)
+
+        conversionPerformed = true
     }
 
     private fun DataForConversion.convertCodeToKotlin(project: Project): ConversionResult {

idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessings.kt

@@ -41,7 +41,6 @@ import org.jetbrains.kotlin.idea.quickfix.RemoveModifierFix
 import org.jetbrains.kotlin.idea.quickfix.RemoveUselessCastFix
 import org.jetbrains.kotlin.idea.refactoring.inline.KotlinInlineValHandler
 import org.jetbrains.kotlin.idea.references.mainReference
-import org.jetbrains.kotlin.idea.util.application.runWriteAction
 import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.getChildOfType
@@ -57,6 +56,8 @@ import java.util.*
 
 interface J2kPostProcessing {
     fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)?
+
+    val writeActionNeeded: Boolean
 }
 
 object J2KPostProcessingRegistrar {
@@ -67,9 +68,7 @@ object J2KPostProcessingRegistrar {
 
     private val processingsToPriorityMap = HashMap<J2kPostProcessing, Int>()
 
-    fun priority(processing: J2kPostProcessing): Int {
-        return processingsToPriorityMap[processing]!!
-    }
+    fun priority(processing: J2kPostProcessing): Int = processingsToPriorityMap[processing]!!
 
     init {
         _processings.add(RemoveExplicitTypeArgumentsProcessing())
@@ -153,6 +152,9 @@ object J2KPostProcessingRegistrar {
             noinline additionalChecker: (TElement) -> Boolean = { true }
     ) {
         _processings.add(object : J2kPostProcessing {
+            // Intention can either need or not need write action
+            override val writeActionNeeded = intention.startInWriteAction()
+
             override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
                 if (!TElement::class.java.isInstance(element)) return null
                 val tElement = element as TElement
@@ -161,11 +163,7 @@ object J2KPostProcessingRegistrar {
                 return {
                     if (intention.applicabilityRange(tElement) != null) { // check availability of the intention again because something could change
                         val apply = { intention.applyTo(element, null) }
-
-                        if (intention.startInWriteAction())
-                            runWriteAction(apply)
-                        else
-                            apply()
+                        apply()
                     }
                 }
             }
@@ -184,6 +182,9 @@ object J2KPostProcessingRegistrar {
             crossinline fixFactory: (TElement, Diagnostic) -> (() -> Unit)?
     ) {
         _processings.add(object : J2kPostProcessing {
+            // ???
+            override val writeActionNeeded = true
+
             override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
                 if (!TElement::class.java.isInstance(element)) return null
                 val diagnostic = diagnostics.forElement(element).firstOrNull { it.factory in diagnosticFactory } ?: return null
@@ -193,6 +194,8 @@ object J2KPostProcessingRegistrar {
     }
 
     private class RemoveExplicitTypeArgumentsProcessing : J2kPostProcessing {
+        override val writeActionNeeded = true
+
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
             if (element !is KtTypeArgumentList || !RemoveExplicitTypeArgumentsIntention.isApplicableTo(element, approximateFlexible = true)) return null
 
@@ -205,6 +208,8 @@ object J2KPostProcessingRegistrar {
     }
 
     private class RemoveRedundantOverrideVisibilityProcessing : J2kPostProcessing {
+        override val writeActionNeeded = true
+
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
             if (element !is KtCallableDeclaration || !element.hasModifier(KtTokens.OVERRIDE_KEYWORD)) return null
             val modifier = element.visibilityModifierType() ?: return null
@@ -213,6 +218,8 @@ object J2KPostProcessingRegistrar {
     }
 
     private class MoveLambdaOutsideParenthesesProcessing : J2kPostProcessing {
+        override val writeActionNeeded = true
+
         private val intention = MoveLambdaOutsideParenthesesIntention()
 
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
@@ -224,6 +231,8 @@ object J2KPostProcessingRegistrar {
     }
 
     private class ConvertToStringTemplateProcessing : J2kPostProcessing {
+        override val writeActionNeeded = true
+
         private val intention = ConvertToStringTemplateIntention()
 
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
@@ -237,6 +246,8 @@ object J2KPostProcessingRegistrar {
     }
 
     private class UsePropertyAccessSyntaxProcessing : J2kPostProcessing {
+        override val writeActionNeeded = true
+
         private val intention = UsePropertyAccessSyntaxIntention()
 
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
@@ -247,6 +258,8 @@ object J2KPostProcessingRegistrar {
     }
 
     private class RemoveRedundantSamAdaptersProcessing : J2kPostProcessing {
+        override val writeActionNeeded = true
+
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
             if (element !is KtCallExpression) return null
 
@@ -261,6 +274,8 @@ object J2KPostProcessingRegistrar {
     }
 
     private class UseExpressionBodyProcessing : J2kPostProcessing {
+        override val writeActionNeeded = true
+
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
             if (element !is KtPropertyAccessor) return null
 
@@ -274,6 +289,9 @@ object J2KPostProcessingRegistrar {
     }
 
     private class UnnecessaryVariableProcessing : J2kPostProcessing {
+        // Refactoring
+        override val writeActionNeeded = false
+
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
             if (element !is KtProperty) return null
 
@@ -286,6 +304,8 @@ object J2KPostProcessingRegistrar {
     }
 
     private class RemoveRedundantCastToNullableProcessing : J2kPostProcessing {
+        override val writeActionNeeded = true
+
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
             if (element !is KtBinaryExpressionWithTypeRHS) return null
 
@@ -305,6 +325,8 @@ object J2KPostProcessingRegistrar {
     }
 
     private class FixObjectStringConcatenationProcessing : J2kPostProcessing {
+        override val writeActionNeeded = true
+
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
             if (element !is KtBinaryExpression ||
                 element.operationToken != KtTokens.PLUS ||
@@ -332,6 +354,8 @@ object J2KPostProcessingRegistrar {
     }
 
     private class UninitializedVariableReferenceFromInitializerToThisReferenceProcessing : J2kPostProcessing {
+        override val writeActionNeeded = true
+
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
             if (element !is KtSimpleNameExpression || diagnostics.forElement(element).none { it.factory == Errors.UNINITIALIZED_VARIABLE }) return null
 
@@ -350,6 +374,8 @@ object J2KPostProcessingRegistrar {
     }
 
     private class UnresolvedVariableReferenceFromInitializerToThisReferenceProcessing : J2kPostProcessing {
+        override val writeActionNeeded = true
+
         override fun createAction(element: KtElement, diagnostics: Diagnostics): (() -> Unit)? {
             if (element !is KtSimpleNameExpression || diagnostics.forElement(element).none { it.factory == Errors.UNRESOLVED_REFERENCE }) return null
 

idea/src/org/jetbrains/kotlin/idea/j2k/J2kPostProcessor.kt

@@ -65,9 +65,16 @@ class J2kPostProcessor(private val formatCode: Boolean) : PostProcessor {
                     }
 
                     run(EDT) {
-                        for ((element, action, _) in elementToActions) {
+                        for ((element, action, _, writeActionNeeded) in elementToActions) {
                             if (element.isValid) {
-                                action()
+                                if (writeActionNeeded) {
+                                    runWriteAction {
+                                        action()
+                                    }
+                                }
+                                else {
+                                    action()
+                                }
                             }
                             else {
                                 modificationStamp = null
@@ -99,7 +106,7 @@ class J2kPostProcessor(private val formatCode: Boolean) : PostProcessor {
             }
 
 
-    private data class ActionData(val element: KtElement, val action: () -> Unit, val priority: Int)
+    private data class ActionData(val element: KtElement, val action: () -> Unit, val priority: Int, val writeActionNeeded: Boolean)
 
     private fun collectAvailableActions(file: KtFile, rangeMarker: RangeMarker?): List<ActionData> {
         val diagnostics = analyzeFileRange(file, rangeMarker)
@@ -118,7 +125,9 @@ class J2kPostProcessor(private val formatCode: Boolean) : PostProcessor {
                         J2KPostProcessingRegistrar.processings.forEach { processing ->
                             val action = processing.createAction(element, diagnostics)
                             if (action != null) {
-                                availableActions.add(ActionData(element, action, J2KPostProcessingRegistrar.priority(processing)))
+                                availableActions.add(ActionData(element, action,
+                                                                J2KPostProcessingRegistrar.priority(processing),
+                                                                processing.writeActionNeeded))
                             }
                         }
                     }