fix #251: compare props on the prototype chain (#267)

* fix #251: compare enumerable props on the prototype

* Negate test: Arguments with same contents as an array is deeply equal

Co-authored-by: Morgan Roderick <[email protected]>

---------

Co-authored-by: Morgan Roderick <[email protected]>

Author: fatso83

lib/deep-equal.js

@@ -4,7 +4,6 @@ var valueToString = require("@sinonjs/commons").valueToString;
 var className = require("@sinonjs/commons").className;
 var typeOf = require("@sinonjs/commons").typeOf;
 var arrayProto = require("@sinonjs/commons").prototypes.array;
-var objectProto = require("@sinonjs/commons").prototypes.object;
 var mapForEach = require("@sinonjs/commons").prototypes.map.forEach;
 
 var getClass = require("./get-class");
@@ -25,22 +24,38 @@ var every = arrayProto.every;
 var push = arrayProto.push;
 
 var getTime = Date.prototype.getTime;
-var hasOwnProperty = objectProto.hasOwnProperty;
 var indexOf = arrayProto.indexOf;
-var keys = Object.keys;
 var getOwnPropertySymbols = Object.getOwnPropertySymbols;
 
+/**
+ * We explicitly want to get props on proto chain
+ *
+ * Objects such as URL in the browser do not have any
+ * enumerable keys of its own. All meaningful props
+ * to compare are enumerable getters further up the chain
+ * @param {object} object
+ * @returns {Array<string>} the list of enumerable keys
+ */
+function allEnumerableKeysInProtoChain(object) {
+    const keys = [];
+
+    // eslint-disable-next-line
+    for (const key in object) {
+        keys.push(key);
+    }
+    return keys;
+}
+
 /**
  * Deep equal comparison. Two values are "deep equal" when:
  *
- *   - They are equal, according to samsam.identical
- *   - They are both date objects representing the same time
- *   - They are both arrays containing elements that are all deepEqual
- *   - They are objects with the same set of properties, and each property
- *     in ``actual`` is deepEqual to the corresponding property in ``expectation``
+ * - They are equal, according to samsam.identical
+ * - They are both date objects representing the same time
+ * - They are both arrays containing elements that are all deepEqual
+ * - They are objects with the same set of properties, and each property
+ * in ``actual`` is deepEqual to the corresponding property in ``expectation``
  *
  * Supports cyclic objects.
- *
  * @alias module:samsam.deepEqual
  * @param {*} actual The object to examine
  * @param {*} expectation The object actual is expected to be equal to
@@ -129,8 +144,8 @@ function deepEqualCyclic(actual, expectation, match) {
 
         var actualClass = getClass(actualObj);
         var expectationClass = getClass(expectationObj);
-        var actualKeys = keys(actualObj);
-        var expectationKeys = keys(expectationObj);
+        var actualKeys = allEnumerableKeysInProtoChain(actualObj);
+        var expectationKeys = allEnumerableKeysInProtoChain(expectationObj);
         var actualName = className(actualObj);
         var expectationName = className(expectationObj);
         var expectationSymbols =
@@ -231,10 +246,6 @@ function deepEqualCyclic(actual, expectation, match) {
         }
 
         return every(expectationKeysAndSymbols, function (key) {
-            if (!hasOwnProperty(actualObj, key)) {
-                return false;
-            }
-
             var actualValue = actualObj[key];
             var expectationValue = expectationObj[key];
             var actualObject = isObject(actualValue);

lib/deep-equal.test.js

@@ -196,6 +196,31 @@ describe("deepEqual", function () {
         assert.isTrue(checkDeep);
     });
 
+    it("compares enumerable props on the prototype", function () {
+        class Url {
+            #value;
+            constructor(str) {
+                this.#value = str;
+            }
+            get proto() {
+                return this.#value.split("://")[0];
+            }
+
+            get domain() {
+                return this.#value.split("://")[1];
+            }
+        }
+        // getters are not enumerable by default, but they are configurable, so let's make them enumerable!
+        Object.defineProperty(Url.prototype, "proto", { enumerable: true });
+        Object.defineProperty(Url.prototype, "domain", { enumerable: true });
+
+        const u1 = new Url("foo://bar");
+        const u2 = new Url("bar://baz");
+        const u3 = new Url("bar://baz");
+        assert.isFalse(samsam.deepEqual(u1, u2));
+        assert.isTrue(samsam.deepEqual(u3, u2));
+    });
+
     it("returns true object without prototype compared to equal object with prototype", function () {
         var obj1 = Object.create(null);
         obj1.a = 1;
@@ -484,7 +509,7 @@ describe("deepEqual", function () {
                 [1, 2, {}, []],
                 gather(1, 2, {}, []),
             );
-            assert.isFalse(checkDeep);
+            assert.isTrue(checkDeep);
         });
 
         it("returns false if array like object to arguments", function () {