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

Throw error when instanceof is passed something that is not a function as constructor #899

Merged
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion lib/chai/core/assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -988,9 +988,29 @@ module.exports = function (chai, _) {

function assertInstanceOf (constructor, msg) {
if (msg) flag(this, 'message', msg);

var target = flag(this, 'object')
var validInstanceOfTarget = constructor === Object(constructor) && (
typeof constructor === 'function' ||
(typeof Symbol !== 'undefined' &&
typeof Symbol.hasInstance !== 'undefined' &&
Symbol.hasInstance in constructor)
);

if (!validInstanceOfTarget) {
var constructorType = constructor === null ? 'null' : typeof constructor;
throw new Error('The instanceof assertion needs a constructor but ' + constructorType + ' was given.');
Copy link
Contributor

Choose a reason for hiding this comment

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

TypeError fits better

Copy link
Contributor

Choose a reason for hiding this comment

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

After this was merged, I was gonna rebase #922 and turn this error into an AssertionError that properly retains ssfi (and then a follow up PR will properly retain the user's custom message).

}

var isInstanceOf = target instanceof constructor
Copy link
Contributor

Choose a reason for hiding this comment

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

Semi-colon :D

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!
I miss linting tools 😅


var name = _.getName(constructor);
if (name === null) {
name = 'an unnamed constructor';
}

this.assert(
flag(this, 'object') instanceof constructor
isInstanceOf
, 'expected #{this} to be an instance of ' + name
, 'expected #{this} to not be an instance of ' + name
);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"assertion-error": "^1.0.1",
"check-error": "^1.0.1",
"deep-eql": "^2.0.1",
"get-func-name": "^1.0.0",
"get-func-name": "^2.0.0",
"pathval": "^1.0.0",
"type-detect": "^4.0.0"
},
Expand Down
95 changes: 95 additions & 0 deletions test/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,60 @@ describe('assert', function () {
function Foo(){}
assert.instanceOf(new Foo(), Foo);

err(function(){
assert.instanceOf(new Foo(), 1);
}, "The instanceof assertion needs a constructor but number was given.");

err(function(){
assert.instanceOf(new Foo(), 'batman');
}, "The instanceof assertion needs a constructor but string was given.");

err(function(){
assert.instanceOf(new Foo(), {});
}, "The instanceof assertion needs a constructor but object was given.");

err(function(){
assert.instanceOf(new Foo(), true);
}, "The instanceof assertion needs a constructor but boolean was given.");

err(function(){
assert.instanceOf(new Foo(), null);
}, "The instanceof assertion needs a constructor but null was given.");

err(function(){
assert.instanceOf(new Foo(), undefined);
}, "The instanceof assertion needs a constructor but undefined was given.");

var expectedError;
try {
t instanceof Thing;
} catch (err) {
errMsg = '[object Object] instanceof function Thing(){} failed: ' + err.message + '.';
}

err(function(){
function Thing(){};
var t = new Thing();
Thing.prototype = 1337;
assert.instanceOf(t, Thing);
}, expectedError);

if (typeof Symbol !== 'undefined' && typeof Symbol.hasInstance !== 'undefined') {
err(function(){
assert.instanceOf(new Foo(), Symbol());
}, "The instanceof assertion needs a constructor but symbol was given.");

err(function() {
var FakeConstructor = {};
var fakeInstanceB = 4;
FakeConstructor[Symbol.hasInstance] = function (val) {
return val === 3;
};

assert.instanceOf(fakeInstanceB, FakeConstructor);
}, 'expected 4 to be an instance of an unnamed constructor')
}

err(function () {
assert.instanceOf(5, Foo);
}, "expected 5 to be an instance of Foo");
Expand All @@ -156,6 +210,47 @@ describe('assert', function () {
function Foo(){}
assert.notInstanceOf(new Foo(), String);

err(function(){
assert.notInstanceOf(new Foo(), 1);
}, "The instanceof assertion needs a constructor but number was given.");

err(function(){
assert.notInstanceOf(new Foo(), 'batman');
}, "The instanceof assertion needs a constructor but string was given.");

err(function(){
assert.notInstanceOf(new Foo(), {});
}, "The instanceof assertion needs a constructor but object was given.");

err(function(){
assert.notInstanceOf(new Foo(), true);
}, "The instanceof assertion needs a constructor but boolean was given.");

err(function(){
assert.notInstanceOf(new Foo(), null);
}, "The instanceof assertion needs a constructor but null was given.");

err(function(){
assert.notInstanceOf(new Foo(), undefined);
}, "The instanceof assertion needs a constructor but undefined was given.");


if (typeof Symbol !== 'undefined' && typeof Symbol.hasInstance !== 'undefined') {
err(function(){
assert.notInstanceOf(new Foo(), Symbol());
}, "The instanceof assertion needs a constructor but symbol was given.");

err(function() {
var FakeConstructor = {};
var fakeInstanceB = 4;
FakeConstructor[Symbol.hasInstance] = function (val) {
return val === 4;
};

assert.notInstanceOf(fakeInstanceB, FakeConstructor);
}, 'expected 4 to not be an instance of an unnamed constructor');
}

err(function () {
assert.notInstanceOf(new Foo(), Foo);
}, "expected {} to not be an instance of Foo");
Expand Down
87 changes: 76 additions & 11 deletions test/expect.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,61 +72,61 @@ describe('expect', 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;
Expand Down Expand Up @@ -366,6 +366,71 @@ describe('expect', function () {
function Foo(){}
expect(new Foo()).to.be.an.instanceof(Foo);

err(function(){
expect(new Foo()).to.an.instanceof(1);
}, "The instanceof assertion needs a constructor but number was given.");

err(function(){
expect(new Foo()).to.an.instanceof('batman');
}, "The instanceof assertion needs a constructor but string was given.");

err(function(){
expect(new Foo()).to.an.instanceof({});
}, "The instanceof assertion needs a constructor but object was given.");

err(function(){
expect(new Foo()).to.an.instanceof(true);
}, "The instanceof assertion needs a constructor but boolean was given.");

err(function(){
expect(new Foo()).to.an.instanceof(null);
}, "The instanceof assertion needs a constructor but null was given.");

err(function(){
expect(new Foo()).to.an.instanceof(undefined);
}, "The instanceof assertion needs a constructor but undefined was given.");

// Different browsers may have different error messages
var expectedError;
try {
t instanceof Thing;
} catch (err) {
errMsg = '[object Object] instanceof function Thing(){} failed: ' + err.message + '.';
}

err(function(){
function Thing(){};
var t = new Thing();
Thing.prototype = 1337;
expect(t).to.an.instanceof(Thing);
}, expectedError)

if (typeof Symbol !== 'undefined' && typeof Symbol.hasInstance !== 'undefined') {
err(function(){
expect(new Foo()).to.an.instanceof(Symbol());
}, "The instanceof assertion needs a constructor but symbol was given.");

err(function() {
var FakeConstructor = {};
var fakeInstanceB = 4;
FakeConstructor[Symbol.hasInstance] = function (val) {
return val === 3;
};

expect(fakeInstanceB).to.be.an.instanceof(FakeConstructor);
}, 'expected 4 to be an instance of an unnamed constructor')

err(function() {
var FakeConstructor = {};
var fakeInstanceB = 4;
FakeConstructor[Symbol.hasInstance] = function (val) {
return val === 4;
};

expect(fakeInstanceB).to.not.be.an.instanceof(FakeConstructor);
}, 'expected 4 to not be an instance of an unnamed constructor')
}

err(function(){
expect(3).to.an.instanceof(Foo, 'blah');
}, "blah: expected 3 to be an instance of Foo");
Expand Down Expand Up @@ -1826,7 +1891,7 @@ describe('expect', function () {
expect(testSet).to.not.have.any.deep.keys([{13: 37}, 'thisDoesNotExist', 'thisToo']);
expect(testSet).to.not.have.any.deep.keys([20, 1, {13: 37}]);
expect(testSet).to.not.have.all.deep.keys([{thisIs: 'anExampleObject'}, {'iDoNot': 'exist'}]);

var weirdSetKey1 = Object.create(null)
, weirdSetKey2 = {toString: NaN}
, weirdSetKey3 = []
Expand Down
Loading