towards separation of IServiceCollection and IInstantiationService, start with createChild

src/vs/editor/common/commonCodeEditor.ts

@@ -12,7 +12,7 @@ import {IDisposable, dispose} from 'vs/base/common/lifecycle';
 import * as objects from 'vs/base/common/objects';
 import * as timer from 'vs/base/common/timer';
 import {TPromise} from 'vs/base/common/winjs.base';
-import {IInstantiationService} from 'vs/platform/instantiation/common/instantiation';
+import {IInstantiationService, ServiceCollection} from 'vs/platform/instantiation/common/instantiation';
 import {IKeybindingContextKey, IKeybindingScopeLocation, IKeybindingService} from 'vs/platform/keybinding/common/keybindingService';
 import {ITelemetryService} from 'vs/platform/telemetry/common/telemetry';
 import {CommonEditorConfiguration} from 'vs/editor/common/config/commonEditorConfig';
@@ -114,9 +114,7 @@ export abstract class CommonCodeEditor extends EventEmitter implements IActionPr
 		this.forcedWidgetFocusCount = 0;
 
 		this._telemetryService = telemetryService;
-		this._instantiationService = instantiationService.createChild({
-			keybindingService: this._keybindingService
-		});
+		this._instantiationService = instantiationService.createChild(new ServiceCollection([IKeybindingService, this._keybindingService]));
 
 		this._attachModel(null);
 

src/vs/editor/contrib/referenceSearch/browser/referenceSearchWidget.ts

@@ -25,7 +25,7 @@ import * as tree from 'vs/base/parts/tree/browser/tree';
 import {DefaultController, LegacyRenderer} from 'vs/base/parts/tree/browser/treeDefaults';
 import {Tree} from 'vs/base/parts/tree/browser/treeImpl';
 import {IEditorService} from 'vs/platform/editor/common/editor';
-import {IInstantiationService} from 'vs/platform/instantiation/common/instantiation';
+import {IInstantiationService, ServiceCollection} from 'vs/platform/instantiation/common/instantiation';
 import {IKeybindingService} from 'vs/platform/keybinding/common/keybindingService';
 import {IWorkspaceContextService} from 'vs/platform/workspace/common/workspace';
 import {DefaultConfig} from 'vs/editor/common/config/defaultConfig';
@@ -34,7 +34,7 @@ import * as editorCommon from 'vs/editor/common/editorCommon';
 import {Model} from 'vs/editor/common/model/model';
 import {ICodeEditor, IMouseTarget} from 'vs/editor/browser/editorBrowser';
 import {EmbeddedCodeEditorWidget} from 'vs/editor/browser/widget/embeddedCodeEditorWidget';
