Skip to content

Commit af9fabb

Browse files
authored
Fix pull up refactoring to properly replace this argument when needed (eclipse-jdt#2193)
- modify PullUpRefactoringProcessor.PullUASTNodeMapper to replace a this argument to a method with the new argument that gets added - modify existing PullUpTests where the behavior has been fixed - add new test to PullUpTests - fixes eclipse-jdt#2192
1 parent 1f40aa1 commit af9fabb

File tree

10 files changed

+87
-5
lines changed

10 files changed

+87
-5
lines changed

org.eclipse.jdt.core.manipulation/refactoring/org/eclipse/jdt/internal/corext/refactoring/structure/PullUpRefactoringProcessor.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2006, 2024 IBM Corporation and others.
2+
* Copyright (c) 2006, 2025 IBM Corporation and others.
33
*
44
* This program and the accompanying materials
55
* are made available under the terms of the Eclipse Public License 2.0
@@ -298,6 +298,27 @@ public final boolean visit(final ThisExpression node) {
298298
final SimpleName ref= ast.newSimpleName(newArgument);
299299
fRewrite.replace(node, ref, null);
300300
}
301+
} else if (node.getLocationInParent() == MethodInvocation.ARGUMENTS_PROPERTY) {
302+
// we are calling a method with "this" as parameter, check what type it needs to be
303+
MethodInvocation methodInvocation= (MethodInvocation)node.getParent();
304+
IMethodBinding methodBinding= methodInvocation.resolveMethodBinding();
305+
if (methodBinding != null) {
306+
ITypeBinding[] parameterTypes= methodBinding.getParameterTypes();
307+
List<Expression> args= methodInvocation.arguments();
308+
int index= 0;
309+
for (int i= 0; i < args.size(); ++i) {
310+
if (args.get(i) == node) {
311+
index= i;
312+
break;
313+
}
314+
}
315+
ITypeBinding thisRequiredType= parameterTypes[index];
316+
if (thisRequiredType.isEqualTo(node.resolveTypeBinding())) {
317+
String newArgument= fNewArgumentMap.get(fEnclosingMethod.getJavaElement());
318+
final SimpleName ref= ast.newSimpleName(newArgument);
319+
fRewrite.replace(node, ref, null);
320+
}
321+
}
301322
}
302323
return true;
303324
}

org.eclipse.jdt.ui.tests.refactoring/resources/PullUp/test56/out/A.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ class A {
44
void a(A a){}
55
public void a(B b){}
66
protected void m(B b) {
7-
a(this);
7+
a(b);
88
}
99
}

org.eclipse.jdt.ui.tests.refactoring/resources/PullUp/test57/out/A.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ class A {
44
void a(A a){}
55
public void a(B b){}
66
protected void m(B b) {
7-
a(this);
7+
a(b);
88
}
99
protected void foo2(B b) {
1010
m(b);

org.eclipse.jdt.ui.tests.refactoring/resources/PullUp/test58/out/A.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ void a(A a){}
55
public void a(B b){}
66
protected void m(B b) {
77
foo2(b);
8-
a(this);
8+
a(b);
99
}
1010
protected void foo2(B b) {
1111
m(b);

org.eclipse.jdt.ui.tests.refactoring/resources/PullUp/test59/out/A.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ void a(A a){}
55
public void a(B b){}
66
protected void m(B b) {
77
foo2(b);
8-
a(this);
8+
a(b);
99
}
1010
protected void foo2(B b) {
1111
m(b);
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package p;
2+
3+
class A {
4+
void a(A a){}
5+
void a(B b){}
6+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package p;
2+
3+
class B extends A {
4+
public void m() {
5+
a(this);
6+
}
7+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package p;
2+
3+
class A {
4+
void a(A a){}
5+
void a(B b){}
6+
public void m(B b) {
7+
a(b);
8+
}
9+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
package p;
2+
3+
class B extends A {
4+
}

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1492,6 +1492,41 @@ public void test62() throws Exception {
14921492
helper1(new String[] { "foo" }, new String[][] { new String[0] }, true, false, 0);
14931493
}
14941494

1495+
@Test
1496+
public void test63() throws Exception {
1497+
// test for bug 241035
1498+
ICompilationUnit cuA= createCUfromTestFile(getPackageP(), "A");
1499+
ICompilationUnit cuB= createCUfromTestFile(getPackageP(), "B");
1500+
1501+
IType typeA= getType(cuA, "A");
1502+
IType typeB= getType(cuB, "B");
1503+
String[] methodNames= new String[] { "m" };
1504+
String[][] signatures= new String[][] { new String[0] };
1505+
IMethod[] methods= getMethods(typeB, methodNames, signatures);
1506+
1507+
PullUpRefactoringProcessor processor= createRefactoringProcessor(methods);
1508+
Refactoring ref= processor.getRefactoring();
1509+
FussyProgressMonitor testMonitor= new FussyProgressMonitor();
1510+
assertTrue("activation", ref.checkInitialConditions(testMonitor).isOK());
1511+
testMonitor.assertUsedUp();
1512+
testMonitor.prepare();
1513+
1514+
setTargetClass(processor, 0);
1515+
processor.setDestinationType(typeA);
1516+
processor.setMembersToMove(methods);
1517+
processor.setDeletedMethods(methods);
1518+
processor.setReplace(true);
1519+
1520+
assertTrue("final", ref.checkFinalConditions(testMonitor).isOK());
1521+
testMonitor.assertUsedUp();
1522+
testMonitor.prepare();
1523+
1524+
performChange(ref, false);
1525+
1526+
assertEqualLines("A", getFileContents(getOutputTestFileName("A")), cuA.getSource());
1527+
assertEqualLines("B", getFileContents(getOutputTestFileName("B")), cuB.getSource());
1528+
}
1529+
14951530
@Test
14961531
public void testFail0() throws Exception {
14971532
// printTestDisabledMessage("6538: searchDeclarationsOf* incorrect");

0 commit comments

Comments
 (0)