Refactoring: inline val / fun now use the common inliner

This prevents their inconsistent work in some situations NB: breaks three tests if used alone
2017-04-04 19:19:03 +03:00
parent e6fa7356e1
commit 4ef0096d46
11 changed files with 111 additions and 221 deletions
@@ -18,6 +18,7 @@ package org.jetbrains.kotlin.idea.refactoring.inline

 import com.intellij.lang.findUsages.DescriptiveNameUtil
 import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiElement
 import com.intellij.psi.search.GlobalSearchScope
 import com.intellij.psi.search.searches.ReferencesSearch
 import com.intellij.refactoring.BaseRefactoringProcessor
@@ -31,25 +32,34 @@ import org.jetbrains.kotlin.idea.codeInliner.replaceUsages
 import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
 import org.jetbrains.kotlin.idea.stubindex.KotlinSourceFilterScope
 import org.jetbrains.kotlin.idea.util.application.runReadAction
+import org.jetbrains.kotlin.psi.KtCallableDeclaration
 import org.jetbrains.kotlin.psi.KtNamedFunction
+import org.jetbrains.kotlin.psi.KtProperty
 import org.jetbrains.kotlin.psi.KtSimpleNameExpression

-class KotlinInlineFunctionProcessor(
+class KotlinInlineCallableProcessor(
        project: Project,
        private val replacementStrategy: CallableUsageReplacementStrategy,
-        private val function: KtNamedFunction,
+        private val declaration: KtCallableDeclaration,
        private val reference: KtSimpleNameReference?,
        private val inlineThisOnly: Boolean,
-        private val deleteAfter: Boolean
+        private val deleteAfter: Boolean,
+        private val assignments: Set<PsiElement> = emptySet()
 ) : BaseRefactoringProcessor(project) {

-    private val commandName = "Inlining function ${DescriptiveNameUtil.getDescriptiveName(function)}"
+    private val kind = when (declaration) {
+        is KtNamedFunction -> "function"
+        is KtProperty -> if (declaration.isLocal) "local variable" else "property"
+        else -> "declaration"
+    }
+
+    private val commandName = "Inlining $kind ${DescriptiveNameUtil.getDescriptiveName(declaration)}"

    override fun findUsages(): Array<UsageInfo> {
        if (inlineThisOnly && reference != null) return arrayOf(UsageInfo(reference))
        val usages = runReadAction {
            val searchScope = KotlinSourceFilterScope.projectSources(GlobalSearchScope.projectScope(myProject), myProject)
-            ReferencesSearch.search(function, searchScope)
+            ReferencesSearch.search(declaration, searchScope)
        }
        return usages.map(::UsageInfo).toTypedArray()
    }
@@ -58,20 +68,21 @@ class KotlinInlineFunctionProcessor(
        val simpleNameUsages = usages.mapNotNull { it.element as? KtSimpleNameExpression }
        replacementStrategy.replaceUsages(
                simpleNameUsages,
-                function,
+                declaration,
                myProject,
                commandName,
                {
                    if (deleteAfter) {
                        if (usages.size == simpleNameUsages.size) {
-                            function.delete()
+                            declaration.delete()
+                            assignments.forEach(PsiElement::delete)
                        }
                        else {
                            CommonRefactoringUtil.showErrorHint(
-                                    function.project,
+                                    declaration.project,
                                    null,
                                    "Cannot inline ${usages.size - simpleNameUsages.size}/${usages.size} usages",
-                                    "Inline Function",
+                                    "Inline $kind",
                                    null
                            )
                        }
@@ -90,9 +101,9 @@ class KotlinInlineFunctionProcessor(
            override fun getCodeReferencesText(usagesCount: Int, filesCount: Int) =
                    RefactoringBundle.message("invocations.to.be.inlined", UsageViewBundle.getReferencesString(usagesCount, filesCount))

-            override fun getElements() = arrayOf(function)
+            override fun getElements() = arrayOf(declaration)

-            override fun getProcessedElementsHeader() = "Function to inline"
+            override fun getProcessedElementsHeader() = "${kind.capitalize()} to inline"
        }
    }
 }
@@ -20,7 +20,6 @@ import com.intellij.openapi.help.HelpManager
 import com.intellij.openapi.project.Project
 import com.intellij.refactoring.HelpID
 import com.intellij.refactoring.JavaRefactoringSettings
-import com.intellij.refactoring.RefactoringBundle
 import com.intellij.refactoring.inline.InlineOptionsDialog
 import org.jetbrains.kotlin.idea.codeInliner.CallableUsageReplacementStrategy
 import org.jetbrains.kotlin.idea.quickfix.createFromUsage.callableBuilder.getReturnTypeReference
@@ -71,7 +70,7 @@ class KotlinInlineFunctionDialog(

    public override fun doAction() {
        invokeRefactoring(
-                KotlinInlineFunctionProcessor(project, replacementStrategy, function, reference,
+                KotlinInlineCallableProcessor(project, replacementStrategy, function, reference,
                                              inlineThisOnly = isInlineThisOnly || allowInlineThisOnly,
                                              deleteAfter = !isInlineThisOnly && !isKeepTheDeclaration && !allowInlineThisOnly)
        )
@@ -17,48 +17,64 @@
 package org.jetbrains.kotlin.idea.refactoring.inline

 import com.intellij.openapi.util.text.StringUtil
-import com.intellij.psi.PsiReference
+import com.intellij.psi.PsiElement
 import com.intellij.refactoring.JavaRefactoringSettings
-import com.intellij.refactoring.inline.AbstractInlineLocalDialog
+import com.intellij.refactoring.inline.InlineOptionsDialog
+import org.jetbrains.kotlin.idea.codeInliner.CallableUsageReplacementStrategy
+import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
 import org.jetbrains.kotlin.psi.KtProperty

 class KotlinInlineValDialog(
-        element: KtProperty,
-        ref: PsiReference?,
-        private val occurrenceCount: Int
-) : AbstractInlineLocalDialog(element.project, element, ref, occurrenceCount) {
-    private val property: KtProperty get() = myElement as KtProperty
+        private val property: KtProperty,
+        private val reference: KtSimpleNameReference?,
+        private val replacementStrategy: CallableUsageReplacementStrategy,
+        private val assignments: Set<PsiElement>
+ ) : InlineOptionsDialog(property.project, true, property) {
+
+    private var occurrenceCount = initOccurrencesNumber(property)

    private val kind = if (property.isLocal) "local variable" else "property"

    private val refactoringName = "Inline ${StringUtil.capitalizeWords(kind, true)}"

    init {
-        myInvokedOnReference = ref != null
+        myInvokedOnReference = reference != null
        title = refactoringName
        init()
    }

+    override fun allowInlineAll() = true
+
    override fun getBorderTitle() = refactoringName

    override fun getNameLabelText() = "${kind.capitalize()} ${property.name}"

-    override fun getInlineAllText(): String? {
-        val occurrencesString = if (occurrenceCount >= 0) {
-            " (" + occurrenceCount + " occurrence" + (if (occurrenceCount == 1) ")" else "s)")
-        } else ""
-        return "Inline all references and remove the $kind" + occurrencesString
-    }
+    private val occurrencesString get() = if (occurrenceCount >= 0) {
+        " (" + occurrenceCount + " occurrence" + (if (occurrenceCount == 1) ")" else "s)")
+    } else ""
+
+    override fun getInlineAllText() =
+            "Inline all references and remove the $kind" + occurrencesString
+
+    override fun getKeepTheDeclarationText(): String? =
+            if (property.isWritable) "Inline all references and keep the $kind" + occurrencesString
+            else super.getKeepTheDeclarationText()

    override fun isInlineThis() = JavaRefactoringSettings.getInstance().INLINE_LOCAL_THIS

    override fun getInlineThisText() = "Inline this occurrence and leave the $kind"

-    override fun doAction() {
+    public override fun doAction() {
+        invokeRefactoring(
+                KotlinInlineCallableProcessor(project, replacementStrategy, property, reference,
+                                              inlineThisOnly = isInlineThisOnly,
+                                              deleteAfter = !isInlineThisOnly && !isKeepTheDeclaration,
+                                              assignments = assignments)
+        )
+
        val settings = JavaRefactoringSettings.getInstance()
        if (myRbInlineThisOnly.isEnabled && myRbInlineAll.isEnabled) {
            settings.INLINE_LOCAL_THIS = isInlineThisOnly
        }
-        close(OK_EXIT_CODE)
    }
 }
@@ -16,51 +16,39 @@

 package org.jetbrains.kotlin.idea.refactoring.inline

-import com.google.common.collect.Sets
+import com.intellij.codeInsight.TargetElementUtil
 import com.intellij.lang.Language
 import com.intellij.lang.refactoring.InlineActionHandler
 import com.intellij.openapi.application.ApplicationManager
 import com.intellij.openapi.editor.Editor
-import com.intellij.openapi.editor.ex.EditorSettingsExternalizable
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.wm.WindowManager
 import com.intellij.psi.PsiElement
-import com.intellij.psi.PsiReference
-import com.intellij.psi.PsiWhiteSpace
 import com.intellij.psi.search.searches.ReferencesSearch
 import com.intellij.refactoring.HelpID
-import com.intellij.refactoring.JavaRefactoringSettings
 import com.intellij.refactoring.RefactoringBundle
 import com.intellij.refactoring.util.CommonRefactoringUtil
 import com.intellij.util.containers.MultiMap
-import org.jetbrains.kotlin.diagnostics.Errors
+import org.jetbrains.kotlin.descriptors.VariableDescriptor
 import org.jetbrains.kotlin.idea.KotlinLanguage
-import org.jetbrains.kotlin.idea.caches.resolve.analyze
+import org.jetbrains.kotlin.idea.analysis.analyzeInContext
 import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
-import org.jetbrains.kotlin.idea.codeInsight.shorten.performDelayedRefactoringRequests
-import org.jetbrains.kotlin.idea.core.ShortenReferences
-import org.jetbrains.kotlin.idea.core.replaced
-import org.jetbrains.kotlin.idea.refactoring.addTypeArgumentsIfNeeded
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor
+import org.jetbrains.kotlin.idea.codeInliner.CallableUsageReplacementStrategy
+import org.jetbrains.kotlin.idea.codeInliner.CodeToInlineBuilder
+import org.jetbrains.kotlin.idea.core.copied
 import org.jetbrains.kotlin.idea.refactoring.checkConflictsInteractively
-import org.jetbrains.kotlin.idea.refactoring.getQualifiedTypeArgumentList
-import org.jetbrains.kotlin.idea.references.mainReference
-import org.jetbrains.kotlin.idea.resolve.ResolutionFacade
-import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers
-import org.jetbrains.kotlin.idea.util.application.executeWriteCommand
+import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
+import org.jetbrains.kotlin.idea.util.getResolutionScope
 import org.jetbrains.kotlin.psi.*
-import org.jetbrains.kotlin.psi.psiUtil.*
+import org.jetbrains.kotlin.psi.psiUtil.getAssignmentByLHS
+import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForSelectorOrThis
 import org.jetbrains.kotlin.resolve.BindingContext
-import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode
-import org.jetbrains.kotlin.types.ErrorUtils
+import org.jetbrains.kotlin.types.TypeUtils
 import org.jetbrains.kotlin.types.expressions.OperatorConventions
 import org.jetbrains.kotlin.utils.addIfNotNull
-import org.jetbrains.kotlin.utils.sure
-import java.util.*

 class KotlinInlineValHandler : InlineActionHandler() {
-    enum class InlineMode {
-        ALL, PRIMARY, NONE
-    }

    override fun isEnabledForLanguage(l: Language) = l == KotlinLanguage.INSTANCE

@@ -69,46 +57,14 @@ class KotlinInlineValHandler : InlineActionHandler() {
        return element.getter == null && element.receiverTypeReference == null
    }

-    private fun doReplace(expression: KtExpression, replacement: KtExpression): List<KtExpression> {
-        val parent = expression.parent
-
-        if (parent is KtStringTemplateEntryWithExpression &&
-            replacement is KtStringTemplateExpression &&
-            // Do not mix raw and non-raw templates
-            parent.parent.firstChild.text == replacement.firstChild.text) {
-            val entriesToAdd = replacement.entries
-            val templateExpression = parent.parent as KtStringTemplateExpression
-            val inlinedExpressions = if (entriesToAdd.isNotEmpty()) {
-                val firstAddedEntry = templateExpression.addRangeBefore(entriesToAdd.first(), entriesToAdd.last(), parent)
-                val lastNewEntry = parent.prevSibling
-                val nextElement = parent.nextSibling
-                if (lastNewEntry is KtSimpleNameStringTemplateEntry &&
-                    lastNewEntry.expression != null &&
-                    !canPlaceAfterSimpleNameEntry(nextElement)) {
-                    lastNewEntry.replace(KtPsiFactory(expression).createBlockStringTemplateEntry(lastNewEntry.expression!!))
-                }
-                firstAddedEntry.siblings()
-                        .take(entriesToAdd.size)
-                        .mapNotNull { (it as? KtStringTemplateEntryWithExpression)?.expression }
-                        .toList()
-            }
-            else emptyList()
-
-            parent.delete()
-            return inlinedExpressions
-        }
-
-        return listOf(expression.replaced(replacement))
-    }
-
    override fun inlineElement(project: Project, editor: Editor?, element: PsiElement) {
        val declaration = element as KtProperty
        val file = declaration.containingKtFile
        val name = declaration.name ?: return

        val references = ReferencesSearch.search(declaration)
-        val referenceExpressions = ArrayList<KtExpression>()
-        val foreignUsages = ArrayList<PsiElement>()
+        val referenceExpressions = mutableListOf<KtExpression>()
+        val foreignUsages = mutableListOf<PsiElement>()
        for (ref in references) {
            val refElement = ref.element ?: continue
            if (refElement !is KtElement) {
@@ -123,7 +79,7 @@ class KotlinInlineValHandler : InlineActionHandler() {
            return showErrorHint(project, editor, "$kind '$name' is never used")
        }

-        val assignments = Sets.newHashSet<PsiElement>()
+        val assignments = hashSetOf<PsiElement>()
        referenceExpressions.forEach { expression ->
            val parent = expression.parent

@@ -147,9 +103,6 @@ class KotlinInlineValHandler : InlineActionHandler() {
            ?: return reportAmbiguousAssignment(project, editor, name, assignments)
        }

-        val typeArgumentsForCall = getQualifiedTypeArgumentList(initializer)
-        val parametersForFunctionLiteral = getParametersForFunctionLiteral(initializer)
-
        val referencesInOriginalFile = referenceExpressions.filter { it.containingFile == file }
        val isHighlighting = referencesInOriginalFile.isNotEmpty()
        highlightElements(project, editor, referencesInOriginalFile)
@@ -158,61 +111,36 @@ class KotlinInlineValHandler : InlineActionHandler() {
            preProcessInternalUsages(initializer, referenceExpressions)
        }

-        fun performRefactoring() {
-            val primaryExpression = if (editor != null) {
-                val offset = editor.caretModel.offset
-                referenceExpressions.firstOrNull { it.textRange.contains(offset) }
-            }
-            else null
-            val primaryRef = primaryExpression?.mainReference
+        val descriptor = element.resolveToDescriptor() as VariableDescriptor
+        val expectedType = if (element.typeReference != null)
+            descriptor.returnType ?: TypeUtils.NO_EXPECTED_TYPE
+        else
+            TypeUtils.NO_EXPECTED_TYPE

-            val inlineMode = showDialog(declaration, primaryRef, referenceExpressions.size)
-            if (inlineMode == InlineMode.NONE) {
-                if (isHighlighting) {
+        val initializerCopy = initializer.copied()
+        fun analyzeInitializerCopy(): BindingContext {
+            return initializerCopy.analyzeInContext(initializer.getResolutionScope(),
+                                                    contextExpression = initializer,
+                                                    expectedType = expectedType)
+        }
+
+        fun performRefactoring() {
+            val reference = editor?.let { TargetElementUtil.findReference(it, it.caretModel.offset) } as? KtSimpleNameReference
+            val replacementBuilder = CodeToInlineBuilder(descriptor, element.getResolutionFacade())
+            val replacement = replacementBuilder.prepareCodeToInline(initializerCopy, emptyList(), ::analyzeInitializerCopy)
+            val replacementStrategy = CallableUsageReplacementStrategy(replacement)
+
+            val dialog = KotlinInlineValDialog(declaration, reference, replacementStrategy, assignments)
+
+            if (!ApplicationManager.getApplication().isUnitTestMode) {
+                dialog.show()
+                if (!dialog.isOK && isHighlighting) {
                    val statusBar = WindowManager.getInstance().getStatusBar(project)
                    statusBar?.info = RefactoringBundle.message("press.escape.to.remove.the.highlighting")
                }
-                return
            }
-
-            val chosenExpressions = if (inlineMode == InlineMode.ALL) referenceExpressions else listOf(primaryExpression)
-
-            project.executeWriteCommand(RefactoringBundle.message("inline.command", name)) {
-                val inlinedExpressions = chosenExpressions
-                        .flatMap { referenceExpression ->
-                            if (assignments.contains(referenceExpression!!.parent)) return@flatMap emptyList<KtExpression>()
-
-                            val importDirective = referenceExpression.getStrictParentOfType<KtImportDirective>()
-                            if (importDirective != null) {
-                                val reference = referenceExpression.getQualifiedElementSelector()?.mainReference
-                                if (reference != null && reference.multiResolve(false).size <= 1) {
-                                    importDirective.delete()
-                                }
-
-                                return@flatMap emptyList<KtExpression>()
-                            }
-
-                            doReplace(referenceExpression, initializer)
-                        }
-                        .mapNotNull { postProcessInternalReferences(it) }
-
-                if (inlineMode == InlineMode.ALL) {
-                    assignments.forEach { it.delete() }
-                    declaration.delete()
-                }
-
-                if (inlinedExpressions.isNotEmpty()) {
-                    if (typeArgumentsForCall != null) {
-                        inlinedExpressions.forEach { addTypeArgumentsIfNeeded(it, typeArgumentsForCall) }
-                    }
-
-                    parametersForFunctionLiteral?.let { addFunctionLiteralParameterTypes(it, inlinedExpressions) }
-
-                    if (isHighlighting) {
-                        highlightElements(project, editor, inlinedExpressions)
-                    }
-                }
-                performDelayedRefactoringRequests(project)
+            else {
+                dialog.doAction()
            }
        }

@@ -238,80 +166,4 @@ class KotlinInlineValHandler : InlineActionHandler() {
        CommonRefactoringUtil.showErrorHint(project, editor, message, RefactoringBundle.message("inline.variable.title"), HelpID.INLINE_VARIABLE)
    }

-    private fun showDialog(property: KtProperty, ref: PsiReference?, occurrenceCount: Int): InlineMode {
-        if (ApplicationManager.getApplication().isUnitTestMode) return InlineMode.ALL
-        if ((ref == null || occurrenceCount <= 1) && !EditorSettingsExternalizable.getInstance().isShowInlineLocalDialog) return InlineMode.ALL
-
-        val dialog = KotlinInlineValDialog(property, ref, occurrenceCount)
-        if (!dialog.showAndGet()) return InlineMode.NONE
-        return if (JavaRefactoringSettings.getInstance().INLINE_LOCAL_THIS) InlineMode.PRIMARY else InlineMode.ALL
-    }
-
-    private fun getParametersForFunctionLiteral(initializer: KtExpression): String? {
-        val functionLiteralExpression = initializer.unpackFunctionLiteral(true) ?: return null
-        val context = initializer.analyze(BodyResolveMode.PARTIAL)
-        val lambdaDescriptor = context.get(BindingContext.FUNCTION, functionLiteralExpression.functionLiteral)
-        if (lambdaDescriptor == null || ErrorUtils.containsErrorType(lambdaDescriptor)) return null
-        return lambdaDescriptor.valueParameters.joinToString {
-            it.name.asString() + ": " + IdeDescriptorRenderers.SOURCE_CODE.renderType(it.type)
-        }
-    }
-
-    private fun addFunctionLiteralParameterTypes(parameters: String, inlinedExpressions: List<KtExpression>) {
-        val containingFile = inlinedExpressions.first().containingKtFile
-        val resolutionFacade = containingFile.getResolutionFacade()
-
-        val functionsToAddParameters = inlinedExpressions.mapNotNull {
-            val lambdaExpr = it.unpackFunctionLiteral(true).sure { "can't find function literal expression for " + it.text }
-            if (needToAddParameterTypes(lambdaExpr, resolutionFacade)) lambdaExpr else null
-        }
-
-        val psiFactory = KtPsiFactory(containingFile)
-        for (lambdaExpr in functionsToAddParameters) {
-            val lambda = lambdaExpr.functionLiteral
-
-            val currentParameterList = lambda.valueParameterList
-            val newParameterList = psiFactory.createParameterList("($parameters)")
-            if (currentParameterList != null) {
-                currentParameterList.replace(newParameterList)
-            }
-            else {
-                // TODO: Ugly code, need refactoring
-                val openBraceElement = lambda.lBrace
-
-                val nextSibling = openBraceElement.nextSibling
-                val whitespaceToAdd = if (nextSibling is PsiWhiteSpace && nextSibling.text.contains("\n"))
-                    nextSibling.copy()
-                else
-                    null
-
-                val whitespaceAndArrow = psiFactory.createWhitespaceAndArrow()
-                lambda.addRangeAfter(whitespaceAndArrow.first, whitespaceAndArrow.second, openBraceElement)
-
-                lambda.addAfter(newParameterList, openBraceElement)
-                if (whitespaceToAdd != null) {
-                    lambda.addAfter(whitespaceToAdd, openBraceElement)
-                }
-            }
-            ShortenReferences.DEFAULT.process(lambdaExpr.valueParameters)
-        }
-    }
-
-    private fun needToAddParameterTypes(
-            lambdaExpression: KtLambdaExpression,
-            resolutionFacade: ResolutionFacade
-    ): Boolean {
-        val functionLiteral = lambdaExpression.functionLiteral
-        val context = resolutionFacade.analyze(lambdaExpression, BodyResolveMode.PARTIAL_WITH_DIAGNOSTICS)
-        return context.diagnostics.any { diagnostic ->
-            val factory = diagnostic.factory
-            val element = diagnostic.psiElement
-            val hasCantInferParameter = factory == Errors.CANNOT_INFER_PARAMETER_TYPE &&
-                                        element.parent.parent == functionLiteral
-            val hasUnresolvedItOrThis = factory == Errors.UNRESOLVED_REFERENCE &&
-                                        element.text == "it" &&
-                                        element.getStrictParentOfType<KtFunctionLiteral>() == functionLiteral
-            hasCantInferParameter || hasUnresolvedItOrThis
-        }
-    }
 }
@@ -1,3 +1,3 @@
 fun foo() {
-    val ff = {(it: Int) -> it }
+    val ff = { it: Int -> it }
 }
@@ -1,6 +1,6 @@
 fun foo() {
    val ff = {
-        (it: Int) ->
+        it: Int ->
        it
    }
 }
@@ -1,3 +1,3 @@
 fun foo() {
-    val ff = (({(x: Int) -> x }))
+    val ff = (({ x: Int -> x }))
 }
@@ -1,3 +1,3 @@
 fun foo() {
-    val ff = {(x: Int) -> x }
+    val ff = { x: Int -> x }
 }
@@ -6,5 +6,6 @@ class Class {
 }

 fun f() {
+    Class()
    println(239)
 }
@@ -0,0 +1,5 @@
+// ERROR: Cannot perform refactoring.\nVariable length has no initializer
+
+fun foo(s: String) {
+    val l = s.<caret>length
+}
@@ -982,6 +982,12 @@ public class InlineTestGenerated extends AbstractInlineTest {
                doTest(fileName);
            }

+            @TestMetadata("Library.kt")
+            public void testLibrary() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/Library.kt");
+                doTest(fileName);
+            }
+
            @TestMetadata("multiplePackages.kt")
            public void testMultiplePackages() throws Exception {
                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/inlineVariableOrProperty/property/multiplePackages.kt");