Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Un-deprecate length and add guard #897

Merged
merged 2 commits into from
Jan 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
82 changes: 42 additions & 40 deletions lib/chai/core/assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -1271,7 +1273,7 @@ module.exports = function (chai, _) {
}

Assertion.addChainableMethod('length', assertLength, assertLengthChain);
Assertion.addMethod('lengthOf', assertLength);
Assertion.addChainableMethod('lengthOf', assertLength, assertLengthChain);

/**
* ### .match(regexp)
Expand Down
2 changes: 1 addition & 1 deletion lib/chai/interface/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

/**
Expand Down
3 changes: 3 additions & 0 deletions lib/chai/utils/addChainableMethod.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Module dependencies
*/

var addLengthGuard = require('./addLengthGuard');
var chai = require('../../chai');
var flag = require('./flag');
var proxify = require('./proxify');
Expand Down Expand Up @@ -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
Expand Down
62 changes: 62 additions & 0 deletions lib/chai/utils/addLengthGuard.js
Original file line number Diff line number Diff line change
@@ -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;
};
2 changes: 2 additions & 0 deletions lib/chai/utils/addMethod.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* MIT Licensed
*/

var addLengthGuard = require('./addLengthGuard');
var chai = require('../../chai');
var flag = require('./flag');
var proxify = require('./proxify');
Expand Down Expand Up @@ -51,5 +52,6 @@ module.exports = function (ctx, name, method) {
return newAssertion;
};

addLengthGuard(fn, name, false);
ctx[name] = proxify(fn, name);
};
6 changes: 6 additions & 0 deletions lib/chai/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ exports.checkError = require('check-error');

exports.proxify = require('./proxify');

/*!
* Proxify util
*/

exports.addLengthGuard = require('./addLengthGuard');

/*!
* isNaN method
*/
Expand Down
6 changes: 3 additions & 3 deletions lib/chai/utils/overwriteChainableMethod.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/chai/utils/overwriteMethod.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* MIT Licensed
*/

var addLengthGuard = require('./addLengthGuard');
var chai = require('../../chai');
var flag = require('./flag');
var proxify = require('./proxify');
Expand Down Expand Up @@ -71,5 +72,6 @@ module.exports = function (ctx, name, method) {
return newAssertion;
}

addLengthGuard(fn, name, false);
ctx[name] = proxify(fn, name);
};