lib: generalize handling in assert.deepEqual()

lib/assert.js

@@ -25,9 +25,7 @@
 'use strict';
 
 // UTILITY
-const compare = process.binding('buffer').compare;
 const util = require('util');
-const Buffer = require('buffer').Buffer;
 const pSlice = Array.prototype.slice;
 
 // 1. The assert module provides functions that throw
@@ -146,24 +144,12 @@ function _deepEqual(actual, expected, strict) {
   // 7.1. All identical values are equivalent, as determined by ===.
   if (actual === expected) {
     return true;
-  } else if (actual instanceof Buffer && expected instanceof Buffer) {
-    return compare(actual, expected) === 0;
 
   // 7.2. If the expected value is a Date object, the actual value is
   // equivalent if it is also a Date object that refers to the same time.
   } else if (util.isDate(actual) && util.isDate(expected)) {
     return actual.getTime() === expected.getTime();
 
-  // 7.3 If the expected value is a RegExp object, the actual value is
-  // equivalent if it is also a RegExp object with the same source and
-  // properties (`global`, `multiline`, `lastIndex`, `ignoreCase`).
-  } else if (util.isRegExp(actual) && util.isRegExp(expected)) {
-    return actual.source === expected.source &&
-           actual.global === expected.global &&
-           actual.multiline === expected.multiline &&
-           actual.lastIndex === expected.lastIndex &&
-           actual.ignoreCase === expected.ignoreCase;
-
   // 7.4. Other pairs that do not both pass typeof value == 'object',
   // equivalence is determined by ==.
   } else if ((actual === null || typeof actual !== 'object') &&
@@ -185,6 +171,14 @@ function isArguments(object) {
   return Object.prototype.toString.call(object) == '[object Arguments]';
 }
 
+function isArrayOrString(object) {
+  return object instanceof Array || object instanceof String;
+}
+
+function getPropertiesAndSymbols(o) {
+  return Object.getOwnPropertyNames(o).concat(Object.getOwnPropertySymbols(o));
+}
+
 function objEquiv(a, b, strict) {
   if (a === null || a === undefined || b === null || b === undefined)
     return false;
@@ -202,9 +196,26 @@ function objEquiv(a, b, strict) {
     b = pSlice.call(b);
     return _deepEqual(a, b, strict);
   }
-  var ka = Object.keys(a),
-      kb = Object.keys(b),
+  var ka = getPropertiesAndSymbols(a),
+      kb = getPropertiesAndSymbols(b),
       key, i;
+
+  // when comparing String/Array to non-Strings/non-Array, ignore length prop
+  const aLengthIndex = ka.indexOf('length');
+  const bLengthIndex = kb.indexOf('length');
+  const aHasLength = (aLengthIndex !== -1);
+  const bHasLength = (bLengthIndex !== -1);
+
+  if (isArrayOrString(a) !== isArrayOrString(b)) {
+    if (aHasLength !== bHasLength) {
+      if (aHasLength) {
+        ka.splice(bLengthIndex);
+      } else {
+        kb.splice(aLengthIndex);
+      }
+    }
+  }
+
   // having the same number of owned properties (keys incorporates
   // hasOwnProperty)
   if (ka.length !== kb.length)

test/parallel/test-assert.js

@@ -362,6 +362,28 @@ try {
   gotError = true;
 }
 
+// https://github.com/nodejs/node/issues/3122
+a.throws(makeBlock(a.deepEqual, Error('a'), Error('b')),
+  a.AssertionError);
+
+// https://github.com/nodejs/node/pull/3124#issuecomment-147416176
+// argh! this const/var blizzard due to current eslint we're using flagging
+//   computed properties as a lint error.
+// Change to computed properties after https://github.com/nodejs/node/pull/2286
+//   lands.
+const symbol = Symbol();
+const symbol2 = Symbol();
+var obj1 = {};
+var obj2 = {};
+var obj3 = {};
+obj1[symbol] = 1;
+obj2[symbol] = 1;
+obj3[symbol2] = 1;
+a.doesNotThrow(makeBlock(a.deepEqual, obj1, obj2));
+a.throws(makeBlock(a.deepEqual, obj1, obj3),
+  a.AssertionError);
+
+
 // GH-7178. Ensure reflexivity of deepEqual with `arguments` objects.
 var args = (function() { return arguments; })();
 a.throws(makeBlock(a.deepEqual, [], args));