diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/baseTransformations.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/baseTransformations.kt index f20908263e1..0dd8b9ad5ef 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/baseTransformations.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/baseTransformations.kt @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.idea.intentions.loopToCallChain import org.jetbrains.kotlin.idea.core.replaced +import org.jetbrains.kotlin.psi.KtBlockExpression import org.jetbrains.kotlin.psi.KtExpression import org.jetbrains.kotlin.psi.KtForExpression import org.jetbrains.kotlin.psi.KtPsiFactory @@ -40,19 +41,34 @@ abstract class AssignToVariableResultTransformation( override val commentSavingRange = PsiChildRange(initialization.initializationStatement, loop.unwrapIfLabeled()) + //TODO: does not work in case of move private val commentRestoringRange = commentSavingRange.withoutLastStatement() override fun commentRestoringRange(convertLoopResult: KtExpression) = commentRestoringRange override fun convertLoop(resultCallChain: KtExpression): KtExpression { initialization.initializer.replace(resultCallChain) + + val previousStatement = loop.unwrapIfLabeled().previousStatement() + loop.deleteWithLabels() if (initialization.variable.isVar && !initialization.variable.hasWriteUsages()) { // change variable to 'val' if possible initialization.variable.valOrVarKeyword.replace(KtPsiFactory(initialization.variable).createValKeyword()) } - return initialization.initializationStatement + // move initializer to the place where the loop was if needed + var initializationStatement = initialization.initializationStatement + if (initializationStatement != previousStatement) { + val block = initializationStatement.parent + assert(block is KtBlockExpression) + val movedInitializationStatement = block.addAfter(initializationStatement, previousStatement) as KtExpression + block.addAfter(KtPsiFactory(block).createNewLine(), previousStatement) + initializationStatement.delete() + initializationStatement = movedInitializationStatement + } + + return initializationStatement } } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/commonUtils.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/commonUtils.kt index 24ea15bedc0..82d83ab1bdd 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/commonUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/commonUtils.kt @@ -32,10 +32,16 @@ import org.jetbrains.kotlin.psi.psiUtil.getQualifiedExpressionForSelector import org.jetbrains.kotlin.psi.psiUtil.parents import org.jetbrains.kotlin.psi.psiUtil.siblings import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstance import org.jetbrains.kotlin.utils.addToStdlib.firstIsInstanceOrNull +fun KtExpression.isConstant(): Boolean { + val bindingContext = analyze(BodyResolveMode.PARTIAL) + return ConstantExpressionEvaluator.getConstant(this, bindingContext) != null +} + fun KtExpression?.isTrueConstant() = this != null && node?.elementType == KtNodeTypes.BOOLEAN_CONSTANT && text == "true" diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt index a6a63f6b27b..ea20435814d 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt @@ -17,6 +17,8 @@ package org.jetbrains.kotlin.idea.intentions.loopToCallChain import com.intellij.openapi.util.Key +import com.intellij.psi.PsiDocumentManager +import com.intellij.psi.codeStyle.CodeStyleManager import org.jetbrains.kotlin.idea.analysis.analyzeInContext import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade @@ -149,6 +151,14 @@ fun convertLoop(loop: KtForExpression, matchResult: MatchResult): KtExpression { commentSaver.restore(resultTransformation.commentRestoringRange(result)) + // need to manually adjust indent of the result because in some cases it's made incorrect when moving closer to the loop + // TODO: use forceAdjustIndent = true of CommentSaver.restore + val file = result.containingFile + val psiDocumentManager = PsiDocumentManager.getInstance(file.project) + psiDocumentManager.doPostponedOperationsAndUnblockDocument(psiDocumentManager.getDocument(file)!!) + val codeStyleManager = CodeStyleManager.getInstance(file.project) + codeStyleManager.adjustLineIndent(file, result.textRange) + return result } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndAssignTransformation.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndAssignTransformation.kt index 250257c94e1..1214af638b6 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndAssignTransformation.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndAssignTransformation.kt @@ -16,7 +16,6 @@ package org.jetbrains.kotlin.idea.intentions.loopToCallChain.result -import org.jetbrains.kotlin.idea.caches.resolve.analyze import org.jetbrains.kotlin.idea.intentions.loopToCallChain.* import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.FilterTransformation import org.jetbrains.kotlin.lexer.KtTokens @@ -24,8 +23,6 @@ import org.jetbrains.kotlin.psi.KtBinaryExpression import org.jetbrains.kotlin.psi.KtBreakExpression import org.jetbrains.kotlin.psi.KtExpression import org.jetbrains.kotlin.psi.KtForExpression -import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator -import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode class FindAndAssignTransformation( loop: KtForExpression, @@ -95,9 +92,7 @@ class FindAndAssignTransformation( if (initialization.variable.countUsages(state.outerLoop) != 1) return null // this should be the only usage of this variable inside the loop // we do not try to convert anything if the initializer is not compile-time constant because of possible side-effects - val initializerIsConstant = ConstantExpressionEvaluator.getConstant( - initialization.initializer, initialization.initializer.analyze(BodyResolveMode.PARTIAL)) != null - if (!initializerIsConstant) return null + if (!initialization.initializer.isConstant()) return null val generator = buildFindOperationGenerator(right, initialization.initializer, state.inputVariable, findFirst) ?: return null diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt index d5dfdd8ffb1..6e117054267 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt @@ -29,6 +29,7 @@ import org.jetbrains.kotlin.idea.imports.importableFqName import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.idea.util.getResolutionScope import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.psi.KtPsiUtil.isOrdinaryAssignment import org.jetbrains.kotlin.psi.psiUtil.* import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.DelegatingBindingTrace @@ -36,6 +37,8 @@ import org.jetbrains.kotlin.resolve.bindingContextUtil.getDataFlowInfo import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator import org.jetbrains.kotlin.resolve.lazy.BodyResolveMode +import org.jetbrains.kotlin.utils.addToStdlib.check +import java.util.* fun generateLambda(inputVariable: KtCallableDeclaration, expression: KtExpression): KtLambdaExpression { val psiFactory = KtPsiFactory(expression) @@ -99,17 +102,38 @@ fun KtExpression.detectInitializationBeforeLoop( if (this !is KtNameReferenceExpression) return null if (getQualifiedExpressionForSelector() != null) return null val variable = this.mainReference.resolve() as? KtProperty ?: return null - val statementBeforeLoop = loop.previousStatement() //TODO: support initialization not right before the loop // do not allow any other usages of this variable inside the loop if (checkNoOtherUsagesInLoop && variable.countUsages(loop) > 1) return null - if (statementBeforeLoop == variable) { + val unwrapped = loop.unwrapIfLabeled() + if (unwrapped.parent !is KtBlockExpression) return null + val prevStatements = unwrapped + .siblings(forward = false, withItself = false) + .filterIsInstance() + + val statementsBetween = ArrayList() + for (statement in prevStatements) { + val variableInitialization = extractVariableInitialization(statement, variable) + if (variableInitialization != null) { + return variableInitialization.check { + statementsBetween.all { canSwapExecutionOrder(variableInitialization.initializationStatement, it) } + } + } + + statementsBetween.add(statement) + } + + return null +} + +private fun extractVariableInitialization(statement: KtExpression, variable: KtProperty): VariableInitialization? { + if (statement == variable) { val initializer = variable.initializer ?: return null return VariableInitialization(variable, variable, initializer) } - val assignment = statementBeforeLoop?.asAssignment() ?: return null + val assignment = statement.asAssignment() ?: return null if (!assignment.left.isVariableReference(variable)) return null val initializer = assignment.right ?: return null @@ -197,3 +221,39 @@ fun KtExpression.hasNoSideEffect(): Boolean { val classFqName = classDescriptor.importableFqName?.asString() return classFqName in NO_SIDE_EFFECT_STANDARD_CLASSES } + +//TODO: we need more correctness checks (if variable is non-local or is local but can be changed by some local functions) +fun canSwapExecutionOrder(expressionBefore: KtExpression, expressionAfter: KtExpression): Boolean { + assert(expressionBefore.isPhysical) + assert(expressionAfter.isPhysical) + + if (expressionBefore is KtDeclaration) { + if (expressionBefore !is KtProperty) return false // local function, class or destructuring declaration - do not bother to handle these rare cases + if (expressionBefore.hasUsages(expressionAfter)) return false + return canSwapExecutionOrder(expressionBefore.initializer ?: return true, expressionAfter) + } + + if (expressionAfter is KtDeclaration) { + if (expressionAfter !is KtProperty) return false // local function, class or destructuring declaration - do not bother to handle these rare cases + return canSwapExecutionOrder(expressionBefore, expressionAfter.initializer ?: return true) + } + + if (expressionBefore is KtBinaryExpression && isOrdinaryAssignment(expressionBefore)) { + val leftName = expressionBefore.left as? KtSimpleNameExpression ?: return false + val target = leftName.mainReference.resolve() as? KtProperty ?: return false + if (target.hasUsages(expressionAfter)) return false + return canSwapExecutionOrder(expressionBefore.right ?: return true, expressionAfter) + } + + if (expressionAfter is KtBinaryExpression && isOrdinaryAssignment(expressionAfter)) { + val leftName = expressionAfter.left as? KtSimpleNameExpression ?: return false + val target = leftName.mainReference.resolve() as? KtProperty ?: return false + if (target.hasUsages(expressionBefore)) return false + return canSwapExecutionOrder(expressionBefore, expressionAfter.right ?: return true) + } + + if (expressionBefore.isConstant() || expressionAfter.isConstant()) return true + + //TODO: more cases + return false +} diff --git a/idea/testData/intentions/loopToCallChain/any_ifAssign_moveDeclaration.kt b/idea/testData/intentions/loopToCallChain/any_ifAssign_moveDeclaration.kt new file mode 100644 index 00000000000..cda382103cd --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/any_ifAssign_moveDeclaration.kt @@ -0,0 +1,12 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'any{}'" +fun foo(list: List) { + var found = false + println("Starting the search") + for (s in list) { + if (s.length > 0) { + found = true + break + } + } +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/any_ifAssign_moveDeclaration.kt.after b/idea/testData/intentions/loopToCallChain/any_ifAssign_moveDeclaration.kt.after new file mode 100644 index 00000000000..78503c57cdf --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/any_ifAssign_moveDeclaration.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'any{}'" +fun foo(list: List) { + println("Starting the search") + val found = list.any { it.length > 0 } +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/any_ifAssign_moveInitialization.kt b/idea/testData/intentions/loopToCallChain/any_ifAssign_moveInitialization.kt new file mode 100644 index 00000000000..2ebf9c61bab --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/any_ifAssign_moveInitialization.kt @@ -0,0 +1,18 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'any{}'" +fun foo(list: List, p: Int) { + var found: Boolean + if (p > 0) { + found = false + println("Starting the search") + for (s in list) { + if (s.length > 0) { + found = true + break + } + } + } + else { + found = true + } +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/any_ifAssign_moveInitialization.kt.after b/idea/testData/intentions/loopToCallChain/any_ifAssign_moveInitialization.kt.after new file mode 100644 index 00000000000..708c28d66bd --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/any_ifAssign_moveInitialization.kt.after @@ -0,0 +1,12 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'any{}'" +fun foo(list: List, p: Int) { + var found: Boolean + if (p > 0) { + println("Starting the search") + found = list.any { it.length > 0 } + } + else { + found = true + } +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/twoInitializationsBeforeLoop.kt b/idea/testData/intentions/loopToCallChain/twoInitializationsBeforeLoop.kt new file mode 100644 index 00000000000..90f9f04d067 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/twoInitializationsBeforeLoop.kt @@ -0,0 +1,15 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'filterIndexed{}.map{}'" +import java.util.* + +fun foo(list: List): List { + val result = ArrayList() + var i = 0 + for (s in list) { + if (s.length > i) { + result.add(s.length) + } + i++ + } + return result +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/twoInitializationsBeforeLoop.kt.after b/idea/testData/intentions/loopToCallChain/twoInitializationsBeforeLoop.kt.after new file mode 100644 index 00000000000..b3484cae220 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/twoInitializationsBeforeLoop.kt.after @@ -0,0 +1,10 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'filterIndexed{}.map{}'" +import java.util.* + +fun foo(list: List): List { + val result = list + .filterIndexed { i, s -> s.length > i } + .map { it.length } + return result +} \ No newline at end of file