Suggest to replace manual range with explicit indices or iterable

#KT-14344 Fixed
2019-03-01 17:23:53 +03:00
parent 8cda5cb849
commit 3451c60f93
17 changed files with 269 additions and 0 deletions
@@ -357,6 +357,10 @@ class KtPsiFactory @JvmOverloads constructor(private val project: Project, val m
        return createClass("class A($text)").primaryConstructorParameters.first()
    }

+    fun createLoopParameter(text: String): KtParameter {
+        return (createExpression("for ($text in list) {}") as KtForExpression).loopParameter!!
+    }
+
    fun createParameterList(text: String): KtParameterList {
        return createFunction("fun foo$text{}").valueParameterList!!
    }
@@ -1820,6 +1820,15 @@
                     language="kotlin"
    />

+    <localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.ReplaceManualRangeWithIndicesCallsInspection"
+                     displayName="Convert manual ranges to indices or iteration over collection"
+                     groupPath="Kotlin"
+                     groupName="Style issues"
+                     enabledByDefault="true"
+                     level="INFO"
+                     language="kotlin"
+    />
+
    <localInspection implementationClass="org.jetbrains.kotlin.idea.intentions.branchedTransformations.intentions.IfThenToElvisInspection"
                     displayName="If-Then foldable to '?:'"
                     groupPath="Kotlin"
