diff --git a/package.json b/package.json index f7af247..4911fac 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "eslint-plugin-import": "^2.2.0", "is-reference": "^1.0.0", "istanbul": "^0.4.3", + "locate-character": "^2.0.0", "magic-string": "^0.15.2", "minimist": "^1.2.0", "mocha": "^3.0.0", diff --git a/src/Bundle.js b/src/Bundle.js index 1166cb2..9332b83 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -566,4 +566,9 @@ export default class Bundle { return ordered; } + + warn ( warning ) { + warning.toString = () => warning.message || warning; + this.onwarn( warning ); + } } diff --git a/src/Module.js b/src/Module.js index 71db738..97f6cca 100644 --- a/src/Module.js +++ b/src/Module.js @@ -1,10 +1,11 @@ -import { timeStart, timeEnd } from './utils/flushTime.js'; import { parse } from 'acorn/src/index.js'; import MagicString from 'magic-string'; +import { locate } from 'locate-character'; +import { timeStart, timeEnd } from './utils/flushTime.js'; import { assign, blank, deepClone, keys } from './utils/object.js'; import { basename, extname } from './utils/path.js'; -import getLocation from './utils/getLocation.js'; import makeLegalIdentifier from './utils/makeLegalIdentifier.js'; +import getCodeFrame from './utils/getCodeFrame.js'; import { SOURCEMAPPING_URL_RE } from './utils/sourceMappingURL.js'; import error from './utils/error.js'; import relativeId from './utils/relativeId.js'; @@ -195,7 +196,7 @@ export default class Module { if ( this.imports[ localName ] ) { const err = new Error( `Duplicated import '${localName}'` ); err.file = this.id; - err.loc = getLocation( this.code, specifier.start ); + err.loc = locate( this.code, specifier.start, { offsetLine: 1 }); throw err; } @@ -361,7 +362,7 @@ export default class Module { 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: getLocation( this.code, importDeclaration.specifier.start ) + loc: locate( this.code, importDeclaration.specifier.start, { offsetLine: 1 }) }); } @@ -381,7 +382,7 @@ export default class Module { error({ message: `'${reexportDeclaration.localName}' is not exported by '${reexportDeclaration.module.id}' (imported by '${this.id}')`, file: this.id, - loc: getLocation( this.code, reexportDeclaration.start ) + loc: locate( this.code, reexportDeclaration.start, { offsetLine: 1 }) }); } @@ -405,4 +406,22 @@ export default class Module { if ( declaration ) return declaration; } } + + warn ( warning, pos ) { + if ( pos ) { + warning.pos = pos; + + const { line, column } = locate( this.code, pos, { offsetLine: 1 }); // TODO trace sourcemaps + + warning.loc = { + file: this.id, + line: line + 1, + column + }; + + warning.frame = getCodeFrame( this.code, line, column ); + } + + this.bundle.warn( warning ); + } } diff --git a/src/ast/Node.js b/src/ast/Node.js index 6a80ede..da36211 100644 --- a/src/ast/Node.js +++ b/src/ast/Node.js @@ -1,5 +1,5 @@ +import { locate } from 'locate-character'; import { UNKNOWN } from './values.js'; -import getLocation from '../utils/getLocation.js'; export default class Node { bind ( scope ) { @@ -74,7 +74,7 @@ export default class Node { locate () { // useful for debugging - const location = getLocation( this.module.code, this.start ); + const location = locate( this.module.code, this.start, { offsetLine: 1 }); location.file = this.module.id; location.toString = () => JSON.stringify( location ); diff --git a/src/ast/nodes/CallExpression.js b/src/ast/nodes/CallExpression.js index d275100..4880cb5 100644 --- a/src/ast/nodes/CallExpression.js +++ b/src/ast/nodes/CallExpression.js @@ -1,4 +1,4 @@ -import getLocation from '../../utils/getLocation.js'; +import { locate } from 'locate-character'; import error from '../../utils/error.js'; import Node from '../Node.js'; import isProgramLevel from '../utils/isProgramLevel.js'; @@ -14,12 +14,16 @@ export default class CallExpression extends Node { message: `Cannot call a namespace ('${this.callee.name}')`, file: this.module.id, pos: this.start, - loc: getLocation( this.module.code, this.start ) + loc: locate( this.module.code, this.start, { offsetLine: 1 }) }); } if ( this.callee.name === 'eval' && declaration.isGlobal ) { - this.module.bundle.onwarn( `Use of \`eval\` (in ${this.module.id}) is strongly discouraged, as it poses security risks and may cause issues with minification. See https://github.com/rollup/rollup/wiki/Troubleshooting#avoiding-eval for more details` ); + this.module.warn({ + code: 'EVAL', + message: `Use of eval is strongly discouraged, as it poses security risks and may cause issues with minification`, + url: 'https://github.com/rollup/rollup/wiki/Troubleshooting#avoiding-eval' + }, this.start ); } } diff --git a/src/ast/nodes/ExportDefaultDeclaration.js b/src/ast/nodes/ExportDefaultDeclaration.js index d70bd10..743287e 100644 --- a/src/ast/nodes/ExportDefaultDeclaration.js +++ b/src/ast/nodes/ExportDefaultDeclaration.js @@ -1,5 +1,5 @@ import Node from '../Node.js'; -import getLocation from '../../utils/getLocation.js'; +import { locate } from 'locate-character'; import relativeId from '../../utils/relativeId.js'; const functionOrClassDeclaration = /^(?:Function|Class)Declaration/; @@ -74,7 +74,7 @@ export default class ExportDefaultDeclaration extends Node { const newlineSeparated = /\n/.test( code.original.slice( start, end ) ); if ( newlineSeparated ) { - const { line, column } = getLocation( this.module.code, this.declaration.start ); + const { line, column } = locate( this.module.code, this.declaration.start, { offsetLine: 1 }); this.module.bundle.onwarn( `${relativeId( this.module.id )} (${line}:${column}) Ambiguous default export (is a call expression, but looks like a function declaration). See https://github.com/rollup/rollup/wiki/Troubleshooting#ambiguous-default-export` ); } } diff --git a/src/ast/nodes/MemberExpression.js b/src/ast/nodes/MemberExpression.js index bcdcf26..4da3031 100644 --- a/src/ast/nodes/MemberExpression.js +++ b/src/ast/nodes/MemberExpression.js @@ -1,5 +1,5 @@ import isReference from 'is-reference'; -import getLocation from '../../utils/getLocation.js'; +import { locate } from 'locate-character'; import relativeId from '../../utils/relativeId.js'; import Node from '../Node.js'; import { UNKNOWN } from '../values.js'; @@ -34,7 +34,7 @@ export default class MemberExpression extends Node { declaration = declaration.module.traceExport( part.name ); if ( !declaration ) { - const { line, column } = getLocation( this.module.code, this.start ); + const { line, column } = locate( this.module.code, this.start, { offsetLine: 1 }); this.module.bundle.onwarn( `${relativeId( this.module.id )} (${line}:${column}) '${part.name}' is not exported by '${relativeId( exporterId )}'. See https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module` ); this.replacement = 'undefined'; return; diff --git a/src/ast/nodes/ThisExpression.js b/src/ast/nodes/ThisExpression.js index ae35173..666be58 100644 --- a/src/ast/nodes/ThisExpression.js +++ b/src/ast/nodes/ThisExpression.js @@ -1,5 +1,5 @@ import Node from '../Node.js'; -import getLocation from '../../utils/getLocation.js'; +import { locate } from 'locate-character'; import relativeId from '../../utils/relativeId.js'; const warning = `The 'this' keyword is equivalent to 'undefined' at the top level of an ES module, and has been rewritten. See https://github.com/rollup/rollup/wiki/Troubleshooting#this-is-undefined for more information`; @@ -11,7 +11,7 @@ export default class ThisExpression extends Node { if ( lexicalBoundary.isModuleScope ) { this.alias = this.module.context; if ( this.alias === 'undefined' ) { - const { line, column } = getLocation( this.module.code, this.start ); + const { line, column } = locate( this.module.code, this.start, { offsetLine: 1 }); const detail = `${relativeId( this.module.id )} (${line}:${column + 1})`; // use one-based column number convention this.module.bundle.onwarn( `${detail} ${warning}` ); } diff --git a/src/ast/nodes/shared/disallowIllegalReassignment.js b/src/ast/nodes/shared/disallowIllegalReassignment.js index a40969f..5d338af 100644 --- a/src/ast/nodes/shared/disallowIllegalReassignment.js +++ b/src/ast/nodes/shared/disallowIllegalReassignment.js @@ -1,4 +1,4 @@ -import getLocation from '../../../utils/getLocation.js'; +import { locate } from 'locate-character'; import error from '../../../utils/error.js'; // TODO tidy this up a bit (e.g. they can both use node.module.imports) @@ -10,7 +10,7 @@ export default function disallowIllegalReassignment ( scope, node ) { message: `Illegal reassignment to import '${node.object.name}'`, file: node.module.id, pos: node.start, - loc: getLocation( node.module.code, node.start ) + loc: locate( node.module.code, node.start, { offsetLine: 1 }) }); } } @@ -21,7 +21,7 @@ export default function disallowIllegalReassignment ( scope, node ) { message: `Illegal reassignment to import '${node.name}'`, file: node.module.id, pos: node.start, - loc: getLocation( node.module.code, node.start ) + loc: locate( node.module.code, node.start, { offsetLine: 1 }) }); } } diff --git a/src/utils/getCodeFrame.js b/src/utils/getCodeFrame.js new file mode 100644 index 0000000..369829b --- /dev/null +++ b/src/utils/getCodeFrame.js @@ -0,0 +1,38 @@ +function spaces ( i ) { + let result = ''; + while ( i-- ) result += ' '; + return result; +} + + +function tabsToSpaces ( str ) { + return str.replace( /^\t+/, match => match.split( '\t' ).join( ' ' ) ); +} + +export default function getCodeFrame ( source, line, column ) { + let lines = source.split( '\n' ); + + const frameStart = Math.max( 0, line - 3 ); + const frameEnd = Math.min( line + 2, lines.length ); + + const digits = String( frameEnd + 1 ).length; + + lines = lines.slice( frameStart, frameEnd ); + while ( !/\S/.test( lines[ lines.length - 1 ] ) ) lines.pop(); + + return lines + .map( ( str, i ) => { + const isErrorLine = frameStart + i + 1 === line; + + let lineNum = String( i + frameStart + 1 ); + while ( lineNum.length < digits ) lineNum = ` ${lineNum}`; + + if ( isErrorLine ) { + const indicator = spaces( digits + 2 + tabsToSpaces( str.slice( 0, column ) ).length ) + '^'; + return `${lineNum}: ${tabsToSpaces( str )}\n${indicator}`; + } + + return `${lineNum}: ${tabsToSpaces( str )}`; + }) + .join( '\n' ); +} diff --git a/src/utils/getLocation.js b/src/utils/getLocation.js deleted file mode 100644 index 41ca492..0000000 --- a/src/utils/getLocation.js +++ /dev/null @@ -1,20 +0,0 @@ -export default function getLocation ( source, charIndex ) { - const lines = source.split( '\n' ); - const len = lines.length; - - let lineStart = 0; - let i; - - for ( i = 0; i < len; i += 1 ) { - const line = lines[i]; - const lineEnd = lineStart + line.length + 1; // +1 for newline - - if ( lineEnd > charIndex ) { - return { line: i + 1, column: charIndex - lineStart }; - } - - lineStart = lineEnd; - } - - throw new Error( 'Could not determine location of character' ); -} diff --git a/test/function/cannot-call-external-namespace/_config.js b/test/function/cannot-call-external-namespace/_config.js index 263c14a..5ed02e2 100644 --- a/test/function/cannot-call-external-namespace/_config.js +++ b/test/function/cannot-call-external-namespace/_config.js @@ -7,6 +7,6 @@ module.exports = { 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, { line: 2, column: 0 }); + assert.deepEqual( err.loc, { character: 28, line: 2, column: 0 }); } }; diff --git a/test/function/cannot-call-internal-namespace/_config.js b/test/function/cannot-call-internal-namespace/_config.js index ceb42c3..d347008 100644 --- a/test/function/cannot-call-internal-namespace/_config.js +++ b/test/function/cannot-call-internal-namespace/_config.js @@ -7,6 +7,6 @@ module.exports = { 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, { line: 2, column: 0 }); + assert.deepEqual( err.loc, { character: 33, line: 2, column: 0 }); } }; diff --git a/test/function/duplicate-import-fails/_config.js b/test/function/duplicate-import-fails/_config.js index 4083690..492ac2f 100644 --- a/test/function/duplicate-import-fails/_config.js +++ b/test/function/duplicate-import-fails/_config.js @@ -4,8 +4,8 @@ var assert = require( 'assert' ); module.exports = { description: 'disallows duplicate imports', error: function ( err ) { - assert.equal( path.normalize(err.file), path.resolve( __dirname, 'main.js' ) ); - assert.deepEqual( err.loc, { line: 2, column: 9 }); + assert.equal( path.normalize( err.file ), path.resolve( __dirname, 'main.js' ) ); + assert.deepEqual( err.loc, { character: 36, line: 2, column: 9 }); assert.ok( /Duplicated import/.test( err.message ) ); } }; diff --git a/test/function/duplicate-import-specifier-fails/_config.js b/test/function/duplicate-import-specifier-fails/_config.js index e3957b5..58446af 100644 --- a/test/function/duplicate-import-specifier-fails/_config.js +++ b/test/function/duplicate-import-specifier-fails/_config.js @@ -4,8 +4,8 @@ var assert = require( 'assert' ); module.exports = { description: 'disallows duplicate import specifiers', error: function ( err ) { - assert.equal( path.normalize(err.file), path.resolve( __dirname, 'main.js' ) ); - assert.deepEqual( err.loc, { line: 1, column: 12 }); + assert.equal( path.normalize( err.file ), path.resolve( __dirname, 'main.js' ) ); + assert.deepEqual( err.loc, { character: 12, line: 1, column: 12 }); assert.ok( /Duplicated import/.test( err.message ) ); } }; diff --git a/test/function/namespace-reassign-import-fails/_config.js b/test/function/namespace-reassign-import-fails/_config.js index 2d606f4..a76a36a 100644 --- a/test/function/namespace-reassign-import-fails/_config.js +++ b/test/function/namespace-reassign-import-fails/_config.js @@ -4,8 +4,8 @@ var assert = require( 'assert' ); module.exports = { description: 'disallows reassignments to namespace exports', error: function ( err ) { - assert.equal( path.normalize(err.file), path.resolve( __dirname, 'main.js' ) ); - assert.deepEqual( err.loc, { line: 3, column: 0 }); + assert.equal( path.normalize( err.file ), path.resolve( __dirname, 'main.js' ) ); + assert.deepEqual( err.loc, { character: 31, line: 3, column: 0 }); assert.ok( /Illegal reassignment/.test( err.message ) ); } }; diff --git a/test/function/namespace-update-import-fails/_config.js b/test/function/namespace-update-import-fails/_config.js index 4cff5c0..dc8e0f3 100644 --- a/test/function/namespace-update-import-fails/_config.js +++ b/test/function/namespace-update-import-fails/_config.js @@ -4,8 +4,8 @@ var assert = require( 'assert' ); module.exports = { description: 'disallows updates to namespace exports', error: function ( err ) { - assert.equal( path.normalize(err.file), path.resolve( __dirname, 'main.js' ) ); - assert.deepEqual( err.loc, { line: 3, column: 0 }); + assert.equal( path.normalize( err.file ), path.resolve( __dirname, 'main.js' ) ); + assert.deepEqual( err.loc, { character: 31, line: 3, column: 0 }); assert.ok( /Illegal reassignment/.test( err.message ) ); } }; diff --git a/test/function/reassign-import-fails/_config.js b/test/function/reassign-import-fails/_config.js index 22591fe..4854d02 100644 --- a/test/function/reassign-import-fails/_config.js +++ b/test/function/reassign-import-fails/_config.js @@ -5,8 +5,8 @@ module.exports = { description: 'disallows assignments to imported bindings', error: function ( err ) { assert.ok( /Illegal reassignment/.test( err.message ) ); - assert.equal( path.normalize(err.file), path.resolve( __dirname, 'main.js' ) ); - assert.deepEqual( err.loc, { line: 8, column: 0 }); + assert.equal( path.normalize( err.file ), path.resolve( __dirname, 'main.js' ) ); + assert.deepEqual( err.loc, { character: 113, line: 8, column: 0 }); } }; diff --git a/test/function/reassign-import-not-at-top-level-fails/_config.js b/test/function/reassign-import-not-at-top-level-fails/_config.js index 6e00786..b4cba92 100644 --- a/test/function/reassign-import-not-at-top-level-fails/_config.js +++ b/test/function/reassign-import-not-at-top-level-fails/_config.js @@ -4,8 +4,8 @@ var assert = require( 'assert' ); module.exports = { description: 'disallows assignments to imported bindings not at the top level', error: function ( err ) { - assert.equal( path.normalize(err.file), path.resolve( __dirname, 'main.js' ) ); - assert.deepEqual( err.loc, { line: 7, column: 2 }); + assert.equal( path.normalize( err.file ), path.resolve( __dirname, 'main.js' ) ); + assert.deepEqual( err.loc, { character: 95, line: 7, column: 2 }); assert.ok( /Illegal reassignment/.test( err.message ) ); } }; diff --git a/test/function/update-expression-of-import-fails/_config.js b/test/function/update-expression-of-import-fails/_config.js index 5f5c27c..cf72241 100644 --- a/test/function/update-expression-of-import-fails/_config.js +++ b/test/function/update-expression-of-import-fails/_config.js @@ -4,8 +4,8 @@ var assert = require( 'assert' ); module.exports = { description: 'disallows updates to imported bindings', error: function ( err ) { - assert.equal( path.normalize(err.file), path.resolve( __dirname, 'main.js' ) ); - assert.deepEqual( err.loc, { line: 3, column: 0 }); + assert.equal( path.normalize( err.file ), path.resolve( __dirname, 'main.js' ) ); + assert.deepEqual( err.loc, { character: 28, line: 3, column: 0 }); assert.ok( /Illegal reassignment/.test( err.message ) ); } }; diff --git a/test/function/warn-on-eval/_config.js b/test/function/warn-on-eval/_config.js index d6ac89f..7b499ef 100644 --- a/test/function/warn-on-eval/_config.js +++ b/test/function/warn-on-eval/_config.js @@ -1,16 +1,16 @@ -var assert = require( 'assert' ); - -var warned = false; - module.exports = { description: 'warns about use of eval', - options: { - onwarn: function ( message ) { - warned = true; - assert.ok( /Use of `eval` \(in .+?main\.js\) is strongly discouraged, as it poses security risks and may cause issues with minification\. See https:\/\/github.com\/rollup\/rollup\/wiki\/Troubleshooting#avoiding-eval for more details/.test( message ) ); + warnings: [ + { + code: 'EVAL', + message: `Use of eval is strongly discouraged, as it poses security risks and may cause issues with minification`, + pos: 13, + loc: { + column: 13, + file: require( 'path' ).resolve( __dirname, 'main.js' ), + line: 2 + }, + url: 'https://github.com/rollup/rollup/wiki/Troubleshooting#avoiding-eval' } - }, - exports: function () { - assert.ok( warned, 'did not warn' ); - } + ] }; diff --git a/test/test.js b/test/test.js index 50f63e8..8b1d3cf 100644 --- a/test/test.js +++ b/test/test.js @@ -270,7 +270,17 @@ describe( 'rollup', function () { } if ( config.warnings ) { - config.warnings( warnings ); + if ( Array.isArray( config.warnings ) ) { + assert.deepEqual( warnings.map( warning => { + const clone = Object.assign( {}, warning ); + delete clone.toString; + delete clone.frame; + + return clone; + }), config.warnings ); + } else { + config.warnings( warnings ); + } } else if ( warnings.length ) { throw new Error( `Got unexpected warnings:\n${warnings.join('\n')}` ); }