implement validate output option and --validate CLI option (#3952)

* implement `validate` output option and `--validate` CLI option * optionally verifies the generated output by parsing it with acorn * disabled by default * enable the validate option in the majority of tests * increase mocha test timeout due to circleci node-v14-latest * Add missing documentation * Refine error output and tests * Normalized options should be normalized * Skip test on windows Co-authored-by: Lukas Taegert-Atkinson <lukas.taegert-atkinson@tngtech.com> Co-authored-by: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com>
rollup · Feb 12, 2021 · 0e3445b · 0e3445b
1 parent 3394ae9
commit 0e3445b
Show file tree

Hide file tree

Showing 19 changed files with 99 additions and 10 deletions.
diff --git a/cli/help.md b/cli/help.md
@@ -72,6 +72,7 @@ Basic options:
 --watch.skipWrite           Do not write files to disk when watching
 --watch.exclude <files>     Exclude files from being watched
 --watch.include <files>     Limit watching to specified files
+--validate                  Validate output
 
 Examples:
 

diff --git a/docs/01-command-line-reference.md b/docs/01-command-line-reference.md
@@ -83,6 +83,7 @@ export default { // can be an array (for multiple inputs)
     sourcemapExcludeSources,
     sourcemapFile,
     sourcemapPathTransform,
+    validate,
 
     // danger zone
     amd,
@@ -330,6 +331,7 @@ Many options have command line equivalents. In those cases, any arguments passed
 --watch.skipWrite           Do not write files to disk when watching
 --watch.exclude <files>     Exclude files from being watched
 --watch.include <files>     Limit watching to specified files
+--validate                  Validate output
 ```
 
 The flags listed below are only available via the command line interface. All other flags correspond to and override their config file equivalents, see the [big list of options](guide/en/#big-list-of-options) for details.

diff --git a/docs/02-javascript-api.md b/docs/02-javascript-api.md
@@ -148,6 +148,7 @@ const outputOptions = {
   sourcemapExcludeSources,
   sourcemapFile,
   sourcemapPathTransform,
+  validate,
 
   // danger zone
   amd,

diff --git a/docs/999-big-list-of-options.md b/docs/999-big-list-of-options.md
@@ -924,6 +924,13 @@ export default ({
 });
 ```
 
+#### output.validate
+Type: `boolean`<br>
+CLI: `--validate`/`--no-validate`<br>
+Default: `false`
+
+Re-parses each generated chunk to detect if the generated code is valid JavaScript. This can be useful to debug output generated by plugins that use the [`renderChunk`](guide/en/#renderchunk) hook to transform code.
+
 #### preserveEntrySignatures
 Type: `"strict" | "allow-extension" | "exports-only" | false`<br>
 CLI: `--preserveEntrySignatures <strict|allow-extension>`/`--no-preserveEntrySignatures`<br>

diff --git a/src/Bundle.ts b/src/Bundle.ts
@@ -14,7 +14,12 @@ import {
 import { Addons, createAddons } from './utils/addons';
 import { getChunkAssignments } from './utils/chunkAssignment';
 import commondir from './utils/commondir';
-import { errCannotAssignModuleToChunk, error, warnDeprecation } from './utils/error';
+import {
+	errCannotAssignModuleToChunk,
+	errChunkInvalid,
+	error,
+	warnDeprecation
+} from './utils/error';
 import { sortByExecutionOrder } from './utils/executionOrder';
 import { FILE_PLACEHOLDER } from './utils/FileEmitter';
 import { basename, isAbsolute } from './utils/path';
@@ -174,6 +179,16 @@ export default class Bundle {
 				);
 				file.type = 'asset';
 			}
+			if (this.outputOptions.validate && typeof file.code == 'string') {
+				try {
+					this.graph.contextParse(file.code, {
+						allowHashBang: true,
+						ecmaVersion: 'latest'
+					});
+				} catch (exception) {
+					return error(errChunkInvalid(file, exception));
+				}
+			}
 		}
 		this.pluginDriver.finaliseAssets();
 	}

diff --git a/src/rollup/types.d.ts b/src/rollup/types.d.ts
@@ -653,6 +653,7 @@ export interface OutputOptions {
 	sourcemapPathTransform?: SourcemapPathTransformOption;
 	strict?: boolean;
 	systemNullSetters?: boolean;
+	validate?: boolean;
 }
 
 export interface NormalizedOutputOptions {
@@ -696,6 +697,7 @@ export interface NormalizedOutputOptions {
 	sourcemapPathTransform: SourcemapPathTransformOption | undefined;
 	strict: boolean;
 	systemNullSetters: boolean;
+	validate: boolean;
 }
 
 export type WarningHandlerWithDefault = (

diff --git a/src/utils/error.ts b/src/utils/error.ts
@@ -45,6 +45,7 @@ export const enum Errors {
 	BAD_LOADER = 'BAD_LOADER',
 	CANNOT_EMIT_FROM_OPTIONS_HOOK = 'CANNOT_EMIT_FROM_OPTIONS_HOOK',
 	CHUNK_NOT_GENERATED = 'CHUNK_NOT_GENERATED',
+	CHUNK_INVALID = 'CHUNK_INVALID',
 	CIRCULAR_REEXPORT = 'CIRCULAR_REEXPORT',
 	CYCLIC_CROSS_CHUNK_REEXPORT = 'CYCLIC_CROSS_CHUNK_REEXPORT',
 	DEPRECATED_FEATURE = 'DEPRECATED_FEATURE',
@@ -93,6 +94,18 @@ export function errChunkNotGeneratedForFileName(name: string) {
 	};
 }
 
+export function errChunkInvalid(
+	{ fileName, code }: { code: string; fileName: string },
+	exception: { loc: { column: number; line: number }; message: string }
+) {
+	const errorProps = {
+		code: Errors.CHUNK_INVALID,
+		message: `Chunk "${fileName}" is not valid JavaScript: ${exception.message}.`
+	};
+	augmentCodeLocation(errorProps, exception.loc, code, fileName);
+	return errorProps;
+}
+
 export function errCircularReexport(exportName: string, importedModule: string) {
 	return {
 		code: Errors.CIRCULAR_REEXPORT,

diff --git a/src/utils/options/mergeOptions.ts b/src/utils/options/mergeOptions.ts
@@ -227,7 +227,8 @@ function mergeOutputOptions(
 		sourcemapFile: getOption('sourcemapFile'),
 		sourcemapPathTransform: getOption('sourcemapPathTransform'),
 		strict: getOption('strict'),
-		systemNullSetters: getOption('systemNullSetters')
+		systemNullSetters: getOption('systemNullSetters'),
+		validate: getOption('validate')
 	};
 
 	warnUnknownOptions(config, Object.keys(outputOptions), 'output options', warn);

diff --git a/src/utils/options/normalizeOutputOptions.ts b/src/utils/options/normalizeOutputOptions.ts
@@ -73,7 +73,8 @@ export function normalizeOutputOptions(
 			| SourcemapPathTransformOption
 			| undefined,
 		strict: (config.strict as boolean | undefined) ?? true,
-		systemNullSetters: (config.systemNullSetters as boolean | undefined) || false
+		systemNullSetters: (config.systemNullSetters as boolean | undefined) || false,
+		validate: (config.validate as boolean | undefined) || false
 	};
 
 	warnUnknownOptions(config, Object.keys(outputOptions), 'output options', inputOptions.onwarn);

diff --git a/test/chunking-form/index.js b/test/chunking-form/index.js
@@ -44,7 +44,8 @@ runTestSuiteWithSamples('chunking form', path.resolve(__dirname, 'samples'), (di
 								dir: `${dir}/_actual/${format}`,
 								exports: 'auto',
 								format,
-								chunkFileNames: 'generated-[name].js'
+								chunkFileNames: 'generated-[name].js',
+								validate: true
 							},
 							(config.options || {}).output || {}
 						),

diff --git a/test/cli/samples/validate/_config.js b/test/cli/samples/validate/_config.js
@@ -0,0 +1,18 @@
+const { assertIncludes } = require('../../../utils.js');
+
+module.exports = {
+	description: 'use CLI --validate to test whether output is well formed',
+	skipIfWindows: true,
+	command: `rollup main.js --silent --outro 'console.log("end"); /*' -o _actual/out.js --validate`,
+	error: () => true,
+	stderr: stderr =>
+		assertIncludes(
+			stderr,
+			`[!] Error: Chunk "out.js" is not valid JavaScript: Unterminated comment (3:20).
+out.js (3:20)
+1: console.log(2 );
+2: 
+3: console.log("end"); /*
+                       ^`
+		)
+};
diff --git a/test/cli/samples/validate/main.js b/test/cli/samples/validate/main.js
@@ -0,0 +1 @@
+console.log(1 ? 2 : 3);
diff --git a/test/form/index.js b/test/form/index.js
@@ -47,7 +47,8 @@ runTestSuiteWithSamples('form', path.resolve(__dirname, 'samples'), (dir, config
 							{
 								exports: 'auto',
 								file: inputFile,
-								format: defaultFormat
+								format: defaultFormat,
+								// validate: true // add when systemjs bugs fixed
 							},
 							(config.options || {}).output || {}
 						),

diff --git a/test/function/samples/output-options-hook/_config.js b/test/function/samples/output-options-hook/_config.js
@@ -45,7 +45,8 @@ module.exports = {
 					sourcemap: false,
 					sourcemapExcludeSources: false,
 					strict: true,
-					systemNullSetters: false
+					systemNullSetters: false,
+					validate: false
 				});
 				assert.strictEqual(options.banner(), 'exports.bar = 43;');
 				assert.ok(/^\d+\.\d+\.\d+/.test(this.meta.rollupVersion));

diff --git a/test/function/samples/validate-output/_config.js b/test/function/samples/validate-output/_config.js
@@ -0,0 +1,23 @@
+module.exports = {
+	description: 'handles validate failure',
+	options: {
+		output: {
+			outro: '/*',
+			validate: true
+		}
+	},
+	generateError: {
+		code: 'CHUNK_INVALID',
+		message: 'Chunk "main.js" is not valid JavaScript: Unterminated comment (5:0).',
+		frame: `
+3: throw new Error('Not executed');
+4:
+5: /*
+   ^`,
+		loc: {
+			column: 0,
+			file: 'main.js',
+			line: 5
+		}
+	}
+};
diff --git a/test/function/samples/validate-output/main.js b/test/function/samples/validate-output/main.js
@@ -0,0 +1 @@
+throw new Error('Not executed');
diff --git a/test/misc/misc.js b/test/misc/misc.js
@@ -1,6 +1,6 @@
 const assert = require('assert');
 const rollup = require('../../dist/rollup');
-const { loader } = require('../utils.js');
+const { assertIncludes, loader } = require('../utils.js');
 
 describe('misc', () => {
 	it('avoids modification of options or their properties', () => {

diff --git a/test/misc/optionList.js b/test/misc/optionList.js
diff --git a/test/test.js b/test/test.js
@@ -1,7 +1,7 @@
 require('source-map-support').install();
 
 describe('rollup', function () {
-	this.timeout(15000);
+	this.timeout(30000);
 	require('./misc/index.js');
 	require('./function/index.js');
 	require('./form/index.js');