Bugzilla – Full Text Bug Listing
| Summary: | [refactor][inline] Inlining statically imported method does not correctly clean up static imports | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Lukas Eder <lukas.eder> | ||||
| Component: | UI | Assignee: | JDT-UI-Inbox <jdt-ui-inbox> | ||||
| Status: | CLOSED DUPLICATE | QA Contact: | |||||
| Severity: | minor | ||||||
| Priority: | P3 | CC: | daniel_megert, jan.opacki, lukas.eder, markus.kell.r | ||||
| Version: | 3.7.2 | Keywords: | helpwanted | ||||
| Target Milestone: | --- | Flags: | markus.kell.r:
review-
|
||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
CallInliner#computeReceiver() should probably detect that the receiver is statically imported and SourceProvider#updateImplicitReceivers(..) should add it as a static import (or add a qualifier if static import is not possible at the old call site. Hi Markus,
I would like to work on this. There is one more scenario to consider here: a case where a body of a method we are inlining uses a static member that is not visible from the place where it is to be inserted.
To ilustrate this, we can modify the class Test1 as following:
---------------------------------------
package pkg;
public class Test1 {
public static void inlineMe() {
callMeDirectly();
}
private static void callMeDirectly() {
}
}
---------------------------------------
Now, inlining inlineMe() won't be successful outside of the Test1 class.
I was thinking about two possible solutions:
1) We do not add a static import for such non-visible member and let the user deal with this.
2) We check it beforehand and if this is the case then we disable the inline option in the context menu so that the user won't be able to perform the inlining through an IDE.
What do you think about it?
Best,
Jan.
3) Another option would be doing this on the way - if we detect that a certain inlining will break the code, we notify a user and let him decide whether he wants to skip this inlining or proceed. Best, Jan. > 3) Another option would be doing this on the way - if we detect that a certain
> inlining will break the code, we notify a user and let him decide whether he
> wants to skip this inlining or proceed.
Yes, that's the best way to do it.
Created attachment 227169 [details]
Proposed patch
Hi there, here is a proposed patch for the bug. If we choose to inline all invocations and to delete the method declaration, static imports will be removed as well.
(In reply to comment #5) > Created attachment 227169 [details] [diff] > Proposed patch > > Hi there, here is a proposed patch for the bug. If we choose to inline all > invocations and to delete the method declaration, static imports will be > removed as well. Thanks for the patch. We are currently quite busy, so it might take a while for the review. The import removal code is not good. It only works in one very specific case and disregards scenarios where
- the method declaration was selected
- the method declaration was not removed
- the static import is an on-demand (star) import
We use ImportRemover to remove imports.
We also need a test case in org.eclipse.jdt.ui.tests.refactoring.InlineMethodTests.
We always have a space between "if (" and no newline in "} else if (..) {".
What about comment 4? I know we also don't recognize this in case the method is not imported, so this could also go to a separate bug.
*** This bug has been marked as a duplicate of bug 108152 *** |
Build Identifier: Build Identifier: Version: Indigo Release Build id: M20120208-0800 When a method is statically imported by client code, then refactor-inline does not correctly clean up the static imports Reproducible: Always Steps to Reproduce: 1. Create these classes --------------------------------------- package pkg; public class Test1 { public static void inlineMe() { callMeDirectly(); } public static void callMeDirectly() { } } --------------------------------------- package pkg; import static pkg.Test1.inlineMe; public class Test2 { public void x() { inlineMe(); } } --------------------------------------- 2. Go to Test1.inlineMe() 3. Choose Refactor > Inline... from the context menu 4. Choose any configuration 5. Test2 now has a compilation error: --------------------------------------- package pkg; // This import doesn't exist anymore import static pkg.Test1.inlineMe; public class Test2 { public void x() { // This, however, is not imported as it should be callMeDirectly(); } } ---------------------------------------