Merge pull request #897 from meeber/proxy-length

Un-deprecate `length` and add guard

lib/chai/core/assertions.js

@@ -674,14 +674,14 @@ module.exports = function (chai, _) {
    *
    *     expect(10).to.be.above(5);
    *
-   * Can also be used in conjunction with `length` to
+   * Can also be used in conjunction with `lengthOf` to
    * assert a minimum length. The benefit being a
    * more informative error message than if the length
    * was supplied directly. In this case the target must
    * have a length property.
    *
-   *     expect('foo').to.have.length.above(2);
-   *     expect([ 1, 2, 3 ]).to.have.length.above(2);
+   *     expect('foo').to.have.lengthOf.above(2);
+   *     expect([ 1, 2, 3 ]).to.have.lengthOf.above(2);
    *
    * @name above
    * @alias gt
@@ -736,14 +736,14 @@ module.exports = function (chai, _) {
    *
    *     expect(10).to.be.at.least(10);
    *
-   * Can also be used in conjunction with `length` to
+   * Can also be used in conjunction with `lengthOf` to
    * assert a minimum length. The benefit being a
    * more informative error message than if the length
    * was supplied directly. In this case the target must
    * have a length property.
    *
-   *     expect('foo').to.have.length.of.at.least(2);
-   *     expect([ 1, 2, 3 ]).to.have.length.of.at.least(3);
+   *     expect('foo').to.have.lengthOf.at.least(2);
+   *     expect([ 1, 2, 3 ]).to.have.lengthOf.at.least(3);
    *
    * @name least
    * @alias gte
@@ -796,14 +796,14 @@ module.exports = function (chai, _) {
    *
    *     expect(5).to.be.below(10);
    *
-   * Can also be used in conjunction with `length` to
+   * Can also be used in conjunction with `lengthOf` to
    * assert a maximum length. The benefit being a
    * more informative error message than if the length
    * was supplied directly. In this case the target must
    * have a length property.
    *
-   *     expect('foo').to.have.length.below(4);
-   *     expect([ 1, 2, 3 ]).to.have.length.below(4);
+   *     expect('foo').to.have.lengthOf.below(4);
+   *     expect([ 1, 2, 3 ]).to.have.lengthOf.below(4);
    *
    * @name below
    * @alias lt
@@ -858,14 +858,14 @@ module.exports = function (chai, _) {
    *
    *     expect(5).to.be.at.most(5);
    *
-   * Can also be used in conjunction with `length` to
+   * Can also be used in conjunction with `lengthOf` to
    * assert a maximum length. The benefit being a
    * more informative error message than if the length
    * was supplied directly. In this case the target must
    * have a length property.
    *
-   *     expect('foo').to.have.length.of.at.most(4);
-   *     expect([ 1, 2, 3 ]).to.have.length.of.at.most(3);
+   *     expect('foo').to.have.lengthOf.at.most(4);
+   *     expect([ 1, 2, 3 ]).to.have.lengthOf.at.most(3);
    *
    * @name most
    * @alias lte
@@ -918,14 +918,14 @@ module.exports = function (chai, _) {
    *
    *     expect(7).to.be.within(5,10);
    *
-   * Can also be used in conjunction with `length` to
+   * Can also be used in conjunction with `lengthOf` to
    * assert a length range. The benefit being a
    * more informative error message than if the length
    * was supplied directly. In this case the target must
    * have a length property.
    *
-   *     expect('foo').to.have.length.within(2,4);
-   *     expect([ 1, 2, 3 ]).to.have.length.within(2,4);
+   *     expect('foo').to.have.lengthOf.within(2,4);
+   *     expect([ 1, 2, 3 ]).to.have.lengthOf.within(2,4);
    *
    * @name within
    * @param {Number} start lowerbound inclusive
@@ -1212,39 +1212,41 @@ module.exports = function (chai, _) {
   Assertion.addMethod('ownPropertyDescriptor', assertOwnPropertyDescriptor);
   Assertion.addMethod('haveOwnPropertyDescriptor', assertOwnPropertyDescriptor);
 
-  /**
-   * ### .length
-   *
-   * Sets the `doLength` flag later used as a chain precursor to a value
-   * comparison for the `length` property.
-   *
-   *     expect('foo').to.have.length.above(2);
-   *     expect([ 1, 2, 3 ]).to.have.length.above(2);
-   *     expect('foo').to.have.length.below(4);
-   *     expect([ 1, 2, 3 ]).to.have.length.below(4);
-   *     expect('foo').to.have.length.within(2,4);
-   *     expect([ 1, 2, 3 ]).to.have.length.within(2,4);
-   *
-   * *Deprecation notice:* Using `length` as an assertion will be deprecated
-   * in version 2.4.0 and removed in 3.0.0. Code using the old style of
-   * asserting for `length` property value using `length(value)` should be
-   * switched to use `lengthOf(value)` instead.
-   *
-   * @name length
-   * @namespace BDD
-   * @api public
-   */
-
   /**
    * ### .lengthOf(value[, message])
    *
-   * Asserts that the target's `length` property has
+   * When invoked as a function, asserts that the target's `length` property has
    * the expected value.
    *
    *     expect([ 1, 2, 3]).to.have.lengthOf(3);
    *     expect('foobar').to.have.lengthOf(6);
    *
+   * When used as a language chain, sets the `doLength` flag which is later used
+   * by the `above`, `below`, `least`, `most`, and `within` assertions for
+   * asserting that the target's `length` property falls within the expected
+   * range of values.
+   *
+   *     expect('foo').to.have.lengthOf.above(2);
+   *     expect([ 1, 2, 3 ]).to.have.lengthOf.above(2);
+   *     expect('foo').to.have.lengthOf.below(4);
+   *     expect([ 1, 2, 3 ]).to.have.lengthOf.below(4);
+   *     expect('foo').to.have.lengthOf.at.least(3);
+   *     expect([ 1, 2, 3 ]).to.have.lengthOf.at.least(3);
+   *     expect('foo').to.have.lengthOf.at.most(3);
+   *     expect([ 1, 2, 3 ]).to.have.lengthOf.at.most(3);
+   *     expect('foo').to.have.lengthOf.within(2,4);
+   *     expect([ 1, 2, 3 ]).to.have.lengthOf.within(2,4);
+   *
+   * Warning: It's recommended to use `lengthOf` instead of its alias `length`,
+   * even though `length` is shorter. This is due to a compatibility issue that
+   * prevents `length` (but not `lengthOf`) from being chained directly off of
+   * an uninvoked method such as `a`. For example:
+   *
+   *     expect('foo').to.have.a.lengthOf(3);  // passes as expected
+   *     expect('foo').to.have.a.length(3);    // throws error
+   *
    * @name lengthOf
+   * @alias length
    * @param {Number} length
    * @param {String} message _optional_
    * @namespace BDD
@@ -1271,7 +1273,7 @@ module.exports = function (chai, _) {
   }
 
   Assertion.addChainableMethod('length', assertLength, assertLengthChain);
-  Assertion.addMethod('lengthOf', assertLength);
+  Assertion.addChainableMethod('lengthOf', assertLength, assertLengthChain);
 
   /**
    * ### .match(regexp)

lib/chai/interface/assert.js

@@ -1456,7 +1456,7 @@ module.exports = function (chai, util) {
    */
 
   assert.lengthOf = function (exp, len, msg) {
-    new Assertion(exp, msg).to.have.length(len);
+    new Assertion(exp, msg).to.have.lengthOf(len);
   };
 
   /**

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,62 @@
+var config = require('../config');
+
+var fnLengthDesc = Object.getOwnPropertyDescriptor(function () {}, 'length');
+
+/*!
+ * 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 (!fnLengthDesc.configurable) return fn;
+
+  Object.defineProperty(fn, 'length', {
+    get: function () {
+      if (isChainable) {
+        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/overwriteChainableMethod.js

@@ -15,7 +15,7 @@ var transferFlags = require('./transferFlags');
  * property.  Must return functions to be used for
  * name.
  *
- *     utils.overwriteChainableMethod(chai.Assertion.prototype, 'length',
+ *     utils.overwriteChainableMethod(chai.Assertion.prototype, 'lengthOf',
  *       function (_super) {
  *       }
  *     , function (_super) {
@@ -28,8 +28,8 @@ var transferFlags = require('./transferFlags');
  *
  * Then can be used as any other assertion.
  *
- *     expect(myFoo).to.have.length(3);
- *     expect(myFoo).to.have.length.above(3);
+ *     expect(myFoo).to.have.lengthOf(3);
+ *     expect(myFoo).to.have.lengthOf.above(3);
  *
  * @param {Object} ctx object whose method / property is to be overwritten
  * @param {String} name of method / property to overwrite

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);
 };