diff --git a/__tests__/hash.ts b/__tests__/hash.ts index 5c2b0ebad1..9d24980243 100644 --- a/__tests__/hash.ts +++ b/__tests__/hash.ts @@ -36,6 +36,13 @@ describe('hash', () => { expect(hash(objA)).not.toBe(hash(objB)); }); + it('generates different hashes for different symbols', () => { + const symA = Symbol(); + const symB = Symbol(); + expect(hash(symA)).toBe(hash(symA)); + expect(hash(symA)).not.toBe(hash(symB)); + }); + it('generates different hashes for different functions', () => { const funA = () => { return; diff --git a/__tests__/issues.ts b/__tests__/issues.ts index ee4963254b..9e079f0f92 100644 --- a/__tests__/issues.ts +++ b/__tests__/issues.ts @@ -101,3 +101,27 @@ describe('Issue #1293', () => { expect(secondState).toEqual(firstState); }); }); + +describe('Issue #1643', () => { + [ + ['a string', 'test'], + ['a number', 5], + ['null', null], + ['undefined', undefined], + ['a boolean', true], + ['an object', {}], + ['an array', []], + ['a function', () => null], + ].forEach(([label, value]) => { + class MyClass { + valueOf() { + return value; + } + } + + it(`Collection#hashCode() should handle objects that return ${label} for valueOf`, () => { + const set = Set().add(new MyClass()); + set.hashCode(); + }); + }); +}); diff --git a/src/Hash.js b/src/Hash.js index f15466ffb9..c63a0b686e 100644 --- a/src/Hash.js +++ b/src/Hash.js @@ -10,41 +10,50 @@ import { smi } from './Math'; const defaultValueOf = Object.prototype.valueOf; export function hash(o) { - switch (typeof o) { + if (o == null) { + return hashNullish(o); + } + + if (typeof o.hashCode === 'function') { + // Drop any high bits from accidentally long hash codes. + return smi(o.hashCode(o)); + } + + const v = valueOf(o); + + if (v == null) { + return hashNullish(v); + } + + switch (typeof v) { case 'boolean': // The hash values for built-in constants are a 1 value for each 5-byte // shift region expect for the first, which encodes the value. This // reduces the odds of a hash collision for these common values. - return o ? 0x42108421 : 0x42108420; + return v ? 0x42108421 : 0x42108420; case 'number': - return hashNumber(o); + return hashNumber(v); case 'string': - return o.length > STRING_HASH_CACHE_MIN_STRLEN - ? cachedHashString(o) - : hashString(o); + return v.length > STRING_HASH_CACHE_MIN_STRLEN + ? cachedHashString(v) + : hashString(v); case 'object': case 'function': - if (o === null) { - return 0x42108422; - } - if (typeof o.hashCode === 'function') { - // Drop any high bits from accidentally long hash codes. - return smi(o.hashCode(o)); - } - if (o.valueOf !== defaultValueOf && typeof o.valueOf === 'function') { - o = o.valueOf(o); - } - return hashJSObj(o); - case 'undefined': - return 0x42108423; + return hashJSObj(v); + case 'symbol': + return hashSymbol(v); default: - if (typeof o.toString === 'function') { - return hashString(o.toString()); + if (typeof v.toString === 'function') { + return hashString(v.toString()); } - throw new Error('Value type ' + typeof o + ' cannot be hashed.'); + throw new Error('Value type ' + typeof v + ' cannot be hashed.'); } } +function hashNullish(nullish) { + return nullish === null ? 0x42108422 : /* undefined */ 0x42108423; +} + // Compress arbitrarily large numbers into smi hashes. function hashNumber(n) { if (n !== n || n === Infinity) { @@ -90,6 +99,19 @@ function hashString(string) { return smi(hashed); } +function hashSymbol(sym) { + let hashed = symbolMap[sym]; + if (hashed !== undefined) { + return hashed; + } + + hashed = nextHash(); + + symbolMap[sym] = hashed; + + return hashed; +} + function hashJSObj(obj) { let hashed; if (usingWeakMap) { @@ -116,10 +138,7 @@ function hashJSObj(obj) { } } - hashed = ++objHashUID; - if (objHashUID & 0x40000000) { - objHashUID = 0; - } + hashed = nextHash(); if (usingWeakMap) { weakMap.set(obj, hashed); @@ -186,6 +205,20 @@ function getIENodeHash(node) { } } +function valueOf(obj) { + return obj.valueOf !== defaultValueOf && typeof obj.valueOf === 'function' + ? obj.valueOf(obj) + : obj; +} + +function nextHash() { + const nextHash = ++_objHashUID; + if (_objHashUID & 0x40000000) { + _objHashUID = 0; + } + return nextHash; +} + // If possible, use a WeakMap. const usingWeakMap = typeof WeakMap === 'function'; let weakMap; @@ -193,7 +226,9 @@ if (usingWeakMap) { weakMap = new WeakMap(); } -let objHashUID = 0; +const symbolMap = Object.create(null); + +let _objHashUID = 0; let UID_HASH_KEY = '__immutablehash__'; if (typeof Symbol === 'function') {