Merge branch 'dbaeumer/linkedMapIterators'

src/vs/base/common/map.ts

@@ -587,11 +587,14 @@ export class LinkedMap<K, V> {
 	private _tail: Item<K, V> | undefined;
 	private _size: number;
 
+	private _state: number;
+
 	constructor() {
 		this._map = new Map<K, Item<K, V>>();
 		this._head = undefined;
 		this._tail = undefined;
 		this._size = 0;
+		this._state = 0;
 	}
 
 	clear(): void {
@@ -599,6 +602,7 @@ export class LinkedMap<K, V> {
 		this._head = undefined;
 		this._tail = undefined;
 		this._size = 0;
+		this._state++;
 	}
 
 	isEmpty(): boolean {
@@ -701,36 +705,44 @@ export class LinkedMap<K, V> {
 		}
 	}
 
-	values(): V[] {
-		const result: V[] = [];
+	keys(): IterableIterator<K> {
+		const map = this;
+		const state = this._state;
 		let current = this._head;
-		while (current) {
-			result.push(current.value);
-			current = current.next;
+		const iterator: IterableIterator<K> = {
+			[Symbol.iterator]() {
+				return iterator;
+			},
+			next(): IteratorResult<K> {
+				if (map._state !== state) {
+					throw new Error(`Map got modified during iteration.`);
 				}
+				if (current) {
+					const result = { value: current.key, done: false };
+					current = current.next;
 					return result;
+				} else {
+					return { value: undefined, done: true };
 				}
-
-	keys(): K[] {
-		const result: K[] = [];
-		let current = this._head;
-		while (current) {
-			result.push(current.key);
-			current = current.next;
 			}
-		return result;
+		};
+		return iterator;
 	}
 
-	/* VS Code / Monaco editor runs on es5 which has no Symbol.iterator
-	keys(): IterableIterator<K> {
-		const current = this._head;
-		const iterator: IterableIterator<K> = {
+	values(): IterableIterator<V> {
+		const map = this;
+		const state = this._state;
+		let current = this._head;
+		const iterator: IterableIterator<V> = {
 			[Symbol.iterator]() {
 				return iterator;
 			},
-			next():IteratorResult<K> {
+			next(): IteratorResult<V> {
+				if (map._state !== state) {
+					throw new Error(`Map got modified during iteration.`);
+				}
 				if (current) {
-					const result = { value: current.key, done: false };
+					const result = { value: current.value, done: false };
 					current = current.next;
 					return result;
 				} else {
@@ -741,15 +753,20 @@ export class LinkedMap<K, V> {
 		return iterator;
 	}
 
-	values(): IterableIterator<V> {
-		const current = this._head;
-		const iterator: IterableIterator<V> = {
+	entries(): IterableIterator<[K, V]> {
+		const map = this;
+		const state = this._state;
+		let current = this._head;
+		const iterator: IterableIterator<[K, V]> = {
 			[Symbol.iterator]() {
 				return iterator;
 			},
-			next():IteratorResult<V> {
+			next(): IteratorResult<[K, V]> {
+				if (map._state !== state) {
+					throw new Error(`Map got modified during iteration.`);
+				}
 				if (current) {
-					const result = { value: current.value, done: false };
+					const result: IteratorResult<[K, V]> = { value: [current.key, current.value], done: false };
 					current = current.next;
 					return result;
 				} else {
@@ -759,7 +776,10 @@ export class LinkedMap<K, V> {
 		};
 		return iterator;
 	}
-	*/
+
+	[Symbol.iterator](): IterableIterator<[K, V]> {
+		return this.entries();
+	}
 
 	protected trimOld(newSize: number) {
 		if (newSize >= this.size) {
@@ -781,6 +801,7 @@ export class LinkedMap<K, V> {
 		if (current) {
 			current.previous = undefined;
 		}
+		this._state++;
 	}
 
 	private addItemFirst(item: Item<K, V>): void {
@@ -794,6 +815,7 @@ export class LinkedMap<K, V> {
 			this._head.previous = item;
 		}
 		this._head = item;
+		this._state++;
 	}
 
 	private addItemLast(item: Item<K, V>): void {
@@ -807,6 +829,7 @@ export class LinkedMap<K, V> {
 			this._tail.next = item;
 		}
 		this._tail = item;
+		this._state++;
 	}
 
 	private removeItem(item: Item<K, V>): void {
@@ -843,6 +866,7 @@ export class LinkedMap<K, V> {
 		}
 		item.next = undefined;
 		item.previous = undefined;
+		this._state++;
 	}
 
 	private touch(item: Item<K, V>, touch: Touch): void {
@@ -879,6 +903,7 @@ export class LinkedMap<K, V> {
 			item.next = this._head;
 			this._head.previous = item;
 			this._head = item;
+			this._state++;
 		} else if (touch === Touch.AsNew) {
 			if (item === this._tail) {
 				return;
@@ -902,6 +927,7 @@ export class LinkedMap<K, V> {
 			item.previous = this._tail;
 			this._tail.next = item;
 			this._tail = item;
+			this._state++;
 		}
 	}
 
@@ -953,8 +979,8 @@ export class LRUCache<K, V> extends LinkedMap<K, V> {
 		this.checkTrim();
 	}
 
-	get(key: K): V | undefined {
-		return super.get(key, Touch.AsNew);
+	get(key: K, touch: Touch = Touch.AsNew): V | undefined {
+		return super.get(key, touch);
 	}
 
 	peek(key: K): V | undefined {

src/vs/base/test/common/map.test.ts

@@ -13,8 +13,8 @@ suite('Map', () => {
 		let map = new LinkedMap<string, string>();
 		map.set('ak', 'av');
 		map.set('bk', 'bv');
-		assert.deepStrictEqual(map.keys(), ['ak', 'bk']);
-		assert.deepStrictEqual(map.values(), ['av', 'bv']);
+		assert.deepStrictEqual([...map.keys()], ['ak', 'bk']);
+		assert.deepStrictEqual([...map.values()], ['av', 'bv']);
 		assert.equal(map.first, 'av');
 		assert.equal(map.last, 'bv');
 	});
@@ -23,16 +23,16 @@ suite('Map', () => {
 		let map = new LinkedMap<string, string>();
 		map.set('ak', 'av');
 		map.set('ak', 'av', Touch.AsOld);
-		assert.deepStrictEqual(map.keys(), ['ak']);
-		assert.deepStrictEqual(map.values(), ['av']);
+		assert.deepStrictEqual([...map.keys()], ['ak']);
+		assert.deepStrictEqual([...map.values()], ['av']);
 	});
 
 	test('LinkedMap - Touch New one', () => {
 		let map = new LinkedMap<string, string>();
 		map.set('ak', 'av');
 		map.set('ak', 'av', Touch.AsNew);
-		assert.deepStrictEqual(map.keys(), ['ak']);
-		assert.deepStrictEqual(map.values(), ['av']);
+		assert.deepStrictEqual([...map.keys()], ['ak']);
+		assert.deepStrictEqual([...map.values()], ['av']);
 	});
 
 	test('LinkedMap - Touch Old two', () => {
@@ -40,8 +40,8 @@ suite('Map', () => {
 		map.set('ak', 'av');
 		map.set('bk', 'bv');
 		map.set('bk', 'bv', Touch.AsOld);
-		assert.deepStrictEqual(map.keys(), ['bk', 'ak']);
-		assert.deepStrictEqual(map.values(), ['bv', 'av']);
+		assert.deepStrictEqual([...map.keys()], ['bk', 'ak']);
+		assert.deepStrictEqual([...map.values()], ['bv', 'av']);
 	});
 
 	test('LinkedMap - Touch New two', () => {
@@ -49,8 +49,8 @@ suite('Map', () => {
 		map.set('ak', 'av');
 		map.set('bk', 'bv');
 		map.set('ak', 'av', Touch.AsNew);
-		assert.deepStrictEqual(map.keys(), ['bk', 'ak']);
-		assert.deepStrictEqual(map.values(), ['bv', 'av']);
+		assert.deepStrictEqual([...map.keys()], ['bk', 'ak']);
+		assert.deepStrictEqual([...map.values()], ['bv', 'av']);
 	});
 
 	test('LinkedMap - Touch Old from middle', () => {
@@ -59,8 +59,8 @@ suite('Map', () => {
 		map.set('bk', 'bv');
 		map.set('ck', 'cv');
 		map.set('bk', 'bv', Touch.AsOld);
-		assert.deepStrictEqual(map.keys(), ['bk', 'ak', 'ck']);
-		assert.deepStrictEqual(map.values(), ['bv', 'av', 'cv']);
+		assert.deepStrictEqual([...map.keys()], ['bk', 'ak', 'ck']);
+		assert.deepStrictEqual([...map.values()], ['bv', 'av', 'cv']);
 	});
 
 	test('LinkedMap - Touch New from middle', () => {
@@ -69,8 +69,8 @@ suite('Map', () => {
 		map.set('bk', 'bv');
 		map.set('ck', 'cv');
 		map.set('bk', 'bv', Touch.AsNew);
-		assert.deepStrictEqual(map.keys(), ['ak', 'ck', 'bk']);
-		assert.deepStrictEqual(map.values(), ['av', 'cv', 'bv']);
+		assert.deepStrictEqual([...map.keys()], ['ak', 'ck', 'bk']);
+		assert.deepStrictEqual([...map.values()], ['av', 'cv', 'bv']);
 	});
 
 	test('LinkedMap - basics', function () {
@@ -129,6 +129,61 @@ suite('Map', () => {
 		assert.ok(!map.has('1'));
 	});
 
+	test('LinkedMap - Iterators', () => {
+		const map = new LinkedMap<number, any>();
+		map.set(1, 1);
+		map.set(2, 2);
+		map.set(3, 3);
+
+		for (const elem of map.keys()) {
+			assert.ok(elem);
+		}
+
+		for (const elem of map.values()) {
+			assert.ok(elem);
+		}
+
+		for (const elem of map.entries()) {
+			assert.ok(elem);
+		}
+
+		{
+			const keys = map.keys();
+			const values = map.values();
+			const entries = map.entries();
+			map.get(1);
+			keys.next();
+			values.next();
+			entries.next();
+		}
+
+		{
+			const keys = map.keys();
+			const values = map.values();
+			const entries = map.entries();
+			map.get(1, Touch.AsNew);
+
+			let exceptions: number = 0;
+			try {
+				keys.next();
+			} catch (err) {
+				exceptions++;
+			}
+			try {
+				values.next();
+			} catch (err) {
+				exceptions++;
+			}
+			try {
+				entries.next();
+			} catch (err) {
+				exceptions++;
+			}
+
+			assert.strictEqual(exceptions, 3);
+		}
+	});
+
 	test('LinkedMap - LRU Cache simple', () => {
 		const cache = new LRUCache<number, number>(5);
 
@@ -136,10 +191,10 @@ suite('Map', () => {
 		assert.strictEqual(cache.size, 5);
 		cache.set(6, 6);
 		assert.strictEqual(cache.size, 5);
-		assert.deepStrictEqual(cache.keys(), [2, 3, 4, 5, 6]);
+		assert.deepStrictEqual([...cache.keys()], [2, 3, 4, 5, 6]);
 		cache.set(7, 7);
 		assert.strictEqual(cache.size, 5);
-		assert.deepStrictEqual(cache.keys(), [3, 4, 5, 6, 7]);
+		assert.deepStrictEqual([...cache.keys()], [3, 4, 5, 6, 7]);
 		let values: number[] = [];
 		[3, 4, 5, 6, 7].forEach(key => values.push(cache.get(key)!));
 		assert.deepStrictEqual(values, [3, 4, 5, 6, 7]);
@@ -150,11 +205,11 @@ suite('Map', () => {
 
 		[1, 2, 3, 4, 5].forEach(value => cache.set(value, value));
 		assert.strictEqual(cache.size, 5);
-		assert.deepStrictEqual(cache.keys(), [1, 2, 3, 4, 5]);
+		assert.deepStrictEqual([...cache.keys()], [1, 2, 3, 4, 5]);
 		cache.get(3);
-		assert.deepStrictEqual(cache.keys(), [1, 2, 4, 5, 3]);
+		assert.deepStrictEqual([...cache.keys()], [1, 2, 4, 5, 3]);
 		cache.peek(4);
-		assert.deepStrictEqual(cache.keys(), [1, 2, 4, 5, 3]);
+		assert.deepStrictEqual([...cache.keys()], [1, 2, 4, 5, 3]);
 		let values: number[] = [];
 		[1, 2, 3, 4, 5].forEach(key => values.push(cache.get(key)!));
 		assert.deepStrictEqual(values, [1, 2, 3, 4, 5]);
@@ -169,7 +224,7 @@ suite('Map', () => {
 		assert.strictEqual(cache.size, 10);
 		cache.limit = 5;
 		assert.strictEqual(cache.size, 5);
-		assert.deepStrictEqual(cache.keys(), [6, 7, 8, 9, 10]);
+		assert.deepStrictEqual([...cache.keys()], [6, 7, 8, 9, 10]);
 		cache.limit = 20;
 		assert.strictEqual(cache.size, 5);
 		for (let i = 11; i <= 20; i++) {
@@ -181,7 +236,7 @@ suite('Map', () => {
 			values.push(cache.get(i)!);
 			assert.strictEqual(cache.get(i), i);
 		}
-		assert.deepStrictEqual(cache.values(), values);
+		assert.deepStrictEqual([...cache.values()], values);
 	});
 
 	test('LinkedMap - LRU Cache limit with ratio', () => {
@@ -193,11 +248,11 @@ suite('Map', () => {
 		assert.strictEqual(cache.size, 10);
 		cache.set(11, 11);
 		assert.strictEqual(cache.size, 5);
-		assert.deepStrictEqual(cache.keys(), [7, 8, 9, 10, 11]);
+		assert.deepStrictEqual([...cache.keys()], [7, 8, 9, 10, 11]);
 		let values: number[] = [];
-		cache.keys().forEach(key => values.push(cache.get(key)!));
+		[...cache.keys()].forEach(key => values.push(cache.get(key)!));
 		assert.deepStrictEqual(values, [7, 8, 9, 10, 11]);
-		assert.deepStrictEqual(cache.values(), values);
+		assert.deepStrictEqual([...cache.values()], values);
 	});
 
 	test('LinkedMap - toJSON / fromJSON', () => {
@@ -222,7 +277,6 @@ suite('Map', () => {
 				assert.equal(key, 'ck');
 				assert.equal(value, 'cv');
 			}
-
 			i++;
 		});
 	});
@@ -237,7 +291,7 @@ suite('Map', () => {
 		map.delete('1');
 		assert.equal(map.get('1'), undefined);
 		assert.equal(map.size, 0);
-		assert.equal(map.keys().length, 0);
+		assert.equal([...map.keys()].length, 0);
 	});
 
 	test('LinkedMap - delete Head', function () {
@@ -251,8 +305,8 @@ suite('Map', () => {
 		map.delete('1');
 		assert.equal(map.get('2'), 2);
 		assert.equal(map.size, 1);
-		assert.equal(map.keys().length, 1);
-		assert.equal(map.keys()[0], 2);
+		assert.equal([...map.keys()].length, 1);
+		assert.equal([...map.keys()][0], 2);
 	});
 
 	test('LinkedMap - delete Tail', function () {
@@ -266,8 +320,8 @@ suite('Map', () => {
 		map.delete('2');
 		assert.equal(map.get('1'), 1);
 		assert.equal(map.size, 1);
-		assert.equal(map.keys().length, 1);
-		assert.equal(map.keys()[0], 1);
+		assert.equal([...map.keys()].length, 1);
+		assert.equal([...map.keys()][0], 1);
 	});
 
 

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

@@ -48,7 +48,7 @@ export class EditorsObserver extends Disposable {
 	}
 
 	get editors(): IEditorIdentifier[] {
-		return this.mostRecentEditorsMap.values();
+		return [...this.mostRecentEditorsMap.values()];
 	}
 
 	hasEditor(resource: URI): boolean {
@@ -283,7 +283,7 @@ export class EditorsObserver extends Disposable {
 
 		// Across all editor groups
 		else {
-			await this.doEnsureOpenedEditorsLimit(limit, this.mostRecentEditorsMap.values(), exclude);
+			await this.doEnsureOpenedEditorsLimit(limit, [...this.mostRecentEditorsMap.values()], exclude);
 		}
 	}
 
@@ -346,7 +346,7 @@ export class EditorsObserver extends Disposable {
 	private serialize(): ISerializedEditorsList {
 		const registry = Registry.as<IEditorInputFactoryRegistry>(Extensions.EditorInputFactories);
 
-		const entries = this.mostRecentEditorsMap.values();
+		const entries = [...this.mostRecentEditorsMap.values()];
 		const mapGroupToSerializableEditorsOfGroup = new Map<IEditorGroup, IEditorInput[]>();
 
 		return {

src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts

@@ -19,7 +19,7 @@ import * as strings from 'vs/base/common/strings';
 import { ValidationStatus, ValidationState } from 'vs/base/common/parsers';
 import * as UUID from 'vs/base/common/uuid';
 import * as Platform from 'vs/base/common/platform';
-import { LRUCache } from 'vs/base/common/map';
+import { LRUCache, Touch } from 'vs/base/common/map';
 
 import { ILifecycleService } from 'vs/platform/lifecycle/common/lifecycle';
 import { IMarkerService } from 'vs/platform/markers/common/markers';
@@ -715,9 +715,10 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 		const folderToTasksMap: Map<string, any> = new Map();
 		const recentlyUsedTasks = this.getRecentlyUsedTasks();
 		const tasks: (Task | ConfiguringTask)[] = [];
-		for (const key of recentlyUsedTasks.keys()) {
+		for (const entry of recentlyUsedTasks.entries()) {
+			const key = entry[0];
+			const task = JSON.parse(entry[1]);
 			const folder = this.getFolderFromTaskKey(key);
-			const task = JSON.parse(recentlyUsedTasks.get(key)!);
 			if (folder && !folderToTasksMap.has(folder)) {
 				folderToTasksMap.set(folder, []);
 			}
@@ -791,13 +792,13 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 		if (quickOpenHistoryLimit === 0) {
 			return;
 		}
-		let keys = this._recentlyUsedTasks.keys();
+		let keys = [...this._recentlyUsedTasks.keys()];
 		if (keys.length > quickOpenHistoryLimit) {
 			keys = keys.slice(0, quickOpenHistoryLimit);
 		}
 		const keyValues: [string, string][] = [];
 		for (const key of keys) {
-			keyValues.push([key, this._recentlyUsedTasks.get(key)!]);
+			keyValues.push([key, this._recentlyUsedTasks.get(key, Touch.None)!]);
 		}
 		this.storageService.store(AbstractTaskService.RecentlyUsedTasks_KeyV2, JSON.stringify(keyValues), StorageScope.WORKSPACE);
 	}
@@ -2325,7 +2326,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
 				taskMap[key] = task;
 			}
 		});
-		const reversed = recentlyUsedTasks.keys().reverse();
+		const reversed = [...recentlyUsedTasks.keys()].reverse();
 		for (const key in reversed) {
 			let task = taskMap[key];
 			if (task) {