Community
Participate
Working Groups
Build ID: I20080502-0100 Steps To Reproduce: 1. Configure Organize imports save action. 2. Create an interface like package org.rssowl.core.internal.persist.service; public interface EntityWithChildren { List<IEntity> getChildEntities(); } 3. Save by using ctrl+s 4. NPE is thrown. Choosing JDT Core, but please move to the appropriate component if that's wrong. More information: eclipse.buildId=I20080502-0100 java.version=1.6.0_02 java.vendor=Sun Microsystems Inc. BootLoader constants: OS=linux, ARCH=x86, WS=gtk, NL=en_US Command-line arguments: -os linux -ws gtk -arch x86 -data /home/ijuma/workspaces/rssowl2/ Error Sun May 04 16:18:11 BST 2008 The save participant 'org.eclipse.jdt.ui.postsavelistener.cleanup' caused an exception: java.lang.NullPointerException java.lang.NullPointerException at org.eclipse.jdt.internal.corext.codemanipulation.OrganizeImportsOperation$TypeReferenceProcessor.add(OrganizeImportsOperation.java:211) at org.eclipse.jdt.internal.corext.codemanipulation.OrganizeImportsOperation.createTextEdit(OrganizeImportsOperation.java:444) at org.eclipse.jdt.internal.corext.fix.ImportsFix.createCleanUp(ImportsFix.java:49) at org.eclipse.jdt.internal.ui.fix.ImportsCleanUp.createFix(ImportsCleanUp.java:60) at org.eclipse.jdt.internal.corext.fix.CleanUpRefactoring.calculateChange(CleanUpRefactoring.java:783) at org.eclipse.jdt.internal.corext.fix.CleanUpPostSaveListener.saved(CleanUpPostSaveListener.java:263) at org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitDocumentProvider$5.run(CompilationUnitDocumentProvider.java:1539) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:37) at org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitDocumentProvider.notifyPostSaveListeners(CompilationUnitDocumentProvider.java:1534) at org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitDocumentProvider.commitWorkingCopy(CompilationUnitDocumentProvider.java:1331) at org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitDocumentProvider$4.execute(CompilationUnitDocumentProvider.java:1400) at org.eclipse.ui.editors.text.TextFileDocumentProvider$DocumentProviderOperation.run(TextFileDocumentProvider.java:130) at org.eclipse.ui.actions.WorkspaceModifyDelegatingOperation.execute(WorkspaceModifyDelegatingOperation.java:68) at org.eclipse.ui.actions.WorkspaceModifyOperation$1.run(WorkspaceModifyOperation.java:104) at org.eclipse.core.internal.resources.Workspace.run(Workspace.java:1800) at org.eclipse.ui.actions.WorkspaceModifyOperation.run(WorkspaceModifyOperation.java:116) at org.eclipse.ui.internal.editors.text.WorkspaceOperationRunner.run(WorkspaceOperationRunner.java:73) at org.eclipse.ui.internal.editors.text.WorkspaceOperationRunner.run(WorkspaceOperationRunner.java:63) at org.eclipse.ui.editors.text.TextFileDocumentProvider.executeOperation(TextFileDocumentProvider.java:450) at org.eclipse.ui.editors.text.TextFileDocumentProvider.saveDocument(TextFileDocumentProvider.java:766) at org.eclipse.ui.texteditor.AbstractTextEditor.performSave(AbstractTextEditor.java:4789) at org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor.performSave(CompilationUnitEditor.java:1228) at org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor.doSave(CompilationUnitEditor.java:1281) at org.eclipse.ui.texteditor.AbstractTextEditor$TextEditorSavable.doSave(AbstractTextEditor.java:6888) at org.eclipse.ui.Saveable.doSave(Saveable.java:212) at org.eclipse.ui.internal.SaveableHelper.doSaveModel(SaveableHelper.java:339) at org.eclipse.ui.internal.SaveableHelper$2.run(SaveableHelper.java:185) at org.eclipse.ui.internal.SaveableHelper$4.run(SaveableHelper.java:266) at org.eclipse.jface.operation.ModalContext.runInCurrentThread(ModalContext.java:444) at org.eclipse.jface.operation.ModalContext.run(ModalContext.java:352) at org.eclipse.jface.window.ApplicationWindow$1.run(ApplicationWindow.java:758) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:67) at org.eclipse.jface.window.ApplicationWindow.run(ApplicationWindow.java:755) at org.eclipse.ui.internal.WorkbenchWindow.run(WorkbenchWindow.java:2476) at org.eclipse.ui.internal.SaveableHelper.runProgressMonitorOperation(SaveableHelper.java:274) at org.eclipse.ui.internal.SaveableHelper.runProgressMonitorOperation(SaveableHelper.java:253) at org.eclipse.ui.internal.SaveableHelper.saveModels(SaveableHelper.java:196) at org.eclipse.ui.internal.SaveableHelper.savePart(SaveableHelper.java:136) at org.eclipse.ui.internal.EditorManager.savePart(EditorManager.java:1350) at org.eclipse.ui.internal.WorkbenchPage.savePart(WorkbenchPage.java:3255) at org.eclipse.ui.internal.WorkbenchPage.saveEditor(WorkbenchPage.java:3268) at org.eclipse.ui.texteditor.SaveAction.run(SaveAction.java:47) at org.eclipse.jface.action.Action.runWithEvent(Action.java:498) at org.eclipse.ui.commands.ActionHandler.execute(ActionHandler.java:185) at org.eclipse.ui.internal.handlers.LegacyHandlerWrapper.execute(LegacyHandlerWrapper.java:109) at org.eclipse.core.commands.Command.executeWithChecks(Command.java:476) at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:508) at org.eclipse.ui.internal.handlers.HandlerService.executeCommand(HandlerService.java:169) at org.eclipse.ui.internal.keys.WorkbenchKeyboard.executeCommand(WorkbenchKeyboard.java:471) at org.eclipse.ui.internal.keys.WorkbenchKeyboard.press(WorkbenchKeyboard.java:822) at org.eclipse.ui.internal.keys.WorkbenchKeyboard.processKeyEvent(WorkbenchKeyboard.java:880) at org.eclipse.ui.internal.keys.WorkbenchKeyboard.filterKeySequenceBindings(WorkbenchKeyboard.java:569) at org.eclipse.ui.internal.keys.WorkbenchKeyboard.access$3(WorkbenchKeyboard.java:511) at org.eclipse.ui.internal.keys.WorkbenchKeyboard$KeyDownFilter.handleEvent(WorkbenchKeyboard.java:126) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Display.filterEvent(Display.java:1433) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1153) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1178) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1163) at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:1190) at org.eclipse.swt.widgets.Widget.gtk_key_press_event(Widget.java:696) at org.eclipse.swt.widgets.Control.gtk_key_press_event(Control.java:2709) at org.eclipse.swt.widgets.Composite.gtk_key_press_event(Composite.java:700) at org.eclipse.swt.widgets.Widget.windowProc(Widget.java:1539) at org.eclipse.swt.widgets.Control.windowProc(Control.java:4450) at org.eclipse.swt.widgets.Display.windowProc(Display.java:4096) 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:5783) at org.eclipse.swt.widgets.Display.eventProc(Display.java:1175) 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:1541) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3028) at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2394) at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2358) at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2210) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:494) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:288) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:489) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:112) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:193) 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:379) 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:549) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:504) at org.eclipse.equinox.launcher.Main.run(Main.java:1236)
Move to JDT/UI
Reproduced in I20080502-0100 Must fix.
This seems to be a JDT/Core bug: ITypeBinding.getTypeDeclaration() returns null, but it is not spec'd to do that. Strange thing is, that if you use the organize imports action Ctrl-Shift-O then getTypeDeclaration returns the correct binding for List<IEntity>, but if you use clean up, or save action then a null is returned. Both the save action and the organize imports action are using the shared AST from the editor. Setting to major: This is very likely to happen and makes the organize imports save action unusable. I can not reproduce in 3.3.1.1.
Created attachment 98608 [details] JDT UI test case
The difference is that Clean up didn't use binding recovery. There a critical bug here with binding recovery == false: - Open the AST view on the given source - In the view menu select: 'Use ASTParser.createAST' and disable 'Use Binding Recovery': AST view crashes (NPE). ITypeBinding.getTypeDeclaration() must not return null.
(In reply to comment #5) > The difference is that Clean up didn't use binding recovery. See bug 230170
Martin, Benno, what would be a good value to return for you in this case? Would the receiver work for you?
If recovered bindings are turned off, the binding for the AST node 'List<IEntity>' should return null.
I agree. If the binding recovery is turned off and any part of a binding can return null, then this means this binding should not be returned.
Created attachment 98638 [details] Proposed fix
Olivier, please see the proposed fix. This will need a regression test as well. It also looks suspicious that we still have paths in the code that return a non-null binding when recovery is off and a binding is tagged has having a missing type. Can you please double-check all paths in the DOM?
Kent, could you please review this case? Added regression tests: org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0298 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0299 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0300 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0301 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0302 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0303 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0304 org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0305
Created attachment 99279 [details] Complete patch (fix + regression tests)
(In reply to comment #4) > Created an attachment (id=98608) [details] > JDT UI test case Benno, I believe your regression test should also have import java.util.List; in the imports after the clean up, not just import java.util.ArrayList;.
With the fix, the test org.eclipse.jdt.ui.tests.quickfix.CleanUpTest#testOrganizeImportsBug229570 is now green.
(In reply to comment #14) > (In reply to comment #4) > > Created an attachment (id=98608) [details] [details] > > JDT UI test case > Benno, I believe your regression test should also have import java.util.List; > in the imports after the clean up, not just import java.util.ArrayList;. > Yes, you're right.
I found the same kind of inconsistency for variable binding. If the type of a variable binding cannot be resolved and the binding recovery is off, no binding should be returned for the variable binding. I'll attach a new patch for this.
Same issue for method declaration that contains types that cannot be resolved: either return type or type for a parameter. I'll run all existing tests (Core and UI).
Frederic, With the next patch I found an inconsistent test in the DOM javadoc tests. One test was expecting to bind a binding for a reference in the javadoc when the return type of the method could not be resolved. I'll fix the test in the patch as well. Benno, Could you please run the patch on JDT/UI tests ? I am doing it right now, but in case of issues, I'd like someone to review them.
(In reply to comment #5) > The difference is that Clean up didn't use binding recovery. I think this is an inconsistency for the user. The same action (Organize imports) runs inside the Java editor or on save doesn't run the same way (the former is using the binding recovery and the latter doesn't). I believe this should be fixed.
(In reply to comment #20) > (In reply to comment #5) > > The difference is that Clean up didn't use binding recovery. > I think this is an inconsistency for the user. The same action (Organize > imports) runs inside the Java editor or on save doesn't run the same way (the > former is using the binding recovery and the latter doesn't). > I believe this should be fixed. > See comment #6 (In reply to comment #19) > Could you please run the patch on JDT/UI tests ? > I am doing it right now, but in case of issues, I'd like someone to review > them. I'm running the tests now.
Created attachment 99314 [details] Complete patch (fix + regression tests) Benno, Here is the patch to test. I won't release if the tests don't pass even if the reason is that the tests are invalid (making wrong assumptions when binding recovery is off). All DOM JDT/Core tests are now green.
(In reply to comment #19) > Frederic, > > With the next patch I found an inconsistent test in the DOM javadoc tests. > One test was expecting to bind a binding for a reference in the javadoc when > the return type of the method could not be resolved. > I'll fix the test in the patch as well. > OK for me to change the test case
Kent, Could you please review the new patch? It covers method and variable bindings as well. Benno confirmed that all JDT/UI tests are green with the patch.
Released for 3.4RC1. Regression tests added: org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0298() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0299() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0300() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0301() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0302() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0303() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0304() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0305() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0306() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0307() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0308() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0309() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0310() org.eclipse.jdt.core.tests.dom.ASTConverter15Test.test0311() org.eclipse.jdt.core.tests.dom.ASTConverterAST3Test.test0234_2() org.eclipse.jdt.core.tests.dom.ASTConverterTest.test0234_2() org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2.test0667_2() org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2.test0666_2() Updated: org.eclipse.jdt.core.tests.dom.ASTConverterAST3Test.test0234() org.eclipse.jdt.core.tests.dom.ASTConverterTest.test0234() org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2.test0666() org.eclipse.jdt.core.tests.dom.ASTConverterTestAST3_2.test0667()
*** Bug 231262 has been marked as a duplicate of this bug. ***
Verified for 3.4RC1 using I20080510-2000
