Hash symbols and valueOf results

__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;

__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();
+    });
+  });
+});

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,14 +205,30 @@ 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;
 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') {