@@ -0,0 +1,5 @@
+<html>
+<body>
+This inspection reports <b>until</b> and <b>..</b> operator usages that are replaceable with <b>Collection.indices</b>
+or iteration over collection inside <b>for</b> loop.</body>
+</html>
@@ -0,0 +1,147 @@
+/*
+ * Copyright 2010-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
+ * that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.idea.inspections
+
+import com.intellij.codeInspection.LocalQuickFix
+import com.intellij.codeInspection.ProblemDescriptor
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.openapi.project.Project
+import com.intellij.psi.search.searches.ReferencesSearch
+import org.jetbrains.kotlin.idea.intentions.getArguments
+import org.jetbrains.kotlin.idea.intentions.isSizeOrLength
+import org.jetbrains.kotlin.lexer.KtTokens
+import org.jetbrains.kotlin.psi.*
+import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
+
+class ReplaceManualRangeWithIndicesCallsInspection : AbstractKotlinInspection() {
+    val rangeFunctions = setOf("until", "rangeTo")
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) = object : KtVisitorVoid() {
+        override fun visitBinaryExpression(binaryExpression: KtBinaryExpression) {
+            val find = rangeFunctions.find { it == binaryExpression.operationReference.text }
+            if (binaryExpression.operationToken == KtTokens.RANGE || find != null) {
+                val operator = find ?: "rangeTo"
+                visitRange(holder, binaryExpression, binaryExpression.left ?: return, binaryExpression.right ?: return, operator)
+            }
+        }
+
+        override fun visitDotQualifiedExpression(expression: KtDotQualifiedExpression) {
+            val find = rangeFunctions.find { it == expression.selectorExpression?.text }
+            if (find != null) {
+                val call = (expression.parent as? KtCallExpression) ?: return
+                val firstArg = call.valueArguments.first().getArgumentExpression() ?: return
+                visitRange(holder, expression, expression.receiverExpression, firstArg, find)
+            }
+        }
+    }
+
+    private fun visitRange(holder: ProblemsHolder, expression: KtExpression, left: KtExpression, right: KtExpression, method: String) {
+        if ((method == "until" && left.toIntConstant() == 0 && right.receiverIfIsSizeOrLengthCall() != null) ||
+            (method == "rangeTo" && left.toIntConstant() == 0 && right.receiverIfIsSizeOrLengthMinusOneCall() != null)
+        ) {
+            visitIndicesRange(holder, expression)
+        }
+    }
+
+    private fun visitIndicesRange(holder: ProblemsHolder, range: KtExpression) {
+        val parent = range.parent.parent
+        if (parent is KtForExpression) {
+            val paramElement = parent.loopParameter?.originalElement ?: return
+            val usageElement = ReferencesSearch.search(paramElement).singleOrNull()?.element
+            val arrayAccess = usageElement?.parent?.parent as? KtArrayAccessExpression
+            if (arrayAccess != null && arrayAccess.indexExpressions.singleOrNull() == usageElement) {
+                val arrayAccessParent = arrayAccess.parent
+                if (arrayAccessParent !is KtBinaryExpression ||
+                    arrayAccessParent.left != arrayAccess ||
+                    arrayAccessParent.operationToken !in KtTokens.ALL_ASSIGNMENTS
+                ) {
+                    holder.registerProblem(
+                        range,
+                        "For loop over indices could be replaced with loop over elements",
+                        ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+                        ReplaceIndexLoopWithCollectionLoopQuickFix()
+                    )
+                    return
+                }
+            }
+        }
+        holder.registerProblem(
+            range,
+            "Range could be replaced with '.indices' call",
+            ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
+            ReplaceManualRangeWithIndicesCallQuickFix()
+        )
+    }
+}
+
+class ReplaceManualRangeWithIndicesCallQuickFix : LocalQuickFix {
+    override fun getName() = "Replace with indices"
+
+    override fun getFamilyName() = name
+
+    override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+        val element = descriptor.psiElement as KtExpression
+        val args = element.getArguments() ?: return
+        if (args.second is KtBinaryExpression) {
+            val second = (args.second as? KtBinaryExpression) ?: return
+            replaceWithIndices(element, (second.left as? KtDotQualifiedExpression)?.receiverExpression ?: return)
+        } else {
+            replaceWithIndices(element, (args.second as? KtDotQualifiedExpression)?.receiverExpression ?: return)
+        }
+    }
+
+    private fun replaceWithIndices(toReplace: KtExpression, receiver: KtExpression) {
+        toReplace.replace(KtPsiFactory(toReplace).createExpressionByPattern("$0.indices", receiver))
+    }
+}
+
+class ReplaceIndexLoopWithCollectionLoopQuickFix : LocalQuickFix {
+    override fun getName() = "Replace with loop over elements"
+
+    override fun getFamilyName() = name
+
+    override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
+        val element = descriptor.psiElement.getStrictParentOfType<KtForExpression>() ?: return
+        val loopParameter = element.loopParameter ?: return
+        val loopRange = element.loopRange ?: return
+        val collectionParent = when (loopRange) {
+            is KtDotQualifiedExpression -> (loopRange.parent as? KtCallExpression)?.valueArguments?.firstOrNull()?.getArgumentExpression()
+            is KtBinaryExpression -> loopRange.right
+            else -> null
+        } ?: return
+        val collection =
+            collectionParent.receiverIfIsSizeOrLengthCall() ?: collectionParent.receiverIfIsSizeOrLengthMinusOneCall() ?: return
+        val paramElement = loopParameter.originalElement ?: return
+        val usageElement = ReferencesSearch.search(paramElement).singleOrNull()?.element ?: return
+        val arrayAccessElement = usageElement.parent.parent as? KtArrayAccessExpression ?: return
+        val factory = KtPsiFactory(project)
+        val newParameter = factory.createLoopParameter("element")
+        val newReferenceExpression = factory.createExpression("element")
+        arrayAccessElement.replace(newReferenceExpression)
+        loopParameter.replace(newParameter)
+        loopRange.replace(collection)
+    }
+}
+
+private fun KtExpression.toIntConstant(): Int? {
+    return (this as? KtConstantExpression)?.text?.toIntOrNull()
+}
+
+fun KtExpression.receiverIfIsSizeOrLengthCall(): KtExpression? {
+    if (this.isSizeOrLength()) {
+        return (this as? KtDotQualifiedExpression)?.receiverExpression ?: return null
+    }
+    return null
+}
+
+fun KtExpression.receiverIfIsSizeOrLengthMinusOneCall(): KtExpression? {
+    if (this !is KtBinaryExpression) return null
+    if (this.operationToken != KtTokens.MINUS) return null
+    val collection = this.left?.receiverIfIsSizeOrLengthCall() ?: return null
+    val constant = this.right?.toIntConstant() ?: return null
+    if (constant == 1) return collection
+    return null
+}
@@ -0,0 +1 @@
+org.jetbrains.kotlin.idea.inspections.ReplaceManualRangeWithIndicesCallsInspection
@@ -0,0 +1,8 @@
+// WITH_RUNTIME
+// PROBLEM: none
+fun test(args: Array<Int>) {
+    val x = arrayOf<String>()
+    for (index in ar<caret>gs) {
+        val out = x[index]
+    }
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun test(args: Array<String>) {
+    for (index in 0..ar<caret>gs.size - 1) {
+        args[index] = "Hello"
+    }
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun test(args: Array<String>) {
+    for (index in args.indices) {
+        args[index] = "Hello"
+    }
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun test(args: Array<String>) {
+    for (index in 0..ar<caret>gs.size - 1) {
+        println(index)
+    }
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun test(args: Array<String>) {
+    for (index in args.indices) {
+        println(index)
+    }
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(args: Array<String>) {
+    val ind = 0..<caret>args.size-1
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(args: Array<String>) {
+    val ind = args.indices
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(args: Array<String>) {
+    val ind = 0 <caret>until args.size
+}
@@ -0,0 +1,4 @@
+// WITH_RUNTIME
+fun test(args: Array<String>) {
+    val ind = args.indices
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun test(args: Array<String>) {
+    for (index in 0..ar<caret>gs.size - 1) {
+        val out = args[index]
+    }
+}
@@ -0,0 +1,6 @@
+// WITH_RUNTIME
+fun test(args: Array<String>) {
+    for (element in args) {
+        val out = element
+    }
+}
@@ -7757,6 +7757,49 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
        }
    }

+    @TestMetadata("idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection")
+    @TestDataPath("$PROJECT_ROOT")
+    @RunWith(JUnit3RunnerWithInners.class)
+    public static class ReplaceManualRangeWithIndicesCallsInspection extends AbstractLocalInspectionTest {
+        private void runTest(String testDataFilePath) throws Exception {
+            KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
+        }
+
+        public void testAllFilesPresentInReplaceManualRangeWithIndicesCallsInspection() throws Exception {
+            KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
+        }
+
+        @TestMetadata("forNotTarget.kt")
+        public void testForNotTarget() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/forNotTarget.kt");
+        }
+
+        @TestMetadata("indexInLvalue.kt")
+        public void testIndexInLvalue() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/indexInLvalue.kt");
+        }
+
+        @TestMetadata("notUsedAsIndex.kt")
+        public void testNotUsedAsIndex() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/notUsedAsIndex.kt");
+        }
+
+        @TestMetadata("simpleExpression.kt")
+        public void testSimpleExpression() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpression.kt");
+        }
+
+        @TestMetadata("simpleExpressionUntil.kt")
+        public void testSimpleExpressionUntil() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpressionUntil.kt");
+        }
+
+        @TestMetadata("simpleFor.kt")
+        public void testSimpleFor() throws Exception {
+            runTest("idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleFor.kt");
+        }
+    }
+
    @TestMetadata("idea/testData/inspectionsLocal/replaceNegatedIsEmptyWithIsNotEmpty")
    @TestDataPath("$PROJECT_ROOT")
    @RunWith(JUnit3RunnerWithInners.class)
				`@@ -0,0 +1 @@`
				`org.jetbrains.kotlin.idea.inspections.ReplaceManualRangeWithIndicesCallsInspection`