Skip to content

Commit 0d7e5c5

Browse files
authored
Fix move refactor to recognize super class name collision (#1704)
- fix MoveInstanceMethodProcessor to recognize when a member being moved has same name as a member of a super class of the target and change existing references to use super qualifier - add new test to MoveInstanceMethodTests - fixes #1676
1 parent 8f8fafd commit 0d7e5c5

File tree

4 files changed

+133
-0
lines changed

4 files changed

+133
-0
lines changed

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2238,6 +2238,7 @@ protected TextChangeManager createChangeManager(final RefactoringStatus status,
22382238
adjustor.rewriteVisibility(Progress.subMonitor(monitor, 1));
22392239
sourceRewrite.rewriteAST(document, fMethod.getCompilationUnit().getOptions(true));
22402240
createMethodSignature(document, declaration, sourceRewrite, rewrites);
2241+
modifyInheritedMethodCalls(document, declaration, sourceRewrite, rewrites);
22412242
ICompilationUnit unit= null;
22422243
CompilationUnitRewrite rewrite= null;
22432244
for (final Iterator<ICompilationUnit> iterator= rewrites.keySet().iterator(); iterator.hasNext();) {
@@ -3035,6 +3036,89 @@ protected void createMethodSignature(final IDocument document, final MethodDecla
30353036
}
30363037
}
30373038

3039+
private class SuperMethodChecker extends ASTVisitor {
3040+
private final CompilationUnitRewrite fRewriter;
3041+
3042+
public SuperMethodChecker(final CompilationUnitRewrite rewriter) {
3043+
fRewriter= rewriter;
3044+
}
3045+
3046+
@Override
3047+
public boolean visit(MethodInvocation node) {
3048+
if (node.getName().getFullyQualifiedName().equals(fMethodName)) {
3049+
IMethodBinding methodBinding= node.resolveMethodBinding();
3050+
if (methodBinding != null) {
3051+
ITypeBinding typeBinding= methodBinding.getDeclaringClass();
3052+
if (typeBinding != null) {
3053+
ITypeBinding targetTypeBinding= fTarget.getType();
3054+
if (!typeBinding.isEqualTo(targetTypeBinding) && findInHierarchy(typeBinding, targetTypeBinding)) {
3055+
ASTRewrite astRewrite= fRewriter.getASTRewrite();
3056+
AST ast= node.getAST();
3057+
SuperMethodInvocation superMethod= ast.newSuperMethodInvocation();
3058+
superMethod.setName(ast.newSimpleName(node.getName().getFullyQualifiedName()));
3059+
ListRewrite typeArgs= astRewrite.getListRewrite(node, MethodInvocation.TYPE_ARGUMENTS_PROPERTY);
3060+
List<Type> originalTypeList= typeArgs.getOriginalList();
3061+
if (originalTypeList.size() > 0) {
3062+
ASTNode typeArgsCopy= typeArgs.createCopyTarget(originalTypeList.get(0), originalTypeList.get(originalTypeList.size() - 1));
3063+
superMethod.typeArguments().add(typeArgsCopy);
3064+
}
3065+
ListRewrite args= astRewrite.getListRewrite(node, MethodInvocation.ARGUMENTS_PROPERTY);
3066+
List<Type> originalArgsList= args.getOriginalList();
3067+
if (originalArgsList.size() > 0) {
3068+
ASTNode argsCopy= typeArgs.createCopyTarget(originalArgsList.get(0), originalArgsList.get(originalTypeList.size() - 1));
3069+
superMethod.arguments().add(argsCopy);
3070+
}
3071+
astRewrite.replace(node, superMethod, null);
3072+
}
3073+
}
3074+
}
3075+
}
3076+
return true;
3077+
}
3078+
3079+
private boolean findInHierarchy(ITypeBinding toFind, ITypeBinding binding) {
3080+
if (binding == null) {
3081+
return false;
3082+
}
3083+
if (toFind.isEqualTo(binding)) {
3084+
return true;
3085+
}
3086+
if (findInHierarchy(toFind, binding.getSuperclass())) {
3087+
return true;
3088+
}
3089+
ITypeBinding[] bindingInterfaces= binding.getInterfaces();
3090+
for (ITypeBinding bindingInterface : bindingInterfaces) {
3091+
if (findInHierarchy(toFind, bindingInterface)) {
3092+
return true;
3093+
}
3094+
}
3095+
return false;
3096+
}
3097+
}
3098+
3099+
/**
3100+
* @param document
3101+
* the buffer containing the source of the source compilation
3102+
* unit
3103+
* @param declaration
3104+
* the method declaration to use as source
3105+
* @param rewrite
3106+
* the ast rewrite to use for the copy of the method body
3107+
* @param rewrites
3108+
* the compilation unit rewrites
3109+
* @throws JavaModelException
3110+
* if the insertion point cannot be found
3111+
*/
3112+
protected void modifyInheritedMethodCalls(final IDocument document, final MethodDeclaration declaration, final ASTRewrite rewrite, final Map<ICompilationUnit, CompilationUnitRewrite> rewrites) throws JavaModelException {
3113+
Assert.isNotNull(document);
3114+
Assert.isNotNull(declaration);
3115+
Assert.isNotNull(rewrite);
3116+
Assert.isNotNull(rewrites);
3117+
final CompilationUnitRewrite rewriter= getCompilationUnitRewrite(rewrites, getTargetType().getCompilationUnit());
3118+
final AbstractTypeDeclaration type= ASTNodeSearchUtil.getAbstractTypeDeclarationNode(getTargetType(), rewriter.getRoot());
3119+
type.accept(new SuperMethodChecker(rewriter));
3120+
}
3121+
30383122
/**
30393123
* Creates the necessary changes to remove method type parameters if they
30403124
* match with enclosing type parameters.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package p1;
2+
3+
class A {
4+
B b;
5+
6+
void method() {
7+
System.out.println("A class method");
8+
}
9+
}
10+
11+
class ParentClass {
12+
void method() {
13+
System.out.println("ParentClass method");
14+
}
15+
}
16+
17+
class B extends ParentClass {
18+
void test() {
19+
method();
20+
}
21+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package p1;
2+
3+
class A {
4+
B b;
5+
}
6+
7+
class ParentClass {
8+
void method() {
9+
System.out.println("ParentClass method");
10+
}
11+
}
12+
13+
class B extends ParentClass {
14+
void test() {
15+
super.method();
16+
}
17+
18+
void method() {
19+
System.out.println("A class method");
20+
}
21+
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,13 @@ public void test78() throws Exception {
708708
public void test79() throws Exception {
709709
helper1(new String[] { "A" }, "A", 11, 17, 11, 18, FIELD, "b", true, true);
710710
}
711+
712+
@Test
713+
public void test80() throws Exception {
714+
helper1(new String[] { "A" }, "A", 6, 10, 6, 16, FIELD, "b", true, true);
715+
}
716+
717+
711718
// Move mA1 to field fB, do not inline delegator
712719
@Test
713720
public void test3() throws Exception {

0 commit comments

Comments
 (0)