From 091e87555179fb1577d5c811ed7b8277b37c36e8 Mon Sep 17 00:00:00 2001
From: Mikhail Glukhikh <Mikhail.Glukhikh@jetbrains.com>
Date: Thu, 30 Mar 2017 15:13:23 +0300
Subject: [PATCH] Inline function: process usages in children-first order

---
 .../codeInliner/UsageReplacementStrategy.kt   | 27 +++++++++++++++++--
 .../MultipleInExpression.kt.after             |  6 ++---
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/idea/src/org/jetbrains/kotlin/idea/codeInliner/UsageReplacementStrategy.kt b/idea/src/org/jetbrains/kotlin/idea/codeInliner/UsageReplacementStrategy.kt
index 8f8a10fc7ef..2af6d76a365 100644
--- a/idea/src/org/jetbrains/kotlin/idea/codeInliner/UsageReplacementStrategy.kt
+++ b/idea/src/org/jetbrains/kotlin/idea/codeInliner/UsageReplacementStrategy.kt
@@ -81,6 +81,29 @@ private fun UsageReplacementStrategy.doRefactoringInside(
     }
 }
 
+// Sort them in the following order: children first, parents second
+// NB: this is topological sort implementation
+private fun Collection<KtSimpleNameExpression>.sortChildrenFirst(): List<KtSimpleNameExpression> {
+    val usagesSet = toSet()
+    val result = mutableListOf<KtSimpleNameExpression>()
+    val visited = hashSetOf<KtSimpleNameExpression>()
+
+    fun visit(expr: KtSimpleNameExpression) {
+        if (expr in visited) return
+        visited += expr
+
+        expr.parent.forEachDescendantOfType<KtSimpleNameExpression> {
+            if (it != expr && it in usagesSet) visit(it)
+        }
+
+        result += expr
+    }
+
+    this.forEach(::visit)
+    assert(result.size == this.size)
+    return result
+}
+
 fun UsageReplacementStrategy.replaceUsages(
         usages: Collection<KtSimpleNameExpression>,
         targetPsiElement: PsiElement,
@@ -98,8 +121,8 @@ fun UsageReplacementStrategy.replaceUsages(
 
             val targetDeclaration = targetPsiElement as? KtNamedDeclaration
 
-            // NB: reversed order is better in case of composition like sqr(sqr(x))
-            for (usage in usages.reversed()) {
+            val usagesChildrenFirst = usages.sortChildrenFirst()
+            for (usage in usagesChildrenFirst) {
                 try {
                     if (!usage.isValid) {
                         invalidUsagesFound = true
diff --git a/idea/testData/refactoring/inline/function/expressionBody/MultipleInExpression.kt.after b/idea/testData/refactoring/inline/function/expressionBody/MultipleInExpression.kt.after
index 304de55b65a..c0fdbaabc5d 100644
--- a/idea/testData/refactoring/inline/function/expressionBody/MultipleInExpression.kt.after
+++ b/idea/testData/refactoring/inline/function/expressionBody/MultipleInExpression.kt.after
@@ -1,8 +1,8 @@
 class Point(val x: Double, val y: Double) {
     // After sqr inlining, only first usage of sqr is replaced
     fun distance(other: Point): Double {
-        val x1 = y - other.y
-        val x2 = x - other.x
-        return Math.sqrt(x2 * x2 + x1 * x1)
+        val x1 = x - other.x
+        val x2 = y - other.y
+        return Math.sqrt(x1 * x1 + x2 * x2)
     }
 }
\ No newline at end of file