Bug 65875 - [extract local] puts declaration at wrong position

Signed-off-by: Nicolaj Hoess <nicohoess@gmail.com>

6 files changed, 77 insertions, 3 deletions

diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test110_in.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test110_in.java
new file mode 100644
index 0000000000..4a19e40ca9
--- /dev/null
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test110_in.java
@@ -0,0 +1,11 @@
+package p; //6, 9, 6, 25
+
+class A {
+	void m() {
+		System.out.println(calculateCount());
+		calculateCount();
+	}
+	private int calculateCount() {
+		return 1;
+	}
+}
diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test110_out.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test110_out.java
new file mode 100644
index 0000000000..be4a6ad142
--- /dev/null
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test110_out.java
@@ -0,0 +1,12 @@
+package p; //6, 9, 6, 25
+
+class A {
+	void m() {
+		int temp= calculateCount();
+		System.out.println(temp);
+		calculateCount();
+	}
+	private int calculateCount() {
+		return 1;
+	}
+}
diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test111_in.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test111_in.java
new file mode 100644
index 0000000000..8e66744c2d
--- /dev/null
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test111_in.java
@@ -0,0 +1,16 @@
+package p; //11, 9, 11, 25
+
+class A {
+	void m() {
+		System.out.println(calculateCount());
+		calculateCount();
+		System.out.println(calculateCount());
+		
+		int x= calculateCount();
+		
+		calculateCount();
+	}
+	private int calculateCount() {
+		return 1;
+	}
+}
diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test111_out.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test111_out.java
new file mode 100644
index 0000000000..44267a1b78
--- /dev/null
+++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractTemp/canExtract/A_test111_out.java
@@ -0,0 +1,17 @@
+package p; //11, 9, 11, 25
+
+class A {
+	void m() {
+		int temp= calculateCount();
+		System.out.println(temp);
+		calculateCount();
+		System.out.println(temp);
+		
+		int x= temp;
+		
+		calculateCount();
+	}
+	private int calculateCount() {
+		return 1;
+	}
+}
diff --git a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractTempTests.java b/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractTempTests.java
index d37024168c..c4dae0e057 100644
--- a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractTempTests.java
+++ b/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractTempTests.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2013 IBM Corporation and others.
+ * Copyright (c) 2000, 2014 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -8,6 +8,7 @@
  * Contributors:
  *     IBM Corporation - initial API and implementation
  *     Nikolay Metchev <nikolaymetchev@gmail.com> - [extract local] Extract to local variable not replacing multiple occurrences in same statement - https://bugs.eclipse.org/406347
+ *     Nicolaj Hoess <nicohoess@gmail.com> - [extract local] puts declaration at wrong position - https://bugs.eclipse.org/65875
  *******************************************************************************/
 package org.eclipse.jdt.ui.tests.refactoring;
 
@@ -681,6 +682,16 @@ public class ExtractTempTests extends RefactoringTest {
 		//test for https://bugs.eclipse.org/bugs/show_bug.cgi?id=335173
 		helper1(5, 20, 5, 29, true, false, "temp", "i");
 	}
+	
+	public void test110() throws Exception {
+		//test for https://bugs.eclipse.org/bugs/show_bug.cgi?id=65875
+		helper1(6, 9, 6, 25, true, false, "temp", "calculateCount");
+	}
+	
+	public void test111() throws Exception {
+		//test for https://bugs.eclipse.org/bugs/show_bug.cgi?id=65875
+		helper1(11, 9, 11, 25, true, false, "temp", "calculateCount");
+	}
 
 	public void test113() throws Exception {
 		//test for https://bugs.eclipse.org/bugs/show_bug.cgi?id=406347
diff --git a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractTempRefactoring.java b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractTempRefactoring.java
index 101e0743e1..b4ecee48e9 100644
--- a/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractTempRefactoring.java
+++ b/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractTempRefactoring.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2013 IBM Corporation and others.
+ * Copyright (c) 2000, 2014 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -8,6 +8,7 @@
  * Contributors:
  *     IBM Corporation - initial API and implementation
  *     Nikolay Metchev <nikolaymetchev@gmail.com> - [extract local] Extract to local variable not replacing multiple occurrences in same statement - https://bugs.eclipse.org/406347
+ *     Nicolaj Hoess <nicohoess@gmail.com> - [extract local] puts declaration at wrong position - https://bugs.eclipse.org/65875
  *******************************************************************************/
 package org.eclipse.jdt.internal.corext.refactoring.code;
 
@@ -666,7 +667,10 @@ public class ExtractTempRefactoring extends Refactoring {
 		Expression initializer= getSelectedExpression().createCopyTarget(fCURewrite.getASTRewrite(), true);
 		VariableDeclarationStatement vds= createTempDeclaration(initializer);
 
-		if ((!fReplaceAllOccurrences) || (retainOnlyReplacableMatches(getMatchingFragments()).length <= 1)) {
+		IASTFragment[] replacableMatches= retainOnlyReplacableMatches(getMatchingFragments());
+		if (!fReplaceAllOccurrences
+				|| replacableMatches.length == 0
+				|| replacableMatches.length == 1 && replacableMatches[0].equals(getSelectedExpression().getAssociatedExpression())) {
 			insertAt(getSelectedExpression().getAssociatedNode(), vds);
 			return;
 		}
@@ -999,6 +1003,9 @@ public class ExtractTempRefactoring extends Refactoring {
 
 	private boolean shouldReplaceSelectedExpressionWithTempDeclaration() throws JavaModelException {
 		IExpressionFragment selectedFragment= getSelectedExpression();
+		IExpressionFragment firstExpression= getFirstReplacedExpression();
+		if (firstExpression.getStartPosition() < selectedFragment.getStartPosition())
+			return false;
 		return selectedFragment.getAssociatedNode().getParent() instanceof ExpressionStatement
 			&& selectedFragment.matches(ASTFragmentFactory.createFragmentForFullSubtree(selectedFragment.getAssociatedNode()));
 	}