detect improper use of t.throws (#742)

jamestalmage · jamestalmage · commit 3201b1b4ff80 · 2016-04-11T06:45:42.000-04:00
* detect improper use of t.throws Protects against a common misuse of t.throws (Like that seen in #739). This required the creation of a custom babel plugin. https://github.com/jamestalmage/babel-plugin-ava-throws-helper * relative file path and colors * protect against null/undefined in _setAssertError * use babel-code-frame to do syntax highlighting on the Error * require `babel-code-frame` inline. It has a sizable dependency graph * remove middle section of message. It is redundant given code-frame * further tests and add documentation. * update readme.md
diff --git a/docs/recipes/babelrc.md b/docs/recipes/babelrc.md
@@ -122,5 +122,6 @@ AVA *always* adds a few custom Babel plugins when transpiling your plugins. They
 
  * Enable `power-assert` support.
  * Rewrite require paths internal AVA dependencies like `babel-runtime` (important if you are still using `npm@2`).
+ * [`ava-throws-helper`](https://github.com/jamestalmage/babel-plugin-ava-throws-helper) helps AVA [detect and report](https://github.com/sindresorhus/ava/pull/742) improper use of the `t.throws` assertion.
  * Generate test metadata to determine which files should be run first (*future*).
  * Static analysis of dependencies for precompilation (*future*).
diff --git a/lib/caching-precompiler.js b/lib/caching-precompiler.js
@@ -52,11 +52,13 @@ CachingPrecompiler.prototype._init = function () {
 	];
 
 	var transformRuntime = require('babel-plugin-transform-runtime');
+	var throwsHelper = require('babel-plugin-ava-throws-helper');
 	var rewriteBabelPaths = this._createRewritePlugin();
 	var powerAssert = this._createEspowerPlugin();
 
 	this.defaultPlugins = [
 		powerAssert,
+		throwsHelper,
 		rewriteBabelPaths,
 		transformRuntime
 	];
diff --git a/lib/fork.js b/lib/fork.js
@@ -24,6 +24,7 @@ if (env.NODE_PATH) {
 module.exports = function (file, opts) {
 	opts = objectAssign({
 		file: file,
+		baseDir: process.cwd(),
 		tty: process.stdout.isTTY ? {
 			columns: process.stdout.columns,
 			rows: process.stdout.rows
diff --git a/lib/test-worker.js b/lib/test-worker.js
@@ -62,6 +62,7 @@ sourceMapSupport.install({
 var loudRejection = require('loud-rejection/api')(process); // eslint-disable-line
 var serializeError = require('./serialize-error');
 var send = require('./send');
+var throwsHelper = require('./throws-helper');
 var installPrecompiler = require('require-precompiled'); // eslint-disable-line
 var cacheDir = opts.cacheDir;
 
@@ -92,7 +93,10 @@ Object.keys(require.extensions).forEach(function (ext) {
 
 require(testPath);
 
+process.on('unhandledRejection', throwsHelper);
+
 process.on('uncaughtException', function (exception) {
+	throwsHelper(exception);
 	send('uncaughtException', {exception: serializeError(exception)});
 });
 
diff --git a/lib/test.js b/lib/test.js
@@ -12,6 +12,7 @@ var plur = require('plur');
 var assert = require('./assert');
 var enhanceAssert = require('./enhance-assert');
 var globals = require('./globals');
+var throwsHelper = require('./throws-helper');
 
 function Test(title, fn, contextRef, report) {
 	if (!(this instanceof Test)) {
@@ -68,6 +69,7 @@ Test.prototype._assert = function (promise) {
 };
 
 Test.prototype._setAssertError = function (err) {
+	throwsHelper(err);
 	if (this.assertError !== undefined) {
 		return;
 	}
diff --git a/lib/throws-helper.js b/lib/throws-helper.js
@@ -0,0 +1,35 @@
+'use strict';
+var fs = require('fs');
+var path = require('path');
+var chalk = require('chalk');
+var globals = require('./globals');
+
+module.exports = function throwsHelper(error) {
+	if (!error || !error._avaThrowsHelperData) {
+		return;
+	}
+	var data = error._avaThrowsHelperData;
+	var codeFrame = require('babel-code-frame');
+	var frame = '';
+	try {
+		var rawLines = fs.readFileSync(data.filename, 'utf8');
+		frame = codeFrame(rawLines, data.line, data.column, {highlightCode: true});
+	} catch (e) {
+		console.warn(e);
+		console.warn(e);
+	}
+	console.error(
+		[
+			'Improper usage of t.throws detected at ' + chalk.bold.yellow('%s (%d:%d)') + ':',
+			frame,
+			'The first argument to t.throws should be wrapped in a function:',
+			chalk.cyan('  t.throws(function() {\n    %s\n  })'),
+			'Visit the following URL for more details:',
+			'  ' + chalk.blue.underline('https://github.com/sindresorhus/ava#throwsfunctionpromise-error-message')
+		].join('\n\n'),
+		path.relative(globals.options.baseDir, data.filename),
+		data.line,
+		data.column,
+		data.source
+	);
+};
diff --git a/package.json b/package.json
@@ -78,7 +78,9 @@
     "array-uniq": "^1.0.2",
     "arrify": "^1.0.0",
     "ava-init": "^0.1.0",
+    "babel-code-frame": "^6.7.5",
     "babel-core": "^6.3.21",
+    "babel-plugin-ava-throws-helper": "0.0.4",
     "babel-plugin-detective": "^1.0.2",
     "babel-plugin-espower": "^2.1.0",
     "babel-plugin-transform-runtime": "^6.3.13",
@@ -138,6 +140,7 @@
     "update-notifier": "^0.6.0"
   },
   "devDependencies": {
+    "babel-code-frame": "^6.7.5",
     "cli-table2": "^0.2.0",
     "coveralls": "^2.11.4",
     "delay": "^1.3.0",
diff --git a/readme.md b/readme.md
@@ -583,7 +583,7 @@ You can also use the special `"inherit"` keyword. This makes AVA defer to the Ba
 
 See AVA's [`.babelrc` recipe](docs/recipes/babelrc.md) for further examples and a more detailed explanation of configuration options.
 
-Note that AVA will *always* apply the [`espower`](https://github.com/power-assert-js/babel-plugin-espower) and [`transform-runtime`](https://babeljs.io/docs/plugins/transform-runtime/) plugins.
+Note that AVA will *always* apply [a few internal plugins](docs/recipes/babelrc.md#notes) regardless of configuration, but they should not impact the behavior of your code.
 
 ### TypeScript support
 
diff --git a/test/caching-precompiler.js b/test/caching-precompiler.js
@@ -6,6 +6,7 @@ var uniqueTempDir = require('unique-temp-dir');
 var sinon = require('sinon');
 var babel = require('babel-core');
 var transformRuntime = require('babel-plugin-transform-runtime');
+var throwsHelper = require('babel-plugin-ava-throws-helper');
 var fromMapFileSource = require('convert-source-map').fromMapFileSource;
 
 var CachingPrecompiler = require('../lib/caching-precompiler');
@@ -145,7 +146,7 @@ test('uses babelConfig for babel options when babelConfig is an object', functio
 	t.true('inputSourceMap' in options);
 	t.false(options.babelrc);
 	t.same(options.presets, ['stage-2', 'es2015']);
-	t.same(options.plugins, [customPlugin, powerAssert, rewrite, transformRuntime]);
+	t.same(options.plugins, [customPlugin, powerAssert, throwsHelper, rewrite, transformRuntime]);
 	t.end();
 });
 
diff --git a/test/cli.js b/test/cli.js
@@ -96,6 +96,30 @@ test('throwing a named function will report the to the console', function (t) {
 	});
 });
 
+test('improper use of t.throws will be reported to the console', function (t) {
+	execCli('fixture/improper-t-throws.js', function (err, stdout, stderr) {
+		t.ok(err);
+		t.match(stderr, /Improper usage of t\.throws detected at .*improper-t-throws.js \(4:10\)/);
+		t.end();
+	});
+});
+
+test('improper use of t.throws from within a Promise will be reported to the console', function (t) {
+	execCli('fixture/improper-t-throws-promise.js', function (err, stdout, stderr) {
+		t.ok(err);
+		t.match(stderr, /Improper usage of t\.throws detected at .*improper-t-throws-promise.js \(5:11\)/);
+		t.end();
+	});
+});
+
+test('improper use of t.throws from within an async callback will be reported to the console', function (t) {
+	execCli('fixture/improper-t-throws-async-callback.js', function (err, stdout, stderr) {
+		t.ok(err);
+		t.match(stderr, /Improper usage of t\.throws detected at .*improper-t-throws-async-callback.js \(5:11\)/);
+		t.end();
+	});
+});
+
 test('babel require hook only applies to the test file', function (t) {
 	t.plan(3);
 
diff --git a/test/fixture/improper-t-throws-async-callback.js b/test/fixture/improper-t-throws-async-callback.js
@@ -0,0 +1,11 @@
+import test from '../../';
+
+test.cb(t => {
+	setTimeout(() => {
+		t.throws(throwSync());
+	});
+});
+
+function throwSync() {
+	throw new Error('should be detected');
+}
diff --git a/test/fixture/improper-t-throws-promise.js b/test/fixture/improper-t-throws-promise.js
@@ -0,0 +1,11 @@
+import test from '../../';
+
+test(t => {
+	return Promise.resolve().then(() => {
+		t.throws(throwSync());
+	});
+});
+
+function throwSync() {
+	throw new Error('should be detected');
+}
diff --git a/test/fixture/improper-t-throws-unhandled-rejection.js b/test/fixture/improper-t-throws-unhandled-rejection.js
@@ -0,0 +1,13 @@
+import test from '../../';
+
+test.cb(t => {
+	Promise.resolve().then(() => {
+		t.throws(throwSync());
+	});
+	
+	setTimeout(t.end, 20);
+});
+
+function throwSync() {
+	throw new Error('should be detected');
+}
diff --git a/test/fixture/improper-t-throws.js b/test/fixture/improper-t-throws.js
@@ -0,0 +1,9 @@
+import test from '../../';
+
+test(t => {
+	t.throws(throwSync());
+});
+
+function throwSync() {
+	throw new Error('should be detected');
+}
