Throw better error messages for parse errors in non-JS files (#2466)

rollup · Sep 21, 2018 · bfb6b03 · bfb6b03
1 parent c962007
commit bfb6b03
Show file tree

Hide file tree

Showing 9 changed files with 72 additions and 10 deletions.
diff --git a/bin/src/logging.ts b/bin/src/logging.ts
@@ -15,7 +15,6 @@ export function handleError(err: RollupError, recover = false) {
 
 	stderr(tc.bold.red(`[!] ${tc.bold(message.toString())}`));
 
-	// TODO should this be "err.url || (err.file && err.loc.file) || err.id"?
 	if (err.url) {
 		stderr(tc.cyan(err.url));
 	}

diff --git a/src/Module.ts b/src/Module.ts
@@ -115,10 +115,16 @@ function tryParse(module: Module, parse: IParse, acornOptions: AcornOptions) {
 				module.comments.push({ block, text, start, end })
 		});
 	} catch (err) {
+		let message = err.message.replace(/ \(\d+:\d+\)$/, '');
+		if (module.id.endsWith('.json')) {
+			message += ' (Note that you need rollup-plugin-json to import JSON files)';
+		} else if (!module.id.endsWith('.js')) {
+			message += ' (Note that you need plugins to import files that are not JavaScript)';
+		}
 		module.error(
 			{
 				code: 'PARSE_ERROR',
-				message: err.message.replace(/ \(\d+:\d+\)$/, '')
+				message
 			},
 			err.pos
 		);

diff --git a/test/function/samples/error-parse-json/_config.js b/test/function/samples/error-parse-json/_config.js
@@ -0,0 +1,22 @@
+const path = require('path');
+
+module.exports = {
+	description:
+		'throws with an extended error message when failing to parse a file with ".json" extension',
+	error: {
+		code: 'PARSE_ERROR',
+		message: 'Unexpected token (Note that you need rollup-plugin-json to import JSON files)',
+		pos: 10,
+		loc: {
+			file: path.resolve(__dirname, 'file.json'),
+			line: 2,
+			column: 8
+		},
+		frame: `
+			1: {
+			2:   "JSON": "is not really JavaScript"
+			           ^
+			3: }
+		`
+	}
+};
diff --git a/test/function/samples/error-parse-json/file.json b/test/function/samples/error-parse-json/file.json
@@ -0,0 +1,3 @@
+{
+  "JSON": "is not really JavaScript"
+}
diff --git a/test/function/samples/error-parse-json/main.js b/test/function/samples/error-parse-json/main.js
@@ -0,0 +1,3 @@
+import json from './file.json';
+
+console.log(json);
diff --git a/test/function/samples/error-parse-unknown-extension/_config.js b/test/function/samples/error-parse-unknown-extension/_config.js
@@ -0,0 +1,23 @@
+const path = require('path');
+
+module.exports = {
+	description:
+		'throws with an extended error message when failing to parse a file without .(m)js extension',
+	error: {
+		code: 'PARSE_ERROR',
+		message:
+			'Unexpected token (Note that you need plugins to import files that are not JavaScript)',
+		pos: 0,
+		loc: {
+			file: path.resolve(__dirname, 'file.css'),
+			line: 1,
+			column: 0
+		},
+		frame: `
+			1: .special-class {
+			   ^
+			2:     color: black;
+			3: }
+		`
+	}
+};
diff --git a/test/function/samples/error-parse-unknown-extension/file.css b/test/function/samples/error-parse-unknown-extension/file.css
@@ -0,0 +1,3 @@
+.special-class {
+    color: black;
+}
diff --git a/test/function/samples/error-parse-unknown-extension/main.js b/test/function/samples/error-parse-unknown-extension/main.js
@@ -0,0 +1,3 @@
+import css from './file.css';
+
+console.log(css);
diff --git a/test/misc/index.js b/test/misc/index.js
@@ -402,8 +402,8 @@ describe('acorn plugins', () => {
 
 		return rollup
 			.rollup({
-				input: 'x',
-				plugins: [loader({ x: `export default 42` })],
+				input: 'x.js',
+				plugins: [loader({ 'x.js': `export default 42` })],
 				acornInjectPlugins: [
 					function pluginA(acorn) {
 						pluginAInjected = true;
@@ -445,8 +445,8 @@ describe('acorn plugins', () => {
 
 		return rollup
 			.rollup({
-				input: 'x',
-				plugins: [loader({ x: `export default 42` })],
+				input: 'x.js',
+				plugins: [loader({ 'x.js': `export default 42` })],
 				acorn: {
 					plugins: {
 						pluginC: true
@@ -473,8 +473,8 @@ describe('acorn plugins', () => {
 	it('throws if acorn.plugins is set and acornInjectPlugins is missing', () => {
 		return rollup
 			.rollup({
-				input: 'x',
-				plugins: [loader({ x: `export default 42` })],
+				input: 'x.js',
+				plugins: [loader({ 'x.js': `export default 42` })],
 				acorn: {
 					plugins: {
 						pluginE: true
@@ -493,8 +493,8 @@ describe('acorn plugins', () => {
 	it('throws if acorn.plugins is set and acornInjectPlugins is empty', () => {
 		return rollup
 			.rollup({
-				input: 'x',
-				plugins: [loader({ x: `export default 42` })],
+				input: 'x.js',
+				plugins: [loader({ 'x.js': `export default 42` })],
 				acorn: {
 					plugins: {
 						pluginF: true