From b70586f57f659ddaf4d40f54882a6c8d9cba6411 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Tue, 19 Mar 2019 15:40:38 +0000 Subject: [PATCH 1/5] Fix circular references in iterable equality --- packages/expect/src/__tests__/utils.test.js | 81 +++++++++++++++++++++ packages/expect/src/utils.ts | 63 ++++++++++++---- 2 files changed, 131 insertions(+), 13 deletions(-) diff --git a/packages/expect/src/__tests__/utils.test.js b/packages/expect/src/__tests__/utils.test.js index 180b514a1a77..b91634157858 100644 --- a/packages/expect/src/__tests__/utils.test.js +++ b/packages/expect/src/__tests__/utils.test.js @@ -15,6 +15,7 @@ const { getPath, hasOwnProperty, subsetEquality, + iterableEquality, } = require('../utils'); describe('getPath()', () => { @@ -202,3 +203,83 @@ describe('subsetEquality()', () => { expect(subsetEquality(undefined, {foo: 'bar'})).not.toBeTruthy(); }); }); + +describe('iterableEquality', () => { + test('returns true when given circular iterators', () => { + class Iter { + *[Symbol.iterator]() { + yield this; + } + } + + const a = new Iter(); + const b = new Iter(); + + expect(iterableEquality(a, b)).toBe(true); + }); + + test('returns true when given circular Set', () => { + const a = new Set(); + a.add(a); + const b = new Set(); + b.add(b); + expect(iterableEquality(a, b)).toBe(true); + }); + + test('returns true when given nested Sets', () => { + expect( + iterableEquality( + new Set([new Set([[1]]), new Set([[2]])]), + new Set([new Set([[2]]), new Set([[1]])]), + ), + ).toBe(true); + expect( + iterableEquality( + new Set([new Set([[1]]), new Set([[2]])]), + new Set([new Set([[3]]), new Set([[1]])]), + ), + ).toBe(false); + }); + + test('returns true when given circular key in Map', () => { + const a = new Map(); + a.set(a, 'a'); + const b = new Map(); + b.set(b, 'a'); + + expect(iterableEquality(a, b)).toBe(true); + }); + + test('returns true when given nested Maps', () => { + expect( + iterableEquality( + new Map([['hello', new Map([['world', 'foobar']])]]), + new Map([['hello', new Map([['world', 'qux']])]]), + ), + ).toBe(false); + expect( + iterableEquality( + new Map([['hello', new Map([['world', 'foobar']])]]), + new Map([['hello', new Map([['world', 'foobar']])]]), + ), + ).toBe(true); + }); + + test('returns true when given circular key and value in Map', () => { + const a = new Map(); + a.set(a, a); + const b = new Map(); + b.set(b, b); + + expect(iterableEquality(a, b)).toBe(true); + }); + + test('returns true when given circular value in Map', () => { + const a = new Map(); + a.set('a', a); + const b = new Map(); + b.set('a', b); + + expect(iterableEquality(a, b)).toBe(true); + }); +}); diff --git a/packages/expect/src/utils.ts b/packages/expect/src/utils.ts index 13047d555ff1..245f227701cf 100644 --- a/packages/expect/src/utils.ts +++ b/packages/expect/src/utils.ts @@ -137,7 +137,13 @@ const IteratorSymbol = Symbol.iterator; const hasIterator = (object: any) => !!(object != null && object[IteratorSymbol]); -export const iterableEquality = (a: any, b: any) => { + +export const iterableEquality = ( + a: any, + b: any, + aStack: Array = [], + bStack: Array = [], +) => { if ( typeof a !== 'object' || typeof b !== 'object' || @@ -152,6 +158,24 @@ export const iterableEquality = (a: any, b: any) => { return false; } + let length = aStack.length; + while (length--) { + // Linear search. Performance is inversely proportional to the number of + // unique nested structures. + // circular references at same depth are equal + // circular reference is not equal to non-circular one + if (aStack[length] === a) { + return bStack[length] === b; + } else if (bStack[length] === b) { + return false; + } + } + aStack.push(a); + bStack.push(b); + + const iterableEqualityWithStack = (a: any, b: any) => + iterableEquality(a, b, aStack, bStack); + if (a.size !== undefined) { if (a.size !== b.size) { return false; @@ -161,7 +185,7 @@ export const iterableEquality = (a: any, b: any) => { if (!b.has(aValue)) { let has = false; for (const bValue of b) { - const isEqual = equals(aValue, bValue, [iterableEquality]); + const isEqual = equals(aValue, bValue, [iterableEqualityWithStack]); if (isEqual === true) { has = true; } @@ -173,25 +197,30 @@ export const iterableEquality = (a: any, b: any) => { } } } - if (allFound) { - return true; - } + + aStack.pop(); + bStack.pop(); + + return allFound; } else if (isA('Map', a) || isImmutableUnorderedKeyed(a)) { let allFound = true; for (const aEntry of a) { if ( !b.has(aEntry[0]) || - !equals(aEntry[1], b.get(aEntry[0]), [iterableEquality]) + !equals(aEntry[1], b.get(aEntry[0]), [iterableEqualityWithStack]) ) { let has = false; for (const bEntry of b) { - const matchedKey = equals(aEntry[0], bEntry[0], [iterableEquality]); + const matchedKey = equals(aEntry[0], bEntry[0], [ + iterableEqualityWithStack, + ]); let matchedValue = false; if (matchedKey === true) { - matchedValue = equals(aEntry[1], bEntry[1], [iterableEquality]); + matchedValue = equals(aEntry[1], bEntry[1], [ + iterableEqualityWithStack, + ]); } - if (matchedValue === true) { has = true; } @@ -203,9 +232,9 @@ export const iterableEquality = (a: any, b: any) => { } } } - if (allFound) { - return true; - } + aStack.pop(); + bStack.pop(); + return allFound; } } @@ -213,13 +242,21 @@ export const iterableEquality = (a: any, b: any) => { for (const aValue of a) { const nextB = bIterator.next(); - if (nextB.done || !equals(aValue, nextB.value, [iterableEquality])) { + if ( + nextB.done || + !equals(aValue, nextB.value, [ + (a: any, b: any) => iterableEquality(a, b, aStack, bStack), + ]) + ) { return false; } } if (!bIterator.next().done) { return false; } + + aStack.pop(); + bStack.pop(); return true; }; From 32d742ea37f56f40d73c05eee700345b9de90703 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Wed, 20 Mar 2019 10:00:11 +0000 Subject: [PATCH 2/5] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fbb3f9d9de4..f106a1a82588 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### Fixes +- `[expect]` Fix circular references in iterable equality ([#8160](https://github.com/facebook/jest/pull/8160)) - `[jest-each]` Fix test function type ([#8145](https://github.com/facebook/jest/pull/8145)) - `[pretty-format]` Print `BigInt` as a readable number instead of `{}` ([#8138](https://github.com/facebook/jest/pull/8138)) - `[jest-fake-timers]` `getTimerCount` not taking immediates and ticks into account ([#8139](https://github.com/facebook/jest/pull/8139)) From ef47fa26f550b904d1f5515d74219434205708ab Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Wed, 20 Mar 2019 10:00:33 +0000 Subject: [PATCH 3/5] Add circular shape checking --- packages/expect/src/__tests__/utils.test.js | 11 +++++++++++ packages/expect/src/utils.ts | 2 -- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/expect/src/__tests__/utils.test.js b/packages/expect/src/__tests__/utils.test.js index b91634157858..7ff609a8ab87 100644 --- a/packages/expect/src/__tests__/utils.test.js +++ b/packages/expect/src/__tests__/utils.test.js @@ -241,6 +241,17 @@ describe('iterableEquality', () => { ).toBe(false); }); + test('returns true when given circular Set shape', () => { + const a1 = new Set(); + const a2 = new Set(); + a1.add(a2); + a2.add(a1); + const b = new Set(); + b.add(b); + + expect(iterableEquality(a1, b)).toBe(true); + }); + test('returns true when given circular key in Map', () => { const a = new Map(); a.set(a, 'a'); diff --git a/packages/expect/src/utils.ts b/packages/expect/src/utils.ts index 245f227701cf..749b20ec597d 100644 --- a/packages/expect/src/utils.ts +++ b/packages/expect/src/utils.ts @@ -166,8 +166,6 @@ export const iterableEquality = ( // circular reference is not equal to non-circular one if (aStack[length] === a) { return bStack[length] === b; - } else if (bStack[length] === b) { - return false; } } aStack.push(a); From 6d9173545d79f65ee65676d4a9626a20bed52acd Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Wed, 20 Mar 2019 10:01:31 +0000 Subject: [PATCH 4/5] Fix changelog conflict --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f106a1a82588..07f64cdc5a61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Fixes - `[expect]` Fix circular references in iterable equality ([#8160](https://github.com/facebook/jest/pull/8160)) +- `[jest-changed-files]` Change method of obtaining git root ([#8052](https://github.com/facebook/jest/pull/8052)) - `[jest-each]` Fix test function type ([#8145](https://github.com/facebook/jest/pull/8145)) - `[pretty-format]` Print `BigInt` as a readable number instead of `{}` ([#8138](https://github.com/facebook/jest/pull/8138)) - `[jest-fake-timers]` `getTimerCount` not taking immediates and ticks into account ([#8139](https://github.com/facebook/jest/pull/8139)) From 2e4611fa6383a16b9ee030b6b309fcf2a65aa4c2 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Wed, 20 Mar 2019 10:23:23 +0000 Subject: [PATCH 5/5] Add comment to remove from stack --- packages/expect/src/utils.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/expect/src/utils.ts b/packages/expect/src/utils.ts index 749b20ec597d..2f3f97ffe2ea 100644 --- a/packages/expect/src/utils.ts +++ b/packages/expect/src/utils.ts @@ -195,10 +195,9 @@ export const iterableEquality = ( } } } - + // Remove the first value from the stack of traversed values. aStack.pop(); bStack.pop(); - return allFound; } else if (isA('Map', a) || isImmutableUnorderedKeyed(a)) { let allFound = true; @@ -230,6 +229,7 @@ export const iterableEquality = ( } } } + // Remove the first value from the stack of traversed values. aStack.pop(); bStack.pop(); return allFound; @@ -242,9 +242,7 @@ export const iterableEquality = ( const nextB = bIterator.next(); if ( nextB.done || - !equals(aValue, nextB.value, [ - (a: any, b: any) => iterableEquality(a, b, aStack, bStack), - ]) + !equals(aValue, nextB.value, [iterableEqualityWithStack]) ) { return false; } @@ -253,6 +251,7 @@ export const iterableEquality = ( return false; } + // Remove the first value from the stack of traversed values. aStack.pop(); bStack.pop(); return true;