Skip to content

Commit

Permalink
Remove locals moved via extract method if they aren't used elsewhere (#…
Browse files Browse the repository at this point in the history
…1399)

- modify ExtractMethodRefactoring.createMethodBody() to check all
  locals that needed to be created in new method to see if the old
  declaration is still needed (i.e. referenced outside of the
  selected statements) and if not, remove the original declaration
- modify all ExtractMethodTests where the locals are left behind
  and not used elsewhere
- add new tests to ExtractMethodTests
- fixes #1357
  • Loading branch information
jjohnstn committed May 10, 2024
1 parent af7a99c commit fc3838c
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1286,9 +1286,28 @@ private Block createMethodBody(ASTNode[] selectedNodes, TextEditGroup substitute

// Locals that are not passed as an arguments since the extracted method only
// writes to them
IRegion selectedNodeRange= fAnalyzer.getSelectedNodeRange();
int endOfSelectedNodes= selectedNodeRange.getOffset() + selectedNodeRange.getLength();
for (IVariableBinding methodLocal : fAnalyzer.getMethodLocals()) {
if (methodLocal != null) {
result.statements().add(createDeclaration(methodLocal, null));
// check if local variable that is used in selected nodes is also used outside of selected nodes, otherwise remove
SimpleName[] nodes= LinkedNodeFinder.findByBinding(fAnalyzer.getEnclosingBodyDeclaration(), methodLocal);
SimpleName firstNode= nodes[0];
VariableDeclarationStatement vdecl= ASTNodes.getFirstAncestorOrNull(firstNode, VariableDeclarationStatement.class);
if (vdecl != null) {
boolean needed= false;
for (int i= 1; i < nodes.length; ++i) {
SimpleName refNode= nodes[i];
if (refNode.getStartPosition() < selectedNodeRange.getOffset() || refNode.getStartPosition() > endOfSelectedNodes) {
needed= true;
break;
}
}
if (!needed) {
fRewriter.remove(vdecl, null);
}
}
}
}
for (ParameterInfo parameter : fParameterInfos) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

public class A_test501 {
public void foo() {
int x= 10;

extracted();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

public class A_test502 {
public void foo() {
int x= 0;
int y= 0;

extracted();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
public class A_test509 {
public void foo() {
int x= 0;
int y= 0;

extracted(x);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

public class A_test518 {
public void foo() {
int i;

extracted();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package validSelection_in;

public class A_testIssue1357_1 {

public synchronized int calculate() {
int result;
/*]*/switch (value) {
case 1:
result = value * 2;
break;
case 2:
result = value * 3;
break;
default:
result = value * 4;
break;
}
return result;/*[*/
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package validSelection_in;

public class A_testIssue1357_2 {

public synchronized int calculate() {
int result;
/*]*/switch (value) {
case 1:
result = value * 2;
break;
case 2:
result = value * 3;
break;
default:
result = value * 4;
break;
}/*[*/
return result;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package validSelection_out;

public class A_testIssue1357_1 {

public synchronized int calculate() {
return extracted();/*[*/
}

protected int extracted() {
int result;
switch (value) {
case 1:
result = value * 2;
break;
case 2:
result = value * 3;
break;
default:
result = value * 4;
break;
}
return result;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package validSelection_out;

public class A_testIssue1357_2 {

public synchronized int calculate() {
int result;
/*]*/result = extracted();/*[*/
return result;
}

protected int extracted() {
int result;
switch (value) {
case 1:
result = value * 2;
break;
case 2:
result = value * 3;
break;
default:
result = value * 4;
break;
}
return result;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ public class A_test2001 {
int field= 0;

void fun() {
int i;
extracted();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2700,4 +2700,14 @@ public void testIssue1356_1() throws Exception {
public void testIssue1356_2() throws Exception {
invalidSelectionTest();
}

@Test
public void testIssue1357_1() throws Exception {
validSelectionTestChecked();
}

@Test
public void testIssue1357_2() throws Exception {
validSelectionTestChecked();
}
}

0 comments on commit fc3838c

Please sign in to comment.