Repository: chaijs/chai (js)

Issue:
Cannot console.log assertions because of proxies
Consider:

```js
const {expect} = require("chai");

it("meep", () => {
  const myAssertion = expect(42).to.equal(42);

  console.log(myAssertion);
});
```

In Chai v3.5, the output is:

> Assertion {
>   __flags: { ssfi: [Function], object: 42, message: undefined } }
>   ✓ meep
> 
>   1 passing (25ms)

But in the current version of Chai off Github, the output is:

>   1) meep
> 
>   0 passing (13ms)
>   1 failing
> 
>   1)  meep:
>      Error: Invalid Chai property: inspect
>       at Error (native)
>       at Object.getProperty [as get] node_modules/chai/lib/chai/utils/proxify.js:30:15
>       at formatValue (util.js:343:19)
>       at inspect (util.js:198:10)
>       at exports.format (util.js:75:24)
>       at Console.log (console.js:43:37)
>       at Context.it (test/index.js:6:11)

Possible solution:

Add `inspect` as a blacklisted property [here](https://github.com/chaijs/chai/blob/fbb4a4e085eca9520146a94ea5fdade7d4508442/lib/chai/utils/proxify.js#L27-L29). `then` already exists as a blacklisted property for promise support. `inspect` is a standard property in Node, as noted in https://github.com/chaijs/chai/issues/727#issuecomment-225373838. 

Rather than hard-coding the blacklist, it might be more appropriate to refactor the blacklist to be a new Chai config value.

Pull request description:
Configurable proxy blacklist
This aims to close #765.

As brilliantly suggested by @meeber this adds a new Array containing keys which should be ignored when checking for non-existing properties on an assertion before throwing an error.

This PR contains:
- [x] An awesome and difficult replacement of a `backtick` which should have been an `'`
- [x] A new setting on `chai.config` called `proxyExcludedKeys` which receives an array of Strings representing the properties which should be ignored by the proxy on environments that support it
- [x] Docs explaining how it works, default values and other peculiarities
- [x] Tests checking:
 * Default values
 * If it does not throw an error on non existing `then` and `inspect` properties
 * If it throws an error on non existing `then` and `inspect` properties if they get removed from the `proxyExcludedKeys` array on environments that do support proxies

Let me know if I missed anything.

Patch:
diff --git a/lib/chai/assertion.js b/lib/chai/assertion.js
index 9a3846b..a5033d4 100644
--- a/lib/chai/assertion.js
+++ b/lib/chai/assertion.js
@@ -104,7 +104,7 @@ module.exports = function (_chai, util) {
     if (true !== config.showDiff) showDiff = false;
 
     if (!ok) {
-      msg = util.getMessage(this, arguments)
+      var msg = util.getMessage(this, arguments)
         , actual = util.getActual(this, arguments);
       throw new AssertionError(msg, {
           actual: actual
diff --git a/lib/chai/config.js b/lib/chai/config.js
index 20f79cb..9c8be3c 100644
--- a/lib/chai/config.js
+++ b/lib/chai/config.js
@@ -64,11 +64,31 @@ module.exports = {
    *     chai.config.useProxy = false;  // disable use of Proxy
    *
    * This feature is automatically disabled regardless of this config value
-   * in environments that don`t support proxies.
+   * in environments that don't support proxies.
    *
    * @param {Boolean}
    * @api public
    */
 
-  useProxy: true
+  useProxy: true,
+
+  /**
+   * ### config.proxyExcludedKeys
+   *
+   * User configurable property, defines which properties should be ignored
+   * instead of throwing an error if they do not exist on the assertion.
+   * This is only applied if the environment Chai is running in supports proxies and
+   * if the `useProxy` configuration setting is enabled.
+   * By default, `then` and `inspect` will not throw an error if they do not exist on the
+   * assertion object because the `.inspect` property is read by `util.inspect` (for example, when
+   * using `console.log` on the assertion object) and `.then` is necessary for promise type-checking.
+   *
+   *     // By default these keys will not throw an error if they do not exist on the assertion object
+   *     chai.config.proxyExcludedKeys = ['then', 'inspect'];
+   *
+   * @param {Array}
+   * @api public
+   */
+
+  proxyExcludedKeys: ['then', 'inspect']
 };
diff --git a/lib/chai/core/assertions.js b/lib/chai/core/assertions.js
index ee442b2..d0f9696 100644
--- a/lib/chai/core/assertions.js
+++ b/lib/chai/core/assertions.js
@@ -481,9 +481,8 @@ module.exports = function (chai, _) {
    */
 
   Assertion.addProperty('exist', function () {
-    var val = flag(this, 'object');
     this.assert(
-        val !== null && val !== undefined
+        null != flag(this, 'object')
       , 'expected #{this} to exist'
       , 'expected #{this} to not exist'
     );
@@ -1754,14 +1753,14 @@ module.exports = function (chai, _) {
     var contains = flag(this, 'contains');
     var ordered = flag(this, 'ordered');
 
-    var subject, failMsg, failNegateMsg, lengthCheck;
+    var failMsg, failNegateMsg, lengthCheck;
 
     if (contains) {
-      subject = ordered ? 'an ordered superset' : 'a superset';
+      var subject = ordered ? 'an ordered superset' : 'a superset';
       failMsg = 'expected #{this} to be ' + subject + ' of #{exp}';
       failNegateMsg = 'expected #{this} to not be ' + subject + ' of #{exp}';
     } else {
-      subject = ordered ? 'ordered members' : 'members';
+      var subject = ordered ? 'ordered members' : 'members';
       failMsg = 'expected #{this} to have the same ' + subject + ' as #{exp}';
       failNegateMsg = 'expected #{this} to not have the same ' + subject + ' as #{exp}';
     }
diff --git a/lib/chai/interface/assert.js b/lib/chai/interface/assert.js
index 898e5b6..bb5171e 100644
--- a/lib/chai/interface/assert.js
+++ b/lib/chai/interface/assert.js
@@ -460,48 +460,6 @@ module.exports = function (chai, util) {
     new Assertion(val, msg).not.to.be.NaN;
   };
 
-  /**
-   * ### .exists
-   *
-   * Asserts that the target is neither `null` nor `undefined`.
-   *
-   *     var foo = 'hi';
-   *
-   *     assert.exists(foo, 'foo is neither `null` nor `undefined`');
-   *
-   * @name exists
-   * @param {Mixed} value
-   * @param {String} message
-   * @namespace Assert
-   * @api public
-   */
-
-  assert.exists = function (val, msg) {
-    new Assertion(val, msg).to.exist;
-  };
-
-  /**
-   * ### .notExists
-   *
-   * Asserts that the target is either `null` or `undefined`.
-   *
-   *     var bar = null
-   *       , baz;
-   *
-   *     assert.notExists(bar);
-   *     assert.notExists(baz, 'baz is either null or undefined');
-   *
-   * @name notExists
-   * @param {Mixed} value
-   * @param {String} message
-   * @namespace Assert
-   * @api public
-   */
-
-  assert.notExists = function (val, msg) {
-    new Assertion(val, msg).to.not.exist;
-  };
-
   /**
    * ### .isUndefined(value, [message])
    *
diff --git a/lib/chai/utils/expectTypes.js b/lib/chai/utils/expectTypes.js
index 59ac62d..4dd5b29 100644
--- a/lib/chai/utils/expectTypes.js
+++ b/lib/chai/utils/expectTypes.js
@@ -23,7 +23,7 @@ var flag = require('./flag');
 var type = require('type-detect');
 
 module.exports = function (obj, types) {
-  obj = flag(obj, 'object');
+  var obj = flag(obj, 'object');
   types = types.map(function (t) { return t.toLowerCase(); });
   types.sort();
 
diff --git a/lib/chai/utils/inspect.js b/lib/chai/utils/inspect.js
index b822515..48b56bf 100644
--- a/lib/chai/utils/inspect.js
+++ b/lib/chai/utils/inspect.js
@@ -8,7 +8,7 @@ var getEnumerableProperties = require('./getEnumerableProperties');
 module.exports = inspect;
 
 /**
- * Echos the value of a value. Trys to print the value out
+ * Echoes the value of a value. Tries to print the value out
  * in the best way possible given the different types.
  *
  * @param {Object} obj The object to print out.
@@ -99,8 +99,6 @@ function formatValue(ctx, value, recurseTimes) {
   var visibleKeys = getEnumerableProperties(value);
   var keys = ctx.showHidden ? getProperties(value) : visibleKeys;
 
-  var name, nameSuffix;
-
   // Some type of object without properties can be shortcutted.
   // In IE, errors have a single `stack` property, or if they are vanilla `Error`,
   // a `stack` plus `description` property; ignore those for consistency.
@@ -109,8 +107,8 @@ function formatValue(ctx, value, recurseTimes) {
       (keys.length === 2 && keys[0] === 'description' && keys[1] === 'stack')
      ))) {
     if (typeof value === 'function') {
-      name = getName(value);
-      nameSuffix = name ? ': ' + name : '';
+      var name = getName(value);
+      var nameSuffix = name ? ': ' + name : '';
       return ctx.stylize('[Function' + nameSuffix + ']', 'special');
     }
     if (isRegExp(value)) {
@@ -134,8 +132,8 @@ function formatValue(ctx, value, recurseTimes) {
 
   // Make functions say that they are functions
   if (typeof value === 'function') {
-    name = getName(value);
-    nameSuffix = name ? ': ' + name : '';
+    var name = getName(value);
+    var nameSuffix = name ? ': ' + name : '';
     base = ' [Function' + nameSuffix + ']';
   }
 
diff --git a/lib/chai/utils/proxify.js b/lib/chai/utils/proxify.js
index 6baaff2..314421b 100644
--- a/lib/chai/utils/proxify.js
+++ b/lib/chai/utils/proxify.js
@@ -24,10 +24,12 @@ module.exports = function proxify (obj) {
 
   return new Proxy(obj, {
     get: function getProperty (target, property) {
-      // Don't throw error on Symbol properties such as Symbol.toStringTag, nor
-      // on .then because it's necessary for promise type-checking.
+      // This check is here because we should not throw errors on Symbol properties
+      // such as `Symbol.toStringTag`.
+      // The values for which an error should be thrown can be configured using
+      // the `config.proxyExcludedKeys` setting.
       if (typeof property === 'string' &&
-          property !== 'then' &&
+          config.proxyExcludedKeys.indexOf(property) === -1 &&
           !Reflect.has(target, property))
         throw Error('Invalid Chai property: ' + property);
 


Test patch:
diff --git a/test/assert.js b/test/assert.js
index 91da9d5..a569ef8 100644
--- a/test/assert.js
+++ b/test/assert.js
@@ -344,31 +344,6 @@ describe('assert', function () {
     }, "expected 'hello' not to be NaN");
   });
 
-  it('exists', function() {
-    var meeber = 'awesome';
-    var iDoNotExist;
-       
-    assert.exists(meeber);
-    assert.exists(0);
-    assert.exists(false);
-    assert.exists('');
-
-    err(function (){
-      assert.exists(iDoNotExist);
-    }, "expected undefined to exist");
-  });
-
-  it('notExists', function() {
-    var meeber = 'awesome';
-    var iDoNotExist;
-       
-    assert.notExists(iDoNotExist);
-
-    err(function (){
-      assert.notExists(meeber);
-    }, "expected 'awesome' to not exist");
-  });
-
   it('isUndefined', function() {
     assert.isUndefined(undefined);
 
diff --git a/test/configuration.js b/test/configuration.js
index 991681c..5eef76c 100644
--- a/test/configuration.js
+++ b/test/configuration.js
@@ -196,4 +196,37 @@ describe('configuration', function () {
       });
     });
   });
+
+  describe('proxyExcludedKeys', function() {
+    var readNoExistentProperty = function(prop) {
+      return function() {
+        var assertion = expect(false);
+        expect(assertion).to.not.have.key(prop);
+        assertion[prop];
+      }
+    };
+
+    it('should have default value equal to `[\'then\', \'inspect\']`', function() {
+      expect(chai.config.proxyExcludedKeys).to.be.deep.equal(['then', 'inspect']);
+    });
+
+    it('should not throw when accessing non-existing `then` and `inspect` in an environment with proxy support', function() {
+      // Since these will not throw if the environment does not support proxies we don't need any `if` clause here
+      expect(readNoExistentProperty('then')).to.not.throw();
+      expect(readNoExistentProperty('inspect')).to.not.throw();
+    });
+
+    it('should throw for properties which are not on the `proxyExcludedKeys` Array in an environment with proxy support', function() {
+      chai.config.proxyExcludedKeys = [];
+
+      if (typeof Proxy !== 'undefined' && typeof Reflect !== 'undefined') {
+        expect(readNoExistentProperty('then')).to.throw('Invalid Chai property: then');
+        expect(readNoExistentProperty('inspect')).to.throw('Invalid Chai property: inspect');
+      } else {
+        expect(readNoExistentProperty('then')).to.not.throw();
+        expect(readNoExistentProperty('inspect')).to.not.throw();
+      }
+    });
+  });
+
 });
diff --git a/test/expect.js b/test/expect.js
index 8560608..4c34b0e 100644
--- a/test/expect.js
+++ b/test/expect.js
@@ -124,17 +124,6 @@ describe('expect', function () {
       , bar;
     expect(foo).to.exist;
     expect(bar).to.not.exist;
-    expect(0).to.exist;
-    expect(false).to.exist;
-    expect('').to.exist;
-
-    err(function () {
-      expect(bar).to.exist;
-    }, "expected undefined to exist");
-
-    err(function () {
-      expect(foo).to.not.exist(foo);
-    }, "expected 'bar' to not exist");
   });
 
   it('arguments', function(){
diff --git a/test/should.js b/test/should.js
index 19a90ac..68579c5 100644
--- a/test/should.js
+++ b/test/should.js
@@ -67,9 +67,6 @@ describe('should', function() {
       , bar = undefined;
     should.exist(foo);
     should.not.exist(bar);
-    should.exist(0);
-    should.exist(false);
-    should.exist('');
 
     err(function () {
       should.exist(bar, 'blah');
@@ -77,7 +74,7 @@ describe('should', function() {
 
     err(function () {
       should.not.exist(foo, 'blah');
-    }, "blah: expected 'foo' to not exist");
+    }, "blah: expected 'foo' to not exist")
   });
 
   it('root equal', function () {