From e32724330f976a72f2353cd345813e4cdd3f28f0 Mon Sep 17 00:00:00 2001 From: Valentin Kipyatkov Date: Fri, 12 Aug 2016 17:14:37 +0300 Subject: [PATCH] Moved shouldUseInputVariable from Transformation to TransformationMatcher and changed its semantics a bit --- .../idea/intentions/loopToCallChain/commonUtils.kt | 4 ++++ .../idea/intentions/loopToCallChain/interfaces.kt | 9 ++++++--- .../intentions/loopToCallChain/matchAndConvert.kt | 10 ++++++++-- .../loopToCallChain/result/CountTransformation.kt | 6 +++--- .../result/FindTransformationMatcher.kt | 9 +++------ .../sequence/IntroduceIndexMatcher.kt | 3 +++ .../loopToCallChain/filter_inputVariableNotUsed.kt | 12 ++++++++++++ .../idea/intentions/IntentionTest2Generated.java | 6 ++++++ .../idea/intentions/IntentionTestGenerated.java | 6 ++++++ 9 files changed, 51 insertions(+), 14 deletions(-) create mode 100644 idea/testData/intentions/loopToCallChain/filter_inputVariableNotUsed.kt diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/commonUtils.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/commonUtils.kt index d919550d6d6..77737f40b11 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/commonUtils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/commonUtils.kt @@ -116,6 +116,10 @@ fun KtVariableDeclaration.hasWriteUsages(inElement: KtElement): Boolean { } } +fun KtCallableDeclaration.hasDifferentSetsOfUsages(elements1: Collection, elements2: Collection): Boolean { + return countUsages(elements1 - elements2) != countUsages(elements2 - elements1) +} + fun KtExpressionWithLabel.targetLoop(): KtLoopExpression? { val label = getTargetLabel() if (label == null) { diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/interfaces.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/interfaces.kt index bb43c9444dc..c10f5c8ee14 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/interfaces.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/interfaces.kt @@ -60,9 +60,6 @@ interface Transformation { val chainCallCount: Int get() = 1 - - val shouldUseInputVariable: Boolean - get() = true } /** @@ -122,6 +119,12 @@ interface TransformationMatcher { */ val indexVariableAllowed: Boolean + /** + * Override with false value if the result of the match should be rejected if the matched part uses neither the input variable nor the index variable. + */ + val shouldUseInputVariables: Boolean + get() = true + /** * Implementors should return true if they match some constructs with expression-embedded break or continue. * In this case they are obliged to deal with them and filter out invalid cases. 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 3b3be5c855c..b1ce4b731a5 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/matchAndConvert.kt @@ -87,17 +87,23 @@ fun match(loop: KtForExpression, useLazySequence: Boolean): MatchResult? { MatchersLoop@ for (matcher in MatcherRegistrar.matchers) { if (state.indexVariable != null && !matcher.indexVariableAllowed) continue + if (matcher.shouldUseInputVariables && !inputVariableUsed && state.indexVariable == null) continue val match = matcher.match(state) if (match != null) { - if (!inputVariableUsed && match.allTransformations.any { it.shouldUseInputVariable }) return null - when (match) { is TransformationMatch.Sequence -> { // check that old input variable is not needed anymore var newState = match.newState if (state.inputVariable != newState.inputVariable && state.inputVariable.hasUsages(newState.statements)) return null + if (matcher.shouldUseInputVariables + && !state.inputVariable.hasDifferentSetsOfUsages(state.statements, newState.statements) + && !(state.indexVariable?.hasDifferentSetsOfUsages(state.statements, newState.statements) ?: false)) { + // matched part of the loop uses neither input variable nor index variable + continue@MatchersLoop + } + if (state.indexVariable != null && match.sequenceTransformations.any { it.affectsIndex }) { // index variable is still needed but index in the new sequence is different if (state.indexVariable!!.hasUsages(newState.statements)) return null 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 f1662a00be1..8f00c6e0649 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 @@ -40,9 +40,6 @@ class CountTransformation( override val presentation: String get() = "count" + (if (filter != null) "{}" else "()") - override val shouldUseInputVariable: Boolean - get() = false - override fun generateCode(chainedCallGenerator: ChainedCallGenerator): KtExpression { val call = if (filter != null) { val lambda = generateLambda(inputVariable, filter) @@ -72,6 +69,9 @@ class CountTransformation( override val indexVariableAllowed: Boolean get() = false + override val shouldUseInputVariables: Boolean + get() = false + override fun match(state: MatchingState): TransformationMatch.Result? { val operand = state.statements.singleOrNull()?.isPlusPlusOf() ?: return null val initialization = operand.isVariableInitializedBeforeLoop(state.outerLoop, checkNoOtherUsagesInLoop = true) ?: return null diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindTransformationMatcher.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindTransformationMatcher.kt index c4fa61e24ff..022fa854cfa 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindTransformationMatcher.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/FindTransformationMatcher.kt @@ -54,6 +54,9 @@ object FindTransformationMatcher : TransformationMatcher { override val indexVariableAllowed: Boolean get() = false + override val shouldUseInputVariables: Boolean + get() = false + override fun match(state: MatchingState): TransformationMatch.Result? { return matchWithFilterBefore(state, null) } @@ -128,9 +131,6 @@ object FindTransformationMatcher : TransformationMatcher { override val chainCallCount: Int get() = generator.chainCallCount - override val shouldUseInputVariable: Boolean - get() = false - override fun generateCode(chainedCallGenerator: ChainedCallGenerator): KtExpression { return generator.generate(chainedCallGenerator) } @@ -157,9 +157,6 @@ object FindTransformationMatcher : TransformationMatcher { override val chainCallCount: Int get() = generator.chainCallCount - override val shouldUseInputVariable: Boolean - get() = false - override fun generateCode(chainedCallGenerator: ChainedCallGenerator): KtExpression { return generator.generate(chainedCallGenerator) } diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/IntroduceIndexMatcher.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/IntroduceIndexMatcher.kt index 94819b230cc..31b6151e731 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/IntroduceIndexMatcher.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/sequence/IntroduceIndexMatcher.kt @@ -37,6 +37,9 @@ object IntroduceIndexMatcher : TransformationMatcher { override val indexVariableAllowed: Boolean get() = false // old index variable is still needed - cannot introduce another one + override val shouldUseInputVariables: Boolean + get() = false + override fun match(state: MatchingState): TransformationMatch.Sequence? { for (statement in state.statements) { val unaryExpressions = statement.collectDescendantsOfType( diff --git a/idea/testData/intentions/loopToCallChain/filter_inputVariableNotUsed.kt b/idea/testData/intentions/loopToCallChain/filter_inputVariableNotUsed.kt new file mode 100644 index 00000000000..6d5ac5809b1 --- /dev/null +++ b/idea/testData/intentions/loopToCallChain/filter_inputVariableNotUsed.kt @@ -0,0 +1,12 @@ +// WITH_RUNTIME +// IS_APPLICABLE: false +// IS_APPLICABLE_2: false +import java.util.* + +fun foo(list: List) { + val random = Random() + for (s in list) { + if (random.nextBoolean()) continue + print(s) + } +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTest2Generated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTest2Generated.java index 9efb7d685c7..f461cb1ff0a 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTest2Generated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTest2Generated.java @@ -473,6 +473,12 @@ public class IntentionTest2Generated extends AbstractIntentionTest2 { doTest(fileName); } + @TestMetadata("filter_inputVariableNotUsed.kt") + public void testFilter_inputVariableNotUsed() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/loopToCallChain/filter_inputVariableNotUsed.kt"); + doTest(fileName); + } + @TestMetadata("filter_mergeMultiple.kt") public void testFilter_mergeMultiple() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/loopToCallChain/filter_mergeMultiple.kt"); diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 20177de8492..434ee7cb7e8 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -7591,6 +7591,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("filter_inputVariableNotUsed.kt") + public void testFilter_inputVariableNotUsed() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/loopToCallChain/filter_inputVariableNotUsed.kt"); + doTest(fileName); + } + @TestMetadata("filter_mergeMultiple.kt") public void testFilter_mergeMultiple() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/loopToCallChain/filter_mergeMultiple.kt");