feat(utils): add length guard to methods

When the `length` assertion is chained directly off of an uninvoked
method, it references `function`'s built-in `length` property instead
of Chai's `length` assertion. This commit adds a guard to Chai methods
to detect this problem and throw a helpful error message that advises
the user on how to correct it.

lib/chai/utils/addChainableMethod.js

@@ -8,6 +8,7 @@
  * Module dependencies
  */
 
+var addLengthGuard = require('./addLengthGuard');
 var chai = require('../../chai');
 var flag = require('./flag');
 var proxify = require('./proxify');
@@ -93,6 +94,8 @@ module.exports = function (ctx, name, method, chainingBehavior) {
           return newAssertion;
         };
 
+        addLengthGuard(assert, name, true);
+
         // Use `__proto__` if available
         if (hasProtoSupport) {
           // Inherit all properties from the object by replacing the `Function` prototype

lib/chai/utils/addLengthGuard.js

@@ -0,0 +1,65 @@
+var config = require('../config');
+
+var fnDesc = Object.getOwnPropertyDescriptor(function () {}, 'length');
+var isFnLengthConfigurable = typeof fnDesc === 'object'
+                           ? fnDesc.configurable
+                           : false;
+
+/*!
+ * Chai - addLengthGuard utility
+ * Copyright(c) 2012-2014 Jake Luer <jake@alogicalparadox.com>
+ * MIT Licensed
+ */
+
+/**
+ * # addLengthGuard(fn, assertionName, isChainable)
+ *
+ * Define `length` as a getter on the given uninvoked method assertion. The
+ * getter acts as a guard against chaining `length` directly off of an uninvoked
+ * method assertion, which is a problem because it references `function`'s
+ * built-in `length` property instead of Chai's `length` assertion. When the
+ * getter catches the user making this mistake, it throws an error with a
+ * helpful message.
+ *
+ * There are two ways in which this mistake can be made. The first way is by
+ * chaining the `length` assertion directly off of an uninvoked chainable
+ * method. In this case, Chai suggests that the user use `lengthOf` instead. The
+ * second way is by chaining the `length` assertion directly off of an uninvoked
+ * non-chainable method. Non-chainable methods must be invoked prior to
+ * chaining. In this case, Chai suggests that the user consult the docs for the
+ * given assertion.
+ *
+ * If the `length` property of functions is unconfigurable, then return `fn`
+ * without modification.
+ *
+ * Note that in ES6, the function's `length` property is configurable, so once
+ * support for legacy environments is dropped, Chai's `length` property can
+ * replace the built-in function's `length` property, and this length guard will
+ * no longer be necessary. In the mean time, maintaining consistency across all
+ * environments is the priority.
+ *
+ * @param {Function} fn
+ * @param {String} assertionName
+ * @param {Boolean} isChainable
+ * @namespace Utils
+ * @name addLengthGuard
+ */
+
+module.exports = function addLengthGuard (fn, assertionName, isChainable) {
+  if (isFnLengthConfigurable !== true) return fn;
+
+  Object.defineProperty(fn, 'length', {
+    get: function () {
+      if (isChainable === true) {
+        throw Error('Invalid Chai property: ' + assertionName + '.length. Due' +
+          ' to a compatibility issue, "length" cannot directly follow "' +
+          assertionName + '". Use "' + assertionName + '.lengthOf" instead.');
+      }
+
+      throw Error('Invalid Chai property: ' + assertionName + '.length. See' +
+        ' docs for proper usage of "' + assertionName + '".');
+    }
+  });
+
+  return fn;
+};

lib/chai/utils/addMethod.js

@@ -4,6 +4,7 @@
  * MIT Licensed
  */
 
+var addLengthGuard = require('./addLengthGuard');
 var chai = require('../../chai');
 var flag = require('./flag');
 var proxify = require('./proxify');
@@ -51,5 +52,6 @@ module.exports = function (ctx, name, method) {
     return newAssertion;
   };
 
+  addLengthGuard(fn, name, false);
   ctx[name] = proxify(fn, name);
 };

lib/chai/utils/index.js

@@ -153,6 +153,12 @@ exports.checkError = require('check-error');
 
 exports.proxify = require('./proxify');
 
+/*!
+ * Proxify util
+ */
+
+exports.addLengthGuard = require('./addLengthGuard');
+
 /*!
  * isNaN method
  */

lib/chai/utils/overwriteMethod.js

@@ -4,6 +4,7 @@
  * MIT Licensed
  */
 
