From 5ac2c48879cf2fb59ac42900e5e9f6f7fe86e26c Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Fri, 22 Apr 2016 13:22:42 +0300 Subject: [PATCH] Merging subsequent filtering checks even when index variable is used --- .../loopToCallChain/matchAndConvert.kt | 5 +- .../result/AddToCollectionTransformation.kt | 73 ++++++------------- .../result/CountTransformation.kt | 1 + .../result/FindAndAssignTransformation.kt | 1 + .../result/FindAndReturnTransformation.kt | 1 + .../sequence/FilterTransformation.kt | 67 +++++++---------- .../idea/intentions/loopToCallChain/utils.kt | 7 +- .../loopToCallChain/filterIndexed_merge1.kt | 11 +++ .../filterIndexed_merge1.kt.after | 7 ++ .../loopToCallChain/filterIndexed_merge2.kt | 11 +++ .../filterIndexed_merge2.kt.after | 7 ++ .../loopToCallChain/filterIndexed_merge3.kt | 11 +++ .../filterIndexed_merge3.kt.after | 7 ++ 13 files changed, 113 insertions(+), 96 deletions(-) create mode 100644 idea/testData/intentions/loopToCallChain/filterIndexed_merge1.kt create mode 100644 idea/testData/intentions/loopToCallChain/filterIndexed_merge1.kt.after create mode 100644 idea/testData/intentions/loopToCallChain/filterIndexed_merge2.kt create mode 100644 idea/testData/intentions/loopToCallChain/filterIndexed_merge2.kt.after create mode 100644 idea/testData/intentions/loopToCallChain/filterIndexed_merge3.kt create mode 100644 idea/testData/intentions/loopToCallChain/filterIndexed_merge3.kt.after 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 53e6fafe072..f4f3313dc57 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt @@ -80,10 +80,7 @@ fun match(loop: KtForExpression): MatchResult? { MatchLoop@ while (true) { - val block = state.statements.singleOrNull() as? KtBlockExpression - if (block != null) { - state = state.copy(statements = block.statements) - } + state = state.unwrapBlock() val inputVariableUsed = state.inputVariable.hasUsages(state.statements) diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/AddToCollectionTransformation.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/AddToCollectionTransformation.kt index e9cbf9f954c..0640cf3c819 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/AddToCollectionTransformation.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/AddToCollectionTransformation.kt @@ -22,7 +22,11 @@ import org.jetbrains.kotlin.descriptors.VariableDescriptor import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptor import org.jetbrains.kotlin.idea.imports.importableFqName import org.jetbrains.kotlin.idea.intentions.loopToCallChain.* -import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.* +import org.jetbrains.kotlin.idea.intentions.loopToCallChain.sequence.FilterNotNullTransformation +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.MapTransformation +import org.jetbrains.kotlin.idea.intentions.negate import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.* @@ -37,11 +41,7 @@ class AddToCollectionTransformation( override fun mergeWithPrevious(previousTransformation: SequenceTransformation): ResultTransformation? { return when (previousTransformation) { is FilterTransformation -> { - FilterToTransformation.create(loop, previousTransformation.inputVariable, targetCollection, previousTransformation.condition, previousTransformation.isInverse) - } - - is FilterIndexedTransformation -> { - FilterIndexedToTransformation.create(loop, previousTransformation.inputVariable, previousTransformation.indexVariable, targetCollection, previousTransformation.condition) + FilterToTransformation.create(loop, previousTransformation.inputVariable, previousTransformation.indexVariable, targetCollection, previousTransformation.condition, previousTransformation.isInverse) } is FilterNotNullTransformation -> { @@ -189,18 +189,28 @@ class AddToCollectionTransformation( class FilterToTransformation private constructor( loop: KtForExpression, private val inputVariable: KtCallableDeclaration, + private val indexVariable: KtCallableDeclaration?, private val targetCollection: KtExpression, - private val filter: KtExpression, - isInverse: Boolean + private val condition: KtExpression, + private val isInverse: Boolean ) : ReplaceLoopResultTransformation(loop) { - private val functionName = if (isInverse) "filterNotTo" else "filterTo" + private fun effectiveCondition() = if (isInverse) condition.negate() else condition + + private val functionName = when { + indexVariable != null -> "filterIndexedTo" + isInverse -> "filterNotTo" + else -> "filterTo" + } override val presentation: String get() = "$functionName(){}" override fun generateCode(chainedCallGenerator: ChainedCallGenerator): KtExpression { - val lambda = generateLambda(inputVariable, filter) + val lambda = if (indexVariable != null) + generateLambda(effectiveCondition(), indexVariable, inputVariable) + else + generateLambda(inputVariable, condition) return chainedCallGenerator.generate("$functionName($0) $1:'{}'", targetCollection, lambda) } @@ -208,53 +218,18 @@ class FilterToTransformation private constructor( fun create( loop: KtForExpression, inputVariable: KtCallableDeclaration, + indexVariable: KtCallableDeclaration?, targetCollection: KtExpression, - filter: KtExpression, + condition: KtExpression, isInverse: Boolean ): ResultTransformation { val initialization = targetCollection.detectInitializationBeforeLoop(loop, checkNoOtherUsagesInLoop = true) if (initialization != null && initialization.initializer.hasNoSideEffect()) { - val transformation = FilterToTransformation(loop, inputVariable, initialization.initializer, filter, isInverse) + val transformation = FilterToTransformation(loop, inputVariable, indexVariable, initialization.initializer, condition, isInverse) return AssignToVariableResultTransformation.createDelegated(transformation, initialization) } else { - return FilterToTransformation(loop, inputVariable, targetCollection, filter, isInverse) - } - } - } -} - -class FilterIndexedToTransformation private constructor( - loop: KtForExpression, - private val inputVariable: KtCallableDeclaration, - private val indexVariable: KtCallableDeclaration, - private val targetCollection: KtExpression, - private val filter: KtExpression -) : ReplaceLoopResultTransformation(loop) { - - override val presentation: String - get() = "filterIndexedTo(){}" - - override fun generateCode(chainedCallGenerator: ChainedCallGenerator): KtExpression { - val lambda = generateLambda(filter, indexVariable, inputVariable) - return chainedCallGenerator.generate("filterIndexedTo($0) $1:'{}'", targetCollection, lambda) - } - - companion object { - fun create( - loop: KtForExpression, - inputVariable: KtCallableDeclaration, - indexVariable: KtCallableDeclaration, - targetCollection: KtExpression, - filter: KtExpression - ): ResultTransformation { - val initialization = targetCollection.detectInitializationBeforeLoop(loop, checkNoOtherUsagesInLoop = true) - if (initialization != null && initialization.initializer.hasNoSideEffect()) { - val transformation = FilterIndexedToTransformation(loop, inputVariable, indexVariable, initialization.initializer, filter) - return AssignToVariableResultTransformation.createDelegated(transformation, initialization) - } - else { - return FilterIndexedToTransformation(loop, inputVariable, indexVariable, targetCollection, filter) + return FilterToTransformation(loop, inputVariable, indexVariable, targetCollection, condition, isInverse) } } } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/CountTransformation.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/CountTransformation.kt index aead2bb61cd..c0805563b2e 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/CountTransformation.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/CountTransformation.kt @@ -32,6 +32,7 @@ class CountTransformation( override fun mergeWithPrevious(previousTransformation: SequenceTransformation): ResultTransformation? { if (previousTransformation !is FilterTransformation) return null + if (previousTransformation.indexVariable != null) return null assert(filter == null) { "Should not happen because no 2 consecutive FilterTransformation's possible"} return CountTransformation(loop, previousTransformation.inputVariable, initialization, previousTransformation.effectiveCondition()) } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndAssignTransformation.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndAssignTransformation.kt index 1214af638b6..bf0b0317d01 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndAssignTransformation.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndAssignTransformation.kt @@ -33,6 +33,7 @@ class FindAndAssignTransformation( override fun mergeWithPrevious(previousTransformation: SequenceTransformation): ResultTransformation? { if (previousTransformation !is FilterTransformation) return null + if (previousTransformation.indexVariable != null) return null assert(filter == null) { "Should not happen because no 2 consecutive FilterTransformation's possible"} return FindAndAssignTransformation(loop, generator, initialization, previousTransformation.effectiveCondition()) } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndReturnTransformation.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndReturnTransformation.kt index 056a2b52601..b0b525d944d 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndReturnTransformation.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindAndReturnTransformation.kt @@ -32,6 +32,7 @@ class FindAndReturnTransformation( override fun mergeWithPrevious(previousTransformation: SequenceTransformation): ResultTransformation? { if (previousTransformation !is FilterTransformation) return null + if (previousTransformation.indexVariable != null) return null assert(filter == null) { "Should not happen because no 2 consecutive FilterTransformation's possible"} return FindAndReturnTransformation(loop, generator, endReturn, previousTransformation.effectiveCondition()) } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/FilterTransformation.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/FilterTransformation.kt index 220ef6443d5..e221a5b5418 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/FilterTransformation.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/FilterTransformation.kt @@ -26,22 +26,19 @@ import org.jetbrains.kotlin.psi.psiUtil.blockExpressionsOrSingle class FilterTransformation( override val loop: KtForExpression, val inputVariable: KtCallableDeclaration, + val indexVariable: KtCallableDeclaration?, val condition: KtExpression, val isInverse: Boolean ) : SequenceTransformation { fun effectiveCondition() = if (isInverse) condition.negate() else condition - override fun mergeWithPrevious(previousTransformation: SequenceTransformation): SequenceTransformation? { - if (previousTransformation !is FilterTransformation) return null - assert(previousTransformation.inputVariable == inputVariable) - val mergedCondition = KtPsiFactory(condition).createExpressionByPattern( - "$0 && $1", previousTransformation.effectiveCondition(), effectiveCondition()) - return FilterTransformation(loop, inputVariable, mergedCondition, isInverse = false) //TODO: build filterNot in some cases? + private val functionName = when { + indexVariable != null -> "filterIndexed" + isInverse -> "filterNot" + else -> "filter" } - private val functionName = if (isInverse) "filterNot" else "filter" - override val affectsIndex: Boolean get() = true @@ -49,7 +46,10 @@ class FilterTransformation( get() = "$functionName{}" override fun generateCode(chainedCallGenerator: ChainedCallGenerator): KtExpression { - val lambda = generateLambda(inputVariable, condition) + val lambda = if (indexVariable != null) + generateLambda(effectiveCondition(), indexVariable, inputVariable) + else + generateLambda(inputVariable, condition) return chainedCallGenerator.generate("$0$1:'{}'", functionName, lambda) } @@ -76,22 +76,29 @@ class FilterTransformation( */ object Matcher : SequenceTransformationMatcher { override fun match(state: MatchingState): SequenceTransformationMatch? { - var (transformation, currentState) = matchOneTransformation(state, takeWhileAllowed = true) ?: return null + // we merge filter transformations here instead of FilterTransformation.mergeWithPrevious() because of filterIndexed that won't merge otherwise -/* - if (transformation is FilterTransformation || transformation is FilterIndexedTransformation) { + var (transformation, currentState) = matchOneTransformation(state) ?: return null + + if (transformation is FilterTransformation) { while (true) { - val (nextTransformation, nextState) = matchOneTransformation(currentState, takeWhileAllowed = false) ?: break - if (nextTransformation !is FilterTransformation && nextTransformation !is FilterIndexedTransformation) break + currentState = currentState.unwrapBlock() + val (nextTransformation, nextState) = matchOneTransformation(currentState) ?: break + if (nextTransformation !is FilterTransformation) break + + val indexVariable = transformation.indexVariable ?: nextTransformation.indexVariable + val mergedCondition = KtPsiFactory(state.outerLoop).createExpressionByPattern( + "$0 && $1", transformation.effectiveCondition(), nextTransformation.effectiveCondition()) + transformation = FilterTransformation(state.outerLoop, transformation.inputVariable, indexVariable, mergedCondition, isInverse = false) //TODO: build filterNot in some cases? + currentState = nextState } } -*/ return SequenceTransformationMatch(transformation, currentState) } - private fun matchOneTransformation(state: MatchingState, takeWhileAllowed: Boolean): Pair? { + private fun matchOneTransformation(state: MatchingState): Pair? { val ifStatement = state.statements.firstOrNull() as? KtIfExpression ?: return null if (ifStatement.`else` != null) return null val condition = ifStatement.condition ?: return null @@ -113,7 +120,6 @@ class FilterTransformation( } is KtBreakExpression -> { - if (!takeWhileAllowed) return null if (statement.targetLoop() != state.outerLoop) return null val transformation = TakeWhileTransformation(state.outerLoop, state.inputVariable, condition.negate()) val newState = state.copy(statements = state.statements.drop(1)) @@ -125,8 +131,6 @@ class FilterTransformation( } } -// private fun mergeFilterTransformations(transformation1: ) - //TODO: choose filter or filterNot depending on condition private fun createFilterTransformation( loop: KtForExpression, @@ -139,7 +143,7 @@ class FilterTransformation( val effectiveCondition = if (isInverse) condition.negate() else condition if (indexVariable != null && indexVariable.hasUsages(condition)) { - return FilterIndexedTransformation(loop, inputVariable, indexVariable, effectiveCondition) + return FilterTransformation(loop, inputVariable, indexVariable, effectiveCondition, isInverse = false) } if (effectiveCondition is KtIsExpression @@ -160,7 +164,7 @@ class FilterTransformation( return FilterNotNullTransformation(loop) } - return FilterTransformation(loop, inputVariable, condition, isInverse) + return FilterTransformation(loop, inputVariable, null, condition, isInverse) } } } @@ -200,27 +204,6 @@ class FilterNotNullTransformation(override val loop: KtForExpression) : Sequence } } -class FilterIndexedTransformation( - override val loop: KtForExpression, - val inputVariable: KtCallableDeclaration, - val indexVariable: KtCallableDeclaration, - val condition: KtExpression -) : SequenceTransformation { - - //TODO: how to handle multiple if's using index? - - override val affectsIndex: Boolean - get() = true - - override val presentation: String - get() = "filterIndexed{}" - - override fun generateCode(chainedCallGenerator: ChainedCallGenerator): KtExpression { - val lambda = generateLambda(condition, indexVariable, inputVariable) - return chainedCallGenerator.generate("filterIndexed $0:'{}'", lambda) - } -} - class TakeWhileTransformation( override val loop: KtForExpression, val inputVariable: KtCallableDeclaration, 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 2f5f7daad06..c8c85b18718 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt @@ -306,4 +306,9 @@ private fun isEmbeddedBreakOrContinue(expression: KtExpressionWithLabel): Boolea else -> return true } -} \ No newline at end of file +} + +fun MatchingState.unwrapBlock(): MatchingState { + val block = statements.singleOrNull() as? KtBlockExpression ?: return this + return this.copy(statements = block.statements) +} diff --git a/idea/testData/intentions/loopToCallChain/filterIndexed_merge1.kt b/idea/testData/intentions/loopToCallChain/filterIndexed_merge1.kt new file mode 100644 index 00000000000..3ba6a90278a --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/filterIndexed_merge1.kt @@ -0,0 +1,11 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'filterIndexed{}.firstOrNull()'" +fun foo(list: List): String? { + for ((index, s) in list.withIndex()) { + if (s.length > index * 10) continue + if (s.length > index) { + return s + } + } + return null +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/filterIndexed_merge1.kt.after b/idea/testData/intentions/loopToCallChain/filterIndexed_merge1.kt.after new file mode 100644 index 00000000000..d1a28dbe654 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/filterIndexed_merge1.kt.after @@ -0,0 +1,7 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'filterIndexed{}.firstOrNull()'" +fun foo(list: List): String? { + return list + .filterIndexed { index, s -> s.length <= index * 10 && s.length > index } + .firstOrNull() +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/filterIndexed_merge2.kt b/idea/testData/intentions/loopToCallChain/filterIndexed_merge2.kt new file mode 100644 index 00000000000..5774814e3e9 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/filterIndexed_merge2.kt @@ -0,0 +1,11 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'filterIndexed{}.firstOrNull()'" +fun foo(list: List): String? { + for ((index, s) in list.withIndex()) { + if (s.isBlank()) continue + if (s.length > index) { + return s + } + } + return null +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/filterIndexed_merge2.kt.after b/idea/testData/intentions/loopToCallChain/filterIndexed_merge2.kt.after new file mode 100644 index 00000000000..48ad88a2b4d --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/filterIndexed_merge2.kt.after @@ -0,0 +1,7 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'filterIndexed{}.firstOrNull()'" +fun foo(list: List): String? { + return list + .filterIndexed { index, s -> !s.isBlank() && s.length > index } + .firstOrNull() +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/filterIndexed_merge3.kt b/idea/testData/intentions/loopToCallChain/filterIndexed_merge3.kt new file mode 100644 index 00000000000..0dfa3dbaaa3 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/filterIndexed_merge3.kt @@ -0,0 +1,11 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'filterIndexed{}.firstOrNull()'" +fun foo(list: List): String? { + for ((index, s) in list.withIndex()) { + if (s.length > index) continue + if (s.isNotBlank()) { + return s + } + } + return null +} \ No newline at end of file diff --git a/idea/testData/intentions/loopToCallChain/filterIndexed_merge3.kt.after b/idea/testData/intentions/loopToCallChain/filterIndexed_merge3.kt.after new file mode 100644 index 00000000000..f1490d63d39 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/filterIndexed_merge3.kt.after @@ -0,0 +1,7 @@ +// WITH_RUNTIME +// INTENTION_TEXT: "Replace with 'filterIndexed{}.firstOrNull()'" +fun foo(list: List): String? { + return list + .filterIndexed { index, s -> s.length <= index && s.isNotBlank() } + .firstOrNull() +} \ No newline at end of file