fix(equal): one-to-one Set/Map matching, symmetric conversions, null-prototype objects

src/internals/utils/objects.spec.ts

@@ -368,6 +368,72 @@ describe('equal', () => {
     expect(equal(a, mismatched)).to.be.false;
   });
 
+  it('should match Set elements one to one', () => {
+    expect(equal(new Set([{ x: 1 }, { x: 1 }]), new Set([{ x: 1 }, { x: 2 }])))
+      .to.be.false;
+    expect(equal(new Set([{ x: 1 }, { x: 1 }]), new Set([{ x: 1 }, { x: 1 }])))
+      .to.be.true;
+  });
+
+  it('should match Map entries one to one', () => {
+    const left = new Map([
+      [{ k: 1 }, 'v'],
+      [{ k: 1 }, 'v'],
+    ]);
+    const right = new Map([
+      [{ k: 1 }, 'v'],
+      [{ k: 2 }, 'v'],
+    ]);
+    expect(equal(left, right)).to.be.false;
+  });
+
+  it('should give the same result in both directions', () => {
+    const valueOf = Object.create({ valueOf: () => 1 });
+    // A `toString` that mimics the default one still differs from it.
+    const toString = Object.create({ toString: () => '[object Object]' });
+
+    expect(equal({}, valueOf)).to.be.false;
+    expect(equal(valueOf, {})).to.be.false;
+    expect(equal({}, toString)).to.be.false;
+    expect(equal(toString, {})).to.be.false;
+  });
+
+  it('should not treat objects pending in different pairs as equal', () => {
+    // `a` and `d` are both pending when they meet, but they differ.
+    const a: Record<string, unknown> = {};
+    const b: Record<string, unknown> = {};
+    const c: Record<string, unknown> = { q: a };
+    const d: Record<string, unknown> = {};
+    a.p = c;
+    b.p = d;
+    d.q = d;
+
+    expect(equal(a, b)).to.be.false;
+  });
+
+  it('should compare null-prototype objects', () => {
+    const create = (value: number) =>
+      Object.assign(Object.create(null), { a: value });
+
+    expect(equal(create(1), create(1))).to.be.true;
+    expect(equal(create(1), create(2))).to.be.false;
+  });
+
+  it('should not throw when only one null-prototype object has valueOf or toString', () => {
+    const bare = Object.create(null);
+    const withValueOf = Object.assign(Object.create(null), {
+      valueOf: () => 1,
+    });
+    const withToString = Object.assign(Object.create(null), {
+      toString: () => 'a',
+    });
+
+    expect(equal(withValueOf, bare)).to.be.false;
+    expect(equal(withToString, bare)).to.be.false;
+    expect(equal(bare, withValueOf)).to.be.false;
+    expect(equal(bare, withToString)).to.be.false;
+  });
+
   it('should still terminate on circular references', () => {
     const a: Record<string, unknown> = { name: 'a' };
     const b: Record<string, unknown> = { name: 'a' };

src/internals/utils/objects.ts

@@ -1,38 +1,31 @@
-import { isObject, isRegExp } from './types.js';
+import { isFunction, isObject, isRegExp } from './types.js';
 
-/** The object pairs the comparison visits at this moment, to stop cycles. */
+/** The object pairs under comparison at this moment. */
 type Visited = WeakMap<object, WeakSet<object>>;
 
 /**
  * Returns whether two values are deeply equal. Handles arrays, Maps, Sets,
  * RegExps, plain objects and circular references.
  */
-export function equal<T>(
+export function equal(
   a: unknown,
-  b: T,
+  b: unknown,
   visited: Visited = new WeakMap()
 ): boolean {
   if (Object.is(a, b)) return true;
 
   if (!isObject(a) || !isObject(b)) return false;
   if (a.constructor !== b.constructor) return false;
 
-  // Record the pair, not each object on its own: the Map and Set branches
-  // below test candidates they expect to fail, and single-object records
-  // would make a later comparison of the same pair return `true`.
-  let pending = visited.get(a);
-  if (pending?.has(b)) return true;
-
-  if (!pending) {
-    pending = new WeakSet();
-    visited.set(a, pending);
-  }
-  pending.add(b);
+  // A pending pair counts as equal, which stops cycles. Track pairs, not
+  // objects: `a` can meet another partner while it is still pending.
+  const pending = visited.get(a) ?? new WeakSet();
+  if (pending.has(b)) return true;
+  visited.set(a, pending.add(b));
 
   try {
     return compare(a, b, visited);
   } finally {
-    // Release the pair, including on an early return in `compare`.
     pending.delete(b);
   }
 }
@@ -41,61 +34,69 @@ function compare(a: object, b: object, visited: Visited): boolean {
   if (isRegExp(a) && isRegExp(b))
     return a.source === b.source && a.flags === b.flags;
 
-  if (a instanceof Map && b instanceof Map) {
-    if (a.size !== b.size) return false;
-    for (const [keyA, valueA] of a.entries()) {
-      let found = false;
-      for (const [keyB, valueB] of b.entries()) {
-        if (equal(keyA, keyB, visited) && equal(valueA, valueB, visited)) {
-          found = true;
-          break;
-        }
-      }
-      if (!found) return false;
-    }
-    return true;
-  }
-
-  if (a instanceof Set && b instanceof Set) {
-    if (a.size !== b.size) return false;
-    for (const valueA of a) {
-      let found = false;
-      for (const valueB of b) {
-        if (equal(valueA, valueB, visited)) {
-          found = true;
-          break;
-        }
-      }
-      if (!found) return false;
-    }
-    return true;
-  }
+  // Map entries iterate as [key, value] arrays.
+  if (
+    (a instanceof Map && b instanceof Map) ||
+    (a instanceof Set && b instanceof Set)
+  )
+    return a.size === b.size && matchOneToOne(a, b, visited);
 
   if (Array.isArray(a) && Array.isArray(b)) {
-    const length = a.length;
-    if (length !== b.length) return false;
-    for (let i = 0; i < length; i++) {
+    if (a.length !== b.length) return false;
+    for (let i = 0; i < a.length; i++) {
       if (!equal(a[i], b[i], visited)) return false;
     }
     return true;
   }
 
-  if (a.valueOf !== Object.prototype.valueOf)
-    return a.valueOf() === b.valueOf();
-  if (a.toString !== Object.prototype.toString)
-    return a.toString() === b.toString();
-
-  const aKeys = Object.keys(a);
-  const bKeys = Object.keys(b);
-  if (aKeys.length !== bKeys.length) return false;
+  // A custom conversion on one side only makes the objects unequal, which
+  // keeps `equal` an equivalence relation.
+  for (const method of ['valueOf', 'toString'] as const) {
+    const left = customConversion(a, method);
+    const right = customConversion(b, method);
 
-  for (const key of aKeys) {
-    if (!Object.hasOwn(b, key)) return false;
+    if (left || right)
+      return (
+        !!left &&
+        !!right &&
+        Reflect.apply(left, a, []) === Reflect.apply(right, b, [])
+      );
   }
 
-  for (const key of aKeys) {
-    if (!equal(a[key as keyof typeof a], b[key as keyof typeof b], visited))
-      return false;
+  const x = a as Record<string, unknown>;
+  const y = b as Record<string, unknown>;
+  const keys = Object.keys(x);
+
+  return (
+    keys.length === Object.keys(y).length &&
+    keys.every((key) => Object.hasOwn(y, key) && equal(x[key], y[key], visited))
+  );
+}
+
+/** Returns `value[method]` unless it is missing or the default. */
+function customConversion(
+  value: object,
+  method: 'valueOf' | 'toString'
+): CallableFunction | undefined {
+  const fn = (value as Record<string, unknown>)[method];
+  return isFunction(fn) && fn !== Object.prototype[method] ? fn : undefined;
+}
+
+/**
+ * Pairs each item of `left` with a distinct equal item of `right`. Greedy
+ * suffices, since `equal` is an equivalence relation.
+ */
+function matchOneToOne(
+  left: Iterable<unknown>,
+  right: Iterable<unknown>,
+  visited: Visited
+): boolean {
+  const pool = [...right];
+
+  for (const a of left) {
+    const index = pool.findIndex((b) => equal(a, b, visited));
+    if (index < 0) return false;
+    pool.splice(index, 1);
   }
 
   return true;