diff --git a/src/Bundle.js b/src/Bundle.js index a532b84..5489797 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -17,6 +17,7 @@ import transformBundle from './utils/transformBundle.js'; import collapseSourcemaps from './utils/collapseSourcemaps.js'; import callIfFunction from './utils/callIfFunction.js'; import relativeId from './utils/relativeId.js'; +import error from './utils/error.js'; import { dirname, isRelative, isAbsolute, normalize, relative, resolve } from './utils/path.js'; import BundleScope from './ast/scopes/BundleScope.js'; @@ -106,7 +107,13 @@ export default class Bundle { // of the entry module's dependencies return this.resolveId( this.entry, undefined ) .then( id => { - if ( id == null ) throw new Error( `Could not resolve entry (${this.entry})` ); + if ( id == null ) { + error({ + code: 'UNRESOLVED_ENTRY', + message: `Could not resolve entry (${this.entry})` + }); + } + this.entryId = id; return this.fetchModule( id, undefined ); }) @@ -367,7 +374,20 @@ export default class Bundle { }); } else { if ( resolvedId === module.id ) { - throw new Error( `A module cannot import itself (${resolvedId})` ); + // need to find the actual import declaration, so we can provide + // a useful error message. Bit hoop-jumpy but what can you do + const name = Object.keys( module.imports ) + .find( name => { + const declaration = module.imports[ name ]; + return declaration.source === source; + }); + + const declaration = module.imports[ name ].specifier.parent; + + module.error({ + code: 'CANNOT_IMPORT_SELF', + message: `A module cannot import itself` + }, declaration.start ); } module.resolvedIds[ source ] = resolvedId; diff --git a/src/Module.js b/src/Module.js index f8e902d..b16f266 100644 --- a/src/Module.js +++ b/src/Module.js @@ -15,25 +15,27 @@ import enhance from './ast/enhance.js'; import clone from './ast/clone.js'; import ModuleScope from './ast/scopes/ModuleScope.js'; -function tryParse ( code, comments, acornOptions, id ) { +function tryParse ( module, acornOptions ) { try { - return parse( code, assign({ + return parse( module.code, assign({ ecmaVersion: 8, sourceType: 'module', - onComment: ( block, text, start, end ) => comments.push({ block, text, start, end }), + onComment: ( block, text, start, end ) => module.comments.push({ block, text, start, end }), preserveParens: false }, acornOptions )); } catch ( err ) { - err.code = 'PARSE_ERROR'; - err.file = id; // see above - not necessarily true, but true enough - err.message += ` in ${id}`; - throw err; + module.error({ + code: 'PARSE_ERROR', + message: err.message.replace( / \(\d+:\d+\)$/, '' ) + }, err.pos ); } } export default class Module { constructor ({ id, code, originalCode, originalSourceMap, ast, sourceMapChain, resolvedIds, bundle }) { this.code = code; + this.id = id; + this.bundle = bundle; this.originalCode = originalCode; this.originalSourceMap = originalSourceMap; this.sourceMapChain = sourceMapChain; @@ -48,14 +50,12 @@ export default class Module { this.ast = clone( ast ); this.astClone = ast; } else { - this.ast = tryParse( code, this.comments, bundle.acornOptions, id ); // TODO what happens to comments if AST is provided? + this.ast = tryParse( this, bundle.acornOptions ); // TODO what happens to comments if AST is provided? this.astClone = clone( this.ast ); } timeEnd( 'ast' ); - this.bundle = bundle; - this.id = id; this.excludeFromSourcemap = /\0/.test( id ); this.context = bundle.getModuleContext( id ); @@ -282,6 +282,19 @@ export default class Module { // } } + error ( props, pos ) { + if ( pos !== undefined ) { + props.pos = pos; + + const { line, column } = locate( this.code, pos, { offsetLine: 1 }); // TODO trace sourcemaps + + props.loc = { file: this.id, line, column }; + props.frame = getCodeFrame( this.code, line, column ); + } + + error( props ); + } + findParent () { // TODO what does it mean if we're here? return null; @@ -372,11 +385,11 @@ export default class Module { const declaration = otherModule.traceExport( importDeclaration.name ); if ( !declaration ) { - error({ - message: `'${importDeclaration.name}' is not exported by ${relativeId( otherModule.id )} (imported by ${relativeId( this.id )}). For help fixing this error see https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module`, - file: this.id, - loc: locate( this.code, importDeclaration.specifier.start, { offsetLine: 1 }) - }); + this.error({ + code: 'MISSING_EXPORT', + message: `'${importDeclaration.name}' is not exported by ${relativeId( otherModule.id )}`, + url: `https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module` + }, importDeclaration.specifier.start ); } return declaration; diff --git a/src/ast/nodes/CallExpression.js b/src/ast/nodes/CallExpression.js index 4880cb5..dc3e6b5 100644 --- a/src/ast/nodes/CallExpression.js +++ b/src/ast/nodes/CallExpression.js @@ -10,12 +10,10 @@ export default class CallExpression extends Node { const declaration = scope.findDeclaration( this.callee.name ); if ( declaration.isNamespace ) { - error({ - message: `Cannot call a namespace ('${this.callee.name}')`, - file: this.module.id, - pos: this.start, - loc: locate( this.module.code, this.start, { offsetLine: 1 }) - }); + this.module.error({ + code: 'CANNOT_CALL_NAMESPACE', + message: `Cannot call a namespace ('${this.callee.name}')` + }, this.start ); } if ( this.callee.name === 'eval' && declaration.isGlobal ) { diff --git a/test/function/cannot-call-external-namespace/_config.js b/test/function/cannot-call-external-namespace/_config.js index 5ed02e2..a891b1f 100644 --- a/test/function/cannot-call-external-namespace/_config.js +++ b/test/function/cannot-call-external-namespace/_config.js @@ -3,10 +3,19 @@ var assert = require( 'assert' ); module.exports = { description: 'errors if code calls an external namespace', - error: function ( err ) { - assert.equal( err.message, 'Cannot call a namespace (\'foo\')' ); - assert.equal( err.file.replace( /\//g, path.sep ), path.resolve( __dirname, 'main.js' ) ); - assert.equal( err.pos, 28 ); - assert.deepEqual( err.loc, { character: 28, line: 2, column: 0 }); + error: { + code: 'CANNOT_CALL_NAMESPACE', + message: `Cannot call a namespace ('foo')`, + pos: 28, + loc: { + file: path.resolve( __dirname, 'main.js' ), + line: 2, + column: 0 + }, + frame: ` + 1: import * as foo from 'foo'; + 2: foo(); + ^ + ` } }; diff --git a/test/function/cannot-call-internal-namespace/_config.js b/test/function/cannot-call-internal-namespace/_config.js index d347008..6067234 100644 --- a/test/function/cannot-call-internal-namespace/_config.js +++ b/test/function/cannot-call-internal-namespace/_config.js @@ -3,10 +3,19 @@ var assert = require( 'assert' ); module.exports = { description: 'errors if code calls an internal namespace', - error: function ( err ) { - assert.equal( err.message, 'Cannot call a namespace (\'foo\')' ); - assert.equal( err.file.replace( /\//g, path.sep ), path.resolve( __dirname, 'main.js' ) ); - assert.equal( err.pos, 33 ); - assert.deepEqual( err.loc, { character: 33, line: 2, column: 0 }); + error: { + code: 'CANNOT_CALL_NAMESPACE', + message: `Cannot call a namespace ('foo')`, + pos: 33, + loc: { + file: path.resolve( __dirname, 'main.js' ), + line: 2, + column: 0 + }, + frame: ` + 1: import * as foo from './foo.js'; + 2: foo(); + ^ + ` } }; diff --git a/test/function/cannot-import-self/_config.js b/test/function/cannot-import-self/_config.js index 2413d03..f2896e9 100644 --- a/test/function/cannot-import-self/_config.js +++ b/test/function/cannot-import-self/_config.js @@ -1,8 +1,20 @@ +var path = require( 'path' ); var assert = require( 'assert' ); module.exports = { description: 'prevents a module importing itself', - error: function ( err ) { - assert.ok( /A module cannot import itself/.test( err.message ) ); + error: { + code: 'CANNOT_IMPORT_SELF', + message: `A module cannot import itself`, + pos: 0, + loc: { + file: path.resolve( __dirname, 'main.js' ), + line: 1, + column: 0 + }, + frame: ` + 1: import me from './main'; + ^ + ` } }; diff --git a/test/function/check-resolve-for-entry/_config.js b/test/function/check-resolve-for-entry/_config.js index a06632d..cd3da5f 100644 --- a/test/function/check-resolve-for-entry/_config.js +++ b/test/function/check-resolve-for-entry/_config.js @@ -5,7 +5,8 @@ module.exports = { options: { entry: '/not/a/path/that/actually/really/exists' }, - error: function ( err ) { - assert.ok( /Could not resolve entry/.test( err.message ) ); + error: { + code: 'UNRESOLVED_ENTRY', + message: 'Could not resolve entry (/not/a/path/that/actually/really/exists)' } }; diff --git a/test/function/custom-path-resolver-plural-b/_config.js b/test/function/custom-path-resolver-plural-b/_config.js index cb2da0c..7ca6fe6 100644 --- a/test/function/custom-path-resolver-plural-b/_config.js +++ b/test/function/custom-path-resolver-plural-b/_config.js @@ -5,21 +5,21 @@ module.exports = { options: { plugins: [ { - resolveId: function () { + resolveId () { throw new Error( 'nope' ); }, - load: function ( id ) { + load ( id ) { if ( id === 'main' ) return 'assert.ok( false );'; } }, { - resolveId: function ( importee, importer ) { + resolveId ( importee, importer ) { return 'main'; } } ] }, - error: function ( err ) { - assert.equal( err.message, 'nope' ); + error: { + message: 'nope' } }; diff --git a/test/function/default-not-reexported/_config.js b/test/function/default-not-reexported/_config.js index 4280924..3bd95b8 100644 --- a/test/function/default-not-reexported/_config.js +++ b/test/function/default-not-reexported/_config.js @@ -1,8 +1,23 @@ +const path = require( 'path' ); const assert = require( 'assert' ); module.exports = { description: 'default export is not re-exported with export *', - error ( error ) { - assert.equal( error.message, `'default' is not exported by foo.js (imported by main.js). For help fixing this error see https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module` ); + error: { + code: 'MISSING_EXPORT', + message: `'default' is not exported by foo.js`, + pos: 7, + loc: { + file: path.resolve( __dirname, 'main.js' ), + line: 1, + column: 7 + }, + frame: ` + 1: import def from './foo.js'; + ^ + 2: + 3: console.log( def ); + `, + url: `https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module` } }; diff --git a/test/function/double-default-export/_config.js b/test/function/double-default-export/_config.js index d5f5254..3a9f85f 100644 --- a/test/function/double-default-export/_config.js +++ b/test/function/double-default-export/_config.js @@ -3,7 +3,19 @@ const assert = require( 'assert' ); module.exports = { description: 'throws on double default exports', - error: err => { - assert.equal( err.message, `Duplicate export 'default' (2:7) in ${path.resolve(__dirname, 'foo.js')}` ); + error: { + code: 'PARSE_ERROR', + message: `Duplicate export 'default'`, + pos: 25, + loc: { + file: path.resolve( __dirname, 'foo.js' ), + line: 2, + column: 7 + }, + frame: ` + 1: export default 1; + 2: export default 2; + ^ + ` } }; diff --git a/test/function/double-named-export/_config.js b/test/function/double-named-export/_config.js index 169ce76..0982751 100644 --- a/test/function/double-named-export/_config.js +++ b/test/function/double-named-export/_config.js @@ -3,7 +3,20 @@ const assert = require( 'assert' ); module.exports = { description: 'throws on duplicate named exports', - error: err => { - assert.equal( err.message, `Duplicate export 'foo' (3:9) in ${path.resolve(__dirname, 'foo.js')}` ); + error: { + code: 'PARSE_ERROR', + message: `Duplicate export 'foo'`, + pos: 38, + loc: { + file: path.resolve( __dirname, 'foo.js' ), + line: 3, + column: 9 + }, + frame: ` + 1: var foo = 1; + 2: export { foo }; + 3: export { foo }; + ^ + ` } }; diff --git a/test/function/double-named-reexport/_config.js b/test/function/double-named-reexport/_config.js index 169ce76..1c7ff1c 100644 --- a/test/function/double-named-reexport/_config.js +++ b/test/function/double-named-reexport/_config.js @@ -3,7 +3,20 @@ const assert = require( 'assert' ); module.exports = { description: 'throws on duplicate named exports', - error: err => { - assert.equal( err.message, `Duplicate export 'foo' (3:9) in ${path.resolve(__dirname, 'foo.js')}` ); + error: { + code: 'PARSE_ERROR', + message: `Duplicate export 'foo'`, + pos: 38, + loc: { + file: path.resolve( __dirname, 'foo.js' ), + line: 3, + column: 9 + }, + frame: ` + 1: var foo = 1; + 2: export { foo }; + 3: export { foo } from './bar.js'; + ^ + ` } }; diff --git a/test/function/export-not-at-top-level-fails/_config.js b/test/function/export-not-at-top-level-fails/_config.js index 2dadeab..7fc1e3f 100644 --- a/test/function/export-not-at-top-level-fails/_config.js +++ b/test/function/export-not-at-top-level-fails/_config.js @@ -3,9 +3,20 @@ var assert = require( 'assert' ); module.exports = { description: 'disallows non-top-level exports', - error: function ( err ) { - assert.equal( path.normalize(err.file), path.resolve( __dirname, 'main.js' ) ); - assert.deepEqual( err.loc, { line: 2, column: 2 }); - assert.ok( /may only appear at the top level/.test( err.message ) ); + error: { + code: 'PARSE_ERROR', + message: `'import' and 'export' may only appear at the top level`, + pos: 19, + loc: { + file: path.resolve( __dirname, 'main.js' ), + line: 2, + column: 2 + }, + frame: ` + 1: function foo() { + 2: export { foo }; + ^ + 3: } + ` } }; diff --git a/test/function/import-not-at-top-level-fails/_config.js b/test/function/import-not-at-top-level-fails/_config.js index 4f873aa..4a68cc6 100644 --- a/test/function/import-not-at-top-level-fails/_config.js +++ b/test/function/import-not-at-top-level-fails/_config.js @@ -3,9 +3,20 @@ var assert = require( 'assert' ); module.exports = { description: 'disallows non-top-level imports', - error: function ( err ) { - assert.equal( path.normalize(err.file), path.resolve( __dirname, 'main.js' ) ); - assert.deepEqual( err.loc, { line: 2, column: 2 }); - assert.ok( /may only appear at the top level/.test( err.message ) ); + error: { + code: 'PARSE_ERROR', + message: `'import' and 'export' may only appear at the top level`, + pos: 19, + loc: { + file: path.resolve( __dirname, 'main.js' ), + line: 2, + column: 2 + }, + frame: ` + 1: function foo() { + 2: import foo from './foo.js'; + ^ + 3: } + ` } }; diff --git a/test/function/import-not-at-top-level-fails/main.js b/test/function/import-not-at-top-level-fails/main.js index 1fa68e1..c88024f 100644 --- a/test/function/import-not-at-top-level-fails/main.js +++ b/test/function/import-not-at-top-level-fails/main.js @@ -1,3 +1,3 @@ function foo() { - import foo from './foo'; + import foo from './foo.js'; } diff --git a/test/function/import-of-unexported-fails/_config.js b/test/function/import-of-unexported-fails/_config.js index 10d11a6..3581bd9 100644 --- a/test/function/import-of-unexported-fails/_config.js +++ b/test/function/import-of-unexported-fails/_config.js @@ -1,8 +1,23 @@ +var path = require( 'path' ); var assert = require( 'assert' ); module.exports = { description: 'marking an imported, but unexported, identifier should throw', - error: function ( err ) { - assert.equal( err.message, `'default' is not exported by empty.js (imported by main.js). For help fixing this error see https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module` ); + error: { + code: 'MISSING_EXPORT', + message: `'default' is not exported by empty.js`, + pos: 7, + loc: { + file: path.resolve( __dirname, 'main.js' ), + line: 1, + column: 7 + }, + frame: ` + 1: import a from './empty.js'; + ^ + 2: + 3: a(); + `, + url: `https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module` } }; diff --git a/test/function/reports-syntax-error-locations/_config.js b/test/function/reports-syntax-error-locations/_config.js deleted file mode 100644 index 2cb1085..0000000 --- a/test/function/reports-syntax-error-locations/_config.js +++ /dev/null @@ -1,8 +0,0 @@ -var assert = require( 'assert' ); - -module.exports = { - description: 'reports syntax error filename', - error: function ( err ) { - assert.ok( /in .+main\.js/.test( err.message ) ); - } -}; diff --git a/test/function/reports-syntax-error-locations/main.js b/test/function/reports-syntax-error-locations/main.js deleted file mode 100644 index d24584b..0000000 --- a/test/function/reports-syntax-error-locations/main.js +++ /dev/null @@ -1 +0,0 @@ -var 42 = answer; diff --git a/test/test.js b/test/test.js index 78fead1..7401b66 100644 --- a/test/test.js +++ b/test/test.js @@ -79,6 +79,23 @@ function compareWarnings ( actual, expected ) { ); } +function compareError ( actual, expected ) { + delete actual.stack; + actual = Object.assign( {}, actual, { + message: actual.message + }); + + if ( actual.frame ) { + actual.frame = actual.frame.replace( /\s+$/gm, '' ); + } + + if ( expected.frame ) { + expected.frame = expected.frame.slice( 1 ).replace( /^\t+/gm, '' ).replace( /\s+$/gm, '' ).trim(); + } + + assert.deepEqual( actual, expected ); +} + describe( 'rollup', function () { this.timeout( 10000 ); @@ -324,7 +341,11 @@ describe( 'rollup', function () { if ( unintendedError ) throw unintendedError; }, err => { if ( config.error ) { - config.error( err ); + if ( typeof config.error === 'object' ) { + compareError( err, config.error ); + } else { + config.error( err ); + } } else { throw err; }