Fix bug when asserting some valid ES6 keys

- Resolution of chaijs#674
- Add compareByInspect utility for use with assertKeys sorts
- Add getOwnEnumerableProperties utility
- Add getOwnEnumerablePropertySymbols utility
- Add Symbol support to the inspect utility
- Add tests to utilities, should, expect, and assert

lib/chai/core/assertions.js

@@ -1170,7 +1170,7 @@ module.exports = function (chai, _) {
       }
 
     } else {
-      actual = Object.keys(obj);
+      actual = _.getOwnEnumerableProperties(obj);
 
       switch (_.type(keys)) {
         case 'array':
@@ -1184,7 +1184,10 @@ module.exports = function (chai, _) {
           keys = Array.prototype.slice.call(arguments);
       }
 
-      keys = keys.map(String);
+      // Only stringify non-Symbols because Symbols would become "Symbol()"
+      keys = keys.map(function (val) {
+        return typeof val === 'symbol' ? val : String(val);
+      });
     }
 
     if (!keys.length) throw new Error('keys required');
@@ -1248,8 +1251,8 @@ module.exports = function (chai, _) {
         ok
       , 'expected #{this} to ' + str
       , 'expected #{this} to not ' + str
-      , expected.slice(0).sort()
-      , actual.sort()
+      , expected.slice(0).sort(_.compareByInspect)
+      , actual.sort(_.compareByInspect)
       , true
     );
   }

lib/chai/utils/compareByInspect.js

@@ -0,0 +1,31 @@
+/*!
+ * Chai - compareByInspect utility
+ * Copyright(c) 2011-2016 Jake Luer <jake@alogicalparadox.com>
+ * MIT Licensed
+ */
+
+/*!
+ * Module dependancies
+ */
+
+var inspect = require('./inspect');
+
+/**
+ * ### .compareByInspect (mixed, mixed)
+ *
+ * To be used as a compareFunction with Array.prototype.sort. Compares elements
+ * using inspect instead of default behavior of using toString so that Symbols
+ * and objects with irregular/missing toString can still be sorted without a
+ * TypeError.
+ *
+ * @param {Mixed} first element to compare
+ * @param {Mixed} second element to compare
+ * @returns {Number} -1 if 'a' should come before 'b'; otherwise 1 
+ * @name compareByInspect
+ * @namespace Utils
+ * @api public
+ */
+
+module.exports = function (a, b) {
+  return inspect(a) < inspect(b) ? -1 : 1;
+};

lib/chai/utils/getOwnEnumerableProperties.js

@@ -0,0 +1,29 @@
+/*!
+ * Chai - getOwnEnumerableProperties utility
+ * Copyright(c) 2011-2016 Jake Luer <jake@alogicalparadox.com>
+ * MIT Licensed
+ */
+
+/*!
+ * Module dependancies
+ */
+
+var getOwnEnumerablePropertySymbols = require('./getOwnEnumerablePropertySymbols');
+
+/**
+ * ### .getOwnEnumerableProperties(object)
+ *
+ * This allows the retrieval of directly-owned enumerable property names and
+ * symbols of an object. This function is necessary because Object.keys only
+ * returns enumerable property names, not enumerable property symbols.
+ *
+ * @param {Object} object
+ * @returns {Array}
+ * @namespace Utils
+ * @name getOwnEnumerableProperties
+ * @api public
+ */
+
+module.exports = function getOwnEnumerableProperties(obj) {
+  return Object.keys(obj).concat(getOwnEnumerablePropertySymbols(obj));
+};

lib/chai/utils/getOwnEnumerablePropertySymbols.js

@@ -0,0 +1,27 @@
+/*!
+ * Chai - getOwnEnumerablePropertySymbols utility
+ * Copyright(c) 2011-2016 Jake Luer <jake@alogicalparadox.com>
+ * MIT Licensed
+ */
+
+/**
+ * ### .getOwnEnumerablePropertySymbols(object)
+ *
+ * This allows the retrieval of directly-owned enumerable property symbols of an
+ * object. This function is necessary because Object.getOwnPropertySymbols
+ * returns both enumerable and non-enumerable property symbols.
+ *
+ * @param {Object} object
+ * @returns {Array}
+ * @namespace Utils
+ * @name getOwnEnumerablePropertySymbols
+ * @api public
+ */
+
+module.exports = function getOwnEnumerablePropertySymbols(obj) {
+  if (typeof Object.getOwnPropertySymbols !== 'function') return [];
+
+  return Object.getOwnPropertySymbols(obj).filter(function (sym) {
+    return Object.getOwnPropertyDescriptor(obj, sym).enumerable;
+  });
+};

