diff --git a/lib/assert.js b/lib/assert.js index 5aca51119484b6..bd36c9a16d1b38 100644 --- a/lib/assert.js +++ b/lib/assert.js @@ -285,9 +285,12 @@ function _deepEqual(actual, expected, strict, memos) { return areEq; } -function setHasSimilarElement(set, val1, strict, memo) { - if (set.has(val1)) +function setHasSimilarElement(set, val1, usedEntries, strict, memo) { + if (set.has(val1)) { + if (usedEntries) + usedEntries.add(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). @@ -296,8 +299,14 @@ function setHasSimilarElement(set, val1, strict, memo) { // Otherwise go looking. for (const val2 of set) { - if (_deepEqual(val1, val2, strict, memo)) + if (usedEntries && usedEntries.has(val2)) + continue; + + if (_deepEqual(val1, val2, strict, memo)) { + if (usedEntries) + usedEntries.add(val2); return true; + } } return false; @@ -314,21 +323,33 @@ function setEquiv(a, b, strict, memo) { if (a.size !== b.size) return false; + // This is a set of the entries in b which have been consumed in our pairwise + // comparison. + // + // When the sets contain only value types (eg, lots of numbers), and we're in + // strict mode, we don't need to match off the entries in a pairwise way. In + // that case this initialization is done lazily to avoid the allocation & + // bookkeeping cost. Unfortunately, we can't get away with that in non-strict + // mode. + let usedEntries = null; + for (const val1 of a) { + if (usedEntries == null && (!strict || typeof val1 === 'object')) + usedEntries = new Set(); + // 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)) { + if (!setHasSimilarElement(b, val1, usedEntries, strict, memo)) return false; - } } return true; } -function mapHasSimilarEntry(map, key1, item1, strict, memo) { +function mapHasSimilarEntry(map, key1, item1, usedEntries, strict, memo) { // To be able to handle cases like: // Map([[1, 'a'], ['1', 'b']]) vs Map([['1', 'a'], [1, 'b']]) // or: @@ -338,8 +359,11 @@ function mapHasSimilarEntry(map, key1, item1, strict, memo) { // 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)) + if (map.has(key1) && _deepEqual(item1, map.get(key1), strict, memo)) { + if (usedEntries) + usedEntries.add(key1); return true; + } if (strict && (util.isPrimitive(key1) || util.isFunction(key1))) return false; @@ -349,8 +373,13 @@ function mapHasSimilarEntry(map, key1, item1, strict, memo) { if (key2 === key1) continue; + if (usedEntries && usedEntries.has(key2)) + continue; + if (_deepEqual(key1, key2, strict, memo) && _deepEqual(item1, item2, strict, memo)) { + if (usedEntries) + usedEntries.add(key2); return true; } } @@ -366,10 +395,15 @@ function mapEquiv(a, b, strict, memo) { if (a.size !== b.size) return false; + let usedEntries = null; + for (const [key1, item1] of a) { + if (usedEntries == null && (!strict || typeof key1 === 'object')) + usedEntries = new Set(); + // 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)) + if (!mapHasSimilarEntry(b, key1, item1, usedEntries, strict, memo)) return false; } diff --git a/test/parallel/test-assert-deep.js b/test/parallel/test-assert-deep.js index f6fed8411b5213..fecb5c89dbe3e7 100644 --- a/test/parallel/test-assert-deep.js +++ b/test/parallel/test-assert-deep.js @@ -187,6 +187,28 @@ assertOnlyDeepEqual(new Map([['a', '1']]), new Map([['a', 1]])); assertDeepAndStrictEqual(new Set([{}]), new Set([{}])); +// Ref: https://github.com/nodejs/node/issues/13347 +assertNotDeepOrStrict( + new Set([{a: 1}, {a: 1}]), + new Set([{a: 1}, {a: 2}]) +); +assertNotDeepOrStrict( + new Set([{a: 1}, {a: 1}, {a: 2}]), + new Set([{a: 1}, {a: 2}, {a: 2}]) +); +assertNotDeepOrStrict( + new Map([[{x: 1}, 5], [{x: 1}, 5]]), + new Map([[{x: 1}, 5], [{x: 2}, 5]]) +); + +assertNotDeepOrStrict(new Set([3, '3']), new Set([3, 4])); +assertNotDeepOrStrict(new Map([[3, 0], ['3', 0]]), new Map([[3, 0], [4, 0]])); + +assertNotDeepOrStrict( + new Set([{a: 1}, {a: 1}, {a: 2}]), + new Set([{a: 1}, {a: 2}, {a: 2}]) +); + // This is an awful case, where a map contains multiple equivalent keys: assertOnlyDeepEqual( new Map([[1, 'a'], ['1', 'b']]),