Community
Participate
Working Groups
1) create the class String.java package pack; public class String { } 2) create the class X.java public class X { public static void foo(String s) { } public void foo(pack.String) { } } 3) select the second 'foo' method (foo(pack.String)) 4) do Refactor->Rename 5) change name to 'bar' 6) do OK an exception occur org.eclipse.jdt.internal.corext.Assert$AssertionFailedException: assertion failed; second condition at org.eclipse.jdt.internal.corext.Assert.isTrue(Assert.java:136) at org.eclipse.jdt.internal.corext.refactoring.rename.RippleMethodFinder.getAllRip pleMethods(RippleMethodFinder.java:115) at org.eclipse.jdt.internal.corext.refactoring.rename.RippleMethodFinder.getRelate dMethods(RippleMethodFinder.java:62) at org.eclipse.jdt.internal.corext.refactoring.rename.RenameMethodProcessor.initia lizeMethodsToRename(RenameMethodProcessor.java:161) at org.eclipse.jdt.internal.corext.refactoring.rename.RenameMethodProcessor.checkF inalConditions(RenameMethodProcessor.java:257) at org.eclipse.jdt.internal.corext.refactoring.rename.RenameVirtualMethodProcessor .checkFinalConditions(RenameVirtualMethodProcessor.java:91) at org.eclipse.ltk.core.refactoring.participants.ProcessorBasedRefactoring.checkFi nalConditions(ProcessorBasedRefactoring.java:134) at org.eclipse.ltk.core.refactoring.CheckConditionsOperation.run (CheckConditionsOperation.java:84) at org.eclipse.ltk.core.refactoring.CreateChangeOperation.run (CreateChangeOperation.java:114) at org.eclipse.ltk.core.refactoring.PerformChangeOperation.run (PerformChangeOperation.java:182) at org.eclipse.core.internal.resources.Workspace.run (Workspace.java:1673) at org.eclipse.ltk.internal.ui.refactoring.WorkbenchRunnableAdapter.run (WorkbenchRunnableAdapter.java:58) at org.eclipse.jface.operation.ModalContext$ModalContextThread.run (ModalContext.java:101) The problem seems to be inside RippleMethodFinder#declaresAsVirtual(). Checks.findSimilarMethod(m, type) return the wrong method (the first method)
We discovered this defect when looking into bug 66271. It is not revealed by fix for bug 66271, as demonstrated with David's testcase, but if we fix 66271, we get further into refactoring and then this bug will occur more often. Scenario is rare, but we have to be careful.
The fix for bug 66271 would IMO not introduce more errors in refactorings. Refactorings which can be started from a code selection of a java element in the editor can always be started by selecting the java element in e.g. the Package Explorer as well. The example in comment 0 reveals a deficiency in the RippleMethodFinder: The RMF uses IMethod#isSimilar(IMethod method) to find ripple methods in the hierarchies. Since IMethod#isSimilar(IMethod method) only compares simple names of the parameters' types, this lookup may fail. The assertion in RippleMethodFinder is causing more harm than good: It fails in the only case where the refactoring would work correctly but doesn't find most of the cases where the current implementation doesn't work correctly. Rename Method (without the offending assert) is correct if the method to rename has only "similar" methods declared in the same type. If "similar" methods are found in the hierarchies, it often renames too many declarations (and their references). Concluding, I opt for giving a go for bug 66271 and removing the assertion in RippleMethodFinder for 3.0. Removing the assertion is a no-risk job. Consequences of getting more declarations by patch 66271 is low-risk for jdt-ui.
We decided to not fix this for 3.0 since it is too risky and the assert could fail since 1.0. It could be triggered using the same test case but executing the rename from the outliner. To get this assertion the code must have - overloaded methods - the simple types names of all parameters of the methods must be the same. I consider this a rare case (that's why we never got a PR for this).
Markus, was is the state of this bug ?
This has been fixed with the new RippleMethodFinder2.
