Fixing bugs of extract method refactoring

org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/RefactoringCoreMessages.java

@@ -515,6 +515,8 @@ public final class RefactoringCoreMessages extends NLS {
 
 	public static String ExtractInterfaceRefactoring_name;
 
+	public static String ExtractMethodAnalyzer_multiple_exits;
+
 	public static String ExtractMethodAnalyzer_after_do_keyword;
 
 	public static String ExtractMethodAnalyzer_ambiguous_return_value;

org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/refactoring.properties

@@ -112,6 +112,7 @@ LocalTypeAnalyzer_local_type_referenced_outside=A local type declared in the sel
 
 FlowAnalyzer_execution_flow=Selected statements contain a return statement but not all possible execution flows end in a return. Semantics may not be preserved if you proceed.
 
+ExtractMethodAnalyzer_multiple_exits=Cannot extract method with multiple exit points.
 ExtractMethodAnalyzer_assignments_to_local=Ambiguous return value: Selected block modifies more than one local variable used in subsequent code. Affected variables are:\n\n{0}
 ExtractMethodAnalyzer_invalid_selection=Cannot extract new method since the selection is not valid.
 ExtractMethodAnalyzer_after_do_keyword=Selection may not start immediately after the \'do\' keyword.

org.eclipse.jdt.core.manipulation/refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodAnalyzer.java

@@ -61,13 +61,15 @@
 import org.eclipse.jdt.core.dom.NodeFinder;
 import org.eclipse.jdt.core.dom.PrimitiveType;
 import org.eclipse.jdt.core.dom.QualifiedName;
+import org.eclipse.jdt.core.dom.ReturnStatement;
 import org.eclipse.jdt.core.dom.SimpleName;
 import org.eclipse.jdt.core.dom.Statement;
 import org.eclipse.jdt.core.dom.StructuralPropertyDescriptor;
 import org.eclipse.jdt.core.dom.SuperConstructorInvocation;
 import org.eclipse.jdt.core.dom.SwitchCase;
 import org.eclipse.jdt.core.dom.SwitchStatement;
 import org.eclipse.jdt.core.dom.ThisExpression;
+import org.eclipse.jdt.core.dom.ThrowStatement;
 import org.eclipse.jdt.core.dom.TryStatement;
 import org.eclipse.jdt.core.dom.Type;
 import org.eclipse.jdt.core.dom.TypeDeclaration;
@@ -206,6 +208,57 @@ public ITypeBinding[] getTypeVariables() {
 		return fTypeVariables;
 	}
 
+	//==new code to prevent extraction where multiple exits exist as per https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/697
+    private boolean hasMultipleExits;
+
+   private class ControlFlowVisitor extends ASTVisitor {
+        private boolean withinBlock;
+
+        @Override
+        public boolean visit(Block node) {
+            withinBlock = true;
+            return super.visit(node);
+        }
+
+        @Override
+        public void endVisit(Block node) {
+            withinBlock = false;
+            super.endVisit(node);
+        }
+
+        @Override
+        public boolean visit(ReturnStatement node) {
+            if (withinBlock) {
+                hasMultipleExits = true;
+            }
+            return super.visit(node);
+        }
+
+        @Override
+        public boolean visit(ThrowStatement node) {
+            if (withinBlock) {
+                hasMultipleExits = true;
+            }
+            return super.visit(node);
+        }
+
+        @Override
+        public boolean visit(BreakStatement node) {
+            if (withinBlock) {
+                hasMultipleExits = true;
+            }
+            return super.visit(node);
+        }
+
+        @Override
+        public boolean visit(ContinueStatement node) {
+            if (withinBlock) {
+                hasMultipleExits = true;
+            }
+            return super.visit(node);
+        }
+    }//===end for https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/697
+
 	//---- Activation checking ---------------------------------------------------------------------------
 
 	public boolean isValidDestination(ASTNode node) {
@@ -248,6 +301,25 @@ public RefactoringStatus checkInitialConditions(ImportRewrite rewriter) {
 			returns++;
 		}
 
+		//==new code to prevent extraction where multiple exits exist as per https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/697
+
+		ASTNode[] selectedNodes = getSelectedNodes();
+
+		// Analyze control flow for each selected node
+		hasMultipleExits = false;
+		ControlFlowVisitor controlFlowVisitor = new ControlFlowVisitor();
+
+		for (ASTNode selectedNode : selectedNodes) {
+		    selectedNode.accept(controlFlowVisitor);
+		}
+
+		if (hasMultipleExits) {
+		    result.addFatalError(RefactoringCoreMessages.ExtractMethodAnalyzer_multiple_exits);
+		    return result;
+		}
+		//===end for https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/697
+
+
 		if (returns > 1) {
 			result.addFatalError(RefactoringCoreMessages.ExtractMethodAnalyzer_ambiguous_return_value, JavaStatusContext.create(fCUnit, getSelection()));
 			fReturnKind= MULTIPLE;

org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/invalidSelection/A_test3000.java

@@ -0,0 +1,23 @@
+package invalidSelection;
+
+public class A_test3000 {
+	public int m(boolean b) {		
+	     int x = 42;
+	      try {
+	    	// from
+	    	  /*]*/if(b) {
+	            x = 23;
+	            throw new Exception();
+	          } /*[*/
+	          // to
+
+	      } catch(Exception e) {
+	    	  return x;
+	      }	      
+	      return x;
+	    }
+	 	public int test(){
+	 		return m(true);
+	 	}
+}
+

org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/invalidSelection/A_test3001.java

@@ -0,0 +1,33 @@
+package invalidSelection;
+
+public class A_test3001 {
+	public int calculateSum(int[] numbers) {
+        int sum = 0;
+        try {
+        	 /*]*/for (int i = 0; i < numbers.length; i++) {
+                if (numbers[i] < 0) {
+
+                    throw new Exception();
+                }
+                sum += numbers[i];
+            }/*[*/
+        } catch (Exception e) {
+            return sum;
+        }
+        return sum;
+    }
+
+
+    public int test() {
+        int[] numbers = {1, 2, -3, 4, 5};
+        int result = calculateSum(numbers);
+        System.out.println("Sum: " + result);
+        return result;
+    }
+
+    public static void main(String[] args) {
+    	A_test3001 b = new A_test3001();
+        b.test();
+    }
+}
+

org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/invalidSelection/A_test3002.java

@@ -0,0 +1,33 @@
+package invalidSelection;
+
+public class A_test3002 {
+	public int calculateSum(int[] numbers) {
+        int sum = 0;
+        try {
+            for (int i = 0; i < numbers.length; i++) {
+            	/*]*/if (numbers[i] < 0) {
+                    //sum++
+                    throw new Exception();
+                }/*[*/
+                sum += numbers[i];
+            }
+        } catch (Exception e) {
+            return sum;
+        }
+        return sum;
+    }
+
+
+    public int test() {
+        int[] numbers = {1, 2, -3, 4, 5};
+        int result = calculateSum(numbers);
+        System.out.println("Sum: " + result);
+        return result;
+    }
+
+    public static void main(String[] args) {
+    	A_test3002 b = new A_test3002();
+        b.test();
+    }
+}
+

org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractMethodTests.java

@@ -239,6 +239,8 @@ public void test4() throws Exception {
 		selectionTest(5, 14, 5, 19);
 	}
 
+
+
 	//=====================================================================================
 	// Testing invalid selections
 	//=====================================================================================
@@ -799,6 +801,23 @@ public void test199() throws Exception {
 	}
 
 
+	@Test
+	public void test3000() throws Exception {
+		invalidSelectionTest();//==https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/697
+	}
+
+	@Test
+	public void test3001() throws Exception {
+		invalidSelectionTest();//==https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/697
+	}
+
+	@Test
+	public void test3002() throws Exception {
+		invalidSelectionTest();//==https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/697
+	}
+
+
+
 	//====================================================================================
 	// Testing valid selections
 	//=====================================================================================
@@ -1096,6 +1115,7 @@ public void test353() throws Exception {
 		validSelectionTest();
 	}
 
+
 	@Test
 	public void test360() throws Exception {
 		validSelectionTestChecked();
@@ -2681,4 +2701,5 @@ public void test2005() throws Exception {
 	public void testIssue671() throws Exception {
 		validSelectionTestChecked();
 	}
+
 }