diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/RefactoringCoreMessages.java b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/RefactoringCoreMessages.java index b14d0660c5c..c634dbc937b 100644 --- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/RefactoringCoreMessages.java +++ b/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; diff --git a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/refactoring.properties b/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/refactoring.properties index 97ca22a8f16..8500b2a2602 100644 --- a/org.eclipse.jdt.core.manipulation/core extension/org/eclipse/jdt/internal/corext/refactoring/refactoring.properties +++ b/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. diff --git a/org.eclipse.jdt.core.manipulation/refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodAnalyzer.java b/org.eclipse.jdt.core.manipulation/refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodAnalyzer.java index f31cebe9f70..f94299a8ee9 100644 --- a/org.eclipse.jdt.core.manipulation/refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodAnalyzer.java +++ b/org.eclipse.jdt.core.manipulation/refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodAnalyzer.java @@ -61,6 +61,7 @@ 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; @@ -68,6 +69,7 @@ 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; diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/invalidSelection/A_test3000.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/invalidSelection/A_test3000.java new file mode 100644 index 00000000000..00443a345ca --- /dev/null +++ b/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); + } +} + diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/invalidSelection/A_test3001.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/invalidSelection/A_test3001.java new file mode 100644 index 00000000000..965b7266da3 --- /dev/null +++ b/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(); + } +} + diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/invalidSelection/A_test3002.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/invalidSelection/A_test3002.java new file mode 100644 index 00000000000..7cbe26e7f15 --- /dev/null +++ b/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(); + } +} + diff --git a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractMethodTests.java b/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractMethodTests.java index fe74750d34c..f0ea3464ba5 100644 --- a/org.eclipse.jdt.ui.tests.refactoring/test cases/org/eclipse/jdt/ui/tests/refactoring/ExtractMethodTests.java +++ b/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(); } + }