"Use withIndex()" intention&inspection

2016-05-13 17:02:40 +03:00
parent 46055dfd2d
commit d658d3b9cf
23 changed files with 248 additions and 15 deletions
@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspection reports for-loops with manually incremented index variable that can be replaced with use of "withIndex()" function
+</body>
+</html>
@@ -0,0 +1,5 @@
+fun foo(list: List<String>) {
+    for (<spot>(index, s) in list.withIndex()</spot>) {
+        print(s.substring(<spot>index</spot>))
+    }
+}
@@ -0,0 +1,6 @@
+fun foo(list: List<String>) {
+    <spot>var index = 0</spot>
+    for (s in list) {
+        print(s.substring(<spot>index++</spot>))
+    }
+}
@@ -0,0 +1,5 @@
+<html>
+<body>
+This intention replaces manually incremented index variable for a for-loop with use of "withIndex()" function
+</body>
+</html>
@@ -1018,6 +1018,11 @@
      <category>Kotlin</category>
    </intentionAction>

+    <intentionAction>
+      <className>org.jetbrains.kotlin.idea.intentions.loopToCallChain.UseWithIndexIntention</className>
+      <category>Kotlin</category>
+    </intentionAction>
+
    <intentionAction>
      <className>org.jetbrains.kotlin.idea.intentions.SwapBinaryExpressionIntention</className>
      <category>Kotlin</category>
@@ -1519,6 +1524,13 @@
                     level="INFO"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.intentions.loopToCallChain.UseWithIndexInspection"
+                     displayName="Manually incremented index variable can be replaced with use of 'withIndex()'"
+                     groupName="Kotlin"
+                     enabledByDefault="true"
+                     level="INFO"
+    />
+
      <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.ConflictingExtensionPropertyInspection"
                       displayName="Extension property conflicting with synthetic one"
                       groupName="Kotlin"
@@ -0,0 +1,59 @@
+/*
+ * Copyright 2010-2016 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.jetbrains.kotlin.idea.intentions.loopToCallChain
+
+import com.intellij.openapi.editor.Editor
+import com.intellij.openapi.util.TextRange
+import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection
+import org.jetbrains.kotlin.idea.intentions.SelfTargetingRangeIntention
+import org.jetbrains.kotlin.psi.KtBlockExpression
+import org.jetbrains.kotlin.psi.KtForExpression
+import org.jetbrains.kotlin.psi.KtPsiFactory
+import org.jetbrains.kotlin.psi.createExpressionByPattern
+
+class UseWithIndexInspection : IntentionBasedInspection<KtForExpression>(UseWithIndexIntention())
+
+class UseWithIndexIntention : SelfTargetingRangeIntention<KtForExpression>(
+        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)
+        }
+    }
+}
@@ -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<KtExpression> = emptyList(),
        val previousTransformations: MutableList<SequenceTransformation> = arrayListOf(),
-        val pseudocodeProvider: () -> Pseudocode
+        val incrementExpressions: Collection<KtUnaryExpression> = emptyList()
 )

 interface TransformationMatcher {
@@ -355,3 +355,23 @@ private fun mergeTransformations(match: TransformationMatch.Result): Transformat
    return TransformationMatch.Result(transformations.last() as ResultTransformation, transformations.dropLast(1) as List<SequenceTransformation>)
 }

+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)
+}
@@ -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?
@@ -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)
    }

@@ -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("{")
@@ -1,5 +1,5 @@
 // WITH_RUNTIME
-// INTENTION_TEXT: "Replace with 'forEachIndexed{}'"
+// IS_APPLICABLE: false
 fun foo(list: List<String>) {
    <caret>for ((index, s) in list.withIndex()) {
        println(s.hashCode() * index)
@@ -1,5 +0,0 @@
-// WITH_RUNTIME
-// INTENTION_TEXT: "Replace with 'forEachIndexed{}'"
-fun foo(list: List<String>) {
-    list.forEachIndexed { index, s -> println(s.hashCode() * index) }
-}
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.intentions.loopToCallChain.UseWithIndexIntention
@@ -0,0 +1,11 @@
+// WITH_RUNTIME
+// IS_APPLICABLE: false
+fun foo(list: List<String>): Int? {
+    var index = 0
+    <caret>for ((i, s) in list.withIndex()) {
+        val x = s.length * index * i
+        index++
+        if (x > 0) return x
+    }
+    return null
+}
@@ -0,0 +1,16 @@
+// IS_APPLICABLE: false
+// WITH_RUNTIME
+class X {
+    operator fun iterator(): Iterator<String>{
+        return emptyList<String>().iterator()
+    }
+}
+
+fun foo(x: X) {
+    var index = 0
+    <caret>for (s in x) {
+        if (s.length > index++) {
+            println(s)
+        }
+    }
+}
@@ -0,0 +1,9 @@
+// WITH_RUNTIME
+fun foo(list: List<String>) {
+    var i = 0
+    <caret>for (s in list) {
+        println(i)
+        val x = s.length * i++
+        if (x > 1000) break
+    }
+}
@@ -0,0 +1,8 @@
+// WITH_RUNTIME
+fun foo(list: List<String>) {
+    for ((i, s) in list.withIndex()) {
+        println(i)
+        val x = s.length * i
+        if (x > 1000) break
+    }
+}
@@ -0,0 +1,9 @@
+// WITH_RUNTIME
+fun foo(list: List<String>) {
+    var i = 0
+    <caret>for (s in list) {
+        println(i)
+        val x = s.length * ++i
+        if (x > 1000) break
+    }
+}
@@ -0,0 +1,8 @@
+// WITH_RUNTIME
+fun foo(list: List<String>) {
+    for ((i, s) in list.withIndex()) {
+        println(i)
+        val x = s.length * (i + 1)
+        if (x > 1000) break
+    }
+}
@@ -0,0 +1,10 @@
+// WITH_RUNTIME
+fun foo(list: List<String>): Int? {
+    var index = 0
+    <caret>for (s in list) {
+        val x = s.length * index
+        index++
+        if (x > 0) return x
+    }
+    return null
+}
@@ -0,0 +1,8 @@
+// WITH_RUNTIME
+fun foo(list: List<String>): Int? {
+    for ((index, s) in list.withIndex()) {
+        val x = s.length * index
+        if (x > 0) return x
+    }
+    return null
+}
@@ -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);
+        }
+    }
 }
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.intentions.loopToCallChain.UseWithIndexIntention`