introduceValue() always adds statement before except for safe call case

2016-10-23 01:36:36 +03:00
parent 8f9f2027f2
commit 330d3a255a
15 changed files with 163 additions and 53 deletions
@@ -17,7 +17,7 @@
    <orderEntry type="module" module-name="frontend.java" />
    <orderEntry type="module" module-name="util" />
    <orderEntry type="library" scope="PROVIDED" name="properties" level="project" />
-    <orderEntry type="module" module-name="idea" scope="RUNTIME" />
+    <orderEntry type="module" module-name="idea" />
    <orderEntry type="module" module-name="formatter" />
  </component>
 </module>
@@ -18,6 +18,7 @@ package org.jetbrains.kotlin.idea.replacement

 import com.intellij.openapi.util.Key
 import org.jetbrains.kotlin.idea.core.replaced
+import org.jetbrains.kotlin.idea.intentions.ConvertToBlockBodyIntention
 import org.jetbrains.kotlin.psi.*
 import org.jetbrains.kotlin.psi.psiUtil.PsiChildRange
 import org.jetbrains.kotlin.psi.psiUtil.findDescendantOfType
@@ -64,11 +65,14 @@ internal class ExpressionReplacementPerformer(
        val insertedStatements = ArrayList<KtExpression>()
        val toInsertedStatementMap = HashMap<KtExpression, KtExpression>()
        for (statement in replacement.statementsBefore) {
+            // copy the statement if it can get invalidated by findOrCreateBlockToInsertStatement()
+            val statementToUse = if (statement.isPhysical) statement.copy() else statement
            val anchor = findOrCreateBlockToInsertStatement()
            val block = anchor.parent as KtBlockExpression

-            val inserted = block.addBefore(statement, anchor) as KtExpression
+            val inserted = block.addBefore(statementToUse, anchor) as KtExpression
            block.addBefore(psiFactory.createNewLine(), anchor)
+            block.addBefore(psiFactory.createNewLine(), inserted)
            toInsertedStatementMap.put(statement, inserted)
            insertedStatements.add(inserted)
        }
@@ -97,7 +101,6 @@ internal class ExpressionReplacementPerformer(
     * Returns statement in a block to insert statement before it
     */
    private fun findOrCreateBlockToInsertStatement(): KtExpression {
-        //TODO: Convert expression function body into block body
        //TODO: Sometimes it's not correct because of side effects

        for (element in elementToBeReplaced.parentsWithSelf) {
@@ -112,6 +115,13 @@ internal class ExpressionReplacementPerformer(
                        return element.replaceWithBlock()
                    }

+                    if (parent is KtDeclarationWithBody) {
+                        withElementToBeReplacedPreserved {
+                            ConvertToBlockBodyIntention.convert(parent)
+                        }
+                        return (parent.bodyExpression as KtBlockExpression).statements.single()
+                    }
+
                    if (parent is KtBlockExpression) return element
                }
            }
@@ -122,11 +132,19 @@ internal class ExpressionReplacementPerformer(
    }

    private fun KtExpression.replaceWithBlock(): KtExpression {
-        elementToBeReplaced.putCopyableUserData(ELEMENT_TO_BE_REPLACED_KEY, Unit)
-        val blockExpression = this.replaced(KtPsiFactory(this).createSingleStatementBlock(this))
-        elementToBeReplaced = blockExpression.findDescendantOfType<KtExpression> { it.getCopyableUserData(ELEMENT_TO_BE_REPLACED_KEY) != null }!!
+        val blockExpression = withElementToBeReplacedPreserved {
+            this.replaced(KtPsiFactory(this).createSingleStatementBlock(this))
+        }
        return blockExpression.statements.single()
    }
+
+    private fun <TElement : KtElement> withElementToBeReplacedPreserved(action: () -> TElement): TElement {
+        elementToBeReplaced.putCopyableUserData(ELEMENT_TO_BE_REPLACED_KEY, Unit)
+        val result = action()
+        elementToBeReplaced = result.findDescendantOfType<KtExpression> { it.getCopyableUserData(ELEMENT_TO_BE_REPLACED_KEY) != null }!!
+        elementToBeReplaced.putCopyableUserData(ELEMENT_TO_BE_REPLACED_KEY, null)
+        return result
+    }
 }

 private val ELEMENT_TO_BE_REPLACED_KEY = Key<Unit>("ELEMENT_TO_BE_REPLACED_KEY")
@@ -51,7 +51,9 @@ internal fun MutableReplacementCode.introduceValue(
    fun replaceUsages(name: Name) {
        val nameInCode = psiFactory.createExpression(name.render())
        for (usage in usages) {
-            usage.replace(nameInCode)
+            // there can be parenthesis around the expression which will become unnecessary
+            val usageToReplace = (usage.parent as? KtParenthesizedExpression) ?: usage
+            usageToReplace.replace(nameInCode)
        }
    }

@@ -67,54 +69,49 @@ internal fun MutableReplacementCode.introduceValue(
    fun isNameUsed(name: String) = collectNameUsages(this, name).any { nameUsage -> usages.none { it.isAncestor(nameUsage) } }

    if (!safeCall) {
-        val block = expressionToBeReplaced.parent as? KtBlockExpression
-        if (block != null) {
-            val resolutionScope = expressionToBeReplaced.getResolutionScope(bindingContext, expressionToBeReplaced.getResolutionFacade())
+        val resolutionScope = expressionToBeReplaced.getResolutionScope(bindingContext, expressionToBeReplaced.getResolutionFacade())

-            if (usages.isNotEmpty()) {
-                var explicitType: KotlinType? = null
-                if (valueType != null && !ErrorUtils.containsErrorType(valueType)) {
-                    val valueTypeWithoutExpectedType = value.computeTypeInContext(
-                            resolutionScope,
-                            expressionToBeReplaced,
-                            dataFlowInfo = bindingContext.getDataFlowInfo(expressionToBeReplaced)
-                    )
-                    if (valueTypeWithoutExpectedType == null || ErrorUtils.containsErrorType(valueTypeWithoutExpectedType)) {
-                        explicitType = valueType
-                    }
+        if (usages.isNotEmpty()) {
+            var explicitType: KotlinType? = null
+            if (valueType != null && !ErrorUtils.containsErrorType(valueType)) {
+                val valueTypeWithoutExpectedType = value.computeTypeInContext(
+                        resolutionScope,
+                        expressionToBeReplaced,
+                        dataFlowInfo = bindingContext.getDataFlowInfo(expressionToBeReplaced)
+                )
+                if (valueTypeWithoutExpectedType == null || ErrorUtils.containsErrorType(valueTypeWithoutExpectedType)) {
+                    explicitType = valueType
                }
-
-                val name = suggestName { name ->
-                    resolutionScope.findLocalVariable(Name.identifier(name)) == null && !isNameUsed(name)
-                }
-
-                val declaration = psiFactory.createDeclarationByPattern<KtVariableDeclaration>("val $0 = $1", name, value)
-                statementsBefore.add(0, declaration)
-                if (explicitType != null) {
-                    addPostInsertionAction(declaration) { it.setType(explicitType!!); it }
-                }
-
-                replaceUsages(name)
            }
-            else {
-                statementsBefore.add(0, value)
+
+            val name = suggestName { name ->
+                resolutionScope.findLocalVariable(Name.identifier(name)) == null && !isNameUsed(name)
            }
-            return
+
+            val declaration = psiFactory.createDeclarationByPattern<KtVariableDeclaration>("val $0 = $1", name, value)
+            statementsBefore.add(0, declaration)
+            if (explicitType != null) {
+                addPostInsertionAction(declaration) { it.setType(explicitType!!); it }
+            }
+
+            replaceUsages(name)
+        }
+        else {
+            statementsBefore.add(0, value)
        }
    }
-
-    //TODO: handle mainExpression == null and statementsBefore!
-
-    val dot = if (safeCall) "?." else "."
-
-    mainExpression = if (!isNameUsed("it")) {
-        replaceUsages(Name.identifier("it"))
-        psiFactory.createExpressionByPattern("$0${dot}let { $1 }", value, mainExpression!!)
-    }
    else {
-        val name = suggestName { !isNameUsed(it) }
-        replaceUsages(name)
-        psiFactory.createExpressionByPattern("$0${dot}let { $1 -> $2 }", value, name, mainExpression!!)
+        //TODO: handle mainExpression == null and statementsBefore!
+
+        mainExpression = if (!isNameUsed("it")) {
+            replaceUsages(Name.identifier("it"))
+            psiFactory.createExpressionByPattern("$0?.let { $1 }", value, mainExpression!!)
+        }
+        else {
+            val name = suggestName { !isNameUsed(it) }
+            replaceUsages(name)
+            psiFactory.createExpressionByPattern("$0?.let { $1 -> $2 }", value, name, mainExpression!!)
+        }
    }
 }

@@ -8,7 +8,8 @@ fun oldFun(p: Int): Int {
 fun newFun(): Int = 0

 fun foo(): Int {
-    return bar().let { newFun() }
+    bar()
+    return newFun()
 }

 fun bar(): Int = 0
@@ -7,6 +7,9 @@ fun oldFun(p: Int?): Int {

 fun newFun(): Int = 0

-fun foo(): Int = bar().<caret>let { newFun() }
+fun foo(): Int {
+    bar()
+    <caret>return newFun()
+}

 fun bar(): Int? = 0
@@ -8,7 +8,10 @@ fun oldFun(p1: Int, p2: Int): Boolean {
 fun newFun(p: Int) = false

 fun foo(list: List<Int>) {
-    list.filter { !bar().let { p1 -> newFun(it) } }
+    list.filter {
+        bar()
+        !newFun(it)
+    }
 }

 fun bar(): Int = 0
@@ -9,5 +9,6 @@ fun newFun(p1: Int, p2: Int): Int = 0

 fun foo(): Int {
    var v = 0
-    return v++.let { newFun(it, it) }
+    val p = v++
+    return newFun(p, p)
 }
@@ -8,7 +8,8 @@ fun oldFun(p: Int?): Int {
 fun newFun(p1: Int?, p2: Int?): Int = 0

 fun foo(): Int {
-    return bar().let { newFun(it, it) }
+    val p = bar()
+    return newFun(p, p)
 }

 fun bar(): Int? = null
@@ -0,0 +1,8 @@
+fun String.<caret>f(p: Int) = hashCode() * p
+
+fun f(s: String?) {
+    s?.f(1)
+    s?.substring(1)?.f(2)
+    val s1 = s?.f(3)
+    val s2 = s?.substring(1)?.f(4)
+}
@@ -0,0 +1,11 @@
+fun f(s: String?) {
+    if (s != null) {
+        s.hashCode() * 1
+    }
+    val substring = s?.substring(1)
+    if (substring != null) {
+        substring.hashCode() * 2
+    }
+    val s1 = s?.let { it.hashCode() * 3 }
+    val s2 = s?.substring(1)?.let { it.hashCode() * 4 }
+}
@@ -0,0 +1,11 @@
+fun <caret>f(p1: Int, p2: Int): Int {
+    println(p1)
+    println(p2)
+    return p1 + p2
+}
+
+fun main(args: Array<String>) {
+    for (i in 1..10) {
+        println(f(i, i + 1))
+    }
+}
@@ -0,0 +1,8 @@
+fun main(args: Array<String>) {
+    for (i in 1..10) {
+        val p2 = i + 1
+        println(i)
+        println(p2)
+        println(i + p2)
+    }
+}
@@ -0,0 +1,12 @@
+fun <caret>f(p1: Int, p2: Int): Int {
+    println(p1)
+    println(p2)
+    return p1 + p2
+}
+
+fun foo1() = f(1, 2)
+
+fun foo2(): Int = f(3, 4)
+
+val v: Int
+    get() = f(5, 6)
@@ -0,0 +1,18 @@
+fun foo1(): Int {
+    println(1)
+    println(2)
+    return 1 + 2
+}
+
+fun foo2(): Int {
+    println(3)
+    println(4)
+    return 3 + 4
+}
+
+val v: Int
+    get() {
+        println(5)
+        println(6)
+        return 5 + 6
+    }
@@ -87,6 +87,12 @@ public class InlineTestGenerated extends AbstractInlineTest {
                doTest(fileName);
            }

+            @TestMetadata("SafeCall.kt")
+            public void testSafeCall() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/expressionBody/SafeCall.kt");
+                doTest(fileName);
+            }
+
            @TestMetadata("Simple.kt")
            public void testSimple() throws Exception {
                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/expressionBody/Simple.kt");
@@ -108,12 +114,24 @@ public class InlineTestGenerated extends AbstractInlineTest {
                KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/refactoring/inline/function/returnAtEnd"), Pattern.compile("^(\\w+)\\.kt$"), true);
            }

+            @TestMetadata("Bug1.kt")
+            public void testBug1() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/returnAtEnd/Bug1.kt");
+                doTest(fileName);
+            }
+
            @TestMetadata("CallArgument.kt")
            public void testCallArgument() throws Exception {
                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/returnAtEnd/CallArgument.kt");
                doTest(fileName);
            }

+            @TestMetadata("ConvertToBlockBody.kt")
+            public void testConvertToBlockBody() throws Exception {
+                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/returnAtEnd/ConvertToBlockBody.kt");
+                doTest(fileName);
+            }
+
            @TestMetadata("MultipleStatements.kt")
            public void testMultipleStatements() throws Exception {
                String fileName = KotlinTestUtils.navigationMetadata("idea/testData/refactoring/inline/function/returnAtEnd/MultipleStatements.kt");