assert: adjust loose assertions

doc/api/assert.md

@@ -163,6 +163,10 @@ An alias of [`assert.ok()`][].
 <!-- YAML
 added: v0.1.21
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/REPLACEME
+    description: The type tags are now properly compared and there are a couple
+                 minor comparison adjustments to make the check less surprising.
   - version: v9.0.0
     pr-url: https://github.com/nodejs/node/pull/15001
     description: The `Error` names and messages are now properly compared
@@ -191,26 +195,39 @@ An alias of [`assert.deepStrictEqual()`][].
 
 > Stability: 0 - Deprecated: Use [`assert.deepStrictEqual()`][] instead.
 
-Tests for deep equality between the `actual` and `expected` parameters.
-Primitive values are compared with the [Abstract Equality Comparison][]
-( `==` ).
+Tests for deep equality between the `actual` and `expected` parameters. Consider
+using [`assert.deepStrictEqual()`][] instead. [`assert.deepEqual()`][] can have
+potentially surprising results.
+
+"Deep" equality means that the enumerable "own" properties of child objects
+are also recursively evaluated by the following rules.
+
+### Comparison details
+
+* Primitive values are compared with the [Abstract Equality Comparison][]
+  ( `==` ).
+* [Type tags][Object.prototype.toString()] of objects should be the same.
+* Only [enumerable "own" properties][] are considered.
+* [`Error`][] names and messages are always compared, even if these are not
+  enumerable properties.
+* [Object wrappers][] are compared both as objects and unwrapped values.
+* `Object` properties are compared unordered.
+* [`Map`][] keys and [`Set`][] items are compared unordered.
+* Recursion stops when both sides differ or both sides encounter a circular
+  reference.
+* Implementation does not test the [`[[Prototype]]`][prototype-spec] of
+  objects.
+* [`Symbol`][] properties are not compared.
+* [`WeakMap`][] and [`WeakSet`][] comparison does not rely on their values.
 
-Only [enumerable "own" properties][] are considered. The
-[`assert.deepEqual()`][] implementation does not test the
-[`[[Prototype]]`][prototype-spec] of objects or enumerable own [`Symbol`][]
-properties. For such checks, consider using [`assert.deepStrictEqual()`][]
-instead. [`assert.deepEqual()`][] can have potentially surprising results. The
-following example does not throw an `AssertionError` because the properties on
-the [`RegExp`][] object are not enumerable:
+The following example does not throw an `AssertionError` because the primitives
+are considered equal by the [Abstract Equality Comparison][] ( `==` ).
 
 ```js
 // WARNING: This does not throw an AssertionError!
-assert.deepEqual(/a/gi, new Date());
+assert.deepEqual('+00000000', false);
 ```
 
-An exception is made for [`Map`][] and [`Set`][]. `Map`s and `Set`s have their
-contained items compared too, as expected.
-
 "Deep" equality means that the enumerable "own" properties of child objects
 are evaluated also:
 
@@ -304,7 +321,7 @@ are recursively evaluated also by the following rules.
 * Enumerable own [`Symbol`][] properties are compared as well.
 * [Object wrappers][] are compared both as objects and unwrapped values.
 * `Object` properties are compared unordered.
-* `Map` keys and `Set` items are compared unordered.
+* [`Map`][] keys and [`Set`][] items are compared unordered.
 * Recursion stops when both sides differ or both sides encounter a circular
   reference.
 * [`WeakMap`][] and [`WeakSet`][] comparison does not rely on their values. See

lib/internal/util/comparisons.js

