diff --git a/compiler/psi/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt b/compiler/psi/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt index 7afb667f81c..335c224d58b 100644 --- a/compiler/psi/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt +++ b/compiler/psi/src/org/jetbrains/kotlin/psi/KtPsiFactory.kt @@ -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!! } diff --git a/idea/resources/META-INF/plugin-common.xml b/idea/resources/META-INF/plugin-common.xml index ed8db941037..6e1af4e3f38 100644 --- a/idea/resources/META-INF/plugin-common.xml +++ b/idea/resources/META-INF/plugin-common.xml @@ -1820,6 +1820,15 @@ language="kotlin" /> + + + +This inspection reports until and .. operator usages that are replaceable with Collection.indices +or iteration over collection inside for loop. + \ No newline at end of file diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/ReplaceManualRangeWithIndicesCallsInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/ReplaceManualRangeWithIndicesCallsInspection.kt new file mode 100644 index 00000000000..1d4ce373162 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/ReplaceManualRangeWithIndicesCallsInspection.kt @@ -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() ?: 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 +} diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/.inspection b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/.inspection new file mode 100644 index 00000000000..05ee16e06a5 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/.inspection @@ -0,0 +1 @@ +org.jetbrains.kotlin.idea.inspections.ReplaceManualRangeWithIndicesCallsInspection \ No newline at end of file diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/forNotTarget.kt b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/forNotTarget.kt new file mode 100644 index 00000000000..43cbe82447f --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/forNotTarget.kt @@ -0,0 +1,8 @@ +// WITH_RUNTIME +// PROBLEM: none +fun test(args: Array) { + val x = arrayOf() + for (index in args) { + val out = x[index] + } +} diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/indexInLvalue.kt b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/indexInLvalue.kt new file mode 100644 index 00000000000..f4ec35cf888 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/indexInLvalue.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun test(args: Array) { + for (index in 0..args.size - 1) { + args[index] = "Hello" + } +} diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/indexInLvalue.kt.after b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/indexInLvalue.kt.after new file mode 100644 index 00000000000..abb561a97d2 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/indexInLvalue.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun test(args: Array) { + for (index in args.indices) { + args[index] = "Hello" + } +} diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/notUsedAsIndex.kt b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/notUsedAsIndex.kt new file mode 100644 index 00000000000..b366b3a1a6c --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/notUsedAsIndex.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun test(args: Array) { + for (index in 0..args.size - 1) { + println(index) + } +} diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/notUsedAsIndex.kt.after b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/notUsedAsIndex.kt.after new file mode 100644 index 00000000000..1abfff5c7ae --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/notUsedAsIndex.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun test(args: Array) { + for (index in args.indices) { + println(index) + } +} diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpression.kt b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpression.kt new file mode 100644 index 00000000000..c9c188a32f3 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpression.kt @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(args: Array) { + val ind = 0..args.size-1 +} diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpression.kt.after b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpression.kt.after new file mode 100644 index 00000000000..adebb077fb3 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpression.kt.after @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(args: Array) { + val ind = args.indices +} diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpressionUntil.kt b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpressionUntil.kt new file mode 100644 index 00000000000..e20fcdb3489 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpressionUntil.kt @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(args: Array) { + val ind = 0 until args.size +} diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpressionUntil.kt.after b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpressionUntil.kt.after new file mode 100644 index 00000000000..adebb077fb3 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleExpressionUntil.kt.after @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun test(args: Array) { + val ind = args.indices +} diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleFor.kt b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleFor.kt new file mode 100644 index 00000000000..2d5b489cf1c --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleFor.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun test(args: Array) { + for (index in 0..args.size - 1) { + val out = args[index] + } +} diff --git a/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleFor.kt.after b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleFor.kt.after new file mode 100644 index 00000000000..06e22c1b267 --- /dev/null +++ b/idea/testData/inspectionsLocal/replaceManualRangeWithIndicesCallsInspection/simpleFor.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME +fun test(args: Array) { + for (element in args) { + val out = element + } +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java index 38e62810593..28f307daebe 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/inspections/LocalInspectionTestGenerated.java @@ -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)