Community
Participate
Working Groups
In the following class: public class BadExtract { void meth() { int x = 9; int y = 10; int j = x + y + x + y; } } Select the first 'x + y'. And execute the extract local variable refactoring. Only one occurrence is replaced. The following exception is thrown: java.lang.reflect.InvocationTargetException at org.eclipse.jface.operation.ModalContext.run(ModalContext.java:421) at org.eclipse.ltk.internal.ui.refactoring.RefactoringWizardDialog2.run(RefactoringWizardDialog2.java:331) at org.eclipse.ltk.ui.refactoring.RefactoringWizard.internalPerformFinish(RefactoringWizard.java:637) at org.eclipse.ltk.ui.refactoring.UserInputWizardPage.performFinish(UserInputWizardPage.java:153) at org.eclipse.ltk.ui.refactoring.RefactoringWizard.performFinish(RefactoringWizard.java:710) at org.eclipse.ltk.internal.ui.refactoring.RefactoringWizardDialog2.okPressed(RefactoringWizardDialog2.java:455) at org.eclipse.jface.dialogs.Dialog.buttonPressed(Dialog.java:472) at org.eclipse.jface.dialogs.Dialog$2.widgetSelected(Dialog.java:624) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:248) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4136) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1458) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1481) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1466) at org.eclipse.swt.widgets.Widget.notifyListeners(Widget.java:1271) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3982) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3621) at org.eclipse.jface.window.Window.runEventLoop(Window.java:825) at org.eclipse.jface.window.Window.open(Window.java:801) at org.eclipse.ltk.ui.refactoring.RefactoringWizardOpenOperation$1.run(RefactoringWizardOpenOperation.java:187) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70) at org.eclipse.ltk.ui.refactoring.RefactoringWizardOpenOperation.run(RefactoringWizardOpenOperation.java:202) at org.eclipse.ltk.ui.refactoring.RefactoringWizardOpenOperation.run(RefactoringWizardOpenOperation.java:122) at org.eclipse.jdt.internal.ui.refactoring.actions.RefactoringStarter.activate(RefactoringStarter.java:38) at org.eclipse.jdt.ui.actions.ExtractTempAction.run(ExtractTempAction.java:88) at org.eclipse.jdt.ui.actions.SelectionDispatchAction.dispatchRun(SelectionDispatchAction.java:279) at org.eclipse.jdt.ui.actions.SelectionDispatchAction.run(SelectionDispatchAction.java:251) at org.eclipse.jface.action.Action.runWithEvent(Action.java:498) at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:584) at org.eclipse.jface.action.ActionContributionItem.access$2(ActionContributionItem.java:501) at org.eclipse.jface.action.ActionContributionItem$5.handleEvent(ActionContributionItem.java:411) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4136) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1458) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1481) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1466) at org.eclipse.swt.widgets.Widget.notifyListeners(Widget.java:1271) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3982) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3621) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1053) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:942) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:86) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:588) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:543) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:124) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:353) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:180) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:601) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:629) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:584) at org.eclipse.equinox.launcher.Main.run(Main.java:1438) at org.eclipse.equinox.launcher.Main.main(Main.java:1414) Caused by: org.eclipse.text.edits.MalformedTreeException: No target edit provided. at org.eclipse.text.edits.CopySourceEdit.performConsistencyCheck(CopySourceEdit.java:239) at org.eclipse.text.edits.TextEdit.traverseConsistencyCheck(TextEdit.java:873) at org.eclipse.text.edits.CopySourceEdit.traverseConsistencyCheck(CopySourceEdit.java:214) at org.eclipse.text.edits.TextEdit.traverseConsistencyCheck(TextEdit.java:869) at org.eclipse.text.edits.TextEdit.traverseConsistencyCheck(TextEdit.java:869) at org.eclipse.text.edits.TextEdit.traverseConsistencyCheck(TextEdit.java:869) at org.eclipse.text.edits.TextEditProcessor.checkIntegrityDo(TextEditProcessor.java:176) at org.eclipse.text.edits.TextEdit.dispatchCheckIntegrity(TextEdit.java:743) at org.eclipse.text.edits.TextEditProcessor.performEdits(TextEditProcessor.java:151) at org.eclipse.ltk.core.refactoring.TextChange.getPreviewDocument(TextChange.java:534) at org.eclipse.ltk.core.refactoring.TextChange.getPreviewDocument(TextChange.java:403) at org.eclipse.ltk.core.refactoring.TextChange.getPreviewContent(TextChange.java:411) at org.eclipse.jdt.internal.corext.refactoring.code.ExtractTempRefactoring.checkNewSource(ExtractTempRefactoring.java:559) at org.eclipse.jdt.internal.corext.refactoring.code.ExtractTempRefactoring.checkFinalConditions(ExtractTempRefactoring.java:504) at org.eclipse.ltk.core.refactoring.CheckConditionsOperation.run(CheckConditionsOperation.java:85) at org.eclipse.ltk.core.refactoring.CreateChangeOperation.run(CreateChangeOperation.java:121) at org.eclipse.ltk.core.refactoring.PerformChangeOperation.run(PerformChangeOperation.java:209) at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:2344) at org.eclipse.ltk.internal.ui.refactoring.WorkbenchRunnableAdapter.run(WorkbenchRunnableAdapter.java:87) at org.eclipse.jface.operation.ModalContext$ModalContextThread.run(ModalContext.java:121)
Used to work in 3.2 and older.
Created attachment 236070 [details] proposed patch This contribution complies with http://www.eclipse.org/legal/CoO.php This was very tough for me to figure out. Hopefully the approach I have come up with is the correct one. This patch also fixes similar problems with Extract Constant Refactoring.
Comment on attachment 236070 [details] proposed patch - Fix is required only in AssociativeInfixExpressionFragment and hence the changes in IASTFragment and SimpleFragment are not relevant. - In AssociativeInfixExpressionFragment, 'matchingFragments' can be obtained in #replace(...) with "getMatchingFragmentsWithNode(groupNode)". - The processed fragments can be ignored in ExtractTempRefactoring#addReplaceExpressionWithTemp(...) by checking the associated node of the matching fragments to replace. - The logic in AssociativeInfixExpressionFragment#replace(...) is fine. The code can be improved by refactoring and looking for minimal changes in the existing code. - ExtractConstantRefactoring can be updated similar to ExtractTempRefactoring and tests can be added in ExtractConstantTests.
Created attachment 236113 [details] Code review changes, bug fix + unit test This contribution complies with http://www.eclipse.org/legal/CoO.php
Thanks for the patch Nikolay. Released after some minor changes: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=a34e4c1c1a8d27c68cfc055933a580c756e7fd10
The approach looked correct, until I realized that there are still cases where we produce exceptions, e.g. if you change j to "int j = x + y + (x + y)". The first underlying problem is a design flaw in InfixExpression: It doesn't handle all operands uniformly. Filed and fixed bug 418924. To work around that, the code always created a new InfixExpression and then copied the other operands to the new expression. Your fix addressed the case where all matching fragments appeared on the same level in an InfixExpression, but still failed for cases where they are more deeply nested. Then there's another caveat: There are many ways how to represent a longer infix expression in an AST. The old code tried to ensure it works no matter whether a chain of operands was parsed into a single InfixExpression or into a nested hierarchy of InfixExpressions. Since the ASTParser doesn't guarantee a certain structure, we shouldn't rely on a patterns we often see. The fix also missed to consider the "replace all occurrences" flag: ExtractConstantTests#test45() should have failed, since the test implementation has replaceAll==false, but the expected output had all occurrences replaced. Released a simpler implementation based on bug 418924 and updated tests: http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=3f132f86580b01fb9661823f29ef36276d6265c9
