diff --git a/idea/idea-replacement-engine/idea-replacement-engine.iml b/idea/idea-replacement-engine/idea-replacement-engine.iml index f749c7a75eb..f1ca7e6921c 100644 --- a/idea/idea-replacement-engine/idea-replacement-engine.iml +++ b/idea/idea-replacement-engine/idea-replacement-engine.iml @@ -17,7 +17,7 @@ - + \ No newline at end of file diff --git a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ReplacementPerformer.kt b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ReplacementPerformer.kt index ae96efe1104..615e976320c 100644 --- a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ReplacementPerformer.kt +++ b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/ReplacementPerformer.kt @@ -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() val toInsertedStatementMap = HashMap() 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 { it.getCopyableUserData(ELEMENT_TO_BE_REPLACED_KEY) != null }!! + val blockExpression = withElementToBeReplacedPreserved { + this.replaced(KtPsiFactory(this).createSingleStatementBlock(this)) + } return blockExpression.statements.single() } + + private fun withElementToBeReplacedPreserved(action: () -> TElement): TElement { + elementToBeReplaced.putCopyableUserData(ELEMENT_TO_BE_REPLACED_KEY, Unit) + val result = action() + elementToBeReplaced = result.findDescendantOfType { 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("ELEMENT_TO_BE_REPLACED_KEY") \ No newline at end of file diff --git a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/introduceValue.kt b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/introduceValue.kt index 068399cf076..73300fb7368 100644 --- a/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/introduceValue.kt +++ b/idea/idea-replacement-engine/src/org/jetbrains/kotlin/idea/replacement/introduceValue.kt @@ -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("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("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!!) + } } } diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed3Runtime.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed3Runtime.kt.after index 408648d7852..5aac156e32e 100644 --- a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed3Runtime.kt.after +++ b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed3Runtime.kt.after @@ -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 diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed4Runtime.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed4Runtime.kt.after index 6165eeee9ed..f68544db8fc 100644 --- a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed4Runtime.kt.after +++ b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed4Runtime.kt.after @@ -7,6 +7,9 @@ fun oldFun(p: Int?): Int { fun newFun(): Int = 0 -fun foo(): Int = bar().let { newFun() } +fun foo(): Int { + bar() + return newFun() +} fun bar(): Int? = 0 diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed5Runtime.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed5Runtime.kt.after index 695836d55cc..4acd770c5b5 100644 --- a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed5Runtime.kt.after +++ b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionNotUsed5Runtime.kt.after @@ -8,7 +8,10 @@ fun oldFun(p1: Int, p2: Int): Boolean { fun newFun(p: Int) = false fun foo(list: List) { - list.filter { !bar().let { p1 -> newFun(it) } } + list.filter { + bar() + !newFun(it) + } } fun bar(): Int = 0 diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionUsedTwice3Runtime.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionUsedTwice3Runtime.kt.after index 67c708a6eeb..4d0a942ef8b 100644 --- a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionUsedTwice3Runtime.kt.after +++ b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionUsedTwice3Runtime.kt.after @@ -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) } diff --git a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionUsedTwice6Runtime.kt.after b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionUsedTwice6Runtime.kt.after index 19bbe9981e1..f9009977292 100644 --- a/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionUsedTwice6Runtime.kt.after +++ b/idea/testData/quickfix/deprecatedSymbolUsage/argumentSideEffects/complexExpressionUsedTwice6Runtime.kt.after @@ -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 diff --git a/idea/testData/refactoring/inline/function/expressionBody/SafeCall.kt b/idea/testData/refactoring/inline/function/expressionBody/SafeCall.kt new file mode 100644 index 00000000000..2b0f9afc075 --- /dev/null +++ b/idea/testData/refactoring/inline/function/expressionBody/SafeCall.kt @@ -0,0 +1,8 @@ +fun String.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) +} \ No newline at end of file diff --git a/idea/testData/refactoring/inline/function/expressionBody/SafeCall.kt.after b/idea/testData/refactoring/inline/function/expressionBody/SafeCall.kt.after new file mode 100644 index 00000000000..847042d2197 --- /dev/null +++ b/idea/testData/refactoring/inline/function/expressionBody/SafeCall.kt.after @@ -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 } +} diff --git a/idea/testData/refactoring/inline/function/returnAtEnd/Bug1.kt b/idea/testData/refactoring/inline/function/returnAtEnd/Bug1.kt new file mode 100644 index 00000000000..762c4b55f5d --- /dev/null +++ b/idea/testData/refactoring/inline/function/returnAtEnd/Bug1.kt @@ -0,0 +1,11 @@ +fun f(p1: Int, p2: Int): Int { + println(p1) + println(p2) + return p1 + p2 +} + +fun main(args: Array) { + for (i in 1..10) { + println(f(i, i + 1)) + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/inline/function/returnAtEnd/Bug1.kt.after b/idea/testData/refactoring/inline/function/returnAtEnd/Bug1.kt.after new file mode 100644 index 00000000000..76baf77763f --- /dev/null +++ b/idea/testData/refactoring/inline/function/returnAtEnd/Bug1.kt.after @@ -0,0 +1,8 @@ +fun main(args: Array) { + for (i in 1..10) { + val p2 = i + 1 + println(i) + println(p2) + println(i + p2) + } +} \ No newline at end of file diff --git a/idea/testData/refactoring/inline/function/returnAtEnd/ConvertToBlockBody.kt b/idea/testData/refactoring/inline/function/returnAtEnd/ConvertToBlockBody.kt new file mode 100644 index 00000000000..046ebdb3da7 --- /dev/null +++ b/idea/testData/refactoring/inline/function/returnAtEnd/ConvertToBlockBody.kt @@ -0,0 +1,12 @@ +fun 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) diff --git a/idea/testData/refactoring/inline/function/returnAtEnd/ConvertToBlockBody.kt.after b/idea/testData/refactoring/inline/function/returnAtEnd/ConvertToBlockBody.kt.after new file mode 100644 index 00000000000..3143d735159 --- /dev/null +++ b/idea/testData/refactoring/inline/function/returnAtEnd/ConvertToBlockBody.kt.after @@ -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 + } 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 71f2c76cce6..be06e2481c9 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/refactoring/inline/InlineTestGenerated.java @@ -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");