Description Knut Wannheden 2011-01-17 04:20:49 EST

The class EMFBasedPersister declares a dependency on the Eclipse IWorkbench:

	@Inject(optional=true)
	private IWorkbench workbench;

The Guice 'optional' parameter only means that there needn't be any binding for the injection key (IWorkbech.class). In this case the point is that the implementation can live with the workbench being 'null'. It should thus instead (or in addition) define the binding as @Nullable:


	@Inject(optional=true) @Nullable
	private IWorkbench workbench;

This then fits together with binding made in the SharedModule:


	bind(IWorkbench.class).toProvider(new Provider<IWorkbench>() {
		public IWorkbench get() {
			if (PlatformUI.isWorkbenchRunning())
				return PlatformUI.getWorkbench();
			return null;
		}
	});
 
The @Nullable should possibly also be used for other bindings in the Xtext code base.

In the case of the EMFBasedPersister I think the IWorkbench binding is wrong altogether. Instead the implementation should use the already present IWorkspace and use addSaveParticipant() to register a callback to save the index.