Community
Participate
Working Groups
3.1 RC2 Consider following snippet: class Top<E> { void add(E[] e) {} void add(String[] t) {}; } class Sub extends Top<String> { void add(String... s) {} } -> Invoke rename refactoring on Top#add(E[]) -> Refactoring does not update declaration and method ripple of Top#add(String [])
Markus, can you please comment on what the expected behaviour should be. IMO this is a bug in search not find all potential declarations. If so please move on to JDT/Core.
Corner case. Although you can't call add(..) on a Top<String> due to an ambiguous method invocation error, the methods are still related. All three should be renamed at once. This is probably a problem in the RippleMethodFinder, which currently assumes that a type declares at most one related method. Another problem is that search with IGNORE_DECLARING_TYPE | IGNORE_RETURN_TYPE, only finds all three methods when started on Sub#add(..), and not when started on a Top#add(..). Have to check whether we can blame the search engine on this.
*** Bug 420265 has been marked as a duplicate of this bug. ***
Created attachment 236879 [details] partial fix This contribution complies with http://www.eclipse.org/legal/CoO.php It seems there is an assumption in the code that you will only have one override per type. This patch removes that assumption from RippleMethodFinder2 and the Rename refactoring. This doesn't fully fix the bug because the search engine doesn't "ripple" properly when you start from the supertype. It does however fix the problem that when you start from the subclass not all relevant super class methods are renamed. I was experimenting with not using the search engine for RippleMethodFinder2.findAllDeclarations as part of bug 351410 which would have the benefit of addressing the rest of this bug. However I still think this partial patch is worthy of being committed.
Created attachment 236881 [details] partial fix This contribution complies with http://www.eclipse.org/legal/CoO.php Missed out one header comment.
Created attachment 236949 [details] proper fix This contribution complies with http://www.eclipse.org/legal/CoO.php Figured out how to fix this properly. This also fixes bug 395349.
(In reply to Nikolay Metchev from comment #6) > Created attachment 236949 [details] > proper fix > > This contribution complies with http://www.eclipse.org/legal/CoO.php > > Figured out how to fix this properly. This also fixes bug 395349. Thanks for the patch, Nikolay. Could you please rebase your patch on the current master branch and upload it to Gerrit for review?
New Gerrit change created: https://git.eclipse.org/r/69935
(In reply to Nikolay Metchev from comment #6) > Figured out how to fix this properly. This also fixes bug 395349. (In reply to Eclipse Genie from comment #8) > New Gerrit change created: https://git.eclipse.org/r/69935 I just tried the patch and found the following issue: Place caret at the first method name in super type and press Alt+Shift+R (or, Ctrl+1 > Rename in workspace). The second method in super type does not enter the linked mode for rename (though it is updated at the end of the refactoring).
Aah.. well spotted Noopur, I'll see if I can fix that.
New Gerrit change created: https://git.eclipse.org/r/70037
(In reply to Eclipse Genie from comment #11) > New Gerrit change created: https://git.eclipse.org/r/70037 I still see the issue mentioned in comment #9 with the above patch set 1. Also, the issue described in this bug report is applicable to "Ctrl+1 > Rename in file" as well, which is still broken. For the next version of your patch, you can amend the existing Gerrit request (70037) instead of creating a new one. You can take your time for the proper fix, as I will be able to do a detailed code review only after I am done with other feature work that needs to go in M7.
Hello Noopur, I apologise, it seems I made a slight change just before I submitted the code review and didn't re-test. Also, this is the first time I am using Gerrit and am still trying to get my head around how it all works. It seems I made a mistake there as well and created a new Gerrit task. I hope to be able to do better shortly. Nikolay
Moving to 4.7.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/70037 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=c8ab3558cf8d482557e53d04f26e2713535580ed
Released for 4.24 M1 Noopur's comment #9 has been addressed by the latest patch
could this have caused https://github.com/eclipse/xtext-xtend/issues/1310
New Gerrit change created: https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/192294
Gerrit change https://git.eclipse.org/r/c/jdt/eclipse.jdt.ui/+/192294 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=cdf5589c767aac592b3ef21dc3baebc88cccb596
Verified for 4.24 M1 using I20220406 build