lib/chai/utils/index.js

@@ -128,3 +128,21 @@ exports.addChainableMethod = require('./addChainableMethod');
  */
 
 exports.overwriteChainableMethod = require('./overwriteChainableMethod');
+
+/*!
+ * Compare by inspect method
+ */
+
+exports.compareByInspect = require('./compareByInspect');
+
+/*!
+ * Get own enumerable property symbols method
+ */
+
+exports.getOwnEnumerablePropertySymbols = require('./getOwnEnumerablePropertySymbols');
+
+/*!
+ * Get own enumerable properties method
+ */
+
+exports.getOwnEnumerableProperties = require('./getOwnEnumerableProperties');

lib/chai/utils/inspect.js

@@ -200,6 +200,9 @@ function formatPrimitive(ctx, value) {
 
     case 'boolean':
       return ctx.stylize('' + value, 'boolean');
+
+    case 'symbol':
+      return ctx.stylize(value.toString(), 'symbol');
   }
   // For some reason typeof null is "object", so special case here.
   if (value === null) {

test/assert.js

@@ -558,6 +558,42 @@ describe('assert', function () {
     assert.doesNotHaveAnyKeys({ foo: 1, bar: 2 }, { baz: 1, biz: 2, fake: 3 });
     assert.doesNotHaveAnyKeys({ foo: 1, bar: 2 }, { baz: 1 });
 
+    var enumProp1 = 'enumProp1'
+      , enumProp2 = 'enumProp2'
+      , nonEnumProp = 'nonEnumProp'
+      , obj = {};
+
+    obj[enumProp1] = 'enumProp1';
+    obj[enumProp2] = 'enumProp2';
+
+    Object.defineProperty(obj, nonEnumProp, {
+        enumerable: false,
+        value: 'nonEnumProp'
+    });
+
+    assert.hasAllKeys(obj, [enumProp1, enumProp2]);
+    assert.doesNotHaveAllKeys(obj, [enumProp1, enumProp2, nonEnumProp]);
+
+    if (typeof Symbol === 'function') {
+      var sym1 = Symbol('sym1')
+        , sym2 = Symbol('sym2')
+        , sym3 = Symbol('sym3')
+        , str = 'str'
+        , obj = {};
+
+      obj[sym1] = 'sym1';
+      obj[sym2] = 'sym2';
+      obj[str] = 'str';
+
+      Object.defineProperty(obj, sym3, {
+        enumerable: false,
+        value: 'sym3'
+      });
+
+      assert.hasAllKeys(obj, [sym1, sym2, str]);
+      assert.doesNotHaveAllKeys(obj, [sym1, sym2, sym3, str]);
+    }
+
     if (typeof Map !== 'undefined') {
       var aKey = {thisIs: 'anExampleObject'};
       var anotherKey = {doingThisBecauseOf: 'referential equality'};
@@ -577,6 +613,25 @@ describe('assert', function () {
       assert.doesNotHaveAnyKeys(new Map([[aKey, 'aValue'], [anotherKey, 'anotherValue']]), [ 'thisDoesNotExist', 'thisToo', {iDoNot: 'exist'} ]);
       assert.doesNotHaveAllKeys(new Map([[aKey, 'aValue'], [anotherKey, 'anotherValue']]), [ aKey, {iDoNot: 'exist'} ]);
 
+      var weirdMapKey1 = Object.create(null)
+        , weirdMapKey2 = {toString: NaN}
+        , weirdMapKey3 = [];
+      assert.hasAllKeys(new Map([[weirdMapKey1, 'val1'], [weirdMapKey2, 'val2']]), [weirdMapKey1, weirdMapKey2]);
+      assert.doesNotHaveAllKeys(new Map([[weirdMapKey1, 'val1'], [weirdMapKey2, 'val2']]), [weirdMapKey1, weirdMapKey3]);
+
+      if (typeof Symbol === 'function') {
+        var symMapKey1 = Symbol()
+          , symMapKey2 = Symbol()
+          , symMapKey3 = Symbol();
+
+        assert.hasAllKeys(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']]), [symMapKey1, symMapKey2]);
+        assert.hasAnyKeys(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']]), [symMapKey1, symMapKey3]);
+        assert.containsAllKeys(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']]), [symMapKey2, symMapKey1]);
+
+        assert.doesNotHaveAllKeys(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']]), [symMapKey1, symMapKey3]);
+        assert.doesNotHaveAnyKeys(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']]), [symMapKey3]);
+      }
+
       err(function(){
         assert.hasAllKeys(new Map([[{1: 20}, 'number']]));
       }, "keys required");
@@ -637,6 +692,25 @@ describe('assert', function () {
       assert.doesNotHaveAnyKeys(new Set([aKey, anotherKey]), [ 20, 1, {iDoNot: 'exist'} ]);
       assert.doesNotHaveAllKeys(new Set([aKey, anotherKey]), [ 'thisDoesNotExist', 'thisToo', {iDoNot: 'exist'} ]);
 
+      var weirdSetKey1 = Object.create(null)
+        , weirdSetKey2 = {toString: NaN}
+        , weirdSetKey3 = [];
+      assert.hasAllKeys(new Set([weirdSetKey1, weirdSetKey2]), [weirdSetKey1, weirdSetKey2]);
+      assert.doesNotHaveAllKeys(new Set([weirdSetKey1, weirdSetKey2]), [weirdSetKey1, weirdSetKey3]);
+
+      if (typeof Symbol === 'function') {
+        var symSetKey1 = Symbol()
+          , symSetKey2 = Symbol()
+          , symSetKey3 = Symbol();
+
+        assert.hasAllKeys(new Set([symSetKey1, symSetKey2]), [symSetKey1, symSetKey2]);
+        assert.hasAnyKeys(new Set([symSetKey1, symSetKey2]), [symSetKey1, symSetKey3]);
+        assert.containsAllKeys(new Set([symSetKey1, symSetKey2]), [symSetKey2, symSetKey1]);
+
+        assert.doesNotHaveAllKeys(new Set([symSetKey1, symSetKey2]), [symSetKey1, symSetKey3]);
+        assert.doesNotHaveAnyKeys(new Set([symSetKey1, symSetKey2]), [symSetKey3]);
+      }
+
       err(function(){
         assert.hasAllKeys(new Set([{1: 20}, 'number']));
       }, "keys required");

test/expect.js

@@ -743,6 +743,42 @@ describe('expect', function () {
     expect({ foo: 1, bar: 2 }).not.have.all.keys({ 'baz': 8, 'foo': 7 });
     expect({ foo: 1, bar: 2 }).not.contain.all.keys({ 'baz': 8, 'foo': 7 });
 
+    var enumProp1 = 'enumProp1'
+      , enumProp2 = 'enumProp2'
+      , nonEnumProp = 'nonEnumProp'
+      , obj = {};
+
+    obj[enumProp1] = 'enumProp1';
+    obj[enumProp2] = 'enumProp2';
+
+    Object.defineProperty(obj, nonEnumProp, {
+        enumerable: false,
+        value: 'nonEnumProp'
+    });
+
+    expect(obj).to.have.all.keys([enumProp1, enumProp2]);
+    expect(obj).to.not.have.all.keys([enumProp1, enumProp2, nonEnumProp]);
+
+    if (typeof Symbol === 'function') {
+      var sym1 = Symbol('sym1')
+        , sym2 = Symbol('sym2')
+        , sym3 = Symbol('sym3')
+        , str = 'str'
+        , obj = {};
+
+      obj[sym1] = 'sym1';
+      obj[sym2] = 'sym2';
+      obj[str] = 'str';
+
+      Object.defineProperty(obj, sym3, {
+        enumerable: false,
+        value: 'sym3'
+      });
+
+      expect(obj).to.have.all.keys([sym1, sym2, str]);
+      expect(obj).to.not.have.all.keys([sym1, sym2, sym3, str]);
+    }
+
     if (typeof Map !== 'undefined') {
       var aKey = {thisIs: 'anExampleObject'};
       var anotherKey = {doingThisBecauseOf: 'referential equality'};
@@ -766,6 +802,28 @@ describe('expect', function () {
       expect(new Map([[aKey, 'aValue'], [anotherKey, 'anotherValue']])).to.not.have.any.keys([20, 1, {13: 37}]);
       expect(new Map([[aKey, 'aValue'], [anotherKey, 'anotherValue']])).to.not.have.all.keys([aKey, {'iDoNot': 'exist'}]);
 
+      var weirdMapKey1 = Object.create(null)
+        , weirdMapKey2 = {toString: NaN}
+        , weirdMapKey3 = [];
+      expect(new Map([[weirdMapKey1, 'val1'], [weirdMapKey2, 'val2']])).to.have.all.keys([weirdMapKey1, weirdMapKey2]);
+      expect(new Map([[weirdMapKey1, 'val1'], [weirdMapKey2, 'val2']])).to.not.have.all.keys([weirdMapKey1, weirdMapKey3]);
+
+      if (typeof Symbol === 'function') {
+        var symMapKey1 = Symbol()
+          , symMapKey2 = Symbol()
+          , symMapKey3 = Symbol();
+
+        expect(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']])).to.have.all.keys(symMapKey1, symMapKey2);
+        expect(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']])).to.have.any.keys(symMapKey1, symMapKey3);
+        expect(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']])).to.contain.all.keys(symMapKey2, symMapKey1);
+        expect(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']])).to.contain.any.keys(symMapKey3, symMapKey1);
+
+        expect(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']])).to.not.have.all.keys(symMapKey1, symMapKey3);
+        expect(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']])).to.not.have.any.keys(symMapKey3);
+        expect(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']])).to.not.contain.all.keys(symMapKey3, symMapKey1);
+        expect(new Map([[symMapKey1, 'symValue1'], [symMapKey2, 'symValue2']])).to.not.contain.any.keys(symMapKey3);
+      }
+
       err(function(){
         expect(new Map().set({ foo: 1 })).to.have.keys();
       }, "keys required");
@@ -806,6 +864,28 @@ describe('expect', function () {
       expect(new Set([aKey, anotherKey])).to.not.have.any.keys([20, 1, {13: 37}]);
       expect(new Set([aKey, anotherKey])).to.not.have.all.keys([aKey, {'iDoNot': 'exist'}]);
 
+      var weirdSetKey1 = Object.create(null)
+        , weirdSetKey2 = {toString: NaN}
+        , weirdSetKey3 = [];
+      expect(new Set([weirdSetKey1, weirdSetKey2])).to.have.all.keys([weirdSetKey1, weirdSetKey2]);
+      expect(new Set([weirdSetKey1, weirdSetKey2])).to.not.have.all.keys([weirdSetKey1, weirdSetKey3]);
+
+      if (typeof Symbol === 'function') {
+        var symSetKey1 = Symbol()
+          , symSetKey2 = Symbol()
+          , symSetKey3 = Symbol();
+
+        expect(new Set([symSetKey1, symSetKey2])).to.have.all.keys(symSetKey1, symSetKey2);
+        expect(new Set([symSetKey1, symSetKey2])).to.have.any.keys(symSetKey1, symSetKey3);
+        expect(new Set([symSetKey1, symSetKey2])).to.contain.all.keys(symSetKey2, symSetKey1);
+        expect(new Set([symSetKey1, symSetKey2])).to.contain.any.keys(symSetKey3, symSetKey1);
+
+        expect(new Set([symSetKey1, symSetKey2])).to.not.have.all.keys(symSetKey1, symSetKey3);
+        expect(new Set([symSetKey1, symSetKey2])).to.not.have.any.keys(symSetKey3);
+        expect(new Set([symSetKey1, symSetKey2])).to.not.contain.all.keys(symSetKey3, symSetKey1);
+        expect(new Set([symSetKey1, symSetKey2])).to.not.contain.any.keys(symSetKey3);
+      }
+
       err(function(){
         expect(new Set().add({ foo: 1 })).to.have.keys();
       }, "keys required");