From f81192d48fdbf29c0b0bfe88102d88832d8bbda4 Mon Sep 17 00:00:00 2001
From: Mikhail Glukhikh <Mikhail.Glukhikh@jetbrains.com>
Date: Wed, 18 May 2016 19:45:28 +0300
Subject: [PATCH] Simplify for: take into account two or more possible local
 variables for the same loop parameter property

---
 .../idea/intentions/SimplifyForIntention.kt     | 17 +++++++++--------
 .../DataClassTwoDifferentLocals.kt              | 12 ++++++++++++
 .../idea/intentions/IntentionTestGenerated.java |  6 ++++++
 3 files changed, 27 insertions(+), 8 deletions(-)
 create mode 100644 idea/testData/intentions/iterationOverMap/DataClassTwoDifferentLocals.kt

diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/SimplifyForIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/SimplifyForIntention.kt
index dbbc803f382..3a5db29dfe5 100644
--- a/idea/src/org/jetbrains/kotlin/idea/intentions/SimplifyForIntention.kt
+++ b/idea/src/org/jetbrains/kotlin/idea/intentions/SimplifyForIntention.kt
@@ -50,7 +50,7 @@ class SimplifyForIntention : SelfTargetingRangeIntention<KtForExpression>(
         val factory = KtPsiFactory(element)
         loopParameter.replace(factory.createDestructuringDeclarationInFor("(${usagesToRemove.joinToString { it.name!! }})"))
         usagesToRemove.forEach { p ->
-            p.property?.delete()
+            p.properties.firstOrNull()?.delete()
             p.usersToReplace.forEach {
                 it.replace(factory.createExpression(p.name!!))
             }
@@ -97,7 +97,7 @@ class SimplifyForIntention : SelfTargetingRangeIntention<KtForExpression>(
                     { otherUsages = true }
             )
 
-            if (!otherUsages && usagesToRemove.all { it != null && it.name != null}) {
+            if (!otherUsages && usagesToRemove.all { it != null && it.name != null && it.properties.size <= 1 }) {
                 return usagesToRemove.mapNotNull { it } to removeSelectorInLoopRange
             }
         }
@@ -116,7 +116,7 @@ class SimplifyForIntention : SelfTargetingRangeIntention<KtForExpression>(
 
             val notNullUsages = usagesToRemove.filterNotNull().filter { it.name != null }
             val droppedLastUnused = usagesToRemove.dropLastWhile { it == null }
-            if (droppedLastUnused.size == notNullUsages.size) {
+            if (droppedLastUnused.size == notNullUsages.size && droppedLastUnused.all { it != null && it.properties.size <= 1 } ) {
                 return notNullUsages to false
             }
         }
@@ -192,13 +192,13 @@ class SimplifyForIntention : SelfTargetingRangeIntention<KtForExpression>(
         return UsageData(property, parentCall, descriptor)
     }
 
-    private data class UsageData(val property: KtProperty?,
+    private data class UsageData(val properties: List<KtProperty>,
                                  val usersToReplace: List<KtExpression>,
                                  val descriptor: CallableDescriptor,
-                                 val name: String? = property?.name
+                                 val name: String? = properties.firstOrNull()?.name
     ) {
         constructor(property: KtProperty?, user: KtExpression, descriptor: CallableDescriptor):
-                this(property, if (property != null) emptyList() else listOf(user), descriptor)
+                this(listOfNotNull(property), if (property != null) emptyList() else listOf(user), descriptor)
 
         fun named(suggested: String) = if (name != null) this else copy(name = suggested)
     }
@@ -206,7 +206,8 @@ class SimplifyForIntention : SelfTargetingRangeIntention<KtForExpression>(
     private operator fun UsageData?.plus(newData: UsageData): UsageData {
         if (this == null) return newData
         val allUsersToReplace = usersToReplace + newData.usersToReplace
-        if (property != null) return copy(usersToReplace = allUsersToReplace)
-        else return newData.copy(usersToReplace = allUsersToReplace)
+        val allProperties = properties + newData.properties
+        if (properties.isNotEmpty()) return copy(properties = allProperties, usersToReplace = allUsersToReplace)
+        else return newData.copy(properties = allProperties, usersToReplace = allUsersToReplace)
     }
 }
\ No newline at end of file
diff --git a/idea/testData/intentions/iterationOverMap/DataClassTwoDifferentLocals.kt b/idea/testData/intentions/iterationOverMap/DataClassTwoDifferentLocals.kt
new file mode 100644
index 00000000000..a6067a9ed52
--- /dev/null
+++ b/idea/testData/intentions/iterationOverMap/DataClassTwoDifferentLocals.kt
@@ -0,0 +1,12 @@
+// IS_APPLICABLE: false
+// WITH_RUNTIME
+
+data class XY(val x: String, val y: Int)
+fun test(xys: Array<XY>) {
+    for (<caret>xy in xys) {
+        val x = xy.x
+        println(x)
+        val xx = xy.x
+        println(xx)
+    }
+}
\ No newline at end of file
diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java
index 65919a11d6c..fd38496e029 100644
--- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java
+++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java
@@ -6579,6 +6579,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest {
             doTest(fileName);
         }
 
+        @TestMetadata("DataClassTwoDifferentLocals.kt")
+        public void testDataClassTwoDifferentLocals() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/DataClassTwoDifferentLocals.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("DataClassUnused.kt")
         public void testDataClassUnused() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/iterationOverMap/DataClassUnused.kt");