diff --git a/idea/resources/inspectionDescriptions/UseWithIndex.html b/idea/resources/inspectionDescriptions/UseWithIndex.html new file mode 100644 index 00000000000..c3831d05d5c --- /dev/null +++ b/idea/resources/inspectionDescriptions/UseWithIndex.html @@ -0,0 +1,5 @@ + + +This inspection reports for-loops with manually incremented index variable that can be replaced with use of "withIndex()" function + + diff --git a/idea/resources/intentionDescriptions/UseWithIndexIntention/after.kt.template b/idea/resources/intentionDescriptions/UseWithIndexIntention/after.kt.template new file mode 100644 index 00000000000..dd478994463 --- /dev/null +++ b/idea/resources/intentionDescriptions/UseWithIndexIntention/after.kt.template @@ -0,0 +1,5 @@ +fun foo(list: List) { + for ((index, s) in list.withIndex()) { + print(s.substring(index)) + } +} \ No newline at end of file diff --git a/idea/resources/intentionDescriptions/UseWithIndexIntention/before.kt.template b/idea/resources/intentionDescriptions/UseWithIndexIntention/before.kt.template new file mode 100644 index 00000000000..18c0bedebf5 --- /dev/null +++ b/idea/resources/intentionDescriptions/UseWithIndexIntention/before.kt.template @@ -0,0 +1,6 @@ +fun foo(list: List) { + var index = 0 + for (s in list) { + print(s.substring(index++)) + } +} \ No newline at end of file diff --git a/idea/resources/intentionDescriptions/UseWithIndexIntention/description.html b/idea/resources/intentionDescriptions/UseWithIndexIntention/description.html new file mode 100644 index 00000000000..f4af95aebe1 --- /dev/null +++ b/idea/resources/intentionDescriptions/UseWithIndexIntention/description.html @@ -0,0 +1,5 @@ + + +This intention replaces manually incremented index variable for a for-loop with use of "withIndex()" function + + diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 3b5012f385a..44edeab90ae 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -1018,6 +1018,11 @@ Kotlin + + org.jetbrains.kotlin.idea.intentions.loopToCallChain.UseWithIndexIntention + Kotlin + + org.jetbrains.kotlin.idea.intentions.SwapBinaryExpressionIntention Kotlin @@ -1519,6 +1524,13 @@ level="INFO" /> + + (UseWithIndexIntention()) + +class UseWithIndexIntention : SelfTargetingRangeIntention( + KtForExpression::class.java, + "Use withIndex() instead of manual index increment" +) { + override fun applicabilityRange(element: KtForExpression): TextRange? { + return if (matchIndexToIntroduce(element) != null) element.forKeyword.textRange else null + } + + override fun applyTo(element: KtForExpression, editor: Editor?) { + val (indexVariable, initializationStatement, incrementExpression) = matchIndexToIntroduce(element)!! + + val factory = KtPsiFactory(element) + val loopRange = element.loopRange!! + val loopParameter = element.loopParameter!! + + val newLoopRange = factory.createExpressionByPattern("$0.withIndex()", loopRange) + loopRange.replace(newLoopRange) + + val multiParameter = (factory.createExpressionByPattern("for(($0, $1) in x){}", indexVariable.nameAsSafeName, loopParameter.text) as KtForExpression).destructuringParameter!! + loopParameter.replace(multiParameter) + + initializationStatement.delete() + if (incrementExpression.parent is KtBlockExpression) { + incrementExpression.delete() + } + else { + removePlusPlus(incrementExpression) + } + } +} \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/interfaces.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/interfaces.kt index 17259cd2b09..e8a6457f97e 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/interfaces.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/interfaces.kt @@ -22,6 +22,7 @@ import org.jetbrains.kotlin.cfg.pseudocode.Pseudocode import org.jetbrains.kotlin.psi.KtCallableDeclaration import org.jetbrains.kotlin.psi.KtExpression import org.jetbrains.kotlin.psi.KtForExpression +import org.jetbrains.kotlin.psi.KtUnaryExpression import org.jetbrains.kotlin.psi.psiUtil.PsiChildRange import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf import org.jetbrains.kotlin.psi.psiUtil.siblings @@ -102,9 +103,10 @@ data class MatchingState( * Matchers can assume that indexVariable is null if it's not used in the rest of the loop */ val indexVariable: KtCallableDeclaration?, + val pseudocodeProvider: () -> Pseudocode, val initializationStatementsToDelete: Collection = emptyList(), val previousTransformations: MutableList = arrayListOf(), - val pseudocodeProvider: () -> Pseudocode + val incrementExpressions: Collection = emptyList() ) interface TransformationMatcher { 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 ba8f2194b6a..066dc170360 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt @@ -355,3 +355,23 @@ private fun mergeTransformations(match: TransformationMatch.Result): Transformat return TransformationMatch.Result(transformations.last() as ResultTransformation, transformations.dropLast(1) as List) } +data class IntroduceIndexData( + val indexVariable: KtCallableDeclaration, + val initializationStatement: KtExpression, + val incrementExpression: KtUnaryExpression +) + +fun matchIndexToIntroduce(loop: KtForExpression): IntroduceIndexData? { + val (inputVariable, indexVariable) = extractLoopData(loop) ?: return null + if (indexVariable != null) return null // loop is already with "withIndex" + + val state = createInitialMatchingState(loop, inputVariable, indexVariable)?.unwrapBlock() ?: return null + + val match = IntroduceIndexMatcher.match(state) ?: return null + assert(match.sequenceTransformations.isEmpty()) + val newState = match.newState + + val initializationStatement = newState.initializationStatementsToDelete.single() + val incrementExpression = newState.incrementExpressions.single() + return IntroduceIndexData(newState.indexVariable!!, initializationStatement, incrementExpression) +} diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/ForEachTransformation.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/ForEachTransformation.kt index 9f971b32a1a..4480d461386 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/ForEachTransformation.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/ForEachTransformation.kt @@ -53,7 +53,7 @@ class ForEachTransformation( get() = true override fun match(state: MatchingState): TransformationMatch.Result? { - if (state.previousTransformations.isEmpty() && state.indexVariable == null) return null // do not suggest conversion to just ".forEach{}" + if (state.previousTransformations.isEmpty()) return null // do not suggest conversion to just ".forEach{}" or ".forEachIndexed{}" val statement = state.statements.singleOrNull() ?: return null //TODO: should we disallow it for complicated statements like loops, if, when? diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/IntroduceIndexMatcher.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/IntroduceIndexMatcher.kt index 79a98716d0d..70beabbe960 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/IntroduceIndexMatcher.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/IntroduceIndexMatcher.kt @@ -72,7 +72,8 @@ object IntroduceIndexMatcher : TransformationMatcher { val restStatements = state.statements - incrementExpression // if it is among statements then drop it, otherwise "index++" will be replaced with "index" by generateLambda() val newState = state.copy(statements = restStatements, indexVariable = variable, - initializationStatementsToDelete = state.initializationStatementsToDelete + variableInitialization.initializationStatement) + initializationStatementsToDelete = state.initializationStatementsToDelete + variableInitialization.initializationStatement, + incrementExpressions = state.incrementExpressions + incrementExpression) return TransformationMatch.Sequence(emptyList(), newState) } 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 319358ba0e2..5a7c140d19f 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt @@ -83,17 +83,21 @@ fun generateLambda(expression: KtExpression, indexVariable: KtCallableDeclaratio } } if (indexPlusPlus != null) { - val operand = indexPlusPlus.baseExpression!! - val replacement = if (indexPlusPlus is KtPostfixExpression) // index++ - operand - else // ++index - KtPsiFactory(operand).createExpressionByPattern("$0 + 1", operand) - indexPlusPlus.replace(replacement) + removePlusPlus(indexPlusPlus) } return lambdaExpression } +fun removePlusPlus(indexPlusPlus: KtUnaryExpression) { + val operand = indexPlusPlus.baseExpression!! + val replacement = if (indexPlusPlus is KtPostfixExpression) // index++ + operand + else // ++index + KtPsiFactory(operand).createExpressionByPattern("$0 + 1", operand) + indexPlusPlus.replace(replacement) +} + fun generateLambda(expression: KtExpression, vararg inputVariables: KtCallableDeclaration): KtLambdaExpression { return KtPsiFactory(expression).buildExpression { appendFixedText("{") diff --git a/idea/testData/intentions/loopToCallChain/forEachIndexed_nothingElse.kt b/idea/testData/intentions/loopToCallChain/forEachIndexed_nothingElse.kt index 0562dc78f1a..7d9c70e2ec5 100644 --- a/idea/testData/intentions/loopToCallChain/forEachIndexed_nothingElse.kt +++ b/idea/testData/intentions/loopToCallChain/forEachIndexed_nothingElse.kt @@ -1,5 +1,5 @@ // WITH_RUNTIME -// INTENTION_TEXT: "Replace with 'forEachIndexed{}'" +// IS_APPLICABLE: false fun foo(list: List) { for ((index, s) in list.withIndex()) { println(s.hashCode() * index) diff --git a/idea/testData/intentions/loopToCallChain/forEachIndexed_nothingElse.kt.after b/idea/testData/intentions/loopToCallChain/forEachIndexed_nothingElse.kt.after deleted file mode 100644 index 5c0035f31ff..00000000000 --- a/idea/testData/intentions/loopToCallChain/forEachIndexed_nothingElse.kt.after +++ /dev/null @@ -1,5 +0,0 @@ -// WITH_RUNTIME -// INTENTION_TEXT: "Replace with 'forEachIndexed{}'" -fun foo(list: List) { - list.forEachIndexed { index, s -> println(s.hashCode() * index) } -} \ No newline at end of file diff --git a/idea/testData/intentions/useWithIndex/.intention b/idea/testData/intentions/useWithIndex/.intention new file mode 100644 index 00000000000..1d67afd291a --- /dev/null +++ b/idea/testData/intentions/useWithIndex/.intention @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.intentions.loopToCallChain.UseWithIndexIntention \ No newline at end of file diff --git a/idea/testData/intentions/useWithIndex/alreadyWithIndex.kt b/idea/testData/intentions/useWithIndex/alreadyWithIndex.kt new file mode 100644 index 00000000000..b60f65f386b --- /dev/null +++ b/idea/testData/intentions/useWithIndex/alreadyWithIndex.kt @@ -0,0 +1,11 @@ +// WITH_RUNTIME +// IS_APPLICABLE: false +fun foo(list: List): Int? { + var index = 0 + for ((i, s) in list.withIndex()) { + val x = s.length * index * i + index++ + if (x > 0) return x + } + return null +} \ No newline at end of file diff --git a/idea/testData/intentions/useWithIndex/customTypeWithIterator.kt b/idea/testData/intentions/useWithIndex/customTypeWithIterator.kt new file mode 100644 index 00000000000..e0dce3a23b7 --- /dev/null +++ b/idea/testData/intentions/useWithIndex/customTypeWithIterator.kt @@ -0,0 +1,16 @@ +// IS_APPLICABLE: false +// WITH_RUNTIME +class X { + operator fun iterator(): Iterator{ + return emptyList().iterator() + } +} + +fun foo(x: X) { + var index = 0 + for (s in x) { + if (s.length > index++) { + println(s) + } + } +} diff --git a/idea/testData/intentions/useWithIndex/indexPlusPlusInsideExpression.kt b/idea/testData/intentions/useWithIndex/indexPlusPlusInsideExpression.kt new file mode 100644 index 00000000000..02e39bacc2a --- /dev/null +++ b/idea/testData/intentions/useWithIndex/indexPlusPlusInsideExpression.kt @@ -0,0 +1,9 @@ +// WITH_RUNTIME +fun foo(list: List) { + var i = 0 + for (s in list) { + println(i) + val x = s.length * i++ + if (x > 1000) break + } +} \ No newline at end of file diff --git a/idea/testData/intentions/useWithIndex/indexPlusPlusInsideExpression.kt.after b/idea/testData/intentions/useWithIndex/indexPlusPlusInsideExpression.kt.after new file mode 100644 index 00000000000..70e29311332 --- /dev/null +++ b/idea/testData/intentions/useWithIndex/indexPlusPlusInsideExpression.kt.after @@ -0,0 +1,8 @@ +// WITH_RUNTIME +fun foo(list: List) { + for ((i, s) in list.withIndex()) { + println(i) + val x = s.length * i + if (x > 1000) break + } +} \ No newline at end of file diff --git a/idea/testData/intentions/useWithIndex/prefixPlusPlusInsideExpression.kt b/idea/testData/intentions/useWithIndex/prefixPlusPlusInsideExpression.kt new file mode 100644 index 00000000000..8e667f37656 --- /dev/null +++ b/idea/testData/intentions/useWithIndex/prefixPlusPlusInsideExpression.kt @@ -0,0 +1,9 @@ +// WITH_RUNTIME +fun foo(list: List) { + var i = 0 + for (s in list) { + println(i) + val x = s.length * ++i + if (x > 1000) break + } +} \ No newline at end of file diff --git a/idea/testData/intentions/useWithIndex/prefixPlusPlusInsideExpression.kt.after b/idea/testData/intentions/useWithIndex/prefixPlusPlusInsideExpression.kt.after new file mode 100644 index 00000000000..e42c07526f4 --- /dev/null +++ b/idea/testData/intentions/useWithIndex/prefixPlusPlusInsideExpression.kt.after @@ -0,0 +1,8 @@ +// WITH_RUNTIME +fun foo(list: List) { + for ((i, s) in list.withIndex()) { + println(i) + val x = s.length * (i + 1) + if (x > 1000) break + } +} \ No newline at end of file diff --git a/idea/testData/intentions/useWithIndex/simple.kt b/idea/testData/intentions/useWithIndex/simple.kt new file mode 100644 index 00000000000..bb0e857593c --- /dev/null +++ b/idea/testData/intentions/useWithIndex/simple.kt @@ -0,0 +1,10 @@ +// WITH_RUNTIME +fun foo(list: List): Int? { + var index = 0 + for (s in list) { + val x = s.length * index + index++ + if (x > 0) return x + } + return null +} \ No newline at end of file diff --git a/idea/testData/intentions/useWithIndex/simple.kt.after b/idea/testData/intentions/useWithIndex/simple.kt.after new file mode 100644 index 00000000000..870c4084c01 --- /dev/null +++ b/idea/testData/intentions/useWithIndex/simple.kt.after @@ -0,0 +1,8 @@ +// WITH_RUNTIME +fun foo(list: List): Int? { + for ((index, s) in list.withIndex()) { + val x = s.length * index + if (x > 0) return x + } + return null +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 3de253d4eb5..d3ec05073b6 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -10517,4 +10517,43 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } } + + @TestMetadata("idea/testData/intentions/useWithIndex") + @TestDataPath("$PROJECT_ROOT") + @RunWith(JUnit3RunnerWithInners.class) + public static class UseWithIndex extends AbstractIntentionTest { + public void testAllFilesPresentInUseWithIndex() throws Exception { + KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/intentions/useWithIndex"), Pattern.compile("^([\\w\\-_]+)\\.kt$"), true); + } + + @TestMetadata("alreadyWithIndex.kt") + public void testAlreadyWithIndex() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/useWithIndex/alreadyWithIndex.kt"); + doTest(fileName); + } + + @TestMetadata("customTypeWithIterator.kt") + public void testCustomTypeWithIterator() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/useWithIndex/customTypeWithIterator.kt"); + doTest(fileName); + } + + @TestMetadata("indexPlusPlusInsideExpression.kt") + public void testIndexPlusPlusInsideExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/useWithIndex/indexPlusPlusInsideExpression.kt"); + doTest(fileName); + } + + @TestMetadata("prefixPlusPlusInsideExpression.kt") + public void testPrefixPlusPlusInsideExpression() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/useWithIndex/prefixPlusPlusInsideExpression.kt"); + doTest(fileName); + } + + @TestMetadata("simple.kt") + public void testSimple() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/useWithIndex/simple.kt"); + doTest(fileName); + } + } }