assert: Add support for Map and Set in deepEqual

doc/api/assert.md

@@ -18,6 +18,9 @@ An alias of [`assert.ok()`][].
 <!-- YAML
 added: v0.1.21
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12142
+    description: Sets and Maps are handled correctly.
   - version: v6.4.0, v4.7.1
     pr-url: https://github.com/nodejs/node/pull/8002
     description: Typed array slices are handled correctly now.
@@ -40,7 +43,7 @@ Only [enumerable "own" properties][] are considered. The
 [`assert.deepEqual()`][] implementation does not test the
 [`[[Prototype]]`][prototype-spec] of objects, attached symbols, or
 non-enumerable properties — for such checks, consider using
-[assert.deepStrictEqual()][] instead. This can lead to some
+[`assert.deepStrictEqual()`][] instead. This can lead to some
 potentially surprising results. For example, the following example does not
 throw an `AssertionError` because the properties on the [`Error`][] object are
 not enumerable:
@@ -50,6 +53,9 @@ not enumerable:
 assert.deepEqual(Error('a'), Error('b'));
 ```
 
+An exception is made for [`Map`][] and [`Set`][]. Maps and Sets have their
+contained items compared too, as expected.
+
 "Deep" equality means that the enumerable "own" properties of child objects
 are evaluated also:
 
@@ -96,6 +102,9 @@ parameter is undefined, a default error message is assigned.
 <!-- YAML
 added: v1.2.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/12142
+    description: Sets and Maps are handled correctly.
   - version: v6.4.0, v4.7.1
     pr-url: https://github.com/nodejs/node/pull/8002
     description: Typed array slices are handled correctly now.
@@ -148,6 +157,21 @@ assert.deepStrictEqual(date, fakeDate);
 // Different type tags
 ```
 
+An exception is made to the strict equality comparison rule for [`Map`][] keys
+and [`Set`][] items. These tests also inherit the rules of `Set.prototype.has`
+and `Map.prototype.has`, which differs from strict equality comparison in
+also allowing NaN to be considered equal to itself:
+
+```js
+assert.deepStrictEqual(NaN, NaN);
+// AssertionError: NaN deepStrictEqual NaN
+// because NaN !== NaN
+
+// But this is ok:
+assert.deepStrictEqual(new Set([NaN]), new Set([NaN]));
+assert.deepStrictEqual(new Map([[NaN, 1]]), new Map([[NaN, 1]]));
+```
+
 If the values are not equal, an `AssertionError` is thrown with a `message`
 property set equal to the value of the `message` parameter. If the `message`
 parameter is undefined, a default error message is assigned.
@@ -580,6 +604,8 @@ For more information, see
 [`TypeError`]: errors.html#errors_class_typeerror
 [Abstract Equality Comparison]: https://tc39.github.io/ecma262/#sec-abstract-equality-comparison
 [Strict Equality Comparison]: https://tc39.github.io/ecma262/#sec-strict-equality-comparison
