search

vazexqi / CodingSpectator

Watches and analyzes code edits in the Eclipse IDE non-invasively
http://codingspectator.cs.illinois.edu
Other
20 stars 14 forks source link

CodingSpectator throws out of bounds exception on extract local variable #232

Closed reprogrammer closed 13 years ago

reprogrammer commented 13 years ago

I got the following exception when trying to extract a local variable from a string literal using quick assist.

java.lang.StringIndexOutOfBoundsException: String index out of range: 2032
at java.lang.String.substring(String.java:1934)
at org.eclipse.jdt.internal.corext.refactoring.codingspectator.CodeSnippetInformationExtractor.getText(CodeSnippetInformationExtractor.java:42)
at org.eclipse.jdt.internal.corext.refactoring.codingspectator.TextSelectionCodeSnippetInformationExtractor.getSelectedText(TextSelectionCodeSnippetInformationExtractor.java:43)
at org.eclipse.jdt.internal.corext.refactoring.codingspectator.TextSelectionCodeSnippetInformationExtractor.extractCodeSnippetInformation(TextSelectionCodeSnippetInformationExtractor.java:37)
at org.eclipse.jdt.internal.corext.refactoring.codingspectator.CodeSnippetInformationFactory.extractCodeSnippetInformation(CodeSnippetInformationFactory.java:39)
at org.eclipse.jdt.internal.corext.refactoring.codingspectator.CodeSnippetInformationFactory.extractCodeSnippetInformation(CodeSnippetInformationFactory.java:43)
at org.eclipse.jdt.internal.corext.refactoring.codingspectator.WatchedJavaRefactoring.getCodeSnippetInformation(WatchedJavaRefactoring.java:58)
at org.eclipse.jdt.internal.corext.refactoring.codingspectator.WatchedJavaRefactoring.addAttributesFromGlobalRefactoringStore(WatchedJavaRefactoring.java:43)
at org.eclipse.jdt.internal.corext.refactoring.codingspectator.WatchedJavaRefactoring.populateInstrumentationData(WatchedJavaRefactoring.java:37)
at org.eclipse.jdt.internal.corext.refactoring.codingspectator.WatchedJavaRefactoring.getSimpleRefactoringDescriptor(WatchedJavaRefactoring.java:28)
at org.eclipse.ltk.core.refactoring.codingspectator.Logger.createRefactoringDescriptor(Logger.java:45)
at org.eclipse.ltk.core.refactoring.codingspectator.Logger.logRefactoringEvent(Logger.java:78)
at org.eclipse.ltk.core.refactoring.codingspectator.Logger.logRefactoringEvent(Logger.java:73)
at org.eclipse.jdt.internal.ui.text.correction.QuickAssistProcessor$RefactoringCorrectionProposal.aboutToPerformChange(QuickAssistProcessor.java:2021)
at org.eclipse.jdt.internal.ui.text.correction.proposals.ChangeCorrectionProposal.performChange(ChangeCorrectionProposal.java:176)
at org.eclipse.jdt.internal.ui.text.correction.proposals.CUCorrectionProposal.performChange(CUCorrectionProposal.java:323)
at org.eclipse.jdt.internal.ui.text.correction.proposals.CUCorrectionProposal.apply(CUCorrectionProposal.java:301)
at org.eclipse.jface.text.contentassist.CompletionProposalPopup.insertProposal(CompletionProposalPopup.java:933)
at org.eclipse.jface.text.contentassist.CompletionProposalPopup.insertSelectedProposalWithMask(CompletionProposalPopup.java:879)
at org.eclipse.jface.text.contentassist.CompletionProposalPopup.verifyKey(CompletionProposalPopup.java:1305)
at org.eclipse.jface.text.contentassist.ContentAssistant$InternalListener.verifyKey(ContentAssistant.java:806)
at org.eclipse.jface.text.TextViewer$VerifyKeyListenersManager.verifyKey(TextViewer.java:489)
at org.eclipse.swt.custom.StyledTextListener.handleEvent(StyledTextListener.java:65)
at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1258)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1282)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1267)
at org.eclipse.swt.widgets.Widget.notifyListeners(Widget.java:1061)
at org.eclipse.swt.custom.StyledText.handleKeyDown(StyledText.java:5957)
at org.eclipse.swt.custom.StyledText$7.handleEvent(StyledText.java:5656)
at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1258)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1282)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1267)
at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:1294)
at org.eclipse.swt.widgets.Widget.gtk_key_press_event(Widget.java:730)
at org.eclipse.swt.widgets.Control.gtk_key_press_event(Control.java:2841)
at org.eclipse.swt.widgets.Composite.gtk_key_press_event(Composite.java:734)
at org.eclipse.swt.widgets.Widget.windowProc(Widget.java:1743)
at org.eclipse.swt.widgets.Control.windowProc(Control.java:4796)
at org.eclipse.swt.widgets.Display.windowProc(Display.java:4360)
at org.eclipse.swt.internal.gtk.OS._gtk_main_do_event(Native Method)
at org.eclipse.swt.internal.gtk.OS.gtk_main_do_event(OS.java:8189)
at org.eclipse.swt.widgets.Display.eventProc(Display.java:1238)
at org.eclipse.swt.internal.gtk.OS._g_main_context_iteration(Native Method)
at org.eclipse.swt.internal.gtk.OS.g_main_context_iteration(OS.java:2237)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3159)
at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2640)
at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2604)
at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2438)
at org.eclipse.ui.internal.Workbench$7.run(Workbench.java:671)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:664)
at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:115)
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:369)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:620)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:575)
at org.eclipse.equinox.launcher.Main.run(Main.java:1408)
at org.eclipse.equinox.launcher.Main.main(Main.java:1384)
reprogrammer commented 13 years ago