+var addLengthGuard = require('./addLengthGuard');
 var chai = require('../../chai');
 var flag = require('./flag');
 var proxify = require('./proxify');
@@ -71,5 +72,6 @@ module.exports = function (ctx, name, method) {
     return newAssertion;
   }
 
+  addLengthGuard(fn, name, false);
   ctx[name] = proxify(fn, name);
 };

test/expect.js

@@ -10,9 +10,7 @@ describe('expect', function () {
     expect('foo').to.equal('foo');
   });
 
-  describe('invalid property', function () {
-    if (typeof Proxy === 'undefined' || typeof Reflect === 'undefined') return;
-
+  describe('safeguards', function () {
     before(function () {
       chai.util.addProperty(chai.Assertion.prototype, 'tmpProperty', function () {
         new chai.Assertion(42).equal(42);
@@ -54,82 +52,163 @@ describe('expect', function () {
       delete chai.Assertion.prototype.tmpChainableMethod;
     });
 
-    it('throws when invalid property follows expect', function () {
-      err(function () {
-        expect(42).pizza;
-      }, 'Invalid Chai property: pizza');
-    });
+    describe('proxify', function () {
+      if (typeof Proxy === 'undefined' || typeof Reflect === 'undefined') return;
 
-    it('throws when invalid property follows language chain', function () {
-      err(function () {
-        expect(42).to.pizza;
-      }, 'Invalid Chai property: pizza');
-    });
+      it('throws when invalid property follows expect', function () {
+        err(function () {
+          expect(42).pizza;
+        }, 'Invalid Chai property: pizza');
+      });
 
-    it('throws when invalid property follows property assertion', function () {
-      err(function () {
-        expect(42).ok.pizza;
-      }, 'Invalid Chai property: pizza');
-    });
+      it('throws when invalid property follows language chain', function () {
+        err(function () {
+          expect(42).to.pizza;
+        }, 'Invalid Chai property: pizza');
+      });
 
-    it('throws when invalid property follows overwritten property assertion', function () {
-      err(function () {
-        expect(42).tmpProperty.pizza;
-      }, 'Invalid Chai property: pizza');
+      it('throws when invalid property follows property assertion', function () {
+        err(function () {
+          expect(42).ok.pizza;
+        }, 'Invalid Chai property: pizza');
+      });
+
+      it('throws when invalid property follows overwritten property assertion', function () {
+        err(function () {
+          expect(42).tmpProperty.pizza;
+        }, 'Invalid Chai property: pizza');
+      });
+
+      it('throws when invalid property follows uncalled method assertion', function () {
+        err(function () {
+          expect(42).equal.pizza;
+        }, 'Invalid Chai property: equal.pizza. See docs for proper usage of "equal".');
+      });
+
+      it('throws when invalid property follows called method assertion', function () {
+        err(function () {
+          expect(42).equal(42).pizza;
+        }, 'Invalid Chai property: pizza');
+      });
+
+      it('throws when invalid property follows uncalled overwritten method assertion', function () {
+        err(function () {
+          expect(42).tmpMethod.pizza;
+        }, 'Invalid Chai property: tmpMethod.pizza. See docs for proper usage of "tmpMethod".');
+      });
+
+      it('throws when invalid property follows called overwritten method assertion', function () {
+        err(function () {
+          expect(42).tmpMethod().pizza;
+        }, 'Invalid Chai property: pizza');
+      });
+
+      it('throws when invalid property follows uncalled chainable method assertion', function () {
+        err(function () {
+          expect(42).a.pizza;
+        }, 'Invalid Chai property: pizza');
+      });
+
+      it('throws when invalid property follows called chainable method assertion', function () {
+        err(function () {
+          expect(42).a('number').pizza;
+        }, 'Invalid Chai property: pizza');
+      });
+
+      it('throws when invalid property follows uncalled overwritten chainable method assertion', function () {
+       err(function () {
+          expect(42).tmpChainableMethod.pizza;
+        }, 'Invalid Chai property: pizza');
+      });
+
+      it('throws when invalid property follows called overwritten chainable method assertion', function () {
+        err(function () {
+          expect(42).tmpChainableMethod().pizza;
+        }, 'Invalid Chai property: pizza');
+      });
+
+      it('doesn\'t throw if invalid property is excluded via config', function () {
+        expect(function () {
+          expect(42).then;
+        }).to.not.throw();
+      });
     });
 
-    it('throws when invalid property follows uncalled method assertion', function () {
-      err(function () {
-        expect(42).equal.pizza;
-      }, 'Invalid Chai property: equal.pizza. See docs for proper usage of "equal".');
-    });
+    describe('length guard', function () {
+      var fnDesc = Object.getOwnPropertyDescriptor(function () {}, 'length');
+      if (typeof fnDesc !== 'object' || fnDesc.configurable !== true) return;
 
-    it('throws when invalid property follows called method assertion', function () {
-      err(function () {
-        expect(42).equal(42).pizza;
-      }, 'Invalid Chai property: pizza');
-    });
+      it('doesn\'t throw when `.length` follows `.expect`', function () {
+        expect(function () {
+          expect('foo').length;
+        }).to.not.throw();
+      });
 
-    it('throws when invalid property follows uncalled overwritten method assertion', function () {
-      err(function () {
-        expect(42).tmpMethod.pizza;
-      }, 'Invalid Chai property: tmpMethod.pizza. See docs for proper usage of "tmpMethod".');
-    });
+      it('doesn\'t throw when `.length` follows language chain', function () {
+        expect(function () {
+          expect('foo').to.length;
+        }).to.not.throw();
+      });
 
-    it('throws when invalid property follows called overwritten method assertion', function () {
-      err(function () {
-        expect(42).tmpMethod().pizza;
-      }, 'Invalid Chai property: pizza');
-    });
+      it('doesn\'t throw when `.length` follows property assertion', function () {
+        expect(function () {
+          expect('foo').ok.length;
+        }).to.not.throw();
+      });
 
-    it('throws when invalid property follows uncalled chainable method assertion', function () {
-      err(function () {
-        expect(42).a.pizza;
-      }, 'Invalid Chai property: pizza');
-    });
+      it('doesn\'t throw when `.length` follows overwritten property assertion', function () {
+        expect(function () {
+          expect('foo').tmpProperty.length;
+        }).to.not.throw();
+      });
 
-    it('throws when invalid property follows called chainable method assertion', function () {
-      err(function () {
-        expect(42).a('number').pizza;
-      }, 'Invalid Chai property: pizza');
-    });
+      it('throws when `.length` follows uncalled method assertion', function () {
+        err(function () {
+          expect('foo').equal.length;
+        }, 'Invalid Chai property: equal.length. See docs for proper usage of "equal".');
+      });
 
-    it('throws when invalid property follows uncalled overwritten chainable method assertion', function () {
-      err(function () {
-        expect(42).tmpChainableMethod.pizza;
-      }, 'Invalid Chai property: pizza');
-    });
+      it('doesn\'t throw when `.length` follows called method assertion', function () {
+        expect(function () {
+          expect('foo').equal('foo').length;
+        }).to.not.throw();
+      });
 
-    it('throws when invalid property follows called overwritten chainable method assertion', function () {
-      err(function () {
-        expect(42).tmpChainableMethod().pizza;
-      }, 'Invalid Chai property: pizza');
-    });
+      it('throws when `.length` follows uncalled overwritten method assertion', function () {
+        err(function () {
+          expect('foo').tmpMethod.length;
+        }, 'Invalid Chai property: tmpMethod.length. See docs for proper usage of "tmpMethod".');
+      });
 
-    it('doesn\'t throw if invalid property is excluded via config', function () {
-      expect(function () {
-        expect(42).then;
-      }).to.not.throw();
+      it('doesn\'t throw when `.length` follows called overwritten method assertion', function () {
+        expect(function () {
+          expect('foo').tmpMethod().length;
+        }).to.not.throw();
+      });
+
+      it('throws when `.length` follows uncalled chainable method assertion', function () {
+        err(function () {
+          expect('foo').a.length;
+        }, 'Invalid Chai property: a.length. Due to a compatibility issue, "length" cannot directly follow "a". Use "a.lengthOf" instead.');
+      });
+
+      it('doesn\'t throw when `.length` follows called chainable method assertion', function () {
+        expect(function () {
+          expect('foo').a('string').length;
+        }).to.not.throw();
+      });
+
+      it('throws when `.length` follows uncalled overwritten chainable method assertion', function () {
+        err(function () {
+          expect('foo').tmpChainableMethod.length;
+        }, 'Invalid Chai property: tmpChainableMethod.length. Due to a compatibility issue, "length" cannot directly follow "tmpChainableMethod". Use "tmpChainableMethod.lengthOf" instead.');
+      });
+
+      it('doesn\'t throw when `.length` follows called overwritten chainable method assertion', function () {
+        expect(function () {
+          expect('foo').tmpChainableMethod().length;
+        }).to.not.throw();
+      });
     });
   });