Merge pull request #884 from meeber/fix-stack

Remove proxy frames from stack traces and improve docs/tests

lib/chai/assertion.js

@@ -26,11 +26,34 @@ module.exports = function (_chai, util) {
    *
    * Creates object for chaining.
    *
+   * `Assertion` objects contain metadata in the form of flags. Three flags can
+   * be assigned during instantiation by passing arguments to this constructor:
+   *
+   * - `object`: This flag contains the target of the assertion. For example, in
+   *   the assertion `expect(numKittens).to.equal(7);`, the `object` flag will
+   *   contain `numKittens` so that the `equal` assertion can reference it when
+   *   needed.
+   *
+   * - `message`: This flag contains an optional custom error message to be
+   *   prepended to the error message that's generated by the assertion when it
+   *   fails.
+   *
+   * - `ssfi`: This flag stands for "start stack function indicator". It
+   *   contains a function reference that serves as the starting point for
+   *   removing frames from the stack trace of the error that's created by the
+   *   assertion when it fails. The goal is to provide a cleaner stack trace to
+   *   end users by removing Chai's internal functions. Note that it only works
+   *   in environments that support `Error.captureStackTrace`, and only when
+   *   `Chai.config.includeStack` hasn't been set to `false`.
+   *
+   * @param {Mixed} obj target of the assertion
+   * @param {String} msg (optional) custom error message
+   * @param {Function} stack (optional) starting point for removing stack frames
    * @api private
    */
 
-  function Assertion (obj, msg, stack) {
-    flag(this, 'ssfi', stack || Assertion);
+  function Assertion (obj, msg, ssfi) {
+    flag(this, 'ssfi', ssfi || Assertion);
     flag(this, 'object', obj);
     flag(this, 'message', msg);
 

lib/chai/utils/addChainableMethod.js

@@ -59,7 +59,7 @@ var call  = Function.prototype.call,
  * @api public
  */
 
-module.exports = function (ctx, name, method, chainingBehavior) {
+module.exports = function addChainableMethod(ctx, name, method, chainingBehavior) {
   if (typeof chainingBehavior !== 'function') {
     chainingBehavior = function () { };
   }
@@ -76,13 +76,18 @@ module.exports = function (ctx, name, method, chainingBehavior) {
   ctx.__methods[name] = chainableBehavior;
 
   Object.defineProperty(ctx, name,
-    { get: function () {
+    { get: function chainableMethodGetter() {
         chainableBehavior.chainingBehavior.call(this);
 
-        var assert = function assert() {
-          var old_ssfi = flag(this, 'ssfi');
-          if (old_ssfi)
-            flag(this, 'ssfi', assert);
+        var chainableMethodWrapper = function () {
+          // Use this chainable method wrapper as the starting point for
+          // removing implementation frames from the stack trace of a failed
+          // assertion. Note that this is the correct starting point even if
+          // this assertion has been overwritten since overwriting a chainable
+          // method merely replaces the saved methods in `ctx.__methods` instead
+          // of completely replacing the overwritten assertion.
+          flag(this, 'ssfi', chainableMethodWrapper);
+
           var result = chainableBehavior.method.apply(this, arguments);
 
           if (result !== undefined) {
@@ -94,12 +99,12 @@ module.exports = function (ctx, name, method, chainingBehavior) {
           return newAssertion;
         };
 
-        addLengthGuard(assert, name, true);
+        addLengthGuard(chainableMethodWrapper, name, true);
 
         // Use `__proto__` if available
         if (hasProtoSupport) {
           // Inherit all properties from the object by replacing the `Function` prototype
-          var prototype = assert.__proto__ = Object.create(this);
+          var prototype = chainableMethodWrapper.__proto__ = Object.create(this);
           // Restore the `call` and `apply` methods from `Function`
           prototype.call = call;
           prototype.apply = apply;
@@ -110,13 +115,13 @@ module.exports = function (ctx, name, method, chainingBehavior) {
           asserterNames.forEach(function (asserterName) {
             if (!excludeNames.test(asserterName)) {
               var pd = Object.getOwnPropertyDescriptor(ctx, asserterName);
-              Object.defineProperty(assert, asserterName, pd);
+              Object.defineProperty(chainableMethodWrapper, asserterName, pd);
             }
           });
         }
 
-        transferFlags(this, assert);
-        return proxify(assert);
+        transferFlags(this, chainableMethodWrapper);
+        return proxify(chainableMethodWrapper);
       }
     , configurable: true
   });

lib/chai/utils/addMethod.js

@@ -36,12 +36,19 @@ var transferFlags = require('./transferFlags');
  * @api public
  */
 
-module.exports = function (ctx, name, method) {
-  var fn = function () {
-    var keep_ssfi = flag(this, 'keep_ssfi');
-    var old_ssfi = flag(this, 'ssfi');
-    if (!keep_ssfi && old_ssfi)
-      flag(this, 'ssfi', fn);
+module.exports = function addMethod(ctx, name, method) {
+  var methodWrapper = function () {
+    // If this assertion hasn't been overwritten, then use this method wrapper
+    // as the starting point for removing implementation frames from the stack
+    // trace of a failed assertion.
+    //
+    // Note: If this assertion has been overwritten, and thus the `keep_ssfi`
+    // flag is set, then the overwriting method wrapper is used as the starting
+    // point instead. This prevents the overwriting method wrapper from showing
+    // up in the stack trace since it's invoked before this method wrapper.
+    if (!flag(this, 'keep_ssfi')) {
+      flag(this, 'ssfi', methodWrapper);
+    }
 
     var result = method.apply(this, arguments);
     if (result !== undefined)
@@ -52,6 +59,6 @@ module.exports = function (ctx, name, method) {
     return newAssertion;
   };
 
-  addLengthGuard(fn, name, false);
-  ctx[name] = proxify(fn, name);
+  addLengthGuard(methodWrapper, name, false);
+  ctx[name] = proxify(methodWrapper, name);
 };

lib/chai/utils/addProperty.js

@@ -6,6 +6,7 @@
 
 var chai = require('../../chai');
 var flag = require('./flag');
+var isProxyEnabled = require('./isProxyEnabled');
 var transferFlags = require('./transferFlags');
 
 /**
@@ -34,15 +35,31 @@ var transferFlags = require('./transferFlags');
  * @api public
  */
 
-module.exports = function (ctx, name, getter) {
+module.exports = function addProperty(ctx, name, getter) {
   getter = getter === undefined ? new Function() : getter;
 
   Object.defineProperty(ctx, name,
-    { get: function addProperty() {
-        var keep_ssfi = flag(this, 'keep_ssfi');
-        var old_ssfi = flag(this, 'ssfi');
-        if (!keep_ssfi && old_ssfi)
-          flag(this, 'ssfi', addProperty);
+    { get: function propertyGetter() {
+        // If proxy protection is disabled and this assertion hasn't been
+        // overwritten, then use this property getter as the starting point for
+        // removing implementation frames from the stack trace of a failed
+        // assertion.
+        //
+        // Notes:
+        //
+        //  - If proxy protection is enabled, then the proxy getter is used as
+        //    the starting point instead. This prevents the proxy getter from
+        //    showing up in the stack trace since it's invoked before this
+        //    property getter.
+        //
+        //  - If proxy protection is disabled but this assertion has been
+        //    overwritten, and thus the `keep_ssfi` flag is set, then the
+        //    overwriting property getter is used as the starting point instead.
+        //    This prevents the overwriting property getter from showing up in
+        //    the stack trace since it's invoked before this property getter.
+        if (!isProxyEnabled() && !flag(this, 'keep_ssfi')) {
+          flag(this, 'ssfi', propertyGetter);
+        }
 
         var result = getter.call(this);
         if (result !== undefined)

lib/chai/utils/compareByInspect.js

@@ -26,6 +26,6 @@ var inspect = require('./inspect');
  * @api public
  */
 
-module.exports = function (a, b) {
+module.exports = function compareByInspect(a, b) {
   return inspect(a) < inspect(b) ? -1 : 1;
 };

lib/chai/utils/expectTypes.js

@@ -22,7 +22,7 @@ var AssertionError = require('assertion-error');
 var flag = require('./flag');
 var type = require('type-detect');
 
-module.exports = function (obj, types) {
+module.exports = function expectTypes(obj, types) {
   obj = flag(obj, 'object');
   types = types.map(function (t) { return t.toLowerCase(); });
   types.sort();

lib/chai/utils/flag.js

@@ -23,7 +23,7 @@
  * @api private
  */
 
-module.exports = function (obj, key, value) {
+module.exports = function flag(obj, key, value) {
   var flags = obj.__flags || (obj.__flags = Object.create(null));
   if (arguments.length === 3) {
     flags[key] = value;

lib/chai/utils/getActual.js

@@ -15,6 +15,6 @@
  * @name getActual
  */
 
-module.exports = function (obj, args) {
+module.exports = function getActual(obj, args) {
   return args.length > 4 ? args[4] : obj._obj;
 };

lib/chai/utils/getMessage.js

@@ -32,7 +32,7 @@ var flag = require('./flag')
  * @api public
  */
 
-module.exports = function (obj, args) {
+module.exports = function getMessage(obj, args) {
   var negate = flag(obj, 'negate')
     , val = flag(obj, 'object')
     , expected = args[3]

lib/chai/utils/index.js

@@ -154,11 +154,17 @@ exports.checkError = require('check-error');
 exports.proxify = require('./proxify');
 
 /*!
- * Proxify util
+ * addLengthGuard util
  */
 
 exports.addLengthGuard = require('./addLengthGuard');
 
+/*!
+ * isProxyEnabled helper
+ */
+
+exports.isProxyEnabled = require('./isProxyEnabled');
+
 /*!
  * isNaN method
  */

lib/chai/utils/isProxyEnabled.js

@@ -0,0 +1,24 @@
+var config = require('../config');
+
+/*!
+ * Chai - isProxyEnabled helper
+ * Copyright(c) 2012-2014 Jake Luer <jake@alogicalparadox.com>
+ * MIT Licensed
+ */
+
+/**
+ * # isProxyEnabled()
+ *
+ * Helper function to check if Chai's proxy protection feature is enabled. If
+ * proxies are unsupported or disabled via the user's Chai config, then return
+ * false. Otherwise, return true.
+ *
+ * @namespace Utils
+ * @name isProxyEnabled
+ */
+
+module.exports = function isProxyEnabled() {
+  return config.useProxy && 
+    typeof Proxy !== 'undefined' &&
+    typeof Reflect !== 'undefined';
+};

lib/chai/utils/objDisplay.js

@@ -24,7 +24,7 @@ var config = require('../config');
  * @api public
  */
 
-module.exports = function (obj) {
+module.exports = function objDisplay(obj) {
   var str = inspect(obj)
     , type = Object.prototype.toString.call(obj);
 

lib/chai/utils/overwriteChainableMethod.js

@@ -40,11 +40,11 @@ var transferFlags = require('./transferFlags');
  * @api public
  */
 
-module.exports = function (ctx, name, method, chainingBehavior) {
+module.exports = function overwriteChainableMethod(ctx, name, method, chainingBehavior) {
   var chainableBehavior = ctx.__methods[name];
 
   var _chainingBehavior = chainableBehavior.chainingBehavior;
-  chainableBehavior.chainingBehavior = function () {
+  chainableBehavior.chainingBehavior = function overwritingChainableMethodGetter() {
     var result = chainingBehavior(_chainingBehavior).call(this);
     if (result !== undefined) {
       return result;
@@ -56,7 +56,7 @@ module.exports = function (ctx, name, method, chainingBehavior) {
   };
 
   var _method = chainableBehavior.method;
-  chainableBehavior.method = function () {
+  chainableBehavior.method = function overwritingChainableMethodWrapper() {
     var result = method(_method).apply(this, arguments);
     if (result !== undefined) {
       return result;

lib/chai/utils/overwriteMethod.js

@@ -44,7 +44,7 @@ var transferFlags = require('./transferFlags');
  * @api public
  */
 
-module.exports = function (ctx, name, method) {
+module.exports = function overwriteMethod(ctx, name, method) {
   var _method = ctx[name]
     , _super = function () {
       throw new Error(name + ' is not a function');
@@ -53,12 +53,24 @@ module.exports = function (ctx, name, method) {
   if (_method && 'function' === typeof _method)
     _super = _method;
 
-  var fn = function () {
-    var keep_ssfi = flag(this, 'keep_ssfi');
-    var old_ssfi = flag(this, 'ssfi');
-    if (!keep_ssfi && old_ssfi)
-      flag(this, 'ssfi', fn);
+  var overwritingMethodWrapper = function () {
+    // If proxy protection is disabled and this overwriting assertion hasn't
+    // been overwritten again by yet another assertion, then use this method
+    // wrapper as the starting point for removing implementation frames from the
+    // stack trace of a failed assertion.
+    //
+    // Note: If this assertion has been overwritten, and thus the `keep_ssfi`
+    // flag is set, then the overwriting method wrapper is used as the starting
+    // point instead. This prevents the overwriting method wrapper from showing
+    // up in the stack trace since it's invoked before this method wrapper.
+    if (!flag(this, 'keep_ssfi')) {
+      flag(this, 'ssfi', overwritingMethodWrapper);
+    }
 
+    // The `keep_ssfi` flag is set so that if this assertion ends up calling
+    // the overwritten assertion, then the overwritten assertion doesn't attempt
+    // to use itself as the starting point for removing implementation frames
+    // from the stack trace of a failed assertion.
     flag(this, 'keep_ssfi', true);
     var result = method(_super).apply(this, arguments);
     flag(this, 'keep_ssfi', false);
@@ -72,6 +84,6 @@ module.exports = function (ctx, name, method) {
     return newAssertion;
   }
 
-  addLengthGuard(fn, name, false);
-  ctx[name] = proxify(fn, name);
+  addLengthGuard(overwritingMethodWrapper, name, false);
+  ctx[name] = proxify(overwritingMethodWrapper, name);
 };