Remove redundant curly braces in string templates on inline

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

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<Unit>("ELEMENT_TO_BE_REPLACED_KEY")

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<KtImportDirective>()
-            val replacements = mutableListOf<KtElement>()
-
-            var invalidUsagesFound = false
-
             val targetDeclaration = targetPsiElement as? KtNamedDeclaration
 
-            val usagesChildrenFirst = usages.sortChildrenFirst()
+            val usagesByFile = usages.groupBy { it.containingFile }
-            usages@ for (usage in usagesChildrenFirst) {
-                try {
-                    if (!usage.isValid) {
-                        invalidUsagesFound = true
-                        continue
-                    }
 
-                    val usageParent = usage.parent
+            val KEY = Key<Unit>("UsageReplacementStrategy.replaceUsages")
-                    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
-                            }
-                        }
 
-                    }
+            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
+                // we should delete imports later to not affect other usages
-                    val importDirective = usage.getStrictParentOfType<KtImportDirective>()
+                val importsToDelete = mutableListOf<KtImportDirective>()
-                    if (importDirective != null) {
-                        if (!importDirective.isAllUnder && importDirective.targetDescriptors().size == 1) {
-                            importsToDelete.add(importDirective)
-                        }
-                        continue
-                    }
 
-                    val replacement = createReplacer(usage)?.invoke()
+                var usagesToProcess = usagesInFile
-                    if (replacement != null) {
+                while (usagesToProcess.isNotEmpty()) {
-                        replacements += replacement
+                    if (processUsages(usagesToProcess, targetDeclaration, importsToDelete)) break
-                    }
+
-                }
+                    // some usages may get invalidated we need to find them in the tree
-                catch (e: Throwable) {
+                    usagesToProcess = file.collectDescendantsOfType<KtSimpleNameExpression> { it.getCopyableUserData(KEY) != null }
-                    LOG.error(e)
                 }
+
+                file.forEachDescendantOfType<KtSimpleNameExpression> { 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<KtSimpleNameExpression>,
+        targetDeclaration: KtNamedDeclaration?,
+        importsToDelete: MutableList<KtImportDirective>
+): 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<KtImportDirective>()
+            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
+}

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<KtBlockStringTemplateEntry>(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)

idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed4Runtime.kt.after

@@ -10,7 +10,7 @@ fun newFun(): Int = 0
 
 fun foo(): Int {
     bar()
-    <caret>return newFun()
+    return <caret>newFun()
 }
 
 fun bar(): Int? = 0

idea/testData/refactoring/inline/function/InStringTemplates.kt

@@ -0,0 +1,14 @@
+class X(val v: Int) {
+    fun <caret>foo() {
+        println("foo()")
+        return v
+    }
+}
+
+fun X.f1() {
+    println("Value: ${foo()}")
+}
+
+fun f2(x: X) {
+    println("Value: ${x.foo()}")
+}

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}")
+}

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");