From fc3838c5ae1ee7066c5498c467c9a0e98dac26f6 Mon Sep 17 00:00:00 2001 From: Jeff Johnston Date: Fri, 10 May 2024 10:57:37 -0400 Subject: [PATCH] Remove locals moved via extract method if they aren't used elsewhere (#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 --- .../code/ExtractMethodRefactoring.java | 19 +++++++++++++ .../locals_out/A_test501.java | 2 -- .../locals_out/A_test502.java | 3 --- .../locals_out/A_test509.java | 2 -- .../locals_out/A_test518.java | 2 -- .../validSelection_in/A_testIssue1357_1.java | 21 +++++++++++++++ .../validSelection_in/A_testIssue1357_2.java | 21 +++++++++++++++ .../validSelection_out/A_testIssue1357_1.java | 25 +++++++++++++++++ .../validSelection_out/A_testIssue1357_2.java | 27 +++++++++++++++++++ .../wiki_out/A_test2001.java | 1 - .../tests/refactoring/ExtractMethodTests.java | 10 +++++++ 11 files changed, 123 insertions(+), 10 deletions(-) create mode 100644 org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_in/A_testIssue1357_1.java create mode 100644 org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_in/A_testIssue1357_2.java create mode 100644 org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_out/A_testIssue1357_1.java create mode 100644 org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_out/A_testIssue1357_2.java diff --git a/org.eclipse.jdt.core.manipulation/refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodRefactoring.java b/org.eclipse.jdt.core.manipulation/refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodRefactoring.java index ab1b4b30b50..9749436e883 100644 --- a/org.eclipse.jdt.core.manipulation/refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodRefactoring.java +++ b/org.eclipse.jdt.core.manipulation/refactoring/org/eclipse/jdt/internal/corext/refactoring/code/ExtractMethodRefactoring.java @@ -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) { diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test501.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test501.java index e28911ff48a..6b660ae2905 100644 --- a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test501.java +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test501.java @@ -2,8 +2,6 @@ public class A_test501 { public void foo() { - int x= 10; - extracted(); } diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test502.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test502.java index ba1eb89132c..57c5e63c8d4 100644 --- a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test502.java +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test502.java @@ -2,9 +2,6 @@ public class A_test502 { public void foo() { - int x= 0; - int y= 0; - extracted(); } diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test509.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test509.java index 9c78abd9b15..de894c6d9ad 100644 --- a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test509.java +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test509.java @@ -3,8 +3,6 @@ public class A_test509 { public void foo() { int x= 0; - int y= 0; - extracted(x); } diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test518.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test518.java index 161986d7e41..090c5a259a9 100644 --- a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test518.java +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/locals_out/A_test518.java @@ -2,8 +2,6 @@ public class A_test518 { public void foo() { - int i; - extracted(); } diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_in/A_testIssue1357_1.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_in/A_testIssue1357_1.java new file mode 100644 index 00000000000..9f06f112389 --- /dev/null +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_in/A_testIssue1357_1.java @@ -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;/*[*/ + } + +} \ No newline at end of file diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_in/A_testIssue1357_2.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_in/A_testIssue1357_2.java new file mode 100644 index 00000000000..386e98bac39 --- /dev/null +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_in/A_testIssue1357_2.java @@ -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; + } + +} \ No newline at end of file diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_out/A_testIssue1357_1.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_out/A_testIssue1357_1.java new file mode 100644 index 00000000000..d584e65726a --- /dev/null +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_out/A_testIssue1357_1.java @@ -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; + } + +} \ No newline at end of file diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_out/A_testIssue1357_2.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_out/A_testIssue1357_2.java new file mode 100644 index 00000000000..8ea3e18dcd3 --- /dev/null +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/validSelection_out/A_testIssue1357_2.java @@ -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; + } + +} \ No newline at end of file diff --git a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/wiki_out/A_test2001.java b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/wiki_out/A_test2001.java index f0fe986bcf2..50a31d82869 100644 --- a/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/wiki_out/A_test2001.java +++ b/org.eclipse.jdt.ui.tests.refactoring/resources/ExtractMethodWorkSpace/ExtractMethodTests/wiki_out/A_test2001.java @@ -5,7 +5,6 @@ public class A_test2001 { int field= 0; void fun() { - int i; extracted(); } 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 7f1b84be9d7..33f0dfaab27 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 @@ -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(); + } }