Repository: chaijs/chai
Pull request #789 (2016-09-09 00:57:07)

Wrap non-chainable methods in proxies
Extends proxy protection to non-chainable methods (including overwritten non-chainable methods) with a helpful error message. Example:

```js
expect(true).to.equal.true;  // Invalid Chai property: equal.true. See docs for proper usage of "equal".
```



Patch:
diff --git a/lib/chai/utils/addMethod.js b/lib/chai/utils/addMethod.js
index 91c048d..b4a51a6 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 9e2c112..c027372 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 b08fe26..a1e41c1 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;


Test patch:
diff --git a/test/expect.js b/test/expect.js
index cca20a8..fb8f8e3 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 c011ef1..c52c9cc 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 0524d0b..48a3c2c 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});