From eac90a52095f9e1f39a54eb0215f364789708573 Mon Sep 17 00:00:00 2001 From: Grant Snodgrass Date: Thu, 8 Sep 2016 20:50:39 -0400 Subject: [PATCH] Wrap non-chainable methods in proxies --- lib/chai/utils/addMethod.js | 7 +- lib/chai/utils/overwriteMethod.js | 7 +- lib/chai/utils/proxify.js | 20 ++++- test/expect.js | 132 ++++++++++++++++++++++++++---- test/should.js | 132 ++++++++++++++++++++++++++---- test/utilities.js | 19 ++++- 6 files changed, 275 insertions(+), 42 deletions(-) diff --git a/lib/chai/utils/addMethod.js b/lib/chai/utils/addMethod.js index 91c048d80..b4a51a6ba 100644 --- a/lib/chai/utils/addMethod.js +++ b/lib/chai/utils/addMethod.js @@ -6,6 +6,7 @@ var chai = require('../../chai'); var flag = require('./flag'); +var proxify = require('./proxify'); var transferFlags = require('./transferFlags'); /** @@ -35,11 +36,11 @@ var transferFlags = require('./transferFlags'); */ module.exports = function (ctx, name, method) { - ctx[name] = function () { + var fn = function () { var keep_ssfi = flag(this, 'keep_ssfi'); var old_ssfi = flag(this, 'ssfi'); if (!keep_ssfi && old_ssfi) - flag(this, 'ssfi', ctx[name]); + flag(this, 'ssfi', fn); var result = method.apply(this, arguments); if (result !== undefined) @@ -49,4 +50,6 @@ module.exports = function (ctx, name, method) { transferFlags(this, newAssertion); return newAssertion; }; + + ctx[name] = proxify(fn, name); }; diff --git a/lib/chai/utils/overwriteMethod.js b/lib/chai/utils/overwriteMethod.js index 9e2c11285..c02737261 100644 --- a/lib/chai/utils/overwriteMethod.js +++ b/lib/chai/utils/overwriteMethod.js @@ -6,6 +6,7 @@ var chai = require('../../chai'); var flag = require('./flag'); +var proxify = require('./proxify'); var transferFlags = require('./transferFlags'); /** @@ -51,11 +52,11 @@ module.exports = function (ctx, name, method) { if (_method && 'function' === typeof _method) _super = _method; - ctx[name] = function () { + var fn = function () { var keep_ssfi = flag(this, 'keep_ssfi'); var old_ssfi = flag(this, 'ssfi'); if (!keep_ssfi && old_ssfi) - flag(this, 'ssfi', ctx[name]); + flag(this, 'ssfi', fn); flag(this, 'keep_ssfi', true); var result = method(_super).apply(this, arguments); @@ -69,4 +70,6 @@ module.exports = function (ctx, name, method) { transferFlags(this, newAssertion); return newAssertion; } + + ctx[name] = proxify(fn, name); }; diff --git a/lib/chai/utils/proxify.js b/lib/chai/utils/proxify.js index b08fe2638..a1e41c127 100644 --- a/lib/chai/utils/proxify.js +++ b/lib/chai/utils/proxify.js @@ -11,15 +11,22 @@ var getProperties = require('./getProperties'); * # proxify(object) * * Return a proxy of given object that throws an error when a non-existent - * property is read. (If Proxy or Reflect is undefined, then return object - * without modification.) + * property is read. By default, the root cause is assumed to be a misspelled + * property, and thus an attempt is made to offer a reasonable suggestion from + * the list of existing properties. However, if a nonChainableMethodName is + * provided, then the root cause is instead a failure to invoke a non-chainable + * method prior to reading the non-existent property. + * + * If proxies are unsupported or disabled via the user's Chai config, then + * return object without modification. * * @param {Object} obj + * @param {String} nonChainableMethodName * @namespace Utils * @name proxify */ -module.exports = function proxify (obj) { +module.exports = function proxify (obj, nonChainableMethodName) { if (!config.useProxy || typeof Proxy === 'undefined' || typeof Reflect === 'undefined') return obj; @@ -32,6 +39,13 @@ module.exports = function proxify (obj) { if (typeof property === 'string' && config.proxyExcludedKeys.indexOf(property) === -1 && !Reflect.has(target, property)) { + // Special message for invalid property access of non-chainable methods. + if (nonChainableMethodName) { + throw Error('Invalid Chai property: ' + nonChainableMethodName + '.' + + property + '. See docs for proper usage of "' + + nonChainableMethodName + '".'); + } + var orderedProperties = getProperties(target).filter(function(property) { return !Object.prototype.hasOwnProperty(property) && ['__flags', '__methods', '_obj', 'assert'].indexOf(property) === -1; diff --git a/test/expect.js b/test/expect.js index cca20a88f..fb8f8e3b2 100644 --- a/test/expect.js +++ b/test/expect.js @@ -10,29 +10,127 @@ describe('expect', function () { expect('foo').to.equal('foo'); }); - it('invalid property', function () { + describe('invalid property', function () { if (typeof Proxy === 'undefined' || typeof Reflect === 'undefined') return; - err(function () { - expect(42).pizza; - }, 'Invalid Chai property: pizza'); + before(function () { + chai.util.addProperty(chai.Assertion.prototype, 'tmpProperty', function () { + new chai.Assertion(42).equal(42); + }); + chai.util.overwriteProperty(chai.Assertion.prototype, 'tmpProperty', function (_super) { + return function () { + _super.call(this); + }; + }); - err(function () { - expect(42).to.pizza; - }, 'Invalid Chai property: pizza'); + chai.util.addMethod(chai.Assertion.prototype, 'tmpMethod', function () { + new chai.Assertion(42).equal(42); + }); + chai.util.overwriteMethod(chai.Assertion.prototype, 'tmpMethod', function (_super) { + return function () { + _super.call(this); + }; + }); - err(function () { - expect(42).to.be.a.pizza; - }, 'Invalid Chai property: pizza'); + chai.util.addChainableMethod(chai.Assertion.prototype, 'tmpChainableMethod', function () { + new chai.Assertion(42).equal(42); + }, function () { + new chai.Assertion(42).equal(42); + }); + chai.util.overwriteChainableMethod(chai.Assertion.prototype, 'tmpChainableMethod', function (_super) { + return function () { + _super.call(this); + }; + }, function (_super) { + return function () { + _super.call(this); + }; + }); + }); - err(function () { - expect(42).to.equal(42).pizza; - }, 'Invalid Chai property: pizza'); + after(function () { + delete chai.Assertion.prototype.tmpProperty; + delete chai.Assertion.prototype.tmpMethod; + delete chai.Assertion.prototype.tmpChainableMethod; + }); - // .then is excluded from property validation for promise support - expect(function () { - expect(42).then; - }).to.not.throw(); + it('throws when invalid property follows expect', function () { + err(function () { + expect(42).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 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('no-op chains', function() { diff --git a/test/should.js b/test/should.js index c011ef197..c52c9cc85 100644 --- a/test/should.js +++ b/test/should.js @@ -7,29 +7,127 @@ describe('should', function() { should.not.equal('foo', 'bar'); }); - it('invalid property', function () { + describe('invalid property', function () { if (typeof Proxy === 'undefined' || typeof Reflect === 'undefined') return; - err(function () { - (42).should.pizza; - }, 'Invalid Chai property: pizza'); + before(function () { + chai.util.addProperty(chai.Assertion.prototype, 'tmpProperty', function () { + new chai.Assertion(42).equal(42); + }); + chai.util.overwriteProperty(chai.Assertion.prototype, 'tmpProperty', function (_super) { + return function () { + _super.call(this); + }; + }); - err(function () { - (42).should.be.pizza; - }, 'Invalid Chai property: pizza'); + chai.util.addMethod(chai.Assertion.prototype, 'tmpMethod', function () { + new chai.Assertion(42).equal(42); + }); + chai.util.overwriteMethod(chai.Assertion.prototype, 'tmpMethod', function (_super) { + return function () { + _super.call(this); + }; + }); - err(function () { - (42).should.be.a.pizza; - }, 'Invalid Chai property: pizza'); + chai.util.addChainableMethod(chai.Assertion.prototype, 'tmpChainableMethod', function () { + new chai.Assertion(42).equal(42); + }, function () { + new chai.Assertion(42).equal(42); + }); + chai.util.overwriteChainableMethod(chai.Assertion.prototype, 'tmpChainableMethod', function (_super) { + return function () { + _super.call(this); + }; + }, function (_super) { + return function () { + _super.call(this); + }; + }); + }); - err(function () { - (42).should.equal(42).pizza; - }, 'Invalid Chai property: pizza'); + after(function () { + delete chai.Assertion.prototype.tmpProperty; + delete chai.Assertion.prototype.tmpMethod; + delete chai.Assertion.prototype.tmpChainableMethod; + }); - // .then is excluded from property validation for promise support - (function () { - (42).should.then; - }).should.not.throw(); + it('throws when invalid property follows should', function () { + err(function () { + (42).should.pizza; + }, 'Invalid Chai property: pizza'); + }); + + it('throws when invalid property follows language chain', function () { + err(function () { + (42).should.to.pizza; + }, 'Invalid Chai property: pizza'); + }); + + it('throws when invalid property follows property assertion', function () { + err(function () { + (42).should.ok.pizza; + }, 'Invalid Chai property: pizza'); + }); + + it('throws when invalid property follows overwritten property assertion', function () { + err(function () { + (42).should.tmpProperty.pizza; + }, 'Invalid Chai property: pizza'); + }); + + it('throws when invalid property follows uncalled method assertion', function () { + err(function () { + (42).should.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 () { + (42).should.equal(42).pizza; + }, 'Invalid Chai property: pizza'); + }); + + it('throws when invalid property follows uncalled overwritten method assertion', function () { + err(function () { + (42).should.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 () { + (42).should.tmpMethod().pizza; + }, 'Invalid Chai property: pizza'); + }); + + it('throws when invalid property follows uncalled chainable method assertion', function () { + err(function () { + (42).should.a.pizza; + }, 'Invalid Chai property: pizza'); + }); + + it('throws when invalid property follows called chainable method assertion', function () { + err(function () { + (42).should.a('number').pizza; + }, 'Invalid Chai property: pizza'); + }); + + it('throws when invalid property follows uncalled overwritten chainable method assertion', function () { + err(function () { + (42).should.tmpChainableMethod.pizza; + }, 'Invalid Chai property: pizza'); + }); + + it('throws when invalid property follows called overwritten chainable method assertion', function () { + err(function () { + (42).should.tmpChainableMethod().pizza; + }, 'Invalid Chai property: pizza'); + }); + + it('doesn\'t throw if invalid property is excluded via config', function () { + (function () { + (42).should.then; + }).should.not.throw(); + }); }); it('no-op chains', function() { diff --git a/test/utilities.js b/test/utilities.js index 0524d0bc9..48a3c2c9a 100644 --- a/test/utilities.js +++ b/test/utilities.js @@ -1178,7 +1178,16 @@ describe('utilities', function () { expect(pizza.mushrooms).to.equal(42); }); - it('throws error if a non-existent property is read', function () { + it('returns property value if an existing property is read when nonChainableMethodName is set', function () { + var bake = function () {}; + bake.numPizzas = 2; + + var bakeProxy = proxify(bake, 'bake'); + + expect(bakeProxy.numPizzas).to.equal(2); + }); + + it('throws invalid property error if a non-existent property is read', function () { var pizza = proxify({}); expect(function () { @@ -1186,6 +1195,14 @@ describe('utilities', function () { }).to.throw('Invalid Chai property: mushrooms'); }); + it('throws invalid use error if a non-existent property is read when nonChainableMethodName is set', function () { + var bake = proxify(function () {}, 'bake'); + + expect(function () { + bake.numPizzas; + }).to.throw('Invalid Chai property: bake.numPizzas. See docs for proper usage of "bake".'); + }); + it('suggests a fix if a non-existent prop looks like a typo', function () { var pizza = proxify({foo: 1, bar: 2, baz: 3});