Repository: unexpectedjs/unexpected
Pull request #426 (2017-11-05 12:24:16)

Fix undefined non numerical keys
This PR adjusts Unexpected to ignore undefined non-numerical keys for array-like types.

In addition to the array-changes modules being bumped there are also changes to the type `equal` function to ignore keys as appropriate.

Closes https://github.com/unexpectedjs/unexpected/issues/424
Closes https://github.com/unexpectedjs/unexpected/issues/427


Patch:
diff --git a/lib/assertions.js b/lib/assertions.js
index e3fca732..58831a25 100644
--- a/lib/assertions.js
+++ b/lib/assertions.js
@@ -1039,8 +1039,13 @@ module.exports = function (expect) {
     expect.addAssertion('<array-like> to [exhaustively] satisfy <array-like>', function (expect, subject, value) {
         expect.errorMode = 'bubble';
         var i;
+        var subjectType = expect.subjectType;
         var valueType = expect.argTypes[0];
-        var valueKeys = valueType.getKeys(value);
+        var valueKeys = valueType.getKeys(value).filter(function (key) {
+            return (utils.numericalRegExp.test(key) || typeof key === 'symbol' ||
+                    // include keys whose value is not undefined on either LHS or RHS
+                    typeof value[key] !== 'undefined' || typeof subject[key] !== 'undefined');
+        });
         var keyPromises = {};
         valueKeys.forEach(function (valueKey) {
             keyPromises[valueKey] = expect.promise(function () {
@@ -1054,11 +1059,29 @@ module.exports = function (expect) {
         });
         return expect.promise.all([
             expect.promise(function () {
-                expect(subject, 'to only have keys', valueKeys);
+                // create subject key presence object
+                var remainingKeysInSubject = {};
+                subjectType.getKeys(subject).forEach(function (key) {
+                    remainingKeysInSubject[key] = 1; // present in subject
+                });
+                // discard or mark missing each previously seen value key
+                valueKeys.forEach(function (key) {
+                    if (!remainingKeysInSubject[key]) {
+                        remainingKeysInSubject[key] = 2; // present in value
+                    } else {
+                        delete remainingKeysInSubject[key];
+                    }
+                });
+                // filter outstanding keys which should not lead to an error
+                var outstandingKeys = Object.keys(remainingKeysInSubject).filter(function (key) {
+                    return (utils.numericalRegExp.test(key) || typeof key === 'symbol' ||
+                            typeof subject[key] !== 'undefined' || remainingKeysInSubject[key] === 2);
+                });
+                // key checking succeeds with no outstanding keys
+                expect(outstandingKeys.length === 0, 'to be truthy');
             }),
             expect.promise.all(keyPromises)
         ]).caught(function () {
-            var subjectType = expect.subjectType;
             return expect.promise.settle(keyPromises).then(function () {
                 var toSatisfyMatrix = new Array(subject.length);
                 for (i = 0 ; i < subject.length ; i += 1) {
diff --git a/lib/styles.js b/lib/styles.js
index ede204d3..70bd3bc0 100644
--- a/lib/styles.js
+++ b/lib/styles.js
@@ -96,7 +96,7 @@ module.exports = function (expect) {
         var isSymbol;
         isSymbol = typeof key === 'symbol';
         if (isSymbol) {
-            this.text('[').sp().appendInspected(key).sp().text(']').text(':');
+            this.text('[').appendInspected(key).text(']').text(':');
         } else {
             key = String(key);
             if (/^[a-z\$\_][a-z0-9\$\_]*$/i.test(key)) {
diff --git a/lib/types.js b/lib/types.js
index 1c0a21b7..e4693fa4 100644
--- a/lib/types.js
+++ b/lib/types.js
@@ -407,37 +407,66 @@ module.exports = function (expect) {
                 keys[i] = i;
             }
             if (!this.numericalPropertiesOnly) {
-                Object.keys(obj).forEach(function (key) {
-                    if (!utils.numericalRegExp.test(key)) {
-                        keys.push(key);
-                    }
-                });
+                keys.push(...this.getKeysNonNumerical(obj));
+            }
+            return keys;
+        },
+        getKeysNonNumerical: Object.getOwnPropertySymbols ? function (obj) {
+            var keys = [];
+            Object.keys(obj).forEach(function (key) {
+                if (!utils.numericalRegExp.test(key)) {
+                    keys.push(key);
+                }
+            });
+            var symbols = Object.getOwnPropertySymbols(obj);
+            if (symbols.length > 0) {
+                keys.push(...symbols);
             }
             return keys;
+        } : function (obj) {
+            var keys = [];
+            Object.keys(obj).forEach(function (key) {
+                if (!utils.numericalRegExp.test(key)) {
+                    keys.push(key);
+                }
+            });
+            return keys;
         },
         equal: function (a, b, equal) {
             if (a === b) {
                 return true;
             } else if (a.constructor === b.constructor && a.length === b.length) {
                 var i;
-                if (this.numericalPropertiesOnly) {
-                    for (i = 0; i < a.length; i += 1) {
-                        if (!equal(a[i], b[i])) {
-                            return false;
-                        }
+
+                // compare numerically indexed elements
+                for (i = 0; i < a.length; i += 1) {
+                    if (!equal(a[i], b[i])) {
+                        return false;
                     }
-                } else {
-                    var aKeys = this.getKeys(a);
-                    var bKeys = this.getKeys(b);
+                }
+
+                // compare non-numerical keys if enabled for the type
+                if (!this.numericalPropertiesOnly) {
+                    var aKeys = this.getKeysNonNumerical(a).filter(function (key) {
+                        // include keys whose value is not undefined
+                        return typeof a[key] !== 'undefined';
+                    });
+                    var bKeys = this.getKeysNonNumerical(b).filter(function (key) {
+                        // include keys whose value is not undefined on either LHS or RHS
+                        return (typeof b[key] !== 'undefined' || typeof a[key] !== 'undefined');
+                    });
+
                     if (aKeys.length !== bKeys.length) {
                         return false;
                     }
+
                     for (i = 0; i < aKeys.length; i += 1) {
                         if (!equal(a[aKeys[i]], b[aKeys[i]])) {
                             return false;
                         }
                     }
                 }
+
                 return true;
             } else {
                 return false;
diff --git a/package.json b/package.json
index a2f4b12c..7288435f 100644
--- a/package.json
+++ b/package.json
@@ -22,8 +22,8 @@
   },
   "main": "./build/lib/index.js",
   "dependencies": {
-    "array-changes": "3.0.0",
-    "array-changes-async": "3.0.0",
+    "array-changes": "3.0.1",
+    "array-changes-async": "3.0.1",
     "babel-runtime": "6.26.0",
     "detect-indent": "3.0.1",
     "diff": "1.1.0",


Test patch:
diff --git a/test/types/Symbol-type.spec.js b/test/types/Symbol-type.spec.js
index 021a0dd3..ce76201c 100644
--- a/test/types/Symbol-type.spec.js
+++ b/test/types/Symbol-type.spec.js
@@ -13,7 +13,7 @@ if (typeof Symbol === 'function' && Symbol('foo').toString() === 'Symbol(foo)')
         it('inspects correctly when used as a key in an object', function () {
             var obj = {};
             obj[symbolA] = 123;
-            expect(obj, 'to inspect as', "{ [ Symbol('a') ]: 123 }");
+            expect(obj, 'to inspect as', "{ [Symbol('a')]: 123 }");
         });
 
         describe('when compared for equality', function () {
@@ -41,16 +41,16 @@ if (typeof Symbol === 'function' && Symbol('foo').toString() === 'Symbol(foo)')
                 expect(function () {
                     expect(a, 'to equal', b);
                 }, 'to throw',
-                    "expected { foo: 123, [ Symbol('a') ]: 'foo', [ Symbol('b') ]: 123 }\n" +
-                    "to equal { foo: 456, [ Symbol('a') ]: 'bar', [ Symbol('b') ]: 123 }\n" +
+                    "expected { foo: 123, [Symbol('a')]: 'foo', [Symbol('b')]: 123 }\n" +
+                    "to equal { foo: 456, [Symbol('a')]: 'bar', [Symbol('b')]: 123 }\n" +
                     "\n" +
                     "{\n" +
                     "  foo: 123, // should equal 456\n" +
-                    "  [ Symbol('a') ]: 'foo', // should equal 'bar'\n" +
-                    "                          //\n" +
-                    "                          // -foo\n" +
-                    "                          // +bar\n" +
-                    "  [ Symbol('b') ]: 123\n" +
+                    "  [Symbol('a')]: 'foo', // should equal 'bar'\n" +
+                    "                        //\n" +
+                    "                        // -foo\n" +
+                    "                        // +bar\n" +
+                    "  [Symbol('b')]: 123\n" +
                     "}"
                 );
             });
@@ -87,16 +87,16 @@ if (typeof Symbol === 'function' && Symbol('foo').toString() === 'Symbol(foo)')
                 expect(function () {
                     expect(a, 'to satisfy', b);
                 }, 'to throw',
-                    "expected { foo: 123, [ Symbol('a') ]: 'foo', [ Symbol('b') ]: 123 }\n" +
-                    "to satisfy { foo: 456, [ Symbol('a') ]: 'bar', [ Symbol('b') ]: 123 }\n" +
+                    "expected { foo: 123, [Symbol('a')]: 'foo', [Symbol('b')]: 123 }\n" +
+                    "to satisfy { foo: 456, [Symbol('a')]: 'bar', [Symbol('b')]: 123 }\n" +
                     "\n" +
                     "{\n" +
                     "  foo: 123, // should equal 456\n" +
-                    "  [ Symbol('a') ]: 'foo', // should equal 'bar'\n" +
-                    "                          //\n" +
-                    "                          // -foo\n" +
-                    "                          // +bar\n" +
-                    "  [ Symbol('b') ]: 123\n" +
+                    "  [Symbol('a')]: 'foo', // should equal 'bar'\n" +
+                    "                        //\n" +
+                    "                        // -foo\n" +
+                    "                        // +bar\n" +
+                    "  [Symbol('b')]: 123\n" +
                     "}"
                 );
             });
diff --git a/test/types/array-like-type.spec.js b/test/types/array-like-type.spec.js
index 1f330b63..6f860786 100644
--- a/test/types/array-like-type.spec.js
+++ b/test/types/array-like-type.spec.js
@@ -1,5 +1,133 @@
 /*global expect*/
 describe('array-like type', function () {
+    describe('equal()', function () {
+        var simpleArrayLikeType = {
+            name: 'simpleArrayLike',
+            base: 'array-like',
+            identify: Array.isArray,
+            numericalPropertiesOnly: false
+        };
+        var clonedExpect = expect.clone().addType(simpleArrayLikeType);
+
+        it('should treat properties with a value of undefined as equivalent to missing properties', function () {
+            var a = [];
+            a.ignoreMe = undefined;
+            var b = [];
+
+            clonedExpect(a, 'to equal', b);
+            clonedExpect(b, 'to equal', a);
+            clonedExpect(a, 'to satisfy', b);
+            clonedExpect(b, 'to satisfy', a);
+        });
+
+        it('should error when a LHS key is undefined on the RHS', function () {
+            var a = [ 'a' ];
+            a.foobar = true;
+            var b = [ 'a' ];
+
+            expect(function () {
+                clonedExpect(a, 'to equal', b);
+            }, 'to throw',
+                "expected [ 'a', foobar: true ] to equal [ 'a' ]\n" +
+                "\n" +
+                "[\n" +
+                "  'a',\n" +
+                "  foobar: true // should be removed\n" +
+                "]"
+            );
+        });
+
+        it('should error when a LHS key is explicitly undefined on the RHS', function () {
+            var a = [ 'a' ];
+            a.foobar = true;
+            var b = [ 'a' ];
+            b.foobar = undefined;
+
+            expect(function () {
+                clonedExpect(a, 'to equal', b);
+            }, 'to throw',
+                "expected [ 'a', foobar: true ] to equal [ 'a', foobar: undefined ]\n" +
+                "\n" +
+                "[\n" +
+                "  'a',\n" +
+                "  foobar: true // should be removed\n" +
+                "]"
+            );
+        });
+
+        it('should error when a LHS key is undefined on the RHS in "to satisfy"', function () {
+            var a = [ 'a' ];
+            a.foobar = true;
+            var b = [ 'a' ];
+            b.foobar = undefined;
+
+            expect(function () {
+                clonedExpect(a, 'to satisfy', b);
+            }, 'to throw',
+                "expected [ 'a', foobar: true ] to satisfy [ 'a', foobar: undefined ]\n" +
+                "\n" +
+                "[\n" +
+                "  'a',\n" +
+                "  foobar: true // should be removed\n" +
+                "]"
+            );
+        });
+
+        if (typeof Symbol === 'function') {
+            it('should error when a LHS key is a Symbol but undefined on the RHS', function () {
+                var a = [ 'a' ];
+                var s = Symbol('foo');
+                a[s] = true;
+                var b = [ 'a' ];
+
+                expect(function () {
+                    clonedExpect(a, 'to equal', b);
+                }, 'to throw',
+                    "expected [ 'a', [Symbol('foo')]: true ] to equal [ 'a' ]\n" +
+                    "\n" +
+                    "[\n" +
+                    "  'a',\n" +
+                    "  [Symbol('foo')]: true // should be removed\n" +
+                    "]"
+                );
+            });
+
+            (typeof weknowhow === 'undefined' ? it : it.skip)('should correctly fetch keys in the absence of symbol support', function () {
+                // stash away then clobber object symbol support
+                var getOwnPropertySymbols = Object.getOwnPropertySymbols;
+                delete Object.getOwnPropertySymbols;
+                // grab a fresh copy of Unexpected with object symbol support disabled
+                if (typeof jest !== 'undefined') {
+                    /*global jest*/
+                    jest.resetModules();
+                } else {
+                    delete require.cache[require.resolve('../../lib/')];
+                }
+                var localExpect = require('../../lib/').clone().addType(simpleArrayLikeType);
+                // restore object symbol support for the rest of the suite
+                Object.getOwnPropertySymbols = getOwnPropertySymbols;
+
+                var a = [ 'a' ];
+                a.foobar = true;
+                var s = Symbol('foo');
+                a[s] = true; // should be ignored
+                var b = [ 'a' ];
+                b.foobar = undefined;
+
+                localExpect(function () {
+                    localExpect(a, 'to equal', b);
+                }, 'to throw',
+                    "expected [ 'a', foobar: true ] to equal [ 'a', foobar: undefined ]\n" +
+                    "\n" +
+                    "[\n" +
+                    "  'a',\n" +
+                    "  foobar: true // should be removed\n" +
+                    "]"
+                );
+            });
+        }
+    });
+
     describe('with a subtype that disables indentation', function () {
         var clonedExpect = expect.clone();
 
@@ -157,9 +285,10 @@ describe('array-like type', function () {
     });
 
     describe('with a custom subtype that comes with its own getKeys', function () {
-        it('should bar', function () {
+        it('should process the elements in both inspection and diff in "to equal"', function () {
             var a = [ 'a' ];
             var b = [ 'a' ];
+            b.foobar = true;
 
             var clonedExpect = expect.clone().addType({
                 name: 'foo',
@@ -167,21 +296,55 @@ describe('array-like type', function () {
                 identify: Array.isArray,
                 numericalPropertiesOnly: false,
                 getKeys: function (obj) {
-                    var keys = this.baseType.getKeys(obj);
+                    // use array-like getKeys() method in non-numerical mode
+                    var keys = this.baseType.getKeys.call(this, obj);
                     if (obj === a) {
                         keys.push('foobar');
                     }
                     return keys;
                 }
             });
+
             expect(function () {
                 clonedExpect(a, 'to equal', b);
             }, 'to throw',
-                "expected [ 'a', foobar: undefined ] to equal [ 'a' ]\n" +
+                "expected [ 'a', foobar: undefined ] to equal [ 'a', foobar: true ]\n" +
+                "\n" +
+                "[\n" +
+                "  'a'\n" +
+                "  // missing foobar: true\n" +
+                "]"
+            );
+        });
+
+        it('should process the elements in both inspection and diff in "to satisfy"', function () {
+            var a = [ 'a' ];
+            var b = [ 'a' ];
+            b.foobar = true;
+
+            var clonedExpect = expect.clone().addType({
+                name: 'foo',
+                base: 'array-like',
+                identify: Array.isArray,
+                numericalPropertiesOnly: false,
+                getKeys: function (obj) {
+                    // use array-like getKeys in non-numerical mode
+                    var keys = this.baseType.getKeys.call(this, obj);
+                    if (obj === a) {
+                        keys.push('foobar');
+                    }
+                    return keys;
+                }
+            });
+
+            expect(function () {
+                clonedExpect(a, 'to satisfy', b);
+            }, 'to throw',
+                "expected [ 'a', foobar: undefined ] to satisfy [ 'a', foobar: true ]\n" +
                 "\n" +
                 "[\n" +
                 "  'a'\n" +
-                "  // missing foobar: undefined\n" +
+                "  // missing foobar: true\n" +
                 "]"
             );
         });