Skip to content

Commit

Permalink
extract method mulriple exit cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
larjean89 committed Nov 4, 2023
1 parent d3f6daf commit 90aff68
Show file tree
Hide file tree
Showing 7 changed files with 185 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}

Original file line number Diff line number Diff line change
@@ -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();
}
}

Original file line number Diff line number Diff line change
@@ -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();
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ public void test4() throws Exception {
selectionTest(5, 14, 5, 19);
}



//=====================================================================================
// Testing invalid selections
//=====================================================================================
Expand Down Expand Up @@ -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
//=====================================================================================
Expand Down Expand Up @@ -1096,6 +1115,7 @@ public void test353() throws Exception {
validSelectionTest();
}


@Test
public void test360() throws Exception {
validSelectionTestChecked();
Expand Down Expand Up @@ -2681,4 +2701,5 @@ public void test2005() throws Exception {
public void testIssue671() throws Exception {
validSelectionTestChecked();
}

}

0 comments on commit 90aff68

Please sign in to comment.