azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Recognizing manually incremented index

Browse Source
This commit is contained in:
Valentin Kipyatkov
2016-04-21 13:40:03 +03:00
parent 14e87b1f2c
commit d61daed461
23 changed files with 283 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/interfaces.kt
+3 -6
View File
@@ -84,12 +84,13 @@ interface ResultTransformation : Transformation {
data class MatchingState(
val outerLoop: KtForExpression,
val innerLoop: KtForExpression,
val statements: Collection<KtExpression>,
val statements: List<KtExpression>,
val inputVariable: KtCallableDeclaration,
/**
* Matchers can assume that indexVariable is null if it's not used in the rest of the loop
*/
val indexVariable: KtCallableDeclaration?
val indexVariable: KtCallableDeclaration?,
val initializationStatementsToDelete: Collection<KtExpression> = emptyList()
)
/**
@@ -103,10 +104,6 @@ class SequenceTransformationMatch(
val transformations: List<SequenceTransformation>,
val newState: MatchingState
) {
init {
assert(transformations.isNotEmpty())
}
constructor(transformation: SequenceTransformation, newState: MatchingState) : this(listOf(transformation), newState)
}
idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt
+8 -3
View File
@@ -26,6 +26,7 @@ import org.jetbrains.kotlin.idea.intentions.loopToCallChain.result.FindAndAssign
import org.jetbrains.kotlin.idea.intentions.loopToCallChain.result.FindAndReturnTransformation
import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.FilterTransformation
import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.FlatMapTransformation
import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.IntroduceIndexMatcher
import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.MapTransformation
import org.jetbrains.kotlin.idea.util.CommentSaver
import org.jetbrains.kotlin.idea.util.getResolutionScope
@@ -42,6 +43,7 @@ import java.util.*
object MatcherRegistrar {
val sequenceMatchers: Collection<SequenceTransformationMatcher> = listOf(
IntroduceIndexMatcher,
FilterTransformation.Matcher,
MapTransformation.Matcher,
FlatMapTransformation.Matcher
@@ -57,7 +59,8 @@ object MatcherRegistrar {
data class MatchResult(
val sequenceExpression: KtExpression,
val transformationMatch: ResultTransformationMatch
val transformationMatch: ResultTransformationMatch,
val initializationStatementsToDelete: Collection<KtExpression>
)
fun match(loop: KtForExpression): MatchResult? {
@@ -100,7 +103,7 @@ fun match(loop: KtForExpression): MatchResult? {
sequenceTransformations.addAll(match.sequenceTransformations)
return ResultTransformationMatch(match.resultTransformation, sequenceTransformations)
.let { mergeTransformations(it) }
.let { MatchResult(sequenceExpression, it) }
.let { MatchResult(sequenceExpression, it, state.initializationStatementsToDelete) }
.check { checkSmartCastsPreserved(loop, it) }
}
}
@@ -141,6 +144,9 @@ fun convertLoop(loop: KtForExpression, matchResult: MatchResult): KtExpression {
val result = resultTransformation.convertLoop(callChain)
//TODO: preserve comments?
matchResult.initializationStatementsToDelete.forEach { it.delete() }
commentSaver.restore(resultTransformation.commentRestoringRange(result))
return result
@@ -154,7 +160,6 @@ data class LoopData(
private fun extractLoopData(loop: KtForExpression): LoopData? {
val loopRange = loop.loopRange ?: return null
//TODO: recognize other patterns for loop with index
val destructuringParameter = loop.destructuringParameter
if (destructuringParameter != null && destructuringParameter.entries.size == 2) {
val qualifiedExpression = loopRange as? KtDotQualifiedExpression
idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/AddToCollectionTransformation.kt
+4 -4
View File
@@ -118,7 +118,7 @@ class AddToCollectionTransformation(
targetCollection: KtExpression,
addOperationArgument: KtExpression
): ResultTransformationMatch? {
val collectionInitialization = targetCollection.detectInitializationBeforeLoop(state.outerLoop) ?: return null
val collectionInitialization = targetCollection.detectInitializationBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = true) ?: return null
val collectionKind = collectionInitialization.initializer.isSimpleCollectionInstantiation() ?: return null
val argumentIsInputVariable = addOperationArgument.isVariableReference(state.inputVariable)
when (collectionKind) {
@@ -204,7 +204,7 @@ class FilterToTransformation private constructor(
targetCollection: KtExpression,
filter: KtExpression
): ResultTransformation {
val initialization = targetCollection.detectInitializationBeforeLoop(loop)
val initialization = targetCollection.detectInitializationBeforeLoop(loop, checkNoOtherUsagesInLoop = true)
if (initialization != null && initialization.initializer.hasNoSideEffect()) {
return AssignFilterToTransformation(loop, inputVariable, initialization, filter)
}
@@ -253,7 +253,7 @@ class MapToTransformation private constructor(
targetCollection: KtExpression,
mapping: KtExpression
): ResultTransformation {
val initialization = targetCollection.detectInitializationBeforeLoop(loop)
val initialization = targetCollection.detectInitializationBeforeLoop(loop, checkNoOtherUsagesInLoop = true)
if (initialization != null && initialization.initializer.hasNoSideEffect()) {
return AssignMapToTransformation(loop, inputVariable, initialization, mapping)
}
@@ -302,7 +302,7 @@ class FlatMapToTransformation private constructor(
targetCollection: KtExpression,
transform: KtExpression
): ResultTransformation {
val initialization = targetCollection.detectInitializationBeforeLoop(loop)
val initialization = targetCollection.detectInitializationBeforeLoop(loop, checkNoOtherUsagesInLoop = true)
if (initialization != null && initialization.initializer.hasNoSideEffect()) {
return AssignFlatMapToTransformation(loop, inputVariable, initialization, transform)
}
idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/CountTransformation.kt
+3 -10
View File
@@ -16,14 +16,11 @@
package org.jetbrains.kotlin.idea.intentions.loopToCallChain.result
import com.intellij.psi.search.LocalSearchScope
import com.intellij.psi.search.searches.ReferencesSearch
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.descriptors.VariableDescriptor
import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny
import org.jetbrains.kotlin.idea.intentions.loopToCallChain.*
import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.FilterTransformation
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.*
class CountTransformation(
@@ -75,14 +72,10 @@ class CountTransformation(
get() = false
override fun match(state: MatchingState): ResultTransformationMatch? {
val statement = state.statements.singleOrNull() as? KtUnaryExpression ?: return null
if (statement.operationToken != KtTokens.PLUSPLUS) return null
val operand = state.statements.singleOrNull()?.isPlusPlusOf() ?: return null
val initialization = operand.detectInitializationBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = true) ?: return null
val operand = statement.baseExpression
val initialization = operand?.detectInitializationBeforeLoop(state.outerLoop) ?: return null
val usageCountInLoop = ReferencesSearch.search(initialization.variable, LocalSearchScope(state.outerLoop)).count()
if (usageCountInLoop != 1) return null // this should be the only usage of this variable inside the loop
if (initialization.variable.countUsages(state.outerLoop) != 1) return null // this should be the only usage of this variable inside the loop
val variableType = (initialization.variable.resolveToDescriptorIfAny() as? VariableDescriptor)?.type ?: return null
if (!KotlinBuiltIns.isInt(variableType)) return null
idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndAssignTransformation.kt
+3 -6
View File
@@ -16,8 +16,6 @@
package org.jetbrains.kotlin.idea.intentions.loopToCallChain.result
import com.intellij.psi.search.LocalSearchScope
import com.intellij.psi.search.searches.ReferencesSearch
import org.jetbrains.kotlin.idea.caches.resolve.analyze
import org.jetbrains.kotlin.idea.intentions.loopToCallChain.*
import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.FilterTransformation
@@ -80,7 +78,7 @@ class FindAndAssignTransformation(
2 -> {
val breakExpression = state.statements.last() as? KtBreakExpression ?: return null
if (!breakExpression.isBreakOrContinueOfLoop(state.outerLoop)) return null
if (breakExpression.targetLoop() != state.outerLoop) return null
}
else -> return null
@@ -92,10 +90,9 @@ class FindAndAssignTransformation(
val left = binaryExpression.left ?: return null
val right = binaryExpression.right ?: return null
val initialization = left.detectInitializationBeforeLoop(state.outerLoop) ?: return null
val initialization = left.detectInitializationBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = true) ?: return null
val usageCountInLoop = ReferencesSearch.search(initialization.variable, LocalSearchScope(state.outerLoop)).count()
if (usageCountInLoop != 1) return null // this should be the only usage of this variable inside the loop
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(
idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/FilterTransformation.kt
+1 -1
View File
@@ -81,7 +81,7 @@ class FilterTransformation(
}
else {
val continueExpression = then.blockExpressionsOrSingle().singleOrNull() as? KtContinueExpression ?: return null
if (!continueExpression.isBreakOrContinueOfLoop(state.innerLoop)) return null
if (continueExpression.targetLoop() != state.innerLoop) return null
val transformation = createFilterTransformation(state.outerLoop, state.inputVariable, state.indexVariable, condition, isInverse = true)
val newState = state.copy(statements = state.statements.drop(1))
return SequenceTransformationMatch(transformation, newState)
idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/IntroduceIndexMatcher.kt
+57
View File
@@ -0,0 +1,57 @@
/*
* 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.sequence
import org.jetbrains.kotlin.idea.intentions.loopToCallChain.*
import org.jetbrains.kotlin.psi.KtConstantExpression
import org.jetbrains.kotlin.psi.KtContinueExpression
import org.jetbrains.kotlin.psi.psiUtil.anyDescendantOfType
import org.jetbrains.kotlin.psi.psiUtil.isAncestor
object IntroduceIndexMatcher : SequenceTransformationMatcher {
override fun match(state: MatchingState): SequenceTransformationMatch? {
if (state.indexVariable != null) return null // old index variable is still needed - cannot introduce another one
if (state.statements.size < 2) return null
val operand = state.statements.last().isPlusPlusOf() ?: return null
val restStatements = state.statements.dropLast(1)
fun isContinueOfThisLoopOrOuter(continueExpression: KtContinueExpression): Boolean {
val targetLoop = continueExpression.targetLoop() ?: return true
return targetLoop.isAncestor(state.innerLoop, strict = false)
}
// there should be no continuation of the loop in statements before index increment
if (restStatements.any { statement -> statement.anyDescendantOfType<KtContinueExpression>(::isContinueOfThisLoopOrOuter) }) return null
val variableInitialization = operand.detectInitializationBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = false)
?: return null
if ((variableInitialization.initializer as? KtConstantExpression)?.text != "0") return null
val variable = variableInitialization.variable
if (variable.countWriteUsages(state.outerLoop) > 1) return null // changed somewhere else
// variable should have no usages except in the initialization + currently matching part of the loop
//TODO: preform more precise analysis when variable can be used earlier or used later but value overwritten before that
if (variable.countUsages() != variable.countUsages(state.statements + variableInitialization.initializationStatement)) return null
val newState = state.copy(statements = restStatements,
indexVariable = variable,
initializationStatementsToDelete = state.initializationStatementsToDelete + variableInitialization.initializationStatement)
return SequenceTransformationMatch(emptyList(), newState)
}
}
idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt
+40 -7
View File
@@ -37,6 +37,7 @@ import org.jetbrains.kotlin.idea.references.KtSimpleNameReference
import org.jetbrains.kotlin.idea.references.mainReference
import org.jetbrains.kotlin.idea.references.readWriteAccess
import org.jetbrains.kotlin.idea.util.getResolutionScope
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.*
@@ -133,6 +134,31 @@ fun KtProperty.hasWriteUsages(): Boolean {
}
}
fun KtProperty.countUsages(inElement: KtElement): Int {
assert(this.isPhysical)
return ReferencesSearch.search(this, LocalSearchScope(inElement)).count()
}
fun KtProperty.countUsages(inElements: Collection<KtElement>): Int {
assert(this.isPhysical)
// TODO: it's a temporary workaround about strange dead-lock when running inspections
return inElements.sumBy { ReferencesSearch.search(this, LocalSearchScope(it)).count() }
}
fun KtProperty.countUsages(): Int {
assert(this.isPhysical)
return ReferencesSearch.search(this, useScope).count()
}
fun KtProperty.countWriteUsages(inElement: KtElement): Int {
assert(this.isPhysical)
if (!isVar) return 0
return ReferencesSearch.search(this, LocalSearchScope(inElement)).count {
(it as? KtSimpleNameReference)?.element?.readWriteAccess(useResolveForReadWrite = true)?.isWrite == true
}
}
interface FindOperatorGenerator {
val functionName: String
@@ -260,19 +286,23 @@ fun buildFindOperationGenerator(
}
}
fun KtExpressionWithLabel.isBreakOrContinueOfLoop(loop: KtLoopExpression): Boolean {
fun KtExpressionWithLabel.targetLoop(): KtLoopExpression? {
val label = getTargetLabel()
if (label == null) {
val closestLoop = parents.firstIsInstance<KtLoopExpression>()
return loop == closestLoop
return parents.firstIsInstance<KtLoopExpression>()
}
else {
//TODO: does PARTIAL always work here?
val targetLoop = analyze(BodyResolveMode.PARTIAL)[BindingContext.LABEL_TARGET, label]
return targetLoop == loop
return analyze(BodyResolveMode.PARTIAL)[BindingContext.LABEL_TARGET, label] as? KtLoopExpression
}
}
fun KtExpression.isPlusPlusOf(): KtExpression? {
if (this !is KtUnaryExpression) return null
if (operationToken != KtTokens.PLUSPLUS) return null
return baseExpression
}
fun KtExpression.previousStatement(): KtExpression? {
val statement = unwrapIfLabeled()
if (statement.parent !is KtBlockExpression) return null
@@ -312,14 +342,17 @@ data class VariableInitialization(
val initializer: KtExpression)
//TODO: we need more correctness checks (if variable is non-local or is local but can be changed by some local functions)
fun KtExpression.detectInitializationBeforeLoop(loop: KtForExpression): VariableInitialization? {
fun KtExpression.detectInitializationBeforeLoop(
loop: KtForExpression,
checkNoOtherUsagesInLoop: Boolean
): VariableInitialization? {
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 (ReferencesSearch.search(variable, LocalSearchScope(loop)).count() > 1) return null
if (checkNoOtherUsagesInLoop && variable.countUsages(loop) > 1) return null
if (statementBeforeLoop == variable) {
val initializer = variable.initializer ?: return null
idea/testData/intentions/loopToCallChain/filterIndexed2.kt
Vendored
+12
View File
@@ -0,0 +1,12 @@
// WITH_RUNTIME
// INTENTION_TEXT: "Replace with 'filterIndexed{}.firstOrNull()'"
fun foo(list: List<String>): String? {
var i = 0
<caret>for (s in list) {
if (s.length > i) {
return s
}
i++
}
return null
}
idea/testData/intentions/loopToCallChain/filterIndexed2.kt.after
Vendored
+7
View File
@@ -0,0 +1,7 @@
// WITH_RUNTIME
// INTENTION_TEXT: "Replace with 'filterIndexed{}.firstOrNull()'"
fun foo(list: List<String>): String? {
<caret>return list
.filterIndexed { i, s -> s.length > i }
.firstOrNull()
}
idea/testData/intentions/loopToCallChain/filterIndexedAndFlatMapWithContinue.kt
Vendored
+14
View File
@@ -0,0 +1,14 @@
// WITH_RUNTIME
// INTENTION_TEXT: "Replace with '...flatMap{}.filterNot{}.mapTo(){}'"
fun foo(list: List<String>, target: MutableCollection<String>) {
var i = 0
<caret>for (s in list) {
if (i % 10 != 0) {
for (j in s.indices) {
if (j == 10) continue
target.add(j.toString())
}
}
i++
}
}
idea/testData/intentions/loopToCallChain/filterIndexedAndFlatMapWithContinue.kt.after
Vendored
+9
View File
@@ -0,0 +1,9 @@
// WITH_RUNTIME
// INTENTION_TEXT: "Replace with '...flatMap{}.filterNot{}.mapTo(){}'"
fun foo(list: List<String>, target: MutableCollection<String>) {
<caret>list
.filterIndexed { i, s -> i % 10 != 0 }
.flatMap { it.indices }
.filterNot { it == 10 }
.mapTo(target) { it.toString() }
}
idea/testData/intentions/loopToCallChain/filterIndexed_indexChangedTwice.kt
Vendored
+12
View File
@@ -0,0 +1,12 @@
// WITH_RUNTIME
// IS_APPLICABLE: false
fun foo(list: List<String>, target: MutableCollection<String>): String? {
var i = 0
<caret>for (s in list) {
if (s.length > i++) {
target.add(s)
}
i++
}
return null
}
idea/testData/intentions/loopToCallChain/filterIndexed_indexStartNotZero.kt
Vendored
+12
View File
@@ -0,0 +1,12 @@
// WITH_RUNTIME
// IS_APPLICABLE: false
fun foo(list: List<String>): String? {
var i = 1
<caret>for (s in list) {
if (s.length > i) {
return s
}
i++
}
return null
}
idea/testData/intentions/loopToCallChain/filterIndexed_indexUsedAfter.kt
Vendored
+13
View File
@@ -0,0 +1,13 @@
// WITH_RUNTIME
// IS_APPLICABLE: false
fun foo(list: List<String>, target: MutableCollection<String>): String? {
var i = 0
<caret>for (s in list) {
if (s.length > i) {
target.add(s)
}
i++
}
println(i)
return null
}
idea/testData/intentions/loopToCallChain/filterIndexed_indexUsedEarlierInLoop.kt
Vendored
+14
View File
@@ -0,0 +1,14 @@
// WITH_RUNTIME
// IS_APPLICABLE: false
fun foo(list: List<String>, target: MutableCollection<String>): String? {
var i = 0
<caret>for (s in list) {
if (s.hashCode() % i == 0) continue
if (s.length > i) {
target.add(s)
}
i++
}
println(i)
return null
}
idea/testData/intentions/loopToCallChain/filterIndexed_twoIndices.kt
Vendored
+12
View File
@@ -0,0 +1,12 @@
// WITH_RUNTIME
// INTENTION_TEXT: "Replace with '+= filterIndexed{}.filterIndexed{}'"
fun foo(list: List<String>, target: MutableCollection<String>) {
var j = 0
<caret>for ((i, s) in list.withIndex()) {
if (s.length > i) continue
if (s.length % j == 0) {
target.add(s)
}
j++
}
}
idea/testData/intentions/loopToCallChain/filterIndexed_twoIndices.kt.after
Vendored
+7
View File
@@ -0,0 +1,7 @@
// WITH_RUNTIME
// INTENTION_TEXT: "Replace with '+= filterIndexed{}.filterIndexed{}'"
fun foo(list: List<String>, target: MutableCollection<String>) {
<caret>target += list
.filterIndexed { i, s -> s.length <= i }
.filterIndexed { j, s -> s.length % j == 0 }
}
idea/testData/intentions/loopToCallChain/filterIndexed_twoIndicesUsed.kt
Vendored
+12
View File
@@ -0,0 +1,12 @@
// WITH_RUNTIME
// IS_APPLICABLE: false
fun foo(list: List<String>, target: MutableCollection<Int>) {
var j = 0
<caret>for ((i, s) in list.withIndex()) {
val x = s.length + i
if (x < i * j) {
target.add(x)
}
j++
}
}
idea/testData/intentions/loopToCallChain/indexWithNestedLoop.kt
Vendored
+12
View File
@@ -0,0 +1,12 @@
// WITH_RUNTIME
// INTENTION_TEXT: "Replace with '+= ...filterNot{}.mapIndexed{}'"
fun foo(list: List<String>, target: MutableCollection<Int>) {
var i = 0
<caret>for (s in list) {
for (j in s.indices) {
if (j == 10) continue
target.add(i + j)
i++
}
}
}
idea/testData/intentions/loopToCallChain/indexWithNestedLoop.kt.after
Vendored
+8
View File
@@ -0,0 +1,8 @@
// WITH_RUNTIME
// INTENTION_TEXT: "Replace with '+= ...filterNot{}.mapIndexed{}'"
fun foo(list: List<String>, target: MutableCollection<Int>) {
<caret>target += list
.flatMap { it.indices }
.filterNot { it == 10 }
.mapIndexed { i, j -> i + j }
}
idea/testData/intentions/loopToCallChain/mapIndexed_afterFilter.kt
Vendored
+12
View File
@@ -0,0 +1,12 @@
// WITH_RUNTIME
// INTENTION_TEXT: "Replace with 'filterNot{}.mapIndexed{}.firstOrNull{}'"
fun foo(list: List<String>): Int? {
var index = 0
<caret>for (s in list) {
if (s.isBlank()) continue
val x = s.length * index
if (x > 0) return x
index++
}
return null
}
idea/testData/intentions/loopToCallChain/mapIndexed_afterFilter.kt.after
Vendored
+8
View File
@@ -0,0 +1,8 @@
// WITH_RUNTIME
// INTENTION_TEXT: "Replace with 'filterNot{}.mapIndexed{}.firstOrNull{}'"
fun foo(list: List<String>): Int? {
return list
.filterNot { it.isBlank() }
.mapIndexed { index, s -> s.length * index }
.firstOrNull { it > 0 }
}