Merge pull request #1231 from rollup/gh-1140

[WIP] [BREAKING] add this.warn method to plugin contexts

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 ) {
+function log ( object, symbol ) {
-	stderr( `${warnSymbol}${chalk.bold( warning.message )}` );
+	const message = object.plugin ? `(${object.plugin} plugin) ${object.message}` : object.message;
+
+	stderr( `${symbol}${chalk.bold( message )}` );
 
-	if ( warning.url ) {
+	if ( object.url ) {
-		stderr( chalk.cyan( warning.url ) );
+		stderr( chalk.cyan( object.url ) );
 	}
 
-	if ( warning.loc ) {
+	if ( object.loc ) {
-		stderr( `${relativeId( warning.loc.file )} (${warning.loc.line}:${warning.loc.column})` );
+		stderr( `${relativeId( object.loc.file )} (${object.loc.line}:${object.loc.column})` );
+	} else if ( object.id ) {
+		stderr( relativeId( object.id ) );
 	}
 
-	if ( warning.frame ) {
+	if ( object.frame ) {
-		stderr( chalk.dim( warning.frame ) );
+		stderr( chalk.dim( object.frame ) );
 	}
 
 	stderr( '' );
 }
 
-export function handleError ( err, recover ) {
+export function handleWarning ( warning ) {
-	stderr( `${errorSymbol}${chalk.bold( err.message )}` );
+	log( warning, warnSymbol );
-
+}
-	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 handleError ( err, recover ) {
+	log( err, errorSymbol );
 	if ( !recover ) process.exit( 1 );
 }

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;
@@ -619,11 +619,13 @@ export default class Bundle {
 
 	warn ( warning ) {
 		warning.toString = () => {
-			if ( warning.loc ) {
+			let str = '';
-				return `${relativeId( warning.loc.file )} (${warning.loc.line}:${warning.loc.column}) ${warning.message}`;
+
-			}
+			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 );

src/utils/transform.js

@@ -1,8 +1,9 @@
 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;
@@ -13,52 +14,88 @@ export default function transform ( source, id, plugins ) {
 
 	const originalCode = source.code;
 	let ast = source.ast;
-	let errored = false;
 
-	return plugins.reduce( ( promise, plugin ) => {
+	let promise = Promise.resolve( source.code );
-		return promise.then( previous => {
-			if ( !plugin.transform ) return previous;
 
-			return Promise.resolve( plugin.transform( previous, id ) ).then( result => {
+	plugins.forEach( plugin => {
-				if ( result == null ) return previous;
+		if ( !plugin.transform ) return;
 
-				if ( typeof result === 'string' ) {
+		promise = promise.then( previous => {
-					result = {
+			function augment ( object, pos, code ) {
-						code: result,
+				if ( typeof object === 'string' ) {
-						ast: null,
+					object = { message: object };
-						map: null
-					};
 				}
-				// `result.map` can only be a string if `result` isn't
+
-				else if ( typeof result.map === 'string' ) {
+				if ( !object.code ) object.code = code;
-					result.map = JSON.parse( result.map );
+
+				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 );
 				}
 
-				if ( result.map && typeof result.map.mappings === 'string' ) {
+				return object;
-					result.map.mappings = decode( result.map.mappings );
+			}
+
+			let err;
+
+			const context = {
+				warn: ( warning, pos ) => {
+					warning = augment( warning, pos, 'PLUGIN_WARNING' );
+					warning.plugin = plugin.name;
+					warning.id = id;
+					bundle.warn( warning );
+				},
+
+				error ( e, pos ) {
+					err = augment( e, pos, 'PLUGIN_ERROR' );
 				}
+			};
+
+			let transformed;
 
-				sourceMapChain.push( result.map || { missing: true, plugin: plugin.name }); // lil' bit hacky but it works
+			try {
-				ast = result.ast;
+				transformed = plugin.transform.call( context, previous, id );
+			} catch ( err ) {
+				context.error( err );
+			}
 
-				return result.code;
+			return Promise.resolve( transformed )
-			});
+				.then( result => {
-		}).catch( err => {
+					if ( err ) throw err;
-			// TODO this all seems a bit hacky
-			if ( errored ) throw err;
-			errored = true;
 
-			err.plugin = plugin.name;
+					if ( result == null ) return previous;
-			throw err;
+
+					if ( typeof result === 'string' ) {
+						result = {
+							code: result,
+							ast: null,
+							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 );
+					}
+
+					if ( result.map && typeof result.map.mappings === 'string' ) {
+						result.map.mappings = decode( result.map.mappings );
+					}
+
+					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({
+	return promise.then( code => ({ code, originalCode, originalSourceMap, ast, sourceMapChain }) );
-				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 }) );
 }

test/function/throws-only-first-transform/_config.js → test/function/plugin-error-only-first-transform/_config.js

@@ -20,8 +20,8 @@ module.exports = {
 		]
 	},
 	error: {
-		code: 'BAD_TRANSFORMER',
+		code: 'PLUGIN_ERROR',
-		message: `Error transforming main.js with 'plugin1' plugin: Something happened 1`,
+		message: `Something happened 1`,
 		plugin: 'plugin1',
 		id: path.resolve( __dirname, 'main.js' )
 	}

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 );
+			                         ^
+		`
+	}
+};

test/function/plugin-error/main.js

@@ -0,0 +1 @@
+assert.equal( 21 * 2, TK );

test/function/plugin-warn/_config.js

@@ -0,0 +1,39 @@
+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',
+			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,
+			loc: {
+				file: path.resolve( __dirname, 'main.js' ),
+				line: 1,
+				column: 22
+			},
+			frame: `
+				1: assert.equal( 21 * 2, TK );
+				                         ^
+			`
+		}
+	]
+};

test/function/plugin-warn/main.js

@@ -0,0 +1 @@
+assert.equal( 21 * 2, TK );

test/function/report-transform-error-file/_config.js

@@ -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' )
-	}
-};

test/function/report-transform-error-file/foo.js

@@ -1,3 +0,0 @@
-export default function () {
-	console.log( 'foo' );
-}

test/function/report-transform-error-file/main.js

@@ -1,3 +0,0 @@
-import foo from './foo.js';
-
-foo();