@@ -14,7 +14,9 @@ const {
   isStringObject,
   isBooleanObject,
   isBigIntObject,
-  isSymbolObject
+  isSymbolObject,
+  isFloat32Array,
+  isFloat64Array
 } = require('internal/util/types');
 const {
   getOwnNonIndexProperties,
@@ -84,18 +86,6 @@ function areEqualArrayBuffers(buf1, buf2) {
     compare(new Uint8Array(buf1), new Uint8Array(buf2)) === 0;
 }
 
-function isFloatTypedArrayTag(tag) {
-  return tag === '[object Float32Array]' || tag === '[object Float64Array]';
-}
-
-function isArguments(tag) {
-  return tag === '[object Arguments]';
-}
-
-function isObjectOrArrayTag(tag) {
-  return tag === '[object Array]' || tag === '[object Object]';
-}
-
 function isEqualBoxedPrimitive(val1, val2) {
   if (isNumberObject(val1)) {
     return isNumberObject(val2) &&
@@ -132,23 +122,45 @@ function isEqualBoxedPrimitive(val1, val2) {
 // For strict comparison, objects should have
 // a) The same built-in type tags
 // b) The same prototypes.
-function strictDeepEqual(val1, val2, memos) {
-  if (typeof val1 !== 'object') {
-    return typeof val1 === 'number' && numberIsNaN(val1) &&
-      numberIsNaN(val2);
+
+function innerDeepEqual(val1, val2, strict, memos) {
+  // All identical values are equivalent, as determined by ===.
+  if (val1 === val2) {
+    if (val1 !== 0)
+      return true;
+    return strict ? objectIs(val1, val2) : true;
   }
-  if (typeof val2 !== 'object' || val1 === null || val2 === null) {
-    return false;
+
+  // Check more closely if val1 and val2 are equal.
+  if (strict) {
+    if (typeof val1 !== 'object') {
+      return typeof val1 === 'number' && numberIsNaN(val1) &&
+        numberIsNaN(val2);
+    }
+    if (typeof val2 !== 'object' || val1 === null || val2 === null) {
+      return false;
+    }
+    if (getPrototypeOf(val1) !== getPrototypeOf(val2)) {
+      return false;
+    }
+  } else {
+    if (val1 === null || typeof val1 !== 'object') {
+      if (val2 === null || typeof val2 !== 'object') {
+        // eslint-disable-next-line eqeqeq
+        return val1 == val2;
+      }
+      return false;
+    }
+    if (val2 === null || typeof val2 !== 'object') {
+      return false;
+    }
   }
   const val1Tag = objectToString(val1);
   const val2Tag = objectToString(val2);
 
   if (val1Tag !== val2Tag) {
     return false;
   }
-  if (getPrototypeOf(val1) !== getPrototypeOf(val2)) {
-    return false;
-  }
   if (Array.isArray(val1)) {
     // Check for sparse arrays and general fast path
     if (val1.length !== val2.length) {
@@ -159,10 +171,10 @@ function strictDeepEqual(val1, val2, memos) {
     if (keys1.length !== keys2.length) {
       return false;
     }
-    return keyCheck(val1, val2, kStrict, memos, kIsArray, keys1);
+    return keyCheck(val1, val2, strict, memos, kIsArray, keys1);
   }
   if (val1Tag === '[object Object]') {
-    return keyCheck(val1, val2, kStrict, memos, kNoIterator);
+    return keyCheck(val1, val2, strict, memos, kNoIterator);
   }
   if (isDate(val1)) {
     if (dateGetTime(val1) !== dateGetTime(val2)) {
@@ -174,13 +186,16 @@ function strictDeepEqual(val1, val2, memos) {
     }
   } else if (isNativeError(val1) || val1 instanceof Error) {
     // Do not compare the stack as it might differ even though the error itself
-    // is otherwise identical. The non-enumerable name should be identical as
-    // the prototype is also identical. Otherwise this is caught later on.
-    if (val1.message !== val2.message) {
+    // is otherwise identical.
+    if (val1.message !== val2.message || val1.name !== val2.name) {
       return false;
     }
   } else if (isArrayBufferView(val1)) {
-    if (!areSimilarTypedArrays(val1, val2)) {
+    if (!strict && (isFloat32Array(val1) || isFloat64Array(val1))) {
+      if (!areSimilarFloatArrays(val1, val2)) {
+        return false;
+      }
+    } else if (!areSimilarTypedArrays(val1, val2)) {
       return false;
     }
     // Buffer.compare returns true, so val1.length === val2.length. If they both
@@ -191,84 +206,25 @@ function strictDeepEqual(val1, val2, memos) {
     if (keys1.length !== keys2.length) {
       return false;
     }
-    return keyCheck(val1, val2, kStrict, memos, kNoIterator, keys1);
+    return keyCheck(val1, val2, strict, memos, kNoIterator, keys1);
   } else if (isSet(val1)) {
     if (!isSet(val2) || val1.size !== val2.size) {
       return false;
     }
-    return keyCheck(val1, val2, kStrict, memos, kIsSet);
+    return keyCheck(val1, val2, strict, memos, kIsSet);
   } else if (isMap(val1)) {
     if (!isMap(val2) || val1.size !== val2.size) {
       return false;
     }
-    return keyCheck(val1, val2, kStrict, memos, kIsMap);
+    return keyCheck(val1, val2, strict, memos, kIsMap);
   } else if (isAnyArrayBuffer(val1)) {
     if (!areEqualArrayBuffers(val1, val2)) {
       return false;
     }
   } else if (isBoxedPrimitive(val1) && !isEqualBoxedPrimitive(val1, val2)) {
     return false;
   }
-  return keyCheck(val1, val2, kStrict, memos, kNoIterator);
-}
-
-function looseDeepEqual(val1, val2, memos) {
-  if (val1 === null || typeof val1 !== 'object') {
-    if (val2 === null || typeof val2 !== 'object') {
-      // eslint-disable-next-line eqeqeq
-      return val1 == val2;
-    }
-    return false;
-  }
-  if (val2 === null || typeof val2 !== 'object') {
-    return false;
-  }
-  const val1Tag = objectToString(val1);
-  const val2Tag = objectToString(val2);
-  if (val1Tag === val2Tag) {
-    if (isObjectOrArrayTag(val1Tag)) {
-      return keyCheck(val1, val2, kLoose, memos, kNoIterator);
-    }
-    if (isArrayBufferView(val1)) {
-      if (isFloatTypedArrayTag(val1Tag)) {
-        return areSimilarFloatArrays(val1, val2);
-      }
-      return areSimilarTypedArrays(val1, val2);
-    }
-    if (isDate(val1) && isDate(val2)) {
-      return val1.getTime() === val2.getTime();
-    }
-    if (isRegExp(val1) && isRegExp(val2)) {
-      return areSimilarRegExps(val1, val2);
-    }
-    if (val1 instanceof Error && val2 instanceof Error) {
-      if (val1.message !== val2.message || val1.name !== val2.name)
-        return false;
-    }
-  // Ensure reflexivity of deepEqual with `arguments` objects.
-  // See https://github.com/nodejs/node-v0.x-archive/pull/7178
-  } else if (isArguments(val1Tag) || isArguments(val2Tag)) {
-    return false;
-  }
-  if (isSet(val1)) {
-    if (!isSet(val2) || val1.size !== val2.size) {
-      return false;
-    }
-    return keyCheck(val1, val2, kLoose, memos, kIsSet);
-  } else if (isMap(val1)) {
-    if (!isMap(val2) || val1.size !== val2.size) {
-      return false;
-    }
-    return keyCheck(val1, val2, kLoose, memos, kIsMap);
-  } else if (isSet(val2) || isMap(val2)) {
-    return false;
-  }
-  if (isAnyArrayBuffer(val1) && isAnyArrayBuffer(val2)) {
-    if (!areEqualArrayBuffers(val1, val2)) {
-      return false;
-    }
-  }
-  return keyCheck(val1, val2, kLoose, memos, kNoIterator);
+  return keyCheck(val1, val2, strict, memos, kNoIterator);
 }
 
 function getEnumerables(val, keys) {
@@ -370,21 +326,6 @@ function keyCheck(val1, val2, strict, memos, iterationType, aKeys) {
   return areEq;
 }
 
-function innerDeepEqual(val1, val2, strict, memos) {
-  // All identical values are equivalent, as determined by ===.
-  if (val1 === val2) {
-    if (val1 !== 0)
-      return true;
-    return strict ? objectIs(val1, val2) : true;
-  }
-
-  // Check more closely if val1 and val2 are equal.
-  if (strict === true)
-    return strictDeepEqual(val1, val2, memos);
-
-  return looseDeepEqual(val1, val2, memos);
-}
-
 function setHasEqualElement(set, val1, strict, memo) {
   // Go looking.
   for (const val2 of set) {

test/es-module/test-esm-dynamic-import.js

@@ -23,9 +23,12 @@ function expectMissingModuleError(result) {
 function expectOkNamespace(result) {
   Promise.resolve(result)
     .then(common.mustCall((ns) => {
-      // Can't deepStrictEqual because ns isn't a normal object
-      // eslint-disable-next-line no-restricted-properties
-      assert.deepEqual(ns, { default: true });
+      const expected = { default: true };
+      Object.defineProperty(expected, Symbol.toStringTag, {
+        value: 'Module'
+      });
+      Object.setPrototypeOf(expected, Object.getPrototypeOf(ns));
+      assert.deepStrictEqual(ns, expected);
     }));
 }
 

test/parallel/test-assert-checktag.js

@@ -18,8 +18,8 @@ if (process.stdout.isTTY)
   FakeDate.prototype = Date.prototype;
   const fake = new FakeDate();
 
-  assert.deepEqual(date, fake);
-  assert.deepEqual(fake, date);
+  assert.notDeepEqual(date, fake);
+  assert.notDeepEqual(fake, date);
 
   // For deepStrictEqual we check the runtime type,
   // then reveal the fakeness of the fake date
@@ -45,7 +45,7 @@ if (process.stdout.isTTY)
   for (const prop of Object.keys(global)) {
     fakeGlobal[prop] = global[prop];
   }
-  assert.deepEqual(fakeGlobal, global);
+  assert.notDeepEqual(fakeGlobal, global);
   // Message will be truncated anyway, don't validate
   assert.throws(() => assert.deepStrictEqual(fakeGlobal, global),
                 assert.AssertionError);
@@ -57,7 +57,7 @@ if (process.stdout.isTTY)
   for (const prop of Object.keys(process)) {
     fakeProcess[prop] = process[prop];
   }
-  assert.deepEqual(fakeProcess, process);
+  assert.notDeepEqual(fakeProcess, process);
   // Message will be truncated anyway, don't validate
   assert.throws(() => assert.deepStrictEqual(fakeProcess, process),
                 assert.AssertionError);