Merge pull request #789 from meeber/proxify-methods

Wrap non-chainable methods in proxies
chaijs · Sep 26, 2016 · 36bbf1c · 36bbf1c
2 parents 190b2b7 + eac90a5
commit 36bbf1c
Show file tree

Hide file tree

Showing 6 changed files with 275 additions and 42 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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() {