Trace sourcemaps in warnings, improve coverage

rollup · Jun 20, 2020 · a3b89e5 · a3b89e5
1 parent c696c1e
commit a3b89e5
Show file tree

Hide file tree

Showing 19 changed files with 57 additions and 51 deletions.
diff --git a/src/Module.ts b/src/Module.ts
@@ -37,11 +37,11 @@ import {
 	PreserveEntrySignaturesOption,
 	ResolvedIdMap,
 	RollupError,
+	RollupLogProps,
 	RollupWarning,
 	TransformModuleJSON
 } from './rollup/types';
-import { error, Errors } from './utils/error';
-import getCodeFrame from './utils/getCodeFrame';
+import { augmentCodeLocation, error, Errors } from './utils/error';
 import { getId } from './utils/getId';
 import { getOriginalLocation } from './utils/getOriginalLocation';
 import { makeLegal } from './utils/identifierHelpers';
@@ -89,7 +89,7 @@ export interface AstContext {
 	addImportMeta: (node: MetaProperty) => void;
 	code: string;
 	deoptimizationTracker: PathTracker;
-	error: (props: RollupError, pos?: number) => never;
+	error: (props: RollupError, pos: number) => never;
 	fileName: string;
 	getExports: () => string[];
 	getModuleExecIndex: () => number;
@@ -107,7 +107,7 @@ export interface AstContext {
 	traceExport: (name: string) => Variable;
 	traceVariable: (name: string) => Variable | null;
 	usesTopLevelAwait: boolean;
-	warn: (warning: RollupWarning, pos?: number) => void;
+	warn: (warning: RollupWarning, pos: number) => void;
 }
 
 function tryParse(module: Module, Parser: typeof acorn.Parser, acornOptions: acorn.Options) {
@@ -256,33 +256,8 @@ export default class Module {
 		this.ast.bind();
 	}
 
-	error(props: RollupError, pos?: number): never {
-		if (typeof pos === 'number') {
-			props.pos = pos;
-			let location: { column: number; line: number } = locate(this.code, pos, { offsetLine: 1 });
-			try {
-				location = getOriginalLocation(this.sourcemapChain, location);
-			} catch (e) {
-				this.warn({
-					code: 'SOURCEMAP_ERROR',
-					loc: {
-						column: location.column,
-						file: this.id,
-						line: location.line
-					},
-					message: `Error when using sourcemap for reporting an error: ${e.message}`,
-					pos
-				});
-			}
-
-			props.loc = {
-				column: location.column,
-				file: this.id,
-				line: location.line
-			};
-			props.frame = getCodeFrame(this.originalCode, location.line, location.column);
-		}
-
+	error(props: RollupError, pos: number): never {
+		this.addLocationToLogProps(props, pos);
 		return error(props);
 	}
 
@@ -484,7 +459,7 @@ export default class Module {
 				return this.exportShimVariable;
 			}
 			const name = exportDeclaration.localName;
-			return this.traceVariable(name) || this.graph.scope.findVariable(name);
+			return this.traceVariable(name)!;
 		}
 
 		if (name !== 'default') {
@@ -756,18 +731,9 @@ export default class Module {
 		return null;
 	}
 
-	warn(warning: RollupWarning, pos?: number) {
-		if (typeof pos === 'number') {
-			warning.pos = pos;
-
-			const { line, column } = locate(this.code, pos, { offsetLine: 1 }); // TODO trace sourcemaps, cf. error()
-
-			warning.loc = { file: this.id, line, column };
-			warning.frame = getCodeFrame(this.code, line, column);
-		}
-
-		warning.id = this.id;
-		this.options.onwarn(warning);
+	warn(props: RollupWarning, pos: number) {
+		this.addLocationToLogProps(props, pos);
+		this.options.onwarn(props);
 	}
 
 	private addDynamicImport(node: ImportExpression) {
@@ -877,6 +843,28 @@ export default class Module {
 		this.importMetas.push(node);
 	}
 
+	private addLocationToLogProps(props: RollupLogProps, pos: number): void {
+		props.id = this.id;
+		props.pos = pos;
+		let { column, line } = locate(this.code, pos, { offsetLine: 1 });
+		try {
+			({ column, line } = getOriginalLocation(this.sourcemapChain, { column, line }));
+		} catch (e) {
+			this.options.onwarn({
+				code: 'SOURCEMAP_ERROR',
+				id: this.id,
+				loc: {
+					column,
+					file: this.id,
+					line
+				},
+				message: `Error when using sourcemap for reporting an error: ${e.message}`,
+				pos
+			});
+		}
+		augmentCodeLocation(props, { column, line }, this.originalCode, this.id);
+	}
+
 	private addModulesToImportDescriptions(importDescription: {
 		[name: string]: ImportDescription | ReexportDescription;
 	}) {

diff --git a/src/utils/error.ts b/src/utils/error.ts
@@ -3,6 +3,7 @@ import Module from '../Module';
 import {
 	NormalizedInputOptions,
 	RollupError,
+	RollupLogProps,
 	RollupWarning,
 	WarningHandler
 } from '../rollup/types';
@@ -15,23 +16,23 @@ export function error(base: Error | RollupError): never {
 }
 
 export function augmentCodeLocation(
-	object: RollupError | RollupWarning,
+	props: RollupLogProps,
 	pos: number | { column: number; line: number },
 	source: string,
 	id: string
 ): void {
 	if (typeof pos === 'object') {
 		const { line, column } = pos;
-		object.loc = { file: id, line, column };
+		props.loc = { file: id, line, column };
 	} else {
-		object.pos = pos;
+		props.pos = pos;
 		const { line, column } = locate(source, pos, { offsetLine: 1 });
-		object.loc = { file: id, line, column };
+		props.loc = { file: id, line, column };
 	}
 
-	if (object.frame === undefined) {
-		const { line, column } = object.loc;
-		object.frame = getCodeFrame(source, line, column);
+	if (props.frame === undefined) {
+		const { line, column } = props.loc;
+		props.frame = getCodeFrame(source, line, column);
 	}
 }
 

diff --git a/test/function/samples/circular-missed-reexports-2/_config.js b/test/function/samples/circular-missed-reexports-2/_config.js
@@ -8,6 +8,7 @@ module.exports = {
 	},
 	error: {
 		code: 'MISSING_EXPORT',
+		id: path.resolve(__dirname, 'dep2.js'),
 		frame: `
 1: export { doesNotExist } from './dep1.js';
             ^`,

diff --git a/test/function/samples/default-not-reexported/_config.js b/test/function/samples/default-not-reexported/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'MISSING_EXPORT',
 		message: `'default' is not exported by foo.js, imported by main.js`,
+		id: path.resolve(__dirname, 'main.js'),
 		pos: 7,
 		watchFiles: [
 			path.resolve(__dirname, 'main.js'),

diff --git a/test/function/samples/double-default-export/_config.js b/test/function/samples/double-default-export/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'PARSE_ERROR',
 		message: `Duplicate export 'default'`,
+		id: path.resolve(__dirname, 'foo.js'),
 		parserError: {
 			loc: {
 				column: 7,

diff --git a/test/function/samples/double-named-export/_config.js b/test/function/samples/double-named-export/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'PARSE_ERROR',
 		message: `Duplicate export 'foo'`,
+		id: path.resolve(__dirname, 'foo.js'),
 		parserError: {
 			loc: {
 				column: 9,

diff --git a/test/function/samples/double-named-reexport/_config.js b/test/function/samples/double-named-reexport/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'PARSE_ERROR',
 		message: `Duplicate export 'foo'`,
+		id: path.resolve(__dirname, 'foo.js'),
 		parserError: {
 			loc: {
 				column: 9,

diff --git a/test/function/samples/duplicate-import-fails/_config.js b/test/function/samples/duplicate-import-fails/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'PARSE_ERROR',
 		message: `Identifier 'a' has already been declared`,
+		id: path.resolve(__dirname, 'main.js'),
 		parserError: {
 			loc: {
 				column: 9,

diff --git a/test/function/samples/duplicate-import-specifier-fails/_config.js b/test/function/samples/duplicate-import-specifier-fails/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'PARSE_ERROR',
 		message: `Identifier 'a' has already been declared`,
+		id: path.resolve(__dirname, 'main.js'),
 		parserError: {
 			loc: {
 				column: 12,

diff --git a/test/function/samples/error-after-transform-should-throw-correct-location/_config.js b/test/function/samples/error-after-transform-should-throw-correct-location/_config.js
@@ -21,6 +21,7 @@ module.exports = {
 	error: {
 		code: 'MISSING_EXPORT',
 		message: `'default' is not exported by empty.js, imported by main.js`,
+		id: path.resolve(__dirname, 'main.js'),
 		pos: 44,
 		watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'empty.js')],
 		loc: {

diff --git a/test/function/samples/error-parse-json/_config.js b/test/function/samples/error-parse-json/_config.js
@@ -6,6 +6,7 @@ module.exports = {
 	error: {
 		code: 'PARSE_ERROR',
 		message: 'Unexpected token (Note that you need @rollup/plugin-json to import JSON files)',
+		id: path.resolve(__dirname, 'file.json'),
 		parserError: {
 			loc: {
 				column: 8,

diff --git a/test/function/samples/error-parse-unknown-extension/_config.js b/test/function/samples/error-parse-unknown-extension/_config.js
@@ -7,6 +7,7 @@ module.exports = {
 		code: 'PARSE_ERROR',
 		message:
 			'Unexpected token (Note that you need plugins to import files that are not JavaScript)',
+		id: path.resolve(__dirname, 'file.css'),
 		parserError: {
 			loc: {
 				column: 0,

diff --git a/test/function/samples/export-not-at-top-level-fails/_config.js b/test/function/samples/export-not-at-top-level-fails/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'PARSE_ERROR',
 		message: `'import' and 'export' may only appear at the top level`,
+		id: path.resolve(__dirname, 'main.js'),
 		parserError: {
 			loc: {
 				column: 2,

diff --git a/test/function/samples/import-not-at-top-level-fails/_config.js b/test/function/samples/import-not-at-top-level-fails/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'PARSE_ERROR',
 		message: `'import' and 'export' may only appear at the top level`,
+		id: path.resolve(__dirname, 'main.js'),
 		parserError: {
 			loc: {
 				column: 2,

diff --git a/test/function/samples/import-of-unexported-fails/_config.js b/test/function/samples/import-of-unexported-fails/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'MISSING_EXPORT',
 		message: `'default' is not exported by empty.js, imported by main.js`,
+		id: path.resolve(__dirname, 'main.js'),
 		pos: 7,
 		watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'empty.js')],
 		loc: {

diff --git a/test/function/samples/reassign-import-fails/_config.js b/test/function/samples/reassign-import-fails/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'ILLEGAL_REASSIGNMENT',
 		message: `Illegal reassignment to import 'x'`,
+		id: path.resolve(__dirname, 'main.js'),
 		pos: 113,
 		watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')],
 		loc: {

diff --git a/test/function/samples/reassign-import-not-at-top-level-fails/_config.js b/test/function/samples/reassign-import-not-at-top-level-fails/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'ILLEGAL_REASSIGNMENT',
 		message: `Illegal reassignment to import 'x'`,
+		id: path.resolve(__dirname, 'main.js'),
 		pos: 95,
 		watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')],
 		loc: {

diff --git a/test/function/samples/reexport-missing-error/_config.js b/test/function/samples/reexport-missing-error/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'MISSING_EXPORT',
 		message: `'foo' is not exported by empty.js, imported by main.js`,
+		id: path.resolve(__dirname, 'main.js'),
 		pos: 9,
 		watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'empty.js')],
 		loc: {

diff --git a/test/function/samples/update-expression-of-import-fails/_config.js b/test/function/samples/update-expression-of-import-fails/_config.js
@@ -5,6 +5,7 @@ module.exports = {
 	error: {
 		code: 'ILLEGAL_REASSIGNMENT',
 		message: `Illegal reassignment to import 'a'`,
+		id: path.resolve(__dirname, 'main.js'),
 		pos: 28,
 		watchFiles: [path.resolve(__dirname, 'main.js'), path.resolve(__dirname, 'foo.js')],
 		loc: {