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: Set and Map content is also compared
   - 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: Set and Map content is also compared
   - version: v6.4.0, v4.7.1
     pr-url: https://github.com/nodejs/node/pull/8002
     description: Typed array slices are handled correctly now.
@@ -113,7 +122,8 @@ changes:
 Generally identical to `assert.deepEqual()` with three exceptions:
 
 1. Primitive values are compared using the [Strict Equality Comparison][]
-  ( `===` ).
+  ( `===` ). Set values and Map keys are compared using the [SameValueZero][]
+  comparison. (Which means they are free of the [caveats][]).
 2. [`[[Prototype]]`][prototype-spec] of objects are compared using
   the [Strict Equality Comparison][] too.
 3. [Type tags][Object.prototype.toString()] of objects should be the same.
@@ -576,10 +586,13 @@ For more information, see
 [`assert.ok()`]: #assert_assert_ok_value_message
 [`assert.throws()`]: #assert_assert_throws_block_error_message
 [`Error`]: errors.html#errors_class_error
+[caveats]: #assert_caveats
 [`RegExp`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions
 [`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,97 @@ function _deepEqual(actual, expected, strict, memos) {
   return objEquiv(actual, expected, strict, memos);
 }
 
+function setHasSimilarElement(set, val1, strict, memo) {
+  if (set.has(val1))
+    return true;
+
+  // In strict mode the only things which can match a primitive or a function
+  // will already be detected by set.has(val1).
+  if (strict && (util.isPrimitive(val1) || util.isFunction(val1)))
+    return false;
+
+  // Otherwise go looking.
+  for (const val2 of set) {
+    if (_deepEqual(val1, val2, strict, memo))
+      return true;
+  }
+
+  return false;
+}
+
+function setEquiv(a, b, strict, memo) {
+  // 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 (!setHasSimilarElement(b, val1, strict, memo)) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+function mapHasSimilarEntry(map, key1, item1, strict, memo) {
+  // 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. The loop performs this check, but
+  // doing it here improves performance of the common case when reference-equal
+  // keys exist (which includes all primitive-valued keys).
+  if (map.has(key1) && _deepEqual(item1, map.get(key1), strict, memo))
+    return true;
+
+  if (strict && (util.isPrimitive(key1) || util.isFunction(key1)))
+    return false;
+
+  for (const [key2, item2] of map) {
+    // This case is checked above.
+    if (key2 === key1)
+      continue;
+
+    if (_deepEqual(key1, key2, strict, memo) &&
+        _deepEqual(item1, item2, strict, memo)) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+function mapEquiv(a, b, strict, memo) {
+  // 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;
+
+  for (const [key1, item1] of a) {
+    // Just like setEquiv above, this hunt makes this function O(n^2) when
+    // using objects and lists as keys
+    if (!mapHasSimilarEntry(b, key1, item1, strict, memo))
+      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 +400,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,172 @@ 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]]));
+
+assertDeepAndStrictEqual(new Set([{}]), new Set([{}]));
+
+// 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 */