Skip to content

Commit

Permalink
Fix pull up scenario referring to outer this type (#1854)
Browse files Browse the repository at this point in the history
- modify PullUpRefactoring.CheckInvalidOuterFieldAccess checker to
  also look for class.this references that won't be accessible after
  pulling up the method
- add new test to PullUpTests
- fixes #1823
  • Loading branch information
jjohnstn authored Dec 14, 2024
1 parent 960a909 commit cedc392
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -1250,6 +1250,18 @@ public boolean visit(SimpleName node) {
return true;
}

@Override
public boolean visit(ThisExpression node) {
if (node.getLocationInParent() == MethodInvocation.ARGUMENTS_PROPERTY) {
ITypeBinding typeBinding= node.resolveTypeBinding();
if (isChildTypeMember(typeBinding, fSourceType) && !isChildTypeMember(typeBinding, fTargetType)) {
fConflictBinding= typeBinding;
throw new AbortSearchException();
}
}
return super.visit(node);
}

private boolean isChildTypeMember(ITypeBinding parentTypeBinding, IType type) {
if (parentTypeBinding.getQualifiedName().equals(type.getFullyQualifiedName('.'))) {
return true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package p;

public class A {
public class BaseInner {
void innerMethodLambda(Outer outer) {
Runnable r = () -> {
System.out.println(outer.x);
outer.foo();
};
r.run();
}
}

public class Outer {
public int x = 0;
public void foo(){};

public class Inner extends BaseInner {
void innerMethod() { // Pull this method up to class BaseInner
innerMethodLambda(Outer.this);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1882,7 +1882,27 @@ public void testFail36() throws Exception {
testMonitor.assertUsedUp();
testMonitor.prepare();
assertFalse("precondition was supposed to fail", checkInputResult.isOK());
// helper2(new String[] { "method" }, new String[][] { new String[0] }, true, false, 0);
}
@Test
public void testFail37() throws Exception {
ICompilationUnit cu= createCUfromTestFile(getPackageP(), "A");
IJavaElement element= cu.getElementAt(409);
assertTrue("element is method", element instanceof IMethod);
IMethod[] methods= new IMethod[] { (IMethod)element };

PullUpRefactoringProcessor processor= createRefactoringProcessor(methods);
Refactoring ref= processor.getRefactoring();

FussyProgressMonitor testMonitor= new FussyProgressMonitor();
assertTrue("activation", ref.checkInitialConditions(testMonitor).isOK());
testMonitor.assertUsedUp();
testMonitor.prepare();
setTargetClass(processor, 0);

RefactoringStatus checkInputResult= ref.checkFinalConditions(testMonitor);
testMonitor.assertUsedUp();
testMonitor.prepare();
assertFalse("precondition was supposed to fail", checkInputResult.isOK());
}
//----------------------------------------------------------
@Test
Expand Down

0 comments on commit cedc392

Please sign in to comment.