Unexpected hash collision. Fixes #86584

src/vs/base/common/hash.ts

@@ -8,7 +8,12 @@ import * as strings from 'vs/base/common/strings';
 /**
  * Return a hash value for an object.
  */
-export function hash(obj: any, hashVal = 0): number {
+export function hash(obj: any): number {
+	return doHash(obj, 0);
+}
+
+
+export function doHash(obj: any, hashVal: number): number {
 	switch (typeof obj) {
 		case 'object':
 			if (obj === null) {
@@ -24,9 +29,9 @@ export function hash(obj: any, hashVal = 0): number {
 		case 'number':
 			return numberHash(obj, hashVal);
 		case 'undefined':
-			return numberHash(0, 937);
+			return numberHash(937, hashVal);
 		default:
-			return numberHash(0, 617);
+			return numberHash(617, hashVal);
 	}
 }
 
@@ -48,14 +53,14 @@ export function stringHash(s: string, hashVal: number) {
 
 function arrayHash(arr: any[], initialHashVal: number): number {
 	initialHashVal = numberHash(104579, initialHashVal);
-	return arr.reduce((hashVal, item) => hash(item, hashVal), initialHashVal);
+	return arr.reduce((hashVal, item) => doHash(item, hashVal), initialHashVal);
 }
 
 function objectHash(obj: any, initialHashVal: number): number {
 	initialHashVal = numberHash(181387, initialHashVal);
 	return Object.keys(obj).sort().reduce((hashVal, key) => {
 		hashVal = stringHash(key, hashVal);
-		return hash(obj[key], hashVal);
+		return doHash(obj[key], hashVal);
 	}, initialHashVal);
 }
 
@@ -68,7 +73,7 @@ export class Hasher {
 	}
 
 	hash(obj: any): number {
-		this._value = hash(obj, this._value);
+		this._value = doHash(obj, this._value);
 		return this._value;
 	}
 }

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

@@ -32,12 +32,18 @@ suite('Hash', () => {
 		assert.equal(hash([1, 2, 3]), hash([1, 2, 3]));
 		assert.equal(hash(['foo', 'bar']), hash(['foo', 'bar']));
 		assert.equal(hash([]), hash([]));
+		assert.equal(hash([]), hash(new Array()));
 		assert.notEqual(hash(['foo', 'bar']), hash(['bar', 'foo']));
 		assert.notEqual(hash(['foo', 'bar']), hash(['bar', 'foo', null]));
+		assert.notEqual(hash(['foo', 'bar', null]), hash(['bar', 'foo', null]));
+		assert.notEqual(hash(['foo', 'bar']), hash(['bar', 'foo', undefined]));
+		assert.notEqual(hash(['foo', 'bar', undefined]), hash(['bar', 'foo', undefined]));
+		assert.notEqual(hash(['foo', 'bar', null]), hash(['foo', 'bar', undefined]));
 	});
 
 	test('object', () => {
 		assert.equal(hash({}), hash({}));
+		assert.equal(hash({}), hash(Object.create(null)));
 		assert.equal(hash({ 'foo': 'bar' }), hash({ 'foo': 'bar' }));
 		assert.equal(hash({ 'foo': 'bar', 'foo2': undefined }), hash({ 'foo2': undefined, 'foo': 'bar' }));
 		assert.notEqual(hash({ 'foo': 'bar' }), hash({ 'foo': 'bar2' }));
@@ -45,14 +51,25 @@ suite('Hash', () => {
 	});
 
 	test('array - unexpected collision', function () {
-		this.skip();
 		const a = hash([undefined, undefined, undefined, undefined, undefined]);
 		const b = hash([undefined, undefined, 'HHHHHH', [{ line: 0, character: 0 }, { line: 0, character: 0 }], undefined]);
-		// console.log(a);
-		// console.log(b);
 		assert.notEqual(a, b);
 	});
 
+	test('all different', () => {
+		const candidates: any[] = [
+			null, undefined, {}, [], 0, false, true, '', ' ', [null], [undefined], [undefined, undefined], { '': undefined }, { [' ']: undefined }
+		];
+		const hashes: number[] = candidates.map(hash);
+		for (let i = 0; i < hashes.length; i++) {
+			assert.equal(hashes[i], hash(candidates[i])); // verify that repeated invocation returns the same hash
+			for (let k = i + 1; k < hashes.length; k++) {
+				assert.notEqual(hashes[i], hashes[k], `Same hash ${hashes[i]} for ${JSON.stringify(candidates[i])} ans ${JSON.stringify(candidates[k])}`);
+			}
+		}
+	});
+
+
 	function checkSHA1(strings: string[], expected: string) {
 		const hash = new StringSHA1();
 		for (const str of strings) {