Repository: chaijs/chai
Pull request #813 (2016-10-02 16:44:32)

Make `utils.getName` more robust & use it in `empty`
Current ponyfill routine does not handle `toString` defined on functions. That is kinda rare, but does not take much effort to support. Beware, proposed "fix" will throw on non-functions (is that desirable?). Also, added basic tests for `getName`.

meeber: Hmm. I think `String` calls `toString` internally. The new tests you added pass without the changes. Do you see any other benefits from doing it this way? Regardless, I think it's worth merging the new tests you added.
shvaikalesh: It does, but if custom `toString` defined directly on a function it will fail. The tests pass without the changes because both Node and Phantom support decent subset of ES6 name property. However, `bar.toString = function()` test will fail in IE10-11 without the change.
lucasfcosta: This LGTM.
I think we should also add those changes to `check-error` btw.
Maybe moving that into a separate module would be nice for a next release such as `5.0.0` so we avoid code duplication.

I'll create an issue for that.
meeber: Okay, I'm convinced. LGTM!

Patch:
diff --git a/lib/chai/assertion.js b/lib/chai/assertion.js
index 2a97d21..107eb08 100644
--- a/lib/chai/assertion.js
+++ b/lib/chai/assertion.js
@@ -105,8 +105,8 @@ module.exports = function (_chai, util) {
     if (true !== config.showDiff) showDiff = false;
 
     if (!ok) {
-      msg = util.getMessage(this, arguments)
-        , actual = util.getActual(this, arguments);
+      msg = util.getMessage(this, arguments);
+      var actual = util.getActual(this, arguments);
       throw new AssertionError(msg, {
           actual: actual
         , expected: expected
diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js
index 829c174..6881ba1 100644
--- a/lib/chai/core/assertions.js
+++ b/lib/chai/core/assertions.js
@@ -536,9 +536,16 @@ module.exports = function (chai, _) {
       case 'string':
         itemsCount = val.length;
         break;
+      case 'map':
+      case 'set':
+        itemsCount = val.size;
+        break;
+      case 'weakmap':
+      case 'weakset':
+        throw new TypeError('.empty was passed a weak collection');
       case 'function':
-        var name = val.name ? ' ' + val.name : '';
-        throw new TypeError('.empty was passed a function' + name);
+        var msg = '.empty was passed a function ' + _.getName(val);
+        throw new TypeError(msg.trim());
       default:
         if (val !== Object(val)) {
           throw new TypeError('.empty was passed non-string primitive ' + _.inspect(val));
diff --git a/lib/chai/utils/getName.js b/lib/chai/utils/getName.js
index 038b63a..67a5e8e 100644
--- a/lib/chai/utils/getName.js
+++ b/lib/chai/utils/getName.js
@@ -14,13 +14,14 @@
  * @name getName
  */
 
+var toString = Function.prototype.toString;
 var functionNameMatch = /\s*function(?:\s|\s*\/\*[^(?:*\/)]+\*\/\s*)*([^\s\(\/]+)/;
 
 module.exports = function (func) {
   var name = '';
   if (typeof func.name === 'undefined') {
     // Here we run a polyfill if func.name is not defined
-    var match = String(func).match(functionNameMatch);
+    var match = toString.call(func).match(functionNameMatch);
     if (match) {
       name = match[1];
     }


Test patch:
diff --git a/test/expect.js b/test/expect.js
index c706021..7b1a75f 100644
--- a/test/expect.js
+++ b/test/expect.js
@@ -646,6 +646,60 @@ describe('expect', function () {
     expect({}).to.be.empty;
     expect({foo: 'bar'}).not.to.be.empty;
 
+    if (typeof WeakMap === 'function') {
+      err(function(){
+        expect(new WeakMap).not.to.be.empty;
+      }, ".empty was passed a weak collection");
+    }
+
+    if (typeof WeakSet === 'function') {
+      err(function(){
+        expect(new WeakSet).not.to.be.empty;
+      }, ".empty was passed a weak collection");
+    }
+
+    if (typeof Map === 'function') {
+      expect(new Map).to.be.empty;
+
+      // Not using Map constructor args because not supported in IE 11.
+      var map = new Map;
+      map.set('a', 1);
+      expect(map).not.to.be.empty;
+
+      err(function(){
+        expect(new Map).not.to.be.empty;
+      }, "expected {} not to be empty");
+
+      map = new Map;
+      map.key = 'val';
+      expect(map).to.be.empty;
+
+      err(function(){
+        expect(map).not.to.be.empty;
+      }, "expected { key: 'val' } not to be empty");
+    }
+
+    if (typeof Set === 'function') {
+      expect(new Set).to.be.empty;
+
+      // Not using Set constructor args because not supported in IE 11.
+      var set = new Set;
+      set.add(1);
+      expect(set).not.to.be.empty;
+
+      err(function(){
+        expect(new Set).not.to.be.empty;
+      }, "expected {} not to be empty");
+
+      set = new Set;
+      set.key = 'val';
+      expect(set).to.be.empty;
+
+      err(function(){
+        expect(set).not.to.be.empty;
+      }, "expected { key: 'val' } not to be empty");
+    }
+
     err(function(){
       expect('').not.to.be.empty;
     }, "expected \'\' not to be empty");
diff --git a/test/should.js b/test/should.js
index b827784..d715bbc 100644
--- a/test/should.js
+++ b/test/should.js
@@ -626,6 +626,60 @@ describe('should', function() {
     ({}).should.be.empty;
     ({foo: 'bar'}).should.not.be.empty;
 
+    if (typeof WeakMap === 'function') {
+      err(function(){
+        (new WeakMap).should.not.be.empty;
+      }, ".empty was passed a weak collection");
+    }
+
+    if (typeof WeakSet === 'function') {
+      err(function(){
+        (new WeakSet).should.not.be.empty;
+      }, ".empty was passed a weak collection");
+    }
+
+    if (typeof Map === 'function') {
+      (new Map).should.be.empty;
+
+      // Not using Map constructor args because not supported in IE 11.
+      var map = new Map;
+      map.set('a', 1);
+      map.should.not.be.empty;
+
+      err(function(){
+        (new Map).should.not.be.empty;
+      }, "expected {} not to be empty");
+
+      map = new Map;
+      map.key = 'val';
+      map.should.be.empty;
+
+      err(function(){
+        map.should.not.be.empty;
+      }, "expected { key: 'val' } not to be empty");
+    }
+
+    if (typeof Set === 'function') {
+      (new Set).should.be.empty;
+
+      // Not using Set constructor args because not supported in IE 11.
+      var set = new Set;
+      set.add(1);
+      set.should.not.be.empty;
+
+      err(function(){
+        (new Set).should.not.be.empty;
+      }, "expected {} not to be empty");
+
+      set = new Set;
+      set.key = 'val';
+      set.should.be.empty;
+
+      err(function(){
+        set.should.not.be.empty;
+      }, "expected { key: 'val' } not to be empty");
+    }
+
     err(function(){
       ''.should.not.be.empty;
     }, "expected \'\' not to be empty");
@@ -680,7 +734,7 @@ describe('should', function() {
       }, ".empty was passed non-string primitive Symbol()");
 
       err(function(){
-        Symbol.iterator.should.to.be.empty;
+        Symbol.iterator.should.be.empty;
       }, ".empty was passed non-string primitive Symbol(Symbol.iterator)");
     }
 
diff --git a/test/utilities.js b/test/utilities.js
index 48a3c2c..44210e7 100644
--- a/test/utilities.js
+++ b/test/utilities.js
@@ -54,6 +54,20 @@ describe('utilities', function () {
     });
   });
 
+  it('getName', function () {
+    chai.use(function (_chai, utils) {
+      var name = utils.getName;
+      expect(name(function () {})).to.equal('');
+      expect(name(function foo() {})).to.equal('foo');
+
+      var bar = function bar() {};
+      bar.toString = function() {
+        return 'function foo() {}';
+      };
+
+      expect(name(bar)).to.equal('bar');
+    });
+  });
 
   it('getPathValue', function () {
     var object = {