Community
Participate
Working Groups
Build Identifier: 20090920-1017 Renaming a class changes class inheritance and program behavior. After the refactoring, the 'extend' expression match with a class of the same package instead of a class imported using wildcard. Reproducible: Always Steps to Reproduce: 1. Create the classes: package a; public class A { public int k() { return 20; } } package a; import b.*; public class B extends C { public int test() { return k(); } } package b; public class C { public int k() { return 10; } } 2. The method test returns 10 3. Apply the rename class refactoring to A renaming it to C package a; public class C { public int k() { return 20; } } package a; import b.*; public class B extends C { public int test() { return k(); } } package b; public class C { public int k() { return 10; } } 4. After the refactoring, the method test returns 20 instead of 10. The program behavior was changed.
Like that since day one.
To avoid this behavior change during rename operation, I would propose two solutions. 1. In addition to the wild card import statement, add an explicit import statement so that the renamed class does not shadow the wild card import. For instance in the given example after the rename operation B.java will have the below import statements. import b.*; import b.C; 2. Inform the user that the renaming might result in a behavior change of the existing system. Either stop the user from performing such a rename or issue a stronger message. Note: In the current implementation we do issue an error message to the user saying "Another type named 'C' is referenced in 'B'". May be we can have a stronger message instead. Dani, let me know if there is a better solution or which of the above can be used to solve the issue.
Similar to bug 356677. RenameTypeProcessor#checkConflictingTypes(..) already finds similar conflicts, but it apparently misses this specific case. Solution 1 probably cannot be implemented right now due to bug 71761. Solution 2 is the way to go. Not critical for 4.3.
Created attachment 230247 [details] Patch using InsertEdit. Went through bug 71761. From the bug report and the subsequent comments, ImportRewrite currently cannot handle an on demand import and an explicit import from the same package. When it encounters the on demand import, it shadows the explicit import. The Javadoc for ImportRewrite says that eventually all the ImportRewrites are converted to text edits and then it is applied to the compilation unit. Hence I tried with InsertEdit to add the import statement in the affected compilation unit and it seems to work. The drawback is that the complete import statement has to be added as a string during the InsertEdit. I am not aware of the implications of using TextEdit instead of ImportRewrite. Markus, the patch uses InsertEdit to add the explicit import to the affected compilation unit. What is the consequence of using InsertEdit instead of ImportRewrite? Kindly have a look when you are free.
(In reply to comment #4) > Created attachment 230247 [details] [diff] > Patch using InsertEdit. Sorry, but that's a no-go. Unless a bug is critical, we don't hack around limitations in Eclipse APIs but we fix the APIs. ImportRewrite is the only acceptable way to manipulate imports. It ensures that all preferences are properly handled, and that adding an import doesn't modify existing references to the same simple name.
Created attachment 230256 [details] Patch with a strong message. Thanks Markus. Since the issue with ImportRewrite is yet to be solved, i have implemented the second proposed solution of issuing a stronger message. The message to the user in this case will be: 'Another type named 'C' is referenced in 'B.java'. Semantics may not be preserved if you proceed.'. If user choose to proceed, then it is up to the user to deal with the consequences.
(In reply to comment #3) > RenameTypeProcessor#checkConflictingTypes(..) already > finds similar conflicts, but it apparently misses this specific case. Nothing is missed in 3.8. Since bug 356677, we show an error. Users who proceed if they see an error are on their own. I probably used Organize Imports, which converted the "import b.*;" to "import b.C;", which makes this whole problem go away. (In reply to comment #6) This looks good and can go into M7, but the bug should stay open for solution 2 (once the blocking bug 71761 made this possible).
Thanks Markus. Released the patch as : http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=6402d5193cf61d970c798bf077f5f31fba3202d6 The bug stays open until bug 71761 is resolved and the right solution of adding the explicit import along with the on demand import can be handled by ImportRewrite.
