diff --git a/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt b/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt index 3120d0c548a..14a38280c3d 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt @@ -381,9 +381,9 @@ class KtPsiFactory @JvmOverloads constructor(private val project: Project, val m return stringTemplateExpression.entries[0] as KtStringTemplateEntryWithExpression } - fun createSimpleNameStringTemplateEntry(name: String): KtStringTemplateEntryWithExpression { + fun createSimpleNameStringTemplateEntry(name: String): KtSimpleNameStringTemplateEntry { val stringTemplateExpression = createExpression("\"\$$name\"") as KtStringTemplateExpression - return stringTemplateExpression.entries[0] as KtStringTemplateEntryWithExpression + return stringTemplateExpression.entries[0] as KtSimpleNameStringTemplateEntry } fun createStringTemplate(content: String) = createExpression("\"$content\"") as KtStringTemplateExpression diff --git a/idea/src/org/jetbrains/kotlin/idea/codeInliner/ReplacementPerformer.kt b/idea/src/org/jetbrains/kotlin/idea/codeInliner/ReplacementPerformer.kt index d208d449ec5..44b6b7375ce 100644 --- a/idea/src/org/jetbrains/kotlin/idea/codeInliner/ReplacementPerformer.kt +++ b/idea/src/org/jetbrains/kotlin/idea/codeInliner/ReplacementPerformer.kt @@ -17,12 +17,17 @@ package org.jetbrains.kotlin.idea.codeInliner import com.intellij.openapi.util.Key -import com.intellij.psi.PsiElement +import com.intellij.psi.PsiTreeChangeAdapter +import com.intellij.psi.PsiTreeChangeEvent import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.core.replaced import org.jetbrains.kotlin.idea.intentions.ConvertToBlockBodyIntention +import org.jetbrains.kotlin.idea.intentions.RemoveCurlyBracesFromTemplateIntention import org.jetbrains.kotlin.psi.* -import org.jetbrains.kotlin.psi.psiUtil.* +import org.jetbrains.kotlin.psi.psiUtil.PsiChildRange +import org.jetbrains.kotlin.psi.psiUtil.canPlaceAfterSimpleNameEntry +import org.jetbrains.kotlin.psi.psiUtil.findDescendantOfType +import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf import org.jetbrains.kotlin.resolve.bindingContextUtil.isUsedAsExpression import java.util.* @@ -109,9 +114,11 @@ internal class ExpressionReplacementPerformer( } val mainExpression = codeToInline.mainExpression - val replaced: PsiElement? = when (mainExpression) { + val replaced: KtExpression? = when (mainExpression) { is KtStringTemplateExpression -> elementToBeReplaced.replacedWithStringTemplate(mainExpression) + is KtExpression -> elementToBeReplaced.replaced(mainExpression) + else -> { // NB: Unit is never used as expression val stub = elementToBeReplaced.replaced(psiFactory.createExpression("0")) @@ -122,7 +129,7 @@ internal class ExpressionReplacementPerformer( null } else { - stub.replace(psiFactory.createExpression("Unit")) + stub.replaced(psiFactory.createExpression("Unit")) } } } @@ -147,9 +154,28 @@ internal class ExpressionReplacementPerformer( } } - range = postProcessing(range) + val listener = replaced?.let { TrackExpressionListener(it) } + listener?.attach() + try { + range = postProcessing(range) + } + finally { + listener?.detach() + } - return range.last as KtExpression? //TODO: return value not correct! + val resultExpression = listener?.result + + // simplify "${x}" to "$x" + val templateEntry = resultExpression?.parent as? KtBlockStringTemplateEntry + if (templateEntry != null) { + val intention = RemoveCurlyBracesFromTemplateIntention() + if (intention.isApplicableTo(templateEntry)) { + val newEntry = intention.applyTo(templateEntry) + return newEntry.expression + } + } + + return resultExpression ?: range.last as? KtExpression } /** @@ -204,6 +230,28 @@ internal class ExpressionReplacementPerformer( elementToBeReplaced.putCopyableUserData(ELEMENT_TO_BE_REPLACED_KEY, null) return result } + + private class TrackExpressionListener(expression: KtExpression) : PsiTreeChangeAdapter() { + private var expression: KtExpression? = expression + private val manager = expression.manager + + fun attach() { + manager.addPsiTreeChangeListener(this) + } + + fun detach() { + manager.removePsiTreeChangeListener(this) + } + + val result: KtExpression? + get() = expression?.takeIf { it.isValid } + + override fun childReplaced(event: PsiTreeChangeEvent) { + if (event.oldChild == expression) { + expression = event.newChild as? KtExpression + } + } + } } private val ELEMENT_TO_BE_REPLACED_KEY = Key("ELEMENT_TO_BE_REPLACED_KEY") \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/codeInliner/UsageReplacementStrategy.kt b/idea/src/org/jetbrains/kotlin/idea/codeInliner/UsageReplacementStrategy.kt index 8abe88bbc83..d65d892c7a1 100644 --- a/idea/src/org/jetbrains/kotlin/idea/codeInliner/UsageReplacementStrategy.kt +++ b/idea/src/org/jetbrains/kotlin/idea/codeInliner/UsageReplacementStrategy.kt @@ -22,6 +22,7 @@ import com.intellij.openapi.progress.ProgressIndicator import com.intellij.openapi.progress.ProgressManager import com.intellij.openapi.progress.Task import com.intellij.openapi.project.Project +import com.intellij.openapi.util.Key import com.intellij.psi.PsiElement import com.intellij.psi.search.GlobalSearchScope import com.intellij.psi.search.searches.ReferencesSearch @@ -39,6 +40,7 @@ import org.jetbrains.kotlin.idea.stubindex.KotlinSourceFilterScope import org.jetbrains.kotlin.idea.util.application.executeWriteCommand import org.jetbrains.kotlin.idea.util.application.runReadAction import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType import org.jetbrains.kotlin.psi.psiUtil.forEachDescendantOfType import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType import org.jetbrains.kotlin.resolve.BindingContext @@ -116,84 +118,104 @@ fun UsageReplacementStrategy.replaceUsages( ) { GuiUtils.invokeLaterIfNeeded({ project.executeWriteCommand(commandName) { - // we should delete imports later to not affect other usages - val importsToDelete = arrayListOf() - val replacements = mutableListOf() - - var invalidUsagesFound = false - val targetDeclaration = targetPsiElement as? KtNamedDeclaration - val usagesChildrenFirst = usages.sortChildrenFirst() - usages@ for (usage in usagesChildrenFirst) { - try { - if (!usage.isValid) { - invalidUsagesFound = true - continue - } + val usagesByFile = usages.groupBy { it.containingFile } - val usageParent = usage.parent - when (usageParent) { - is KtCallableReferenceExpression -> { - val grandParent = usageParent.parent - ConvertReferenceToLambdaIntention().applyTo(usageParent, null) - (grandParent as? KtElement)?.let { - doRefactoringInside(it, targetDeclaration?.name, targetDeclaration?.descriptor) - } - continue@usages - } - is KtCallElement -> { - val lambdaArguments = usageParent.lambdaArguments - if (lambdaArguments.isNotEmpty()) { - val grandParent = usageParent.parent - val specifySignature = SpecifyExplicitLambdaSignatureIntention() - for (lambdaArgument in lambdaArguments) { - val lambdaExpression = lambdaArgument.getLambdaExpression() - val functionDescriptor = - lambdaExpression.functionLiteral.resolveToDescriptorIfAny() as? FunctionDescriptor ?: continue - if (functionDescriptor.valueParameters.isNotEmpty()) { - specifySignature.applyTo(lambdaExpression, null) - } - } - (grandParent as? KtElement)?.let { - doRefactoringInside(it, targetDeclaration?.name, targetDeclaration?.descriptor) - } - continue@usages - } - } + val KEY = Key("UsageReplacementStrategy.replaceUsages") - } + for ((file, usagesInFile) in usagesByFile) { + usagesInFile.forEach { it.putCopyableUserData(KEY, Unit) } - //TODO: keep the import if we don't know how to replace some of the usages - val importDirective = usage.getStrictParentOfType() - if (importDirective != null) { - if (!importDirective.isAllUnder && importDirective.targetDescriptors().size == 1) { - importsToDelete.add(importDirective) - } - continue - } + // we should delete imports later to not affect other usages + val importsToDelete = mutableListOf() - val replacement = createReplacer(usage)?.invoke() - if (replacement != null) { - replacements += replacement - } - } - catch (e: Throwable) { - LOG.error(e) + var usagesToProcess = usagesInFile + while (usagesToProcess.isNotEmpty()) { + if (processUsages(usagesToProcess, targetDeclaration, importsToDelete)) break + + // some usages may get invalidated we need to find them in the tree + usagesToProcess = file.collectDescendantsOfType { it.getCopyableUserData(KEY) != null } } + + file.forEachDescendantOfType { it.putCopyableUserData(KEY, null) } + + importsToDelete.forEach { it.delete() } } - if (invalidUsagesFound && targetDeclaration != null) { - val name = targetDeclaration.name - val descriptor = targetDeclaration.descriptor - for (replacement in replacements) { - doRefactoringInside(replacement, name, descriptor) - } - } - - importsToDelete.forEach { it.delete() } - postAction() } }, ModalityState.NON_MODAL) } + +/** + * @return false if some usages were invalidated + */ +private fun UsageReplacementStrategy.processUsages( + usages: List, + targetDeclaration: KtNamedDeclaration?, + importsToDelete: MutableList +): Boolean { + var invalidUsagesFound = false + for (usage in usages.sortChildrenFirst()) { + try { + if (!usage.isValid) { + invalidUsagesFound = true + continue + } + + if (specialUsageProcessing(usage, targetDeclaration)) continue + + //TODO: keep the import if we don't know how to replace some of the usages + val importDirective = usage.getStrictParentOfType() + if (importDirective != null) { + if (!importDirective.isAllUnder && importDirective.targetDescriptors().size == 1) { + importsToDelete.add(importDirective) + } + continue + } + + createReplacer(usage)?.invoke() + } + catch (e: Throwable) { + LOG.error(e) + } + } + return !invalidUsagesFound +} + +private fun UsageReplacementStrategy.specialUsageProcessing(usage: KtSimpleNameExpression, targetDeclaration: KtNamedDeclaration?): Boolean { + val usageParent = usage.parent + when (usageParent) { + is KtCallableReferenceExpression -> { + val grandParent = usageParent.parent + ConvertReferenceToLambdaIntention().applyTo(usageParent, null) + (grandParent as? KtElement)?.let { + doRefactoringInside(it, targetDeclaration?.name, targetDeclaration?.descriptor) + } + return true + } + + is KtCallElement -> { + val lambdaArguments = usageParent.lambdaArguments + if (lambdaArguments.isNotEmpty()) { + val grandParent = usageParent.parent + val specifySignature = SpecifyExplicitLambdaSignatureIntention() + for (lambdaArgument in lambdaArguments) { + val lambdaExpression = lambdaArgument.getLambdaExpression() + val functionDescriptor = + lambdaExpression.functionLiteral.resolveToDescriptorIfAny() as? FunctionDescriptor ?: continue + if (functionDescriptor.valueParameters.isNotEmpty()) { + specifySignature.applyTo(lambdaExpression, null) + } + } + (grandParent as? KtElement)?.let { + doRefactoringInside(it, targetDeclaration?.name, targetDeclaration?.descriptor) + } + return true + } + } + + } + return false +} diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/RemoveCurlyBracesFromTemplateIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/RemoveCurlyBracesFromTemplateIntention.kt index 3aa644a4afa..d4cb958a624 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/RemoveCurlyBracesFromTemplateIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/RemoveCurlyBracesFromTemplateIntention.kt @@ -22,7 +22,7 @@ import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection import org.jetbrains.kotlin.psi.KtBlockStringTemplateEntry import org.jetbrains.kotlin.psi.KtNameReferenceExpression import org.jetbrains.kotlin.psi.KtPsiFactory -import org.jetbrains.kotlin.psi.KtStringTemplateEntryWithExpression +import org.jetbrains.kotlin.psi.KtSimpleNameStringTemplateEntry import org.jetbrains.kotlin.psi.psiUtil.canPlaceAfterSimpleNameEntry class RemoveCurlyBracesFromTemplateInspection : IntentionBasedInspection(RemoveCurlyBracesFromTemplateIntention::class) @@ -37,7 +37,7 @@ class RemoveCurlyBracesFromTemplateIntention : SelfTargetingOffsetIndependentInt applyTo(element) } - fun applyTo(element: KtBlockStringTemplateEntry): KtStringTemplateEntryWithExpression { + fun applyTo(element: KtBlockStringTemplateEntry): KtSimpleNameStringTemplateEntry { val name = (element.expression as KtNameReferenceExpression).getReferencedName() val newEntry = KtPsiFactory(element).createSimpleNameStringTemplateEntry(name) return element.replaced(newEntry) diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed4Runtime.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed4Runtime.kt.after index 42e8c29f315..7cf25e57d85 100644 --- a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed4Runtime.kt.after +++ b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed4Runtime.kt.after @@ -10,7 +10,7 @@ fun newFun(): Int = 0 fun foo(): Int { bar() - return newFun() + return newFun() } fun bar(): Int? = 0 diff --git a/idea/testData/refactoring/inline/function/InStringTemplates.kt b/idea/testData/refactoring/inline/function/InStringTemplates.kt new file mode 100644 index 00000000000..092e5e35027 --- /dev/null +++ b/idea/testData/refactoring/inline/function/InStringTemplates.kt @@ -0,0 +1,14 @@ +class X(val v: Int) { + fun foo() { + println("foo()") + return v + } +} + +fun X.f1() { + println("Value: ${foo()}") +} + +fun f2(x: X) { + println("Value: ${x.foo()}") +} \ No newline at end of file diff --git a/idea/testData/refactoring/inline/function/InStringTemplates.kt.after b/idea/testData/refactoring/inline/function/InStringTemplates.kt.after new file mode 100644 index 00000000000..8a523e6e088 --- /dev/null +++ b/idea/testData/refactoring/inline/function/InStringTemplates.kt.after @@ -0,0 +1,12 @@ +class X(val v: Int) { +} + +fun X.f1() { + println("foo()") + println("Value: $v") +} + +fun f2(x: X) { + println("foo()") + println("Value: ${x.v}") +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java index 67c9651a79e..dd982101c63 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java @@ -50,6 +50,12 @@ public class InlineTestGenerated extends AbstractInlineTest { doTest(fileName); } + @TestMetadata("InStringTemplates.kt") + public void testInStringTemplates() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/InStringTemplates.kt"); + doTest(fileName); + } + @TestMetadata("LocalCapturing.kt") public void testLocalCapturing() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/LocalCapturing.kt");