-import {PeekViewWidget} from 'vs/editor/contrib/zoneWidget/browser/peekViewWidget';
+import {PeekViewWidget, IPeekViewService} from 'vs/editor/contrib/zoneWidget/browser/peekViewWidget';
 import {EventType, FileReferences, OneReference, ReferencesModel} from './referenceSearchModel';
 
 class DecorationsManager implements IDisposable {
@@ -398,7 +398,7 @@ export class ReferenceWidget extends PeekViewWidget {
 		super(editor, keybindingService, ReferenceWidget.INNER_EDITOR_CONTEXT_KEY, { frameColor: '#007ACC', showFrame: false, showArrow: true });
 		this.editorService = editorService;
 		this.contextService = contextService;
-		this.instantiationService = instantiationService.createChild({ peekViewService: this });
+		this.instantiationService = instantiationService.createChild(new ServiceCollection([IPeekViewService, this]));
 
 		this.callOnModel = [];
 

src/vs/platform/instantiation/common/instantiation.ts

@@ -5,6 +5,7 @@
 'use strict';
 
 import {TPromise} from 'vs/base/common/winjs.base';
+import {binarySearch} from 'vs/base/common/arrays';
 import * as descriptors from './descriptors';
 
 // ----------------------- internal util -----------------------
@@ -103,6 +104,11 @@ export interface IFunctionSignature8<A1, A2, A3, A4, A5, A6, A7, A8, R> {
 	(accessor: ServicesAccessor, first: A1, second: A2, third: A3, forth: A4, fifth: A5, sixth: A6, seventh: A7, eigth: A8): R;
 }
 
+export interface IServiceCollection {
+	add<T>(id: ServiceIdentifier<T>, instanceOrDescriptor: T | descriptors.SyncDescriptor<T>): void;
+	forEach(callback: (id: ServiceIdentifier<any>, instanceOrDescriptor: any) => any): void;
+	has(id: ServiceIdentifier<any>): boolean;
+}
 
 export var IInstantiationService = createDecorator<IInstantiationService>('instantiationService');
 
@@ -175,7 +181,7 @@ export interface IInstantiationService {
 	 * Creates a child of this service which inherts all current services
 	 * and adds/overwrites the given services
 	 */
-	createChild(services: any): IInstantiationService;
+	createChild(services: IServiceCollection): IInstantiationService;
 
 	/**
 	 * Registers a new service to this instantation service.
@@ -223,10 +229,47 @@ export function createDecorator<T>(serviceId: string): { (...args: any[]): void;
 	return <any>ret;
 }
 
-/**
- * A service context which can be used to retrieve services
- * given a valid service identifer is being presented
- */
-export interface Context {
-	get<T>(id: ServiceIdentifier<T>): T;
+type Entry = [ServiceIdentifier<any>, any];
+
+export class ServiceCollection implements IServiceCollection {
+
+	private _entries: Entry[] = [];
+
+	constructor(...entries:[ServiceIdentifier<any>, any][]) {
+		for (let entry of entries) {
+			this.add(entry[0], entry[1]);
+		}
+	}
+
+	add<T>(id: ServiceIdentifier<T>, instanceOrDescriptor: T | descriptors.SyncDescriptor<T>): void {
+		const entry: Entry = [id, instanceOrDescriptor];
+		const idx = ~binarySearch(this._entries, entry, ServiceCollection._entryCompare);
+		if (idx < 0) {
+			throw new Error(`service with that identifier already registered`);
+		}
+		this._entries.splice(idx, 0, entry);
+	}
+
+	forEach(callback: (id: ServiceIdentifier<any>, instanceOrDescriptor: any) => any): void {
+		for (let entry of this._entries) {
+			let [id, instanceOrDescriptor] = entry;
+			callback(id, instanceOrDescriptor);
+		}
+	}
+
+	has(id: ServiceIdentifier<any>): boolean {
+		return binarySearch(this._entries, <Entry>[id,], ServiceCollection._entryCompare) >= 0;
+	}
+
+	private static _entryCompare(a: Entry, b: Entry): number {
+		const _a = _util.getServiceId(a[0]);
+		const _b = _util.getServiceId(b[0]);
+		if (_a < _b) {
+			return -1;
+		} else if (_a > _b) {
+			return 1;
+		} else {
+			return 0;
+		}
+	}
 }

src/vs/platform/instantiation/common/instantiationService.ts

@@ -4,13 +4,12 @@
  *--------------------------------------------------------------------------------------------*/
 'use strict';
 
-import * as winjs from 'vs/base/common/winjs.base';
-import * as errors from 'vs/base/common/errors';
-import * as strings from 'vs/base/common/strings';
-import * as types from 'vs/base/common/types';
-import * as collections from 'vs/base/common/collections';
-import * as descriptors from './descriptors';
+import {TPromise} from 'vs/base/common/winjs.base';
+import {illegalArgument, illegalState, canceled} from 'vs/base/common/errors';
+import {create} from 'vs/base/common/types';
+import {forEach} from 'vs/base/common/collections';
 import {Graph} from 'vs/base/common/graph';
+import * as descriptors from './descriptors';
 import * as instantiation from './instantiation';
 
 import IInstantiationService = instantiation.IInstantiationService;
@@ -45,7 +44,7 @@ class AccessLock {
 class ServicesMap {
 
 	constructor(private _services: any, private _lock: AccessLock) {
-		collections.forEach(this._services, (entry) => {
+		forEach(this._services, (entry) => {
 
 			// add a accessor to myselves
 			this.registerService(entry.key, entry.value);
@@ -57,10 +56,10 @@ class ServicesMap {
 		Object.defineProperty(this, name, {
 			get: () => {
 				if (this._lock.locked) {
-					throw errors.illegalState('the services map can only be used during construction');
+					throw illegalState('the services map can only be used during construction');
 				}
 				if (!service) {
-					throw errors.illegalArgument(strings.format('service with \'{0}\' not found', name));
+					throw illegalArgument(`service with '${name}' not found`);
 				}
 				if (service instanceof descriptors.SyncDescriptor) {
 					let cached = this._services[name];
@@ -74,7 +73,7 @@ class ServicesMap {
 				return service;
 			},
 			set: (value: any) => {
-				throw errors.illegalState('services cannot be changed');
+				throw illegalState('services cannot be changed');
 			},
 			configurable: false,
 			enumerable: false
@@ -172,12 +171,12 @@ class ServicesMap {
 		let fixedArguments = descriptor.staticArguments().concat(args);
 		let expectedFirstServiceIndex = fixedArguments.length;
 		let actualFirstServiceIndex = Number.MAX_VALUE;
-		serviceInjections.forEach(si => {
+		serviceInjections.forEach(serviceInjection => {
 			// @IServiceName
-			let {serviceId, index} = si;
+			let {serviceId, index} = serviceInjection;
 			let service = this._lock.runUnlocked(() => this[serviceId]);
 			allArguments[index] = service;
-			actualFirstServiceIndex = Math.min(actualFirstServiceIndex, si.index);
+			actualFirstServiceIndex = Math.min(actualFirstServiceIndex, index);
 		});
 
 		// insert the fixed arguments into the array of all ctor
@@ -209,7 +208,7 @@ class ServicesMap {
 		}
 
 		return this._lock.runUnlocked(() => {
-			const instance = types.create.apply(null, allArguments);
+			const instance = create.apply(null, allArguments);
 			descriptor._validate(instance);
 			return <T>instance;
 		});
@@ -226,17 +225,24 @@ class InstantiationService implements IInstantiationService {
 		this._servicesMap = new ServicesMap(services, lock);
 	}
 
-	createChild(services: any): IInstantiationService {
-		const childServices = {};
-		// copy existing services
-		collections.forEach(this._servicesMap.services, (entry) => {
-			childServices[entry.key] = entry.value;
+	createChild(services: instantiation.ServiceCollection): IInstantiationService {
+
+		const result = new InstantiationService(Object.create(null), this._servicesMap.lock);
+
+		// insert new services
+		services.forEach((id, instanceOrDescriptor) => {
+			result.addSingleton(id, instanceOrDescriptor);
 		});
-		// insert new services (might overwrite)
-		collections.forEach(services, (entry) => {
-			childServices[entry.key] = entry.value;
+
+		// fill gaps existing services
+		forEach(this._servicesMap.services, (entry) => {
+			let id = instantiation.createDecorator(entry.key);
+			if (!services.has(id) && entry.key !== 'instantiationService') {
+				result.addSingleton(id, entry.value);
+			}
 		});
-		return new InstantiationService(childServices, this._servicesMap.lock);
+
+		return result;
 	}
 
 	registerService(name: string, service: any): void {
@@ -274,18 +280,18 @@ class InstantiationService implements IInstantiationService {
 	createInstance<A1, A2, A3, A4, A5, A6, A7, T>(descriptor: descriptors.SyncDescriptor7<A1, A2, A3, A4, A5, A6, A7, T>, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5, a6: A6, a7: A7): T;
 	createInstance<A1, A2, A3, A4, A5, A6, A7, A8, T>(descriptor: descriptors.SyncDescriptor8<A1, A2, A3, A4, A5, A6, A7, A8, T>, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5, a6: A6, a7: A7, a8: A8): T;
 
-	createInstance<T>(descriptor: descriptors.AsyncDescriptor0<T>): winjs.TPromise<T>;
-	createInstance<A1, T>(descriptor: descriptors.AsyncDescriptor1<A1, T>, a1: A1): winjs.TPromise<T>;
-	createInstance<A1, A2, T>(descriptor: descriptors.AsyncDescriptor2<A1, A2, T>, a1: A1, a2: A2): winjs.TPromise<T>;
-	createInstance<A1, A2, A3, T>(descriptor: descriptors.AsyncDescriptor3<A1, A2, A3, T>, a1: A1, a2: A2, a3: A3): winjs.TPromise<T>;
-	createInstance<A1, A2, A3, A4, T>(descriptor: descriptors.AsyncDescriptor4<A1, A2, A3, A4, T>, a1: A1, a2: A2, a3: A3, a4: A4): winjs.TPromise<T>;
-	createInstance<A1, A2, A3, A4, A5, T>(descriptor: descriptors.AsyncDescriptor5<A1, A2, A3, A4, A5, T>, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5): winjs.TPromise<T>;
-	createInstance<A1, A2, A3, A4, A5, A6, T>(descriptor: descriptors.AsyncDescriptor6<A1, A2, A3, A4, A5, A6, T>, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5, a6: A6): winjs.TPromise<T>;
-	createInstance<A1, A2, A3, A4, A5, A6, A7, T>(descriptor: descriptors.AsyncDescriptor7<A1, A2, A3, A4, A5, A6, A7, T>, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5, a6: A6, a7: A7): winjs.TPromise<T>;
-	createInstance<A1, A2, A3, A4, A5, A6, A7, A8, T>(descriptor: descriptors.AsyncDescriptor8<A1, A2, A3, A4, A5, A6, A7, A8, T>, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5, a6: A6, a7: A7, a8: A8): winjs.TPromise<T>;
+	createInstance<T>(descriptor: descriptors.AsyncDescriptor0<T>): TPromise<T>;
+	createInstance<A1, T>(descriptor: descriptors.AsyncDescriptor1<A1, T>, a1: A1): TPromise<T>;
+	createInstance<A1, A2, T>(descriptor: descriptors.AsyncDescriptor2<A1, A2, T>, a1: A1, a2: A2): TPromise<T>;
+	createInstance<A1, A2, A3, T>(descriptor: descriptors.AsyncDescriptor3<A1, A2, A3, T>, a1: A1, a2: A2, a3: A3): TPromise<T>;
+	createInstance<A1, A2, A3, A4, T>(descriptor: descriptors.AsyncDescriptor4<A1, A2, A3, A4, T>, a1: A1, a2: A2, a3: A3, a4: A4): TPromise<T>;
+	createInstance<A1, A2, A3, A4, A5, T>(descriptor: descriptors.AsyncDescriptor5<A1, A2, A3, A4, A5, T>, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5): TPromise<T>;
+	createInstance<A1, A2, A3, A4, A5, A6, T>(descriptor: descriptors.AsyncDescriptor6<A1, A2, A3, A4, A5, A6, T>, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5, a6: A6): TPromise<T>;
+	createInstance<A1, A2, A3, A4, A5, A6, A7, T>(descriptor: descriptors.AsyncDescriptor7<A1, A2, A3, A4, A5, A6, A7, T>, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5, a6: A6, a7: A7): TPromise<T>;
+	createInstance<A1, A2, A3, A4, A5, A6, A7, A8, T>(descriptor: descriptors.AsyncDescriptor8<A1, A2, A3, A4, A5, A6, A7, A8, T>, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5, a6: A6, a7: A7, a8: A8): TPromise<T>;
 
 	createInstance<T>(descriptor: descriptors.SyncDescriptor<T>, ...args: any[]): T;
-	createInstance<T>(descriptor: descriptors.AsyncDescriptor<T>, ...args: any[]): winjs.TPromise<T>;
+	createInstance<T>(descriptor: descriptors.AsyncDescriptor<T>, ...args: any[]): TPromise<T>;
 
 	createInstance<T>(param: any): any {
 
@@ -303,18 +309,18 @@ class InstantiationService implements IInstantiationService {
 		}
 	}
 
-	_createInstanceAsync<T>(descriptor: descriptors.AsyncDescriptor<T>, args: any[]): winjs.TPromise<T> {
+	_createInstanceAsync<T>(descriptor: descriptors.AsyncDescriptor<T>, args: any[]): TPromise<T> {
 
-		let canceled: Error;
+		let canceledError: Error;
 
-		return new winjs.TPromise((c, e, p) => {
+		return new TPromise((c, e, p) => {
 			require([descriptor.moduleName], (_module?: any) => {
-				if (canceled) {
-					e(canceled);
+				if (canceledError) {
+					e(canceledError);
 				}
 
 				if (!_module) {
-					return e(errors.illegalArgument('module not found: ' + descriptor.moduleName));
+					return e(illegalArgument('module not found: ' + descriptor.moduleName));
 				}
 
 				let ctor: Function;
@@ -325,7 +331,7 @@ class InstantiationService implements IInstantiationService {
 				}
 
 				if (typeof ctor !== 'function') {
-					return e(errors.illegalArgument('not a function: ' + descriptor.ctorName || descriptor.moduleName));
+					return e(illegalArgument('not a function: ' + descriptor.ctorName || descriptor.moduleName));
 				}
 
 				try {
@@ -336,7 +342,7 @@ class InstantiationService implements IInstantiationService {
 				}
 			}, e);
 		}, () => {
-			canceled = errors.canceled();
+			canceledError = canceled();
 		});
 	}
 

src/vs/platform/instantiation/test/common/instantiationService.test.ts

@@ -123,6 +123,29 @@ class ServiceLoop2 implements IService2 {
 }
 
 suite('Instantiation Service', () => {
+
+	test('service collection, cannot overwrite', function () {
+		let collection = new instantiation.ServiceCollection();
+		collection.add(IService1, null);
+		assert.throws(() => collection.add(IService1, null));
+	});
+
+	test('service collection, add/has', function () {
+		let collection = new instantiation.ServiceCollection();
+		collection.add(IService1, null);
+		assert.ok(collection.has(IService1));
+
+		collection.add(IService2, null);
+		assert.ok(collection.has(IService1));
+		assert.ok(collection.has(IService2));
+	});
+
+	test('addSingleton - cannot overwrite service', function() {
+		let service = instantiationService.createInstantiationService(Object.create(null));
+		service.addSingleton(IService1, new Service1());
+		assert.throws(() => service.addSingleton(IService1, new Service1()));
+	});
+
 	test('@Param - simple clase', function() {
 		let service = instantiationService.createInstantiationService(Object.create(null));
 		service.addSingleton(IService1, new Service1());

src/vs/workbench/browser/parts/compositePart.ts

@@ -30,7 +30,7 @@ import {IPartService} from 'vs/workbench/services/part/common/partService';
 import {IStorageService, StorageScope} from 'vs/platform/storage/common/storage';
 import {IContextMenuService} from 'vs/platform/contextview/browser/contextView';
 import {IEventService} from 'vs/platform/event/common/event';
-import {IInstantiationService} from 'vs/platform/instantiation/common/instantiation';
+import {IInstantiationService, ServiceCollection} from 'vs/platform/instantiation/common/instantiation';
 import {IMessageService, Severity} from 'vs/platform/message/common/message';
 import {IProgressService} from 'vs/platform/progress/common/progress';
 import {ITelemetryService} from 'vs/platform/telemetry/common/telemetry';
@@ -176,10 +176,7 @@ export abstract class CompositePart<T extends Composite> extends Part {
 			let loaderPromise = this.compositeLoaderPromises[id];
 			if (!loaderPromise) {
 				let progressService = new WorkbenchProgressService(this.eventService, this.progressBar, compositeDescriptor.id, isActive);
-				let services = {
-					progressService: progressService
-				};
-				let compositeInstantiationService = this.instantiationService.createChild(services);
+				let compositeInstantiationService = this.instantiationService.createChild(new ServiceCollection([IProgressService, progressService]));
 
 				loaderPromise = compositeInstantiationService.createInstance(compositeDescriptor).then((composite: Composite) => {
 					this.mapProgressServiceToComposite[composite.getId()] = progressService;

src/vs/workbench/browser/parts/editor/editorPart.ts

@@ -35,9 +35,10 @@ import {IPartService} from 'vs/workbench/services/part/common/partService';
 import {Position, POSITIONS} from 'vs/platform/editor/common/editor';
 import {IStorageService} from 'vs/platform/storage/common/storage';
 import {IEventService} from 'vs/platform/event/common/event';
-import {IInstantiationService} from 'vs/platform/instantiation/common/instantiation';
+import {IInstantiationService, ServiceCollection} from 'vs/platform/instantiation/common/instantiation';
 import {IMessageService, IMessageWithAction, Severity} from 'vs/platform/message/common/message';
 import {ITelemetryService} from 'vs/platform/telemetry/common/telemetry';
+import {IProgressService} from 'vs/platform/progress/common/progress';
 
 const EDITOR_STATE_STORAGE_KEY = 'editorpart.editorState';
 
@@ -755,11 +756,10 @@ export class EditorPart extends Part implements IEditorPart {
 	}
 
 	private createEditor(editorDescriptor: EditorDescriptor, editorDomNode: HTMLElement, position: Position): TPromise<BaseEditor> {
-		let services = {
-			progressService: new WorkbenchProgressService(this.eventService, this.sideBySideControl.getProgressBar(position), editorDescriptor.getId(), true)
-		};
 
-		let editorInstantiationService = this.instantiationService.createChild(services);
+		let progressService = new WorkbenchProgressService(this.eventService, this.sideBySideControl.getProgressBar(position), editorDescriptor.getId(), true);
+
+		let editorInstantiationService = this.instantiationService.createChild(new ServiceCollection([IProgressService, progressService]));
 
 		return editorInstantiationService.createInstance(editorDescriptor);
 	}

src/vs/workbench/browser/parts/editor/textDiffEditor.ts

@@ -31,7 +31,7 @@ import {IWorkspaceContextService} from 'vs/workbench/services/workspace/common/c
 import {IStorageService} from 'vs/platform/storage/common/storage';
 import {IConfigurationService} from 'vs/platform/configuration/common/configuration';
 import {IEventService} from 'vs/platform/event/common/event';
-import {IInstantiationService} from 'vs/platform/instantiation/common/instantiation';
+import {IInstantiationService, ServiceCollection} from 'vs/platform/instantiation/common/instantiation';
 import {IMessageService} from 'vs/platform/message/common/message';
 import {IWorkbenchEditorService} from 'vs/workbench/services/editor/common/editorService';
 import {IModeService} from 'vs/editor/common/services/modeService';
@@ -111,9 +111,7 @@ export class TextDiffEditor extends BaseTextEditor {
 		});
 
 		// Create a special child of instantiator that will delegate all calls to openEditor() to the same diff editor if the input matches with the modified one
-		let diffEditorInstantiator = this.instantiationService.createChild({
-			editorService: delegatingService
-		});
+		let diffEditorInstantiator = this.instantiationService.createChild(new ServiceCollection([IWorkbenchEditorService, delegatingService]));
 
 		return diffEditorInstantiator.createInstance(DiffEditorWidget, parent.getHTMLElement(), this.getCodeEditorOptions());
 	}