@vazexqi, @Wanderer777: I couldn't reproduce this problem. However, while trying to reproduce his exception, I got different exceptions when I extracted constants via quick assist. But, I couldn't reproduce any of these problems deterministically. I got suspicious about the way we capture ITypeRoot when the user invokes a refactoring via quick assists. The problem might be that we get the ITypeRoot from non-primary working copies. When the user invokes the quick assist menu and begins to hover the mouse on the menu items, Eclipse creates the change for each menu item and previews it to the user. Eclipse might prepare the preview by applying the changes to non-primary working copies. Therefore, CodingSpectator might get the ITypeRoot from a working copy whose content is different from that of the the primary working copy. I haven't found a way to confirm my hypothesis yet.

reprogrammer commented 13 years ago

@vazexqi, @Wanderer777: I managed to reproduce the exception. The exception happens when the user tries to perform the refactoring on an unsaved file. Since the contents of the file don't reflect the code the user has invoked the refactoring on, CodingSpectator throws an exception when trying to extract the selected text from the file. Note that CodingSpectator used to extract the selected text from the buffer of ITypeRoot. But, issue #207 led me to use the file instead of the buffer. In summary, the offsets of the selection may not be applicable to the contents of the file or the buffer. Do you have any suggestions for extracting the selections safely?

Wanderer777 commented 13 years ago

The selection should be extracted from the corresponding IDocument. If the document already exists (which means that the file is opened in an editor, either saved or unsaved), the document's contents would reflect exactly what the user (and the invoked refactoring) sees. If the document does not exist yet, a newly created document would reflect the content of the underlying file (which is fine, since the file is not edited). As the starting point, please see method IDocumentProvider#getDocument and an example of its use in method EditorHelper#getEditedDocument.

reprogrammer commented 13 years ago

@Wanderer777, @vazexqi: I didn't know that IDocument reflects what the user sees in the editor. But, switching to IDocument for computing the code snippet information seems nontrivial, because IDocument doesn't provide an ITypeRoot. Currently, CodingSpectator needs an ITypeRoot to compute the code snippet. CodingSpectator first finds the AST node covering the selected text. Then, it traverses the AST up until the AST node gets big enough. Finally, it uses the text of the found AST node as the code snippet. CodingSpectator doesn't have to rely on the AST for computing code snippets. For example, it could have captured up to a fixed number of characters around the selected text as the code snippet. That is, there are ways to compute the code snippet information without an ITypeRoot. But, breaking this dependency on ITypeRoot requires a lot of changes.

Instead of changing the way code snippet information is computed, I'd suggest to compute this information early before the buffer changes. Currently, CodingSpectator stores the ITypeRoot and ITextSelection in the RefactoringGlobalStore in order to compute the code snippet and selected text later when it's about to serialize the refactoring descriptor. I suggest to extract the code snippet information when the selection object is stored in RefactoringGlobalStore. This approach may not address issue #207. If early computation of code snippet information doesn't fix this issue and issue #207, CodingSpectator has to compute the code snippet information just from the underlying IDocument.

Wanderer777 commented 13 years ago

AST could be obtained from parsing the content of the underlying IDocument. But first, we need to make sure that this document indeed represents an ICompilationUnit (which is ITypeRoot, and we do not need IClassFile as we are interested only in source code). Thus, we could use IDocument and still avoid changing the current code snippet computation.

reprogrammer commented 13 years ago

@Wanderer777: You're right. We should be able to get the ITypeRoot from IDocument. However, I don't know which method of getting the ITypeRoot is superior.

Currently, action classes such as ExtractTempAction get the ITypeRoot from the editor using the expression EditorUtility.getEditorInputJavaElement(fEditor, false).

Do you know which method is more reliable? That is, which method provides us a better ITypeRoot to extract the code snippet information from (selection offset and length and the code snippet around the selection).

reprogrammer commented 13 years ago

dd44da620afd4c38ab17ddc7a3eb7ace16910090 .. 679f9041d773d5dac95520ec011640f9770b685a compute the code snippet information immediately after a selection for a refactoring is made and before the invocation of the refactoring proceeds. Immediate computation of code snippet information fixes the problem reported in this issue. But, we still need to investigate if getting the ITypeRoot from IDocument is better than how action classes are currently getting the ITypeRoot.

Wanderer777 commented 13 years ago

As we discussed on the phone, if the current approach works, lets keep it. I do not know whether changing it to get ITypeRoot directly from IDocument is superior, so we can just keep in mind that there is such alternative.