Checking that target collection is always the same

idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/commonUtils.kt

@@ -67,7 +67,7 @@ fun KtCallableDeclaration.hasUsages(inElements: Collection<KtElement>): Boolean
 //    return ReferencesSearch.search(this, LocalSearchScope(inElements.toTypedArray())).any()
 }
 
-fun KtProperty.hasWriteUsages(): Boolean {
+fun KtVariableDeclaration.hasWriteUsages(): Boolean {
     assert(this.isPhysical)
     if (!isVar) return false
     return ReferencesSearch.search(this, useScope).any {
@@ -75,23 +75,23 @@ fun KtProperty.hasWriteUsages(): Boolean {
     }
 }
 
-fun KtProperty.countUsages(inElement: KtElement): Int {
+fun KtCallableDeclaration.countUsages(inElement: KtElement): Int {
     assert(this.isPhysical)
     return ReferencesSearch.search(this, LocalSearchScope(inElement)).count()
 }
 
-fun KtProperty.countUsages(inElements: Collection<KtElement>): Int {
+fun KtCallableDeclaration.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 {
+fun KtCallableDeclaration.countUsages(): Int {
     assert(this.isPhysical)
     return ReferencesSearch.search(this, useScope).count()
 }
 
-fun KtProperty.countWriteUsages(inElement: KtElement): Int {
+fun KtVariableDeclaration.countWriteUsages(inElement: KtElement): Int {
     assert(this.isPhysical)
     if (!isVar) return 0
     return ReferencesSearch.search(this, LocalSearchScope(inElement)).count {
@@ -99,6 +99,14 @@ fun KtProperty.countWriteUsages(inElement: KtElement): Int {
     }
 }
 
+fun KtVariableDeclaration.hasWriteUsages(inElement: KtElement): Boolean {
+    assert(this.isPhysical)
+    if (!isVar) return false
+    return ReferencesSearch.search(this, LocalSearchScope(inElement)).any {
+        (it as? KtSimpleNameReference)?.element?.readWriteAccess(useResolveForReadWrite = true)?.isWrite == true
+    }
+}
+
 fun KtExpressionWithLabel.targetLoop(): KtLoopExpression? {
     val label = getTargetLabel()
     if (label == null) {
@@ -148,4 +156,3 @@ fun PsiChildRange.withoutLastStatement(): PsiChildRange {
     val newLast = last!!.siblings(forward = false, withItself = false).first { it !is PsiWhiteSpace }
     return PsiChildRange(first, newLast)
 }
-

idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/result/AddToCollectionTransformation.kt

@@ -93,13 +93,15 @@ class AddToCollectionTransformation(
             //TODO: it can be implicit 'this' too
             val qualifiedExpression = statement as? KtDotQualifiedExpression ?: return null
             val targetCollection = qualifiedExpression.receiverExpression
-            //TODO: check that receiver is stable!
             val callExpression = qualifiedExpression.selectorExpression as? KtCallExpression ?: return null
             if (callExpression.getCallNameExpression()?.getReferencedName() != "add") return null
             //TODO: check that it's MutableCollection's add
             val argument = callExpression.valueArguments.singleOrNull() ?: return null
             val argumentValue = argument.getArgumentExpression() ?: return null
 
+            //TODO: should work when ".asSequence()" used when there are other read-usages in the loop
+            if (!targetCollection.isStableInLoop(state.outerLoop, checkNoOtherUsagesInLoop = true)) return null
+
             if (state.indexVariable == null) {
                 matchWithCollectionInitializationReplacement(state, targetCollection, argumentValue)
                         ?.let { return it }

idea/src/org/jetbrains/kotlin/idea/intentions/loopToCallChain/utils.kt

@@ -19,10 +19,12 @@ package org.jetbrains.kotlin.idea.intentions.loopToCallChain
 import com.intellij.openapi.util.Key
 import com.intellij.openapi.util.UserDataHolderBase
 import org.jetbrains.kotlin.descriptors.ConstructorDescriptor
+import org.jetbrains.kotlin.descriptors.VariableDescriptor
 import org.jetbrains.kotlin.diagnostics.Severity
 import org.jetbrains.kotlin.idea.analysis.analyzeInContext
 import org.jetbrains.kotlin.idea.caches.resolve.analyze
 import org.jetbrains.kotlin.idea.caches.resolve.getResolutionFacade
+import org.jetbrains.kotlin.idea.caches.resolve.resolveToDescriptorIfAny
 import org.jetbrains.kotlin.idea.core.copied
 import org.jetbrains.kotlin.idea.core.replaced
 import org.jetbrains.kotlin.idea.imports.importableFqName
@@ -257,3 +259,28 @@ fun canSwapExecutionOrder(expressionBefore: KtExpression, expressionAfter: KtExp
     //TODO: more cases
     return false
 }
+
+fun KtExpression.isStableInLoop(loop: KtLoopExpression, checkNoOtherUsagesInLoop: Boolean): Boolean {
+    when {
+        isConstant() -> return true
+
+        this is KtSimpleNameExpression -> {
+            val declaration = mainReference.resolve() as? KtCallableDeclaration ?: return false
+            if (loop.isAncestor(declaration)) return false // should be declared outside the loop
+            val variable = declaration.resolveToDescriptorIfAny() as? VariableDescriptor ?: return false
+
+            if (checkNoOtherUsagesInLoop && declaration.countUsages(loop) > 1) return false
+
+            if (!variable.isVar) return true
+            if (declaration !is KtVariableDeclaration) return false
+            if (!KtPsiUtil.isLocal(declaration)) return false // it's difficult to analyze non-local declarations
+            //TODO: check that there are no local functions or lambdas that can modify it implicitly
+            return !declaration.hasWriteUsages(loop)
+        }
+
+        //TODO: qualified expression?
+        //TODO: this
+
+        else -> return false
+    }
+}

idea/testData/intentions/loopToCallChain/addToCollection_badReceiver1.kt

@@ -0,0 +1,7 @@
+// WITH_RUNTIME
+// IS_APPLICABLE: false
+fun foo(list: List<MutableCollection<Int>>) {
+    <caret>for (collection in list) {
+        collection.add(collection.size)
+    }
+}

idea/testData/intentions/loopToCallChain/addToCollection_badReceiver2.kt

@@ -0,0 +1,11 @@
+// WITH_RUNTIME
+// IS_APPLICABLE: false
+import java.util.ArrayList
+
+var globalCollection = ArrayList<Int>()
+
+fun foo(list: List<Collection<Int>>) {
+    <caret>for (collection in list) {
+        globalCollection.add(collection.size)
+    }
+}

idea/testData/intentions/loopToCallChain/addToCollection_goodReceiver.kt

@@ -0,0 +1,16 @@
+// WITH_RUNTIME
+// INTENTION_TEXT: "Replace with 'mapTo(){}'"
+import java.util.ArrayList
+
+fun foo(list: List<MutableCollection<Int>>) {
+    var target: MutableCollection<Int>()
+    target = ArrayList<Int>()
+
+    <caret>for (collection in list) {
+        target.add(collection.size)
+    }
+
+    if (target.size > 100) {
+        target = ArrayList<Int>()
+    }
+}

idea/testData/intentions/loopToCallChain/addToCollection_goodReceiver.kt.after

@@ -0,0 +1,12 @@
+// WITH_RUNTIME
+// INTENTION_TEXT: "Replace with 'mapTo(){}'"
+import java.util.ArrayList
+
+fun foo(list: List<MutableCollection<Int>>) {
+    var target: MutableCollection<Int>()
+    <caret>target = list.mapTo(ArrayList<Int>()) { it.size }
+
+    if (target.size > 100) {
+        target = ArrayList<Int>()
+    }
+}

idea/testData/intentions/loopToCallChain/resultCollectionUsedInsideLoop.kt

@@ -1,5 +1,5 @@
 // WITH_RUNTIME
-// INTENTION_TEXT: "Replace with 'filter{}.mapTo(){}'"
+// IS_APPLICABLE: false
 import java.util.ArrayList
 
 fun foo(list: List<String>): List<Int> {

idea/testData/intentions/loopToCallChain/resultCollectionUsedInsideLoop.kt.after

@@ -1,11 +0,0 @@
-// WITH_RUNTIME
-// INTENTION_TEXT: "Replace with 'filter{}.mapTo(){}'"
-import java.util.ArrayList
-
-fun foo(list: List<String>): List<Int> {
-    val result = ArrayList<Int>()
-    <caret>list
-            .filter { it.length > result.size }
-            .mapTo(result) { it.hashCode() }
-    return result
-}