+[`Map`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Map
+[`Set`]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Set
 [`Object.is()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is
 [SameValueZero]: https://tc39.github.io/ecma262/#sec-samevaluezero
 [prototype-spec]: https://tc39.github.io/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots

lib/assert.js

@@ -23,6 +23,7 @@
 // UTILITY
 const compare = process.binding('buffer').compare;
 const util = require('util');
+const { isSet, isMap } = process.binding('util');
 const objectToString = require('internal/util').objectToString;
 const Buffer = require('buffer').Buffer;
 
@@ -262,11 +263,12 @@ function _deepEqual(actual, expected, strict, memos) {
     }
   }
 
-  // For all other Object pairs, including Array objects,
+  // For all other Object pairs, including Array objects and Maps,
   // equivalence is determined by having:
   // a) The same number of owned enumerable properties
   // b) The same set of keys/indexes (although not necessarily the same order)
   // c) Equivalent values for every corresponding key/index
+  // d) For Sets and Maps, equal contents
   // Note: this accounts for both named and indexed properties on Arrays.
 
   // Use memos to handle cycles.
@@ -283,6 +285,86 @@ function _deepEqual(actual, expected, strict, memos) {
   return objEquiv(actual, expected, strict, memos);
 }
 
+function setHasSimilarElement(set, val1, strict, actualVisitedObjects) {
+  for (const val2 of set) {
+    if (_deepEqual(val1, val2, strict, actualVisitedObjects))
+      return true;
+  }
+  return false;
+}
+
+function setEquiv(a, b, strict, actualVisitedObjects) {
+  // This code currently returns false for this pair of sets:
+  //   assert.deepEqual(new Set(['1', 1]), new Set([1]))
+  //
+  // In theory, all the items in the first set have a corresponding == value in
+  // the second set, but the sets have different sizes. Its a silly case,
+  // and more evidence that deepStrictEqual should always be preferred over
+  // deepEqual.
+  if (a.size !== b.size)
+    return false;
+
+  for (const val1 of a) {
+    // If the value doesn't exist in the second set by reference, and its an
+    // object or an array we'll need to go hunting for something thats
+    // deep-equal to it. Note that this is O(n^2) complexity, and will get
+    // slower if large, very similar sets / maps are nested inside.
+    // Unfortunately there's no real way around this.
+    if (!b.has(val1)
+        && (!strict || (!util.isObject(val1) && !util.isFunction(val1)))
+        && !setHasSimilarElement(b, val1, strict, actualVisitedObjects)) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+function mapEquiv(a, b, strict, actualVisitedObjects) {
+  // Caveat: In non-strict mode, this implementation does not handle cases
+  // where maps contain two equivalent-but-not-reference-equal keys.
+  //
+  // For example, maps like this are currently considered not equivalent:
+  if (a.size !== b.size)
+    return false;
+
+  outer: for (const [key1, item1] of a) {
+    // To be able to handle cases like:
+    //   Map([[1, 'a'], ['1', 'b']]) vs Map([['1', 'a'], [1, 'b']])
+    // or:
+    //   Map([[{}, 'a'], [{}, 'b']]) vs Map([[{}, 'b'], [{}, 'a']])
+    // ... we need to consider *all* matching keys, not just the first we find.
+
+    // This check is not strictly necessary if we run the loop below, but it
+    // improves performance of the common case when reference-equal keys exist
+    // (which includes all primitive-valued keys).
+    if (b.has(key1)) {
+      if (_deepEqual(item1, b.get(key1), strict, actualVisitedObjects))
+        continue outer;
+    }
+
+    // Hunt for keys which are deep-equal to key1 in b. Just like setEquiv
+    // above, this hunt makes this function O(n^2) when using objects and lists
+    // as keys
+    if (!strict || (typeof key1 === 'object' && key1 !== null)) {
+      for (const [key2, item2] of b) {
+        // Just for performance. We already checked these keys above.
+        if (key2 === key1)
+          continue;
+
+        if (_deepEqual(key1, key2, strict, actualVisitedObjects) &&
+            _deepEqual(item1, item2, strict, actualVisitedObjects)) {
+          continue outer;
+        }
+      }
+    }
+
+    return false;
+  }
+
+  return true;
+}
+
 function objEquiv(a, b, strict, actualVisitedObjects) {
   // If one of them is a primitive, the other must be the same.
   if (util.isPrimitive(a) || util.isPrimitive(b))
@@ -307,6 +389,22 @@ function objEquiv(a, b, strict, actualVisitedObjects) {
       return false;
   }
 
+  // Sets and maps don't have their entries accessible via normal object
+  // properties.
+  if (isSet(a)) {
+    if (!isSet(b) || !setEquiv(a, b, strict, actualVisitedObjects))
+      return false;
+  } else if (isSet(b)) {
+    return false;
+  }
+
+  if (isMap(a)) {
+    if (!isMap(b) || !mapEquiv(a, b, strict, actualVisitedObjects))
+      return false;
+  } else if (isMap(b)) {
+    return false;
+  }
+
   // The pair must have equivalent values for every corresponding key.
   // Possibly expensive deep test:
   for (i = aKeys.length - 1; i >= 0; i--) {

test/parallel/test-assert-deep.js

@@ -100,11 +100,170 @@ const similar = new Set([
 for (const a of similar) {
   for (const b of similar) {
     if (a !== b) {
-      assert.doesNotThrow(() => assert.deepEqual(a, b));
+      assert.deepEqual(a, b);
       assert.throws(() => assert.deepStrictEqual(a, b),
                     re`${a} deepStrictEqual ${b}`);
     }
   }
 }
 
+function assertDeepAndStrictEqual(a, b) {
+  assert.deepEqual(a, b);
+  assert.deepStrictEqual(a, b);
+
+  assert.deepEqual(b, a);
+  assert.deepStrictEqual(b, a);
+}
+
+function assertNotDeepOrStrict(a, b) {
+  assert.throws(() => assert.deepEqual(a, b));
+  assert.throws(() => assert.deepStrictEqual(a, b));
+
+  assert.throws(() => assert.deepEqual(b, a));
+  assert.throws(() => assert.deepStrictEqual(b, a));
+}
+
+function assertOnlyDeepEqual(a, b) {
+  assert.doesNotThrow(() => assert.deepEqual(a, b));
+  assert.throws(() => assert.deepStrictEqual(a, b));
+
+  assert.doesNotThrow(() => assert.deepEqual(b, a));
+  assert.throws(() => assert.deepStrictEqual(b, a));
+}
+
+// es6 Maps and Sets
+assertDeepAndStrictEqual(new Set(), new Set());
+assertDeepAndStrictEqual(new Map(), new Map());
+
+assertDeepAndStrictEqual(new Set([1, 2, 3]), new Set([1, 2, 3]));
+assertNotDeepOrStrict(new Set([1, 2, 3]), new Set([1, 2, 3, 4]));
+assertNotDeepOrStrict(new Set([1, 2, 3, 4]), new Set([1, 2, 3]));
+assertDeepAndStrictEqual(new Set(['1', '2', '3']), new Set(['1', '2', '3']));
+assertDeepAndStrictEqual(new Set([[1, 2], [3, 4]]), new Set([[3, 4], [1, 2]]));
+
+assertDeepAndStrictEqual(new Map([[1, 1], [2, 2]]), new Map([[1, 1], [2, 2]]));
+assertDeepAndStrictEqual(new Map([[1, 1], [2, 2]]), new Map([[2, 2], [1, 1]]));
+assertNotDeepOrStrict(new Map([[1, 1], [2, 2]]), new Map([[1, 2], [2, 1]]));
+
+assertNotDeepOrStrict(new Set([1]), [1]);
+assertNotDeepOrStrict(new Set(), []);
+assertNotDeepOrStrict(new Set(), {});
+
+assertNotDeepOrStrict(new Map([['a', 1]]), {a: 1});
+assertNotDeepOrStrict(new Map(), []);
+assertNotDeepOrStrict(new Map(), {});
+
+assertOnlyDeepEqual(new Set(['1']), new Set([1]));
+
+assertOnlyDeepEqual(new Map([['1', 'a']]), new Map([[1, 'a']]));
+assertOnlyDeepEqual(new Map([['a', '1']]), new Map([['a', 1]]));
+
+// This is an awful case, where a map contains multiple equivalent keys:
+assertOnlyDeepEqual(
+  new Map([[1, 'a'], ['1', 'b']]),
+  new Map([['1', 'a'], [1, 'b']])
+);
+assertDeepAndStrictEqual(
+  new Map([[{}, 'a'], [{}, 'b']]),
+  new Map([[{}, 'b'], [{}, 'a']])
+);
+
+{
+  const values = [
+    123,
+    Infinity,
+    0,
+    null,
+    undefined,
+    false,
+    true,
+    {},
+    [],
+    () => {},
+  ];
+  assertDeepAndStrictEqual(new Set(values), new Set(values));
+  assertDeepAndStrictEqual(new Set(values), new Set(values.reverse()));
+
+  const mapValues = values.map((v) => [v, {a: 5}]);
+  assertDeepAndStrictEqual(new Map(mapValues), new Map(mapValues));
+  assertDeepAndStrictEqual(new Map(mapValues), new Map(mapValues.reverse()));
+}
+
+{
+  const s1 = new Set();
+  const s2 = new Set();
+  s1.add(1);
+  s1.add(2);
+  s2.add(2);
+  s2.add(1);
+  assertDeepAndStrictEqual(s1, s2);
+}
+
+{
+  const m1 = new Map();
+  const m2 = new Map();
+  const obj = {a: 5, b: 6};
+  m1.set(1, obj);
+  m1.set(2, 'hi');
+  m1.set(3, [1, 2, 3]);
+
+  m2.set(2, 'hi'); // different order
+  m2.set(1, obj);
+  m2.set(3, [1, 2, 3]); // deep equal, but not reference equal.
+
+  assertDeepAndStrictEqual(m1, m2);
+}
+
+{
+  const m1 = new Map();
+  const m2 = new Map();
+
+  // m1 contains itself.
+  m1.set(1, m1);
+  m2.set(1, new Map());
+
+  assertNotDeepOrStrict(m1, m2);
+}
+
+assert.deepEqual(new Map([[1, 1]]), new Map([[1, '1']]));
+assert.throws(() =>
+  assert.deepStrictEqual(new Map([[1, 1]]), new Map([[1, '1']]))
+);
+
+{
+  // Two equivalent sets / maps with different key/values applied shouldn't be
+  // the same. This is a terrible idea to do in practice, but deepEqual should
+  // still check for it.
+  const s1 = new Set();
+  const s2 = new Set();
+  s1.x = 5;
+  assertNotDeepOrStrict(s1, s2);
+
+  const m1 = new Map();
+  const m2 = new Map();
+  m1.x = 5;
+  assertNotDeepOrStrict(m1, m2);
+}
+
+{
+  // Circular references.
+  const s1 = new Set();
+  s1.add(s1);
+  const s2 = new Set();
+  s2.add(s2);
+  assertDeepAndStrictEqual(s1, s2);
+
+  const m1 = new Map();
+  m1.set(2, m1);
+  const m2 = new Map();
+  m2.set(2, m2);
+  assertDeepAndStrictEqual(m1, m2);
+
+  const m3 = new Map();
+  m3.set(m3, 2);
+  const m4 = new Map();
+  m4.set(m4, 2);
+  assertDeepAndStrictEqual(m3, m4);
+}
+
 /* eslint-enable */