Extract Function: Take duplicates into account when choosing placement of extracted function

#KT-5916 Fixed

idea/src/org/jetbrains/jet/plugin/refactoring/extractFunction/extractorUtil.kt

@@ -58,6 +58,8 @@ import org.jetbrains.jet.plugin.util.psi.patternMatching.UnificationResult.Weakl
 import org.jetbrains.jet.plugin.util.psi.patternMatching.UnificationResult.StronglyMatched
 import org.jetbrains.jet.lang.psi.JetDeclarationWithBody
 import org.jetbrains.jet.plugin.util.IdeDescriptorRenderers
+import org.jetbrains.jet.lang.psi.psiUtil.parents
+import java.util.ArrayList
 
 fun ExtractableCodeDescriptor.getDeclarationText(
         options: ExtractionGeneratorOptions = ExtractionGeneratorOptions.DEFAULT,
@@ -481,19 +483,19 @@ fun ExtractableCodeDescriptor.generateDeclaration(options: ExtractionGeneratorOp
         }
     }
 
-    fun insertDeclaration(declaration: JetNamedDeclaration): JetNamedDeclaration {
+    fun insertDeclaration(declaration: JetNamedDeclaration, anchor: PsiElement): JetNamedDeclaration {
         return with(extractionData) {
-            val targetContainer = targetSibling.getParent()!!
+            val targetContainer = anchor.getParent()!!
             val emptyLines = psiFactory.createWhiteSpace("\n\n")
             if (insertBefore) {
-                val declarationInFile = targetContainer.addBefore(declaration, targetSibling) as JetNamedDeclaration
+                val declarationInFile = targetContainer.addBefore(declaration, anchor) as JetNamedDeclaration
-                targetContainer.addBefore(emptyLines, targetSibling)
+                targetContainer.addBefore(emptyLines, anchor)
 
                 declarationInFile
             }
             else {
-                val declarationInFile = targetContainer.addAfter(declaration, targetSibling) as JetNamedDeclaration
+                val declarationInFile = targetContainer.addAfter(declaration, anchor) as JetNamedDeclaration
-                targetContainer.addAfter(emptyLines, targetSibling)
+                targetContainer.addAfter(emptyLines, anchor)
 
                 declarationInFile
             }
@@ -502,7 +504,22 @@ fun ExtractableCodeDescriptor.generateDeclaration(options: ExtractionGeneratorOp
 
     val duplicates = if (options.inTempFile) Collections.emptyList() else findDuplicates()
 
-    val declaration = createDeclaration().let { if (options.inTempFile) it else insertDeclaration(it) }
+    val anchor = with(extractionData) {
+        val anchorCandidates = duplicates.mapTo(ArrayList<PsiElement>()) { it.range.elements.first() }
+        anchorCandidates.add(targetSibling)
+
+        val marginalCandidate = if (insertBefore) {
+            anchorCandidates.minBy { it.getTextRange().getStartOffset() }!!
+        }
+        else {
+            anchorCandidates.maxBy { it.getTextRange().getStartOffset() }!!
+        }
+
+        val targetParent = targetSibling.getParent()
+        marginalCandidate.parents().first { it.getParent() == targetParent }
+    }
+
+    val declaration = createDeclaration().let { if (options.inTempFile) it else insertDeclaration(it, anchor) }
     adjustDeclarationBody(declaration)
 
     if (options.inTempFile) return ExtractionResult(declaration, Collections.emptyMap(), nameByOffset)

idea/testData/refactoring/extractFunction/duplicates/branchingMatch1.kt.after

@@ -11,17 +11,6 @@ fun test(a: Int): Int {
     return t
 }
 
-private fun i(a: Int, b: Int): Int {
-    var b1 = b
-    return if (a > 0) {
-        b1++
-        b1 + a
-    } else {
-        b1--
-        b1 - a
-    }
-}
-
 fun foo1() {
     val x = 1
     var y: Int = x
@@ -68,3 +57,14 @@ fun foo5(x: Int): Int {
     var p: Int = 1
     i(x, p)
 }
+
+private fun i(a: Int, b: Int): Int {
+    var b1 = b
+    return if (a > 0) {
+        b1++
+        b1 + a
+    } else {
+        b1--
+        b1 - a
+    }
+}

idea/testData/refactoring/extractFunction/duplicates/branchingMatch2.kt.after

@@ -11,16 +11,6 @@ fun test(a: Int): Int {
     return b
 }
 
-private fun i(a: Int, b: Int): Int {
-    var b1 = b
-    if (a > 0) {
-        b1 = b1 + a
-    } else {
-        b1 = b1 - a
-    }
-    return b1
-}
-
 fun foo1() {
     val x = 1
     var y: Int = x
@@ -62,3 +52,13 @@ fun foo5(x: Int): Int {
     var p: Int = 1
     i(x, p)
 }
+
+private fun i(a: Int, b: Int): Int {
+    var b1 = b
+    if (a > 0) {
+        b1 = b1 + a
+    } else {
+        b1 = b1 - a
+    }
+    return b1
+}

idea/testData/refactoring/extractFunction/duplicates/defaultCF.kt.after

@@ -8,15 +8,15 @@ fun foo(a: Int, b: Int) {
     unit(a, b)
 }
 
-private fun unit(a: Int, b: Int) {
-    println("a = $a")
-    println("b = $b")
-    println(a + b * a)
-}
-
 fun bar() {
     val x = 1
     val y = 2
 
     unit(x, y)
+}
+
+private fun unit(a: Int, b: Int) {
+    println("a = $a")
+    println("b = $b")
+    println(a + b * a)
 }

idea/testData/refactoring/extractFunction/duplicates/insertAfterDuplicates.kt

@@ -0,0 +1,4 @@
+class A {
+    val x = <selection>1 + 1</selection>
+    val y = 1 + 1
+}

idea/testData/refactoring/extractFunction/duplicates/insertAfterDuplicates.kt.after

@@ -0,0 +1,8 @@
+class A {
+    val x = i()
+    val y = i()
+
+    private fun i(): Int {
+        return 1 + 1
+    }
+}

idea/testData/refactoring/extractFunction/duplicates/insertBeforeDuplicates.kt

@@ -0,0 +1,4 @@
+fun foo() {
+    val x = 1 + 1
+    val y = <selection>1 + 1</selection>
+}

idea/testData/refactoring/extractFunction/duplicates/insertBeforeDuplicates.kt.after

@@ -0,0 +1,8 @@
+fun foo() {
+    fun i(): Int {
+        return 1 + 1
+    }
+
+    val x = i()
+    val y = i()
+}

idea/testData/refactoring/extractFunction/duplicates/multipleOutputValuesMatching.kt.after

@@ -8,12 +8,6 @@ fun test(a: Int): Int {
     return b*c
 }
 
-private fun pair(a: Int): Pair<Int, Int> {
-    val b = a + 1
-    val c = a - 1
-    return Pair(b, c)
-}
-
 fun foo1(a: Int) {
     var (x, y) = pair(a)
     println(x + y)
@@ -28,8 +22,14 @@ fun foo2(): Int {
     return p + q
 }
 
+private fun pair(a: Int): Pair<Int, Int> {
+    val b = a + 1
+    val c = a - 1
+    return Pair(b, c)
+}
+
 fun foo4(a: Int) {
     var b = a
     b = b + 1
     return b - 1
-}
+}

idea/testData/refactoring/extractFunction/duplicates/outputValueAndUnitMatching.kt.after

@@ -10,11 +10,6 @@ fun test(): () -> Int {
     }
 }
 
-private fun i(z: Int): Int {
-    println(z)
-    return z + 1
-}
-
 fun foo1(a: Int): Int {
     val t = println(a)
     return a + 1
@@ -22,4 +17,9 @@ fun foo1(a: Int): Int {
 
 fun foo2(a: Int) {
     i(a)
+}
+
+private fun i(z: Int): Int {
+    println(z)
+    return z + 1
 }

idea/testData/refactoring/extractFunction/duplicates/outputValueMatching.kt.after

@@ -7,11 +7,6 @@ fun test(a: Int): Int {
     return i(a)
 }
 
-private fun i(a: Int): Int {
-    println(a)
-    return a + 1
-}
-
 fun foo1(): Int {
     val x = 1
     val y = i(x)
@@ -38,4 +33,9 @@ fun foo4(a: Int): Int {
 
 fun foo5(a: Int) {
     i(a)
+}
+
+private fun i(a: Int): Int {
+    println(a)
+    return a + 1
 }

idea/testData/refactoring/extractFunction/duplicates/singleExpression.kt.after

@@ -8,10 +8,6 @@ fun foo(a: Int, b: Int): Int {
     return i(a, b) + 1
 }
 
-private fun i(a: Int, b: Int): Int {
-    return a + b * a
-}
-
 fun bar() {
     fun f() = 1
 
@@ -26,4 +22,8 @@ fun bar() {
     a + c
     i(f(), a)
     i(f(), f())
+}
+
+private fun i(a: Int, b: Int): Int {
+    return a + b * a
 }

idea/tests/org/jetbrains/jet/plugin/refactoring/introduce/introduceVariable/JetExtractionTestGenerated.java

@@ -1080,7 +1080,19 @@ public class JetExtractionTestGenerated extends AbstractJetExtractionTest {
                 String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/extractFunction/duplicates/defaultCF.kt");
                 doExtractFunctionTest(fileName);
             }
-
+            
+            @TestMetadata("insertAfterDuplicates.kt")
+            public void testInsertAfterDuplicates() throws Exception {
+                String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/extractFunction/duplicates/insertAfterDuplicates.kt");
+                doExtractFunctionTest(fileName);
+            }
+            
+            @TestMetadata("insertBeforeDuplicates.kt")
+            public void testInsertBeforeDuplicates() throws Exception {
+                String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/extractFunction/duplicates/insertBeforeDuplicates.kt");
+                doExtractFunctionTest(fileName);
+            }
+            
             @TestMetadata("multipleOutputValuesMatching.kt")
             public void testMultipleOutputValuesMatching() throws Exception {
                 String fileName = JetTestUtils.navigationMetadata("idea/testData/refactoring/extractFunction/duplicates/multipleOutputValuesMatching.kt");