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

Make 'empty' throw on non-string primitives and functions #812

Merged
merged 5 commits into from
Oct 1, 2016
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
26 changes: 21 additions & 5 deletions lib/chai/core/assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -515,8 +515,8 @@ module.exports = function (chai, _) {
* ### .empty
*
* Asserts that the target's length is `0`. For arrays and strings, it checks
* the `length` property. For objects, it gets the count of
* enumerable keys.
* the `length` property. For non-function objects, it gets the count of own
* enumerable string keys.
*
* expect([]).to.be.empty;
* expect('').to.be.empty;
Expand All @@ -528,10 +528,26 @@ module.exports = function (chai, _) {
*/

Assertion.addProperty('empty', function () {
var obj = flag(this, 'object');
new Assertion(obj).to.exist;
var val = flag(this, 'object')
, itemsCount;

switch (_.type(val)) {
case 'array':
case 'string':
itemsCount = val.length;
break;
case 'function':
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should handle generators too (because of recent updates to type-detect) and async function in the future.

var name = val.name ? ' ' + val.name : '';
throw new TypeError('.empty was passed a function' + name);
default:
if (val !== Object(val)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other option is to add case clause for other primitives' types.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great use of Object(). 😍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That blew my mind to!

throw new TypeError('.empty was passed non-string primitive ' + _.inspect(val));
}
itemsCount = Object.keys(val).length;
}

this.assert(
Object.keys(Object(obj)).length === 0
0 === itemsCount
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any ESLint config we can add to the project? This will greatly improve contribution/review experience: yoda comparisons and comma-first are not quite popular and it would be awesome to automate codestyle checks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shvaikalesh Yes, we thought about that too, we already discussed that here and we came to the conclusion that by moving all our code to separate repos with their own linting rules would be better, however, if you disagree you're welcome to share your thoughts with us 😄 .

, 'expected #{this} to be empty'
, 'expected #{this} not to be empty'
);
Expand Down
47 changes: 41 additions & 6 deletions test/expect.js
Original file line number Diff line number Diff line change
Expand Up @@ -680,28 +680,63 @@ describe('expect', function () {

err(function(){
expect(null).to.be.empty;
}, "expected null to exist");
}, ".empty was passed non-string primitive null");

err(function(){
expect(undefined).to.be.empty;
}, "expected undefined to exist");
}, ".empty was passed non-string primitive undefined");

err(function(){
expect().to.be.empty;
}, "expected undefined to exist");
}, ".empty was passed non-string primitive undefined");

err(function(){
expect(null).to.not.be.empty;
}, "expected null to exist");
}, ".empty was passed non-string primitive null");

err(function(){
expect(undefined).to.not.be.empty;
}, "expected undefined to exist");
}, ".empty was passed non-string primitive undefined");

err(function(){
expect().to.not.be.empty;
}, "expected undefined to exist");
}, ".empty was passed non-string primitive undefined");

err(function(){
expect(0).to.be.empty;
}, ".empty was passed non-string primitive 0");

err(function(){
expect(1).to.be.empty;
}, ".empty was passed non-string primitive 1");

err(function(){
expect(true).to.be.empty;
}, ".empty was passed non-string primitive true");

err(function(){
expect(false).to.be.empty;
}, ".empty was passed non-string primitive false");

if (typeof Symbol !== 'undefined') {
err(function(){
expect(Symbol()).to.be.empty;
}, ".empty was passed non-string primitive Symbol()");

err(function(){
expect(Symbol.iterator).to.be.empty;
}, ".empty was passed non-string primitive Symbol(Symbol.iterator)");
}

err(function(){
expect(function() {}).to.be.empty;
}, ".empty was passed a function");

if (FakeArgs.name === 'FakeArgs') {
err(function(){
expect(FakeArgs).to.be.empty;
}, ".empty was passed a function FakeArgs");
}
});

it('NaN', function() {
Expand Down
36 changes: 36 additions & 0 deletions test/should.js
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,42 @@ describe('should', function() {
err(function(){
({foo: 'bar'}).should.be.empty;
}, "expected { foo: \'bar\' } to be empty");

err(function(){
(0).should.be.empty;
}, ".empty was passed non-string primitive 0");

err(function(){
(1).should.be.empty;
}, ".empty was passed non-string primitive 1");

err(function(){
true.should.be.empty;
}, ".empty was passed non-string primitive true");

err(function(){
false.should.be.empty;
}, ".empty was passed non-string primitive false");

if (typeof Symbol !== 'undefined') {
err(function(){
Symbol().should.be.empty;
}, ".empty was passed non-string primitive Symbol()");

err(function(){
Symbol.iterator.should.to.be.empty;
}, ".empty was passed non-string primitive Symbol(Symbol.iterator)");
}

err(function(){
(function() {}).should.be.empty;
}, ".empty was passed a function");

if (FakeArgs.name === 'FakeArgs') {
err(function(){
FakeArgs.should.be.empty;
}, ".empty was passed a function FakeArgs");
}
});

it('finite(value)', function() {
Expand Down