Bugzilla – Full Text Bug Listing
| Summary: | [move method] super method invocation does not compile after refactoring | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Gustavo Soares <gsoares> | ||||||||
| Component: | UI | Assignee: | Nikolay Metchev <nikolaymetchev> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P3 | CC: | daniel_megert, markus.kell.r, noopur_gupta | ||||||||
| Version: | 3.8 | Flags: | noopur_gupta:
review+
|
||||||||
| Target Milestone: | 4.4 M2 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
Refactoring is to move A#m(int) to B (move to the subclass). The problem is that we don't handle the super.m(2) call in B at all. Should at least issue an error in this case. *** Bug 355329 has been marked as a duplicate of this bug. *** Created attachment 234680 [details] propsed patch This contribution complies with http://www.eclipse.org/legal/CoO.php This fixes the reported issue. However I can't help feeling that there are corner cases that I have missed out. Even if I have missed out some cases it is still worth checking this patch in because it improves the situation. Few more common cases can also be handled in this patch.
Have a look at the impl for "if (node instanceof MethodInvocation)" just below your fix to cover such cases, like :
1. super method invocation changes to "null.m(2);" :
public class A {
public long m(B b, int i) {
return 1;
}
}
public class B extends A {
public long test() {
return super.m(null, 2);
}
}
2. Results in compilation error if target node was inserted while moving:
public class A {
public int i= 0;
public long m(B b, int i) {
return this.i + i;
}
}
public class B extends A {
public long test() {
return super.m(new B(), 2);
}
}
etc.
Created attachment 234705 [details] More test cases This contribution complies with http://www.eclipse.org/legal/CoO.php Ok I think I got all the corner cases. Comment on attachment 234705 [details] More test cases - You are always adding ThisExpression as the *first* argument of new method invocation. This won't work here: public class A { public int i= 0; public long m(int i, B b) { return this.i + i; } } public class B extends A { public long test() { return super.m(2, new B()); } } - It is good to use 'if-else if' instead of consecutive 'if's wherever possible. - Change the copyright addition to follow this one line pattern: Name <e-mail> - summary - https://bugs.eclipse.org/BUG_NUMBER - Please refer http://wiki.eclipse.org/JDT_UI/How_to_Contribute to configure your workspace with JDT UI specific Save Actions. Created attachment 234747 [details] even more test cases covered This contribution complies with http://www.eclipse.org/legal/CoO.php Thanks Noopur, Here is a patch with your comments incorporated Thanks Nikolay, the patch looks good now. Released after some refactoring and formatting, please have a look: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=bd8e7065888ed4d82ba11a6485106ef4ebad93ae Thanks Noopur, Looks good |
Build Identifier: 20110615-0604 Applying the move method refactoring introduces a compilation error Reproducible: Always Steps to Reproduce: 1. Create the classes public class A { public B b; private long m(long l) { return 0; } public long m(int i) { return 1; } } public class B extends A { public long test() { return super.m(2); } } 2. Apply the move method refactoring to move m to B public class A { public B b; private long m(long l) { return 0; } } public class B extends A { public long test() { return super.m(2); } public long m(int i) { return 1; } } 3. The resulting program does not compile: The method m from the type A is not visible