From 6d418687c07c683e0cb21898f0e916e49a6802da Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Fri, 6 Jan 2017 15:13:15 -0500
Subject: [PATCH 1/3] add this.warn method to plugin contexts (#1140)

---
 src/Bundle.js                        |  2 +-
 src/utils/transform.js               | 27 ++++++++++++++++++--
 test/function/plugin-warn/_config.js | 37 ++++++++++++++++++++++++++++
 test/function/plugin-warn/main.js    |  1 +
 4 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 test/function/plugin-warn/_config.js
 create mode 100644 test/function/plugin-warn/main.js

diff --git a/src/Bundle.js b/src/Bundle.js
index 0cfe895..26e6229 100644
--- a/src/Bundle.js
+++ b/src/Bundle.js
@@ -293,7 +293,7 @@ export default class Bundle {
 					return this.cachedModules.get( id );
 				}
 
-				return transform( source, id, this.plugins );
+				return transform( this, source, id, this.plugins );
 			})
 			.then( source => {
 				const { code, originalCode, originalSourceMap, ast, sourceMapChain, resolvedIds } = source;
diff --git a/src/utils/transform.js b/src/utils/transform.js
index 7e90a74..0dd813c 100644
--- a/src/utils/transform.js
+++ b/src/utils/transform.js
@@ -1,8 +1,10 @@
 import { decode } from 'sourcemap-codec';
+import { locate } from 'locate-character';
 import error from './error.js';
 import relativeId from './relativeId.js';
+import getCodeFrame from './getCodeFrame.js';
 
-export default function transform ( source, id, plugins ) {
+export default function transform ( bundle, source, id, plugins ) {
 	const sourceMapChain = [];
 
 	const originalSourceMap = typeof source.map === 'string' ? JSON.parse( source.map ) : source.map;
@@ -19,7 +21,27 @@ export default function transform ( source, id, plugins ) {
 		return promise.then( previous => {
 			if ( !plugin.transform ) return previous;
 
-			return Promise.resolve( plugin.transform( previous, id ) ).then( result => {
+			const context = {
+				warn: ( warning, pos ) => {
+					if ( typeof warning === 'string' ) {
+						warning = { message: warning };
+					}
+
+					warning.plugin = plugin.name;
+					if ( !warning.code ) warning.code = 'PLUGIN_WARNING';
+
+					if ( pos !== undefined ) {
+						warning.pos = pos;
+						const { line, column } = locate( previous, pos, { offsetLine: 1 });
+						warning.loc = { file: id, line, column };
+						warning.frame = getCodeFrame( previous, line, column );
+					}
+
+					bundle.warn( warning );
+				}
+			};
+
+			return Promise.resolve( plugin.transform.call( context, previous, id ) ).then( result => {
 				if ( result == null ) return previous;
 
 				if ( typeof result === 'string' ) {
@@ -29,6 +51,7 @@ export default function transform ( source, id, plugins ) {
 						map: null
 					};
 				}
+
 				// `result.map` can only be a string if `result` isn't
 				else if ( typeof result.map === 'string' ) {
 					result.map = JSON.parse( result.map );
diff --git a/test/function/plugin-warn/_config.js b/test/function/plugin-warn/_config.js
new file mode 100644
index 0000000..1fd2f8c
--- /dev/null
+++ b/test/function/plugin-warn/_config.js
@@ -0,0 +1,37 @@
+const path = require( 'path' );
+
+module.exports = {
+	description: 'plugin transform hooks can use `this.warn({...}, char)` (#1140)',
+	options: {
+		plugins: [{
+			name: 'test',
+			transform ( code, id ) {
+				this.warn({ message: 'foo' });
+				this.warn( 'bar', 22 );
+				return 'assert.equal( 21 * 2, 42 );';
+			}
+		}]
+	},
+	warnings: [
+		{
+			code: 'PLUGIN_WARNING',
+			plugin: 'test',
+			message: 'foo'
+		},
+		{
+			code: 'PLUGIN_WARNING',
+			plugin: 'test',
+			message: 'bar',
+			pos: 22,
+			loc: {
+				file: path.resolve( __dirname, 'main.js' ),
+				line: 1,
+				column: 22
+			},
+			frame: `
+				1: assert.equal( 21 * 2, TK );
+				                         ^
+			`
+		}
+	]
+};
diff --git a/test/function/plugin-warn/main.js b/test/function/plugin-warn/main.js
new file mode 100644
index 0000000..04164ee
--- /dev/null
+++ b/test/function/plugin-warn/main.js
@@ -0,0 +1 @@
+assert.equal( 21 * 2, TK );

From 7831164748478c2b38f26d86fc95e96badb31450 Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Fri, 6 Jan 2017 16:29:01 -0500
Subject: [PATCH 2/3] implement this.error

---
 src/utils/transform.js                        | 121 ++++++++++--------
 .../_config.js                                |   0
 .../main.js                                   |   0
 .../_config.js                                |   4 +-
 .../main.js                                   |   0
 test/function/plugin-error/_config.js         |  29 +++++
 test/function/plugin-error/main.js            |   1 +
 .../report-transform-error-file/_config.js    |  22 ----
 .../report-transform-error-file/foo.js        |   3 -
 .../report-transform-error-file/main.js       |   3 -
 10 files changed, 99 insertions(+), 84 deletions(-)
 rename test/function/{throws-only-first-transform-bundle => plugin-error-only-first-transform-bundle}/_config.js (100%)
 rename test/function/{throws-only-first-transform-bundle => plugin-error-only-first-transform-bundle}/main.js (100%)
 rename test/function/{throws-only-first-transform => plugin-error-only-first-transform}/_config.js (80%)
 rename test/function/{throws-only-first-transform => plugin-error-only-first-transform}/main.js (100%)
 create mode 100644 test/function/plugin-error/_config.js
 create mode 100644 test/function/plugin-error/main.js
 delete mode 100644 test/function/report-transform-error-file/_config.js
 delete mode 100644 test/function/report-transform-error-file/foo.js
 delete mode 100644 test/function/report-transform-error-file/main.js

diff --git a/src/utils/transform.js b/src/utils/transform.js
index 0dd813c..8d0f9bb 100644
--- a/src/utils/transform.js
+++ b/src/utils/transform.js
@@ -1,7 +1,6 @@
 import { decode } from 'sourcemap-codec';
 import { locate } from 'locate-character';
 import error from './error.js';
-import relativeId from './relativeId.js';
 import getCodeFrame from './getCodeFrame.js';
 
 export default function transform ( bundle, source, id, plugins ) {
@@ -15,73 +14,87 @@ export default function transform ( bundle, source, id, plugins ) {
 
 	const originalCode = source.code;
 	let ast = source.ast;
-	let errored = false;
 
-	return plugins.reduce( ( promise, plugin ) => {
-		return promise.then( previous => {
-			if ( !plugin.transform ) return previous;
+	let promise = Promise.resolve( source.code );
 
-			const context = {
-				warn: ( warning, pos ) => {
-					if ( typeof warning === 'string' ) {
-						warning = { message: warning };
-					}
+	plugins.forEach( plugin => {
+		if ( !plugin.transform ) return;
 
-					warning.plugin = plugin.name;
-					if ( !warning.code ) warning.code = 'PLUGIN_WARNING';
+		promise = promise.then( previous => {
+			function augment ( object, pos, code ) {
+				if ( typeof object === 'string' ) {
+					object = { message: object };
+				}
 
-					if ( pos !== undefined ) {
-						warning.pos = pos;
-						const { line, column } = locate( previous, pos, { offsetLine: 1 });
-						warning.loc = { file: id, line, column };
-						warning.frame = getCodeFrame( previous, line, column );
-					}
+				if ( !object.code ) object.code = code;
 
+				if ( pos !== undefined ) {
+					object.pos = pos;
+					const { line, column } = locate( previous, pos, { offsetLine: 1 });
+					object.loc = { file: id, line, column };
+					object.frame = getCodeFrame( previous, line, column );
+				}
+
+				return object;
+			}
+
+			let err;
+
+			const context = {
+				warn: ( warning, pos ) => {
+					warning = augment( warning, pos, 'PLUGIN_WARNING' );
+					warning.plugin = plugin.name;
 					bundle.warn( warning );
+				},
+
+				error ( e, pos ) {
+					err = augment( e, pos, 'PLUGIN_ERROR' );
 				}
 			};
 
-			return Promise.resolve( plugin.transform.call( context, previous, id ) ).then( result => {
-				if ( result == null ) return previous;
+			let transformed;
 
-				if ( typeof result === 'string' ) {
-					result = {
-						code: result,
-						ast: null,
-						map: null
-					};
-				}
+			try {
+				transformed = plugin.transform.call( context, previous, id );
+			} catch ( err ) {
+				context.error( err );
+			}
 
-				// `result.map` can only be a string if `result` isn't
-				else if ( typeof result.map === 'string' ) {
-					result.map = JSON.parse( result.map );
-				}
+			return Promise.resolve( transformed )
+				.then( result => {
+					if ( err ) throw err;
 
-				if ( result.map && typeof result.map.mappings === 'string' ) {
-					result.map.mappings = decode( result.map.mappings );
-				}
+					if ( result == null ) return previous;
 
-				sourceMapChain.push( result.map || { missing: true, plugin: plugin.name }); // lil' bit hacky but it works
-				ast = result.ast;
+					if ( typeof result === 'string' ) {
+						result = {
+							code: result,
+							ast: null,
+							map: null
+						};
+					}
 
-				return result.code;
-			});
-		}).catch( err => {
-			// TODO this all seems a bit hacky
-			if ( errored ) throw err;
-			errored = true;
+					// `result.map` can only be a string if `result` isn't
+					else if ( typeof result.map === 'string' ) {
+						result.map = JSON.parse( result.map );
+					}
+
+					if ( result.map && typeof result.map.mappings === 'string' ) {
+						result.map.mappings = decode( result.map.mappings );
+					}
 
-			err.plugin = plugin.name;
-			throw err;
+					sourceMapChain.push( result.map || { missing: true, plugin: plugin.name }); // lil' bit hacky but it works
+					ast = result.ast;
+
+					return result.code;
+				})
+				.catch( err => {
+					err.plugin = plugin.name;
+					err.id = id;
+					error( err );
+				});
 		});
-	}, Promise.resolve( source.code ) )
-		.catch( err => {
-			error({
-				code: 'BAD_TRANSFORMER',
-				message: `Error transforming ${relativeId( id )}${err.plugin ? ` with '${err.plugin}' plugin` : ''}: ${err.message}`,
-				plugin: err.plugin,
-				id
-			});
-		})
-		.then( code => ({ code, originalCode, originalSourceMap, ast, sourceMapChain }) );
+	});
+
+	return promise.then( code => ({ code, originalCode, originalSourceMap, ast, sourceMapChain }) );
 }
diff --git a/test/function/throws-only-first-transform-bundle/_config.js b/test/function/plugin-error-only-first-transform-bundle/_config.js
similarity index 100%
rename from test/function/throws-only-first-transform-bundle/_config.js
rename to test/function/plugin-error-only-first-transform-bundle/_config.js
diff --git a/test/function/throws-only-first-transform-bundle/main.js b/test/function/plugin-error-only-first-transform-bundle/main.js
similarity index 100%
rename from test/function/throws-only-first-transform-bundle/main.js
rename to test/function/plugin-error-only-first-transform-bundle/main.js
diff --git a/test/function/throws-only-first-transform/_config.js b/test/function/plugin-error-only-first-transform/_config.js
similarity index 80%
rename from test/function/throws-only-first-transform/_config.js
rename to test/function/plugin-error-only-first-transform/_config.js
index 4bfd127..34f1e65 100644
--- a/test/function/throws-only-first-transform/_config.js
+++ b/test/function/plugin-error-only-first-transform/_config.js
@@ -20,8 +20,8 @@ module.exports = {
 		]
 	},
 	error: {
-		code: 'BAD_TRANSFORMER',
-		message: `Error transforming main.js with 'plugin1' plugin: Something happened 1`,
+		code: 'PLUGIN_ERROR',
+		message: `Something happened 1`,
 		plugin: 'plugin1',
 		id: path.resolve( __dirname, 'main.js' )
 	}
diff --git a/test/function/throws-only-first-transform/main.js b/test/function/plugin-error-only-first-transform/main.js
similarity index 100%
rename from test/function/throws-only-first-transform/main.js
rename to test/function/plugin-error-only-first-transform/main.js
diff --git a/test/function/plugin-error/_config.js b/test/function/plugin-error/_config.js
new file mode 100644
index 0000000..ca424e1
--- /dev/null
+++ b/test/function/plugin-error/_config.js
@@ -0,0 +1,29 @@
+const path = require( 'path' );
+
+module.exports = {
+	description: 'plugin transform hooks can use `this.error({...}, char)` (#1140)',
+	options: {
+		plugins: [{
+			name: 'test',
+			transform ( code, id ) {
+				this.error( 'nope', 22 );
+			}
+		}]
+	},
+	error: {
+		code: 'PLUGIN_ERROR',
+		plugin: 'test',
+		message: 'nope',
+		id: path.resolve( __dirname, 'main.js' ),
+		pos: 22,
+		loc: {
+			file: path.resolve( __dirname, 'main.js' ),
+			line: 1,
+			column: 22
+		},
+		frame: `
+			1: assert.equal( 21 * 2, TK );
+			                         ^
+		`
+	}
+};
diff --git a/test/function/plugin-error/main.js b/test/function/plugin-error/main.js
new file mode 100644
index 0000000..04164ee
--- /dev/null
+++ b/test/function/plugin-error/main.js
@@ -0,0 +1 @@
+assert.equal( 21 * 2, TK );
diff --git a/test/function/report-transform-error-file/_config.js b/test/function/report-transform-error-file/_config.js
deleted file mode 100644
index 56dc1bc..0000000
--- a/test/function/report-transform-error-file/_config.js
+++ /dev/null
@@ -1,22 +0,0 @@
-var path = require( 'path' );
-var assert = require( 'assert' );
-
-module.exports = {
-	description: 'reports which file caused a transform error',
-	options: {
-		plugins: [{
-			name: 'bad-plugin',
-			transform: function ( code, id ) {
-				if ( /foo/.test( id ) ) {
-					throw new Error( 'nope' );
-				}
-			}
-		}]
-	},
-	error: {
-		code: 'BAD_TRANSFORMER',
-		message: `Error transforming foo.js with 'bad-plugin' plugin: nope`,
-		plugin: 'bad-plugin',
-		id: path.resolve( __dirname, 'foo.js' )
-	}
-};
diff --git a/test/function/report-transform-error-file/foo.js b/test/function/report-transform-error-file/foo.js
deleted file mode 100644
index a2db33b..0000000
--- a/test/function/report-transform-error-file/foo.js
+++ /dev/null
@@ -1,3 +0,0 @@
-export default function () {
-	console.log( 'foo' );
-}
diff --git a/test/function/report-transform-error-file/main.js b/test/function/report-transform-error-file/main.js
deleted file mode 100644
index e1fb5c4..0000000
--- a/test/function/report-transform-error-file/main.js
+++ /dev/null
@@ -1,3 +0,0 @@
-import foo from './foo.js';
-
-foo();

From be7c90f0c758e2e1f8e4f4725149a0c4410c724d Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Fri, 6 Jan 2017 16:51:58 -0500
Subject: [PATCH 3/3] use plugin names in CLI logs

---
 bin/src/logging.js                   | 41 ++++++++++++----------------
 src/Bundle.js                        | 10 ++++---
 src/utils/transform.js               |  1 +
 test/function/plugin-warn/_config.js |  2 ++
 4 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/bin/src/logging.js b/bin/src/logging.js
index 33b84af..16dd8b5 100644
--- a/bin/src/logging.js
+++ b/bin/src/logging.js
@@ -8,40 +8,33 @@ const errorSymbol = process.stderr.isTTY ? `🚨   ` : `Error: `;
 // log to stderr to keep `rollup main.js > bundle.js` from breaking
 export const stderr = console.error.bind( console ); // eslint-disable-line no-console
 
-export function handleWarning ( warning ) {
-	stderr( `${warnSymbol}${chalk.bold( warning.message )}` );
+function log ( object, symbol ) {
+	const message = object.plugin ? `(${object.plugin} plugin) ${object.message}` : object.message;
+
+	stderr( `${symbol}${chalk.bold( message )}` );
 
-	if ( warning.url ) {
-		stderr( chalk.cyan( warning.url ) );
+	if ( object.url ) {
+		stderr( chalk.cyan( object.url ) );
 	}
 
-	if ( warning.loc ) {
-		stderr( `${relativeId( warning.loc.file )} (${warning.loc.line}:${warning.loc.column})` );
+	if ( object.loc ) {
+		stderr( `${relativeId( object.loc.file )} (${object.loc.line}:${object.loc.column})` );
+	} else if ( object.id ) {
+		stderr( relativeId( object.id ) );
 	}
 
-	if ( warning.frame ) {
-		stderr( chalk.dim( warning.frame ) );
+	if ( object.frame ) {
+		stderr( chalk.dim( object.frame ) );
 	}
 
 	stderr( '' );
 }
 
-export function handleError ( err, recover ) {
-	stderr( `${errorSymbol}${chalk.bold( err.message )}` );
-
-	if ( err.url ) {
-		stderr( chalk.cyan( err.url ) );
-	}
-
-	if ( err.loc ) {
-		stderr( `${relativeId( err.loc.file )} (${err.loc.line}:${err.loc.column})` );
-	}
-
-	if ( err.frame ) {
-		stderr( chalk.dim( err.frame ) );
-	}
-
-	stderr( '' );
+export function handleWarning ( warning ) {
+	log( warning, warnSymbol );
+}
 
+export function handleError ( err, recover ) {
+	log( err, errorSymbol );
 	if ( !recover ) process.exit( 1 );
 }
diff --git a/src/Bundle.js b/src/Bundle.js
index 26e6229..6ce3024 100644
--- a/src/Bundle.js
+++ b/src/Bundle.js
@@ -619,11 +619,13 @@ export default class Bundle {
 
 	warn ( warning ) {
 		warning.toString = () => {
-			if ( warning.loc ) {
-				return `${relativeId( warning.loc.file )} (${warning.loc.line}:${warning.loc.column}) ${warning.message}`;
-			}
+			let str = '';
+
+			if ( warning.plugin ) str += `(${warning.plugin} plugin) `;
+			if ( warning.loc ) str += `${relativeId( warning.loc.file )} (${warning.loc.line}:${warning.loc.column}) `;
+			str += warning.message;
 
-			return warning.message;
+			return str;
 		};
 
 		this.onwarn( warning );
diff --git a/src/utils/transform.js b/src/utils/transform.js
index 8d0f9bb..6581882 100644
--- a/src/utils/transform.js
+++ b/src/utils/transform.js
@@ -44,6 +44,7 @@ export default function transform ( bundle, source, id, plugins ) {
 				warn: ( warning, pos ) => {
 					warning = augment( warning, pos, 'PLUGIN_WARNING' );
 					warning.plugin = plugin.name;
+					warning.id = id;
 					bundle.warn( warning );
 				},
 
diff --git a/test/function/plugin-warn/_config.js b/test/function/plugin-warn/_config.js
index 1fd2f8c..78ee253 100644
--- a/test/function/plugin-warn/_config.js
+++ b/test/function/plugin-warn/_config.js
@@ -15,11 +15,13 @@ module.exports = {
 	warnings: [
 		{
 			code: 'PLUGIN_WARNING',
+			id: path.resolve( __dirname, 'main.js' ),
 			plugin: 'test',
 			message: 'foo'
 		},
 		{
 			code: 'PLUGIN_WARNING',
+			id: path.resolve( __dirname, 'main.js' ),
 			plugin: 'test',
 			message: 'bar',
 			pos: 22,