From 4e27d2e6582d85d3848939087929a77c57d3be74 Mon Sep 17 00:00:00 2001 From: Ilya Kirillov Date: Wed, 18 Sep 2019 20:28:16 +0300 Subject: [PATCH] New J2K: do not use global write action for some post-processings which may use resolving while applying Also, modify that post-processings & inspections to explicitly use write action when modifying PSI elements #KT-33875 fixed --- .../kotlin/idea/core/ShortenReferences.kt | 23 +++++---- .../AddOperatorModifierInspection.kt | 8 +++- .../FoldInitializerAndIfToElvisInspection.kt | 27 ++++++----- .../ConvertToStringTemplateIntention.kt | 6 ++- .../idea/intentions/DestructureIntention.kt | 47 ++++++++++++------- .../ObjectLiteralToLambdaIntention.kt | 12 +++-- ...SpecifyExplicitLambdaSignatureIntention.kt | 7 ++- .../UsePropertyAccessSyntaxIntention.kt | 6 ++- .../idea/quickfix/AddConstModifierFix.kt | 20 +++++--- .../inspectionLikePostProcessings.kt | 2 - .../processings/inspeсtionLikeProcessings.kt | 36 +++++++++++--- .../jetbrains/kotlin/nj2k/ImportStorage.kt | 19 +++++++- 12 files changed, 148 insertions(+), 65 deletions(-) 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 bd58301347b..5aad8c6b786 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 @@ -19,6 +19,7 @@ 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.* @@ -207,20 +208,24 @@ class ShortenReferences(val options: (KtElement) -> Options = { Options.DEFAULT processors.forEach { it.analyzeCollectedElements(bindingContext) } // step 3: shorten elements that can be shortened right now - processors.forEach { it.shortenElements(elementSetToUpdate = elementsToUse, options = options) } + runWriteAction { + 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 - for (descriptor in descriptorsToImport) { - assert(descriptor !in failedToImportDescriptors) + runWriteAction { + for (descriptor in descriptorsToImport) { + assert(descriptor !in failedToImportDescriptors) - val result = helper.importDescriptor(file, descriptor) - if (result != ImportDescriptorResult.ALREADY_IMPORTED) { - anyChange = true - } - if (result == ImportDescriptorResult.FAIL) { - failedToImportDescriptors.add(descriptor) + val result = helper.importDescriptor(file, descriptor) + if (result != ImportDescriptorResult.ALREADY_IMPORTED) { + anyChange = true + } + if (result == ImportDescriptorResult.FAIL) { + failedToImportDescriptors.add(descriptor) + } } } if (!anyChange) break diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/AddOperatorModifierInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/AddOperatorModifierInspection.kt index be54c84dd32..a3c8ba82aff 100644 --- a/idea/src/org/jetbrains/kotlin/idea/inspections/AddOperatorModifierInspection.kt +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/AddOperatorModifierInspection.kt @@ -9,6 +9,7 @@ import com.intellij.openapi.editor.Editor import com.intellij.openapi.project.Project import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny import org.jetbrains.kotlin.idea.refactoring.withExpectedActuals +import org.jetbrains.kotlin.idea.util.application.runWriteAction import org.jetbrains.kotlin.idea.util.nameIdentifierTextRangeInThis import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.KtNamedFunction @@ -28,8 +29,11 @@ class AddOperatorModifierInspection : AbstractApplicabilityBasedInspection= margin || element.then?.hasComments() == true) val elvis = factory.createExpressionByPattern(pattern, initializer, ifNullExpr) as KtBinaryExpression - if (typeReference != null) { - elvis.left!!.replace(factory.createExpressionByPattern("$0 as? $1", initializer, typeReference)) + + return runWriteAction { + if (typeReference != null) { + elvis.left!!.replace(factory.createExpressionByPattern("$0 as? $1", initializer, typeReference)) + } + val newElvis = initializer.replaced(elvis) + element.delete() + + if (explicitTypeToSet != null && !explicitTypeToSet.isError) { + declaration.setType(explicitTypeToSet) + } + + commentSaver.restore(childRangeAfter) + newElvis } - val newElvis = initializer.replaced(elvis) - element.delete() - - if (explicitTypeToSet != null && !explicitTypeToSet.isError) { - declaration.setType(explicitTypeToSet) - } - - commentSaver.restore(childRangeAfter) - - return newElvis } private fun calcData(ifExpression: KtIfExpression): Data? { diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToStringTemplateIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToStringTemplateIntention.kt index e8ace0bdc4a..eba76b609be 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToStringTemplateIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertToStringTemplateIntention.kt @@ -23,6 +23,7 @@ import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.core.replaced import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection +import org.jetbrains.kotlin.idea.util.application.runWriteAction import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator @@ -47,7 +48,10 @@ open class ConvertToStringTemplateIntention : SelfTargetingOffsetIndependentInte } override fun applyTo(element: KtBinaryExpression, editor: Editor?) { - element.replaced(buildReplacement(element)) + val replacement = buildReplacement(element) + runWriteAction { + element.replaced(replacement) + } } companion object { diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt index 14635e59c90..c5678dca84e 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/DestructureIntention.kt @@ -34,6 +34,7 @@ import org.jetbrains.kotlin.idea.core.isVisible import org.jetbrains.kotlin.idea.findUsages.ReferencesSearchScopeHelper import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection import org.jetbrains.kotlin.idea.project.languageVersionSettings +import org.jetbrains.kotlin.idea.util.application.runWriteAction import org.jetbrains.kotlin.incremental.components.NoLookupLocation import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.name.Name @@ -89,9 +90,12 @@ class DestructureIntention : SelfTargetingRangeIntention( else { name ?: KotlinNameSuggester.suggestNameByName(descriptor.name.asString(), validator) } - variableToDrop?.delete() - usagesToReplace.forEach { - it.replace(factory.createExpression(suggestedName)) + + runWriteAction { + variableToDrop?.delete() + usagesToReplace.forEach { + it.replace(factory.createExpression(suggestedName)) + } } names.add(suggestedName) } @@ -100,31 +104,42 @@ class DestructureIntention : SelfTargetingRangeIntention( when (element) { is KtParameter -> { val loopRange = (element.parent as? KtForExpression)?.loopRange - element.replace(factory.createDestructuringParameter("($joinedNames)")) - if (removeSelectorInLoopRange && loopRange is KtDotQualifiedExpression) { - loopRange.replace(loopRange.receiverExpression) + runWriteAction { + element.replace(factory.createDestructuringParameter("($joinedNames)")) + if (removeSelectorInLoopRange && loopRange is KtDotQualifiedExpression) { + loopRange.replace(loopRange.receiverExpression) + } } } is KtFunctionLiteral -> { val lambda = element.parent as KtLambdaExpression SpecifyExplicitLambdaSignatureIntention().applyTo(lambda, editor) - lambda.functionLiteral.valueParameters.singleOrNull()?.replace( + runWriteAction { + lambda.functionLiteral.valueParameters.singleOrNull()?.replace( factory.createDestructuringParameter("($joinedNames)") - ) + ) + } } is KtVariableDeclaration -> { val rangeAfterEq = PsiChildRange(element.initializer, element.lastChild) val modifierList = element.modifierList - if (modifierList == null) { - element.replace(factory.createDestructuringDeclarationByPattern( - "val ($joinedNames) = $0", rangeAfterEq)) - } - else { - val rangeBeforeVal = PsiChildRange(element.firstChild, modifierList) - element.replace(factory.createDestructuringDeclarationByPattern( - "$0:'@xyz' val ($joinedNames) = $1", rangeBeforeVal, rangeAfterEq)) + runWriteAction { + if (modifierList == null) { + element.replace( + factory.createDestructuringDeclarationByPattern( + "val ($joinedNames) = $0", rangeAfterEq + ) + ) + } else { + val rangeBeforeVal = PsiChildRange(element.firstChild, modifierList) + element.replace( + factory.createDestructuringDeclarationByPattern( + "$0:'@xyz' val ($joinedNames) = $1", rangeBeforeVal, rangeAfterEq + ) + ) + } } } } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/ObjectLiteralToLambdaIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/ObjectLiteralToLambdaIntention.kt index 7e2894622a6..c994d545051 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/ObjectLiteralToLambdaIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/ObjectLiteralToLambdaIntention.kt @@ -35,6 +35,7 @@ import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection import org.jetbrains.kotlin.idea.inspections.RedundantSamConstructorInspection import org.jetbrains.kotlin.idea.util.CommentSaver import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers +import org.jetbrains.kotlin.idea.util.application.runWriteAction import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.load.java.sam.SingleAbstractMethodUtils import org.jetbrains.kotlin.psi.* @@ -157,21 +158,22 @@ class ObjectLiteralToLambdaIntention : SelfTargetingRangeIntention() - if (call != null && it.element == call.methodExpression) { - val fieldRef = factory.createExpressionFromText(fieldFQName, it.element) - call.replace(fieldRef) + runWriteAction { + getterUsages.forEach { + val call = it.element.getNonStrictParentOfType() + if (call != null && it.element == call.methodExpression) { + val fieldRef = factory.createExpressionFromText(fieldFQName, it.element) + call.replace(fieldRef) + } } } } diff --git a/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/inspectionLikePostProcessings.kt b/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/inspectionLikePostProcessings.kt index 522ef768e99..97ec72ace90 100644 --- a/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/inspectionLikePostProcessings.kt +++ b/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/inspectionLikePostProcessings.kt @@ -5,7 +5,6 @@ package org.jetbrains.kotlin.nj2k.postProcessing -import com.intellij.openapi.command.CommandProcessor import com.intellij.openapi.editor.RangeMarker import com.intellij.openapi.util.TextRange import com.intellij.psi.PsiElement @@ -13,7 +12,6 @@ import com.intellij.psi.PsiRecursiveElementVisitor import org.jetbrains.kotlin.idea.inspections.AbstractApplicabilityBasedInspection import org.jetbrains.kotlin.idea.intentions.SelfTargetingRangeIntention import org.jetbrains.kotlin.idea.util.application.runReadAction -import org.jetbrains.kotlin.idea.util.application.runWriteAction import org.jetbrains.kotlin.j2k.ConverterSettings import org.jetbrains.kotlin.nj2k.NewJ2kConverterContext import org.jetbrains.kotlin.psi.KtElement diff --git a/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/processings/inspeсtionLikeProcessings.kt b/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/processings/inspeсtionLikeProcessings.kt index 57312fa8fb5..44ce6109a76 100644 --- a/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/processings/inspeсtionLikeProcessings.kt +++ b/nj2k/nj2k-services/src/org/jetbrains/kotlin/nj2k/postProcessing/processings/inspeсtionLikeProcessings.kt @@ -25,6 +25,8 @@ import org.jetbrains.kotlin.idea.quickfix.AddConstModifierFix import org.jetbrains.kotlin.idea.references.KtSimpleNameReference import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.idea.references.readWriteAccess +import org.jetbrains.kotlin.idea.util.CommentSaver +import org.jetbrains.kotlin.idea.util.application.runWriteAction import org.jetbrains.kotlin.idea.util.getResolutionScope import org.jetbrains.kotlin.j2k.ConverterSettings import org.jetbrains.kotlin.lexer.KtTokens @@ -127,13 +129,29 @@ class RemoveRedundantOverrideVisibilityProcessing : } } -class UseExpressionBodyProcessing : InspectionLikeProcessingForElement(KtPropertyAccessor::class) { - private val inspection = UseExpressionBodyInspection(convertEmptyToUnit = false) - override fun isApplicableTo(element: KtPropertyAccessor, settings: ConverterSettings?): Boolean = - inspection.isActiveFor(element) +class ReplaceGetterBodyWithSingleReturnStatementWithExpressionBody : + InspectionLikeProcessingForElement(KtPropertyAccessor::class) { + + private fun KtPropertyAccessor.singleBodyStatementExpression() = + bodyBlockExpression?.statements + ?.singleOrNull() + ?.safeAs() + ?.takeIf { it.labeledExpression == null } + ?.returnedExpression + + override fun isApplicableTo(element: KtPropertyAccessor, settings: ConverterSettings?): Boolean { + if (!element.isGetter) return false + return element.singleBodyStatementExpression() != null + } override fun apply(element: KtPropertyAccessor) { - inspection.simplify(element, false) + val body = element.bodyExpression ?: return + val returnedExpression = element.singleBodyStatementExpression() ?: return + + val commentSaver = CommentSaver(body) + element.addBefore(KtPsiFactory(element).createEQ(), body) + val newBody = body.replaced(returnedExpression) + commentSaver.restore(newBody) } } @@ -156,13 +174,17 @@ class RemoveRedundantCastToNullableProcessing : class RemoveRedundantSamAdaptersProcessing : InspectionLikeProcessingForElement(KtCallExpression::class) { + override val writeActionNeeded = false override fun isApplicableTo(element: KtCallExpression, settings: ConverterSettings?): Boolean = RedundantSamConstructorInspection.samConstructorCallsToBeConverted(element).isNotEmpty() override fun apply(element: KtCallExpression) { - RedundantSamConstructorInspection.samConstructorCallsToBeConverted(element).forEach { call -> - RedundantSamConstructorInspection.replaceSamConstructorCall(call) + val callsToBeConverted = RedundantSamConstructorInspection.samConstructorCallsToBeConverted(element) + runWriteAction { + for (call in callsToBeConverted) { + RedundantSamConstructorInspection.replaceSamConstructorCall(call) + } } } } diff --git a/nj2k/src/org/jetbrains/kotlin/nj2k/ImportStorage.kt b/nj2k/src/org/jetbrains/kotlin/nj2k/ImportStorage.kt index 85bbdbe1a83..47997d5e243 100644 --- a/nj2k/src/org/jetbrains/kotlin/nj2k/ImportStorage.kt +++ b/nj2k/src/org/jetbrains/kotlin/nj2k/ImportStorage.kt @@ -6,8 +6,12 @@ package org.jetbrains.kotlin.nj2k import com.intellij.psi.CommonClassNames +import org.jetbrains.kotlin.idea.refactoring.fqName.getKotlinFqName +import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.load.java.NULLABILITY_ANNOTATIONS import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.psi.KtQualifiedExpression +import org.jetbrains.kotlin.resolve.calls.callUtil.getCalleeExpressionIfAny class ImportStorage { private val imports = mutableSetOf() @@ -27,6 +31,16 @@ class ImportStorage { return true } + fun isImportNeededForCall(qualifiedExpression: KtQualifiedExpression): Boolean { + val shortName = qualifiedExpression.getCalleeExpressionIfAny()?.text ?: return true + if (shortName !in SHORT_NAMES) return true + val fqName = qualifiedExpression.selectorExpression?.mainReference?.resolve()?.getKotlinFqName() ?: return true + return isImportNeeded(fqName) + } + + fun isImportNeeded(fqName: String): Boolean = isImportNeeded(FqName(fqName)) + + private val JAVA_TYPE_WRAPPERS_WHICH_HAVE_CONFLICTS_WITH_KOTLIN_ONES = setOf( FqName(CommonClassNames.JAVA_LANG_BYTE), FqName(CommonClassNames.JAVA_LANG_SHORT), @@ -35,6 +49,9 @@ class ImportStorage { FqName(CommonClassNames.JAVA_LANG_DOUBLE) ) - fun isImportNeeded(fqName: String): Boolean = isImportNeeded(FqName(fqName)) + private val SHORT_NAMES = + (NULLABILITY_ANNOTATIONS + JAVA_TYPE_WRAPPERS_WHICH_HAVE_CONFLICTS_WITH_KOTLIN_ONES) + .map { it.shortName().asString() } + .toSet() } } \ No newline at end of file