Improve coverage, make non-deferred handlers immediate by default

cli/run/batchWarnings.ts

@@ -10,7 +10,7 @@ export interface BatchWarnings {
 }
 
 export default function batchWarnings() {
-	let allWarnings = new Map<string, RollupWarning[]>();
+	let deferredWarnings = new Map<keyof typeof deferredHandlers, RollupWarning[]>();
 	let count = 0;
 
 	return {
@@ -19,53 +19,43 @@ export default function batchWarnings() {
 		},
 
 		add: (warning: RollupWarning) => {
-			if (warning.code! in immediateHandlers) {
+			count += 1;
+
+			if (warning.code! in deferredHandlers) {
+				if (!deferredWarnings.has(warning.code!)) deferredWarnings.set(warning.code!, []);
+				deferredWarnings.get(warning.code!)!.push(warning);
+			} else if (warning.code! in immediateHandlers) {
 				immediateHandlers[warning.code!](warning);
-				return;
-			}
+			} else {
+				title(warning.message);
 
-			if (!allWarnings.has(warning.code!)) allWarnings.set(warning.code!, []);
-			allWarnings.get(warning.code!)!.push(warning);
+				if (warning.url) info(warning.url);
 
-			count += 1;
+				const id = (warning.loc && warning.loc.file) || warning.id;
+				if (id) {
+					const loc = warning.loc
+						? `${relativeId(id)}: (${warning.loc.line}:${warning.loc.column})`
+						: relativeId(id);
+
+					stderr(tc.bold(relativeId(loc)));
+				}
+
+				if (warning.frame) info(warning.frame);
+			}
 		},
 
 		flush: () => {
 			if (count === 0) return;
 
-			const codes = Array.from(allWarnings.keys()).sort((a, b) => {
-				if (deferredHandlers[a]) return -1;
-				if (deferredHandlers[b]) return 1;
-				return allWarnings.get(b)!.length - allWarnings.get(a)!.length;
-			});
+			const codes = Array.from(deferredWarnings.keys()).sort(
+				(a, b) => deferredWarnings.get(b)!.length - deferredWarnings.get(a)!.length
+			);
 
 			for (const code of codes) {
-				const handler = deferredHandlers[code];
-				const warnings = allWarnings.get(code)!;
-
-				if (handler) {
-					handler(warnings);
-				} else {
-					for (const warning of warnings) {
-						title(warning.message);
-
-						if (warning.url) info(warning.url);
-
-						const id = (warning.loc && warning.loc.file) || warning.id;
-						if (id) {
-							const loc = warning.loc
-								? `${relativeId(id)}: (${warning.loc.line}:${warning.loc.column})`
-								: relativeId(id);
-
-							stderr(tc.bold(relativeId(loc)));
-						}
-
-						if (warning.frame) info(warning.frame);
-					}
-				}
+				deferredHandlers[code](deferredWarnings.get(code)!);
 			}
 
-			allWarnings = new Map();
+			deferredWarnings = new Map();
 			count = 0;
 		}
 	};

cli/run/build.ts

@@ -81,8 +81,8 @@ export default function build(
 				}
 			}
 		})
-		.catch((err: any) => {
-			if (warnings.count > 0) warnings.flush();
+		.catch((err: Error) => {
+			warnings.flush();
 			handleError(err);
 		});
 }

test/cli/samples/warn-import-export/_config.js

@@ -11,20 +11,43 @@ module.exports = {
 		);
 		assertStderrIncludes(
 			stderr,
-
 			'(!) Unused external imports\n' +
 				"default imported from external module 'external' but never used\n"
 		);
 		assertStderrIncludes(
 			stderr,
-
 			'(!) Import of non-existent export\n' +
 				'main.js\n' +
 				"1: import unused from 'external';\n" +
-				"2: import alsoUnused from './dep.js';\n" +
+				"2: import * as dep from './dep.js';\n" +
+				"3: import alsoUnused from './dep.js';\n" +
 				'          ^\n' +
-				'3: export const foo = 1;\n' +
-				'4: export default 42;\n'
+				"4: import 'unresolvedExternal';\n"
+		);
+		assertStderrIncludes(
+			stderr,
+			'(!) Missing exports\n' +
+				'https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module\n' +
+				'main.js\n' +
+				'missing is not exported by dep.js\n' +
+				"4: import 'unresolvedExternal';\n" +
+				'5: \n' +
+				'6: export const missing = dep.missing;\n' +
+				'                              ^\n' +
+				'7: export default 42;\n'
+		);
+		assertStderrIncludes(
+			stderr,
+			'(!) Conflicting re-exports\n' +
+				"main.js re-exports 'foo' from both dep.js and dep2.js (will be ignored)\n" +
+				"main.js re-exports 'bar' from both dep.js and dep2.js (will be ignored)\n"
+		);
+		assertStderrIncludes(
+			stderr,
+			'(!) Unresolved dependencies\n' +
+				'https://rollupjs.org/guide/en/#warning-treating-module-as-external-dependency\n' +
+				'unresolvedExternal (imported by main.js, dep.js)\n' +
+				'otherUnresolvedExternal (imported by dep.js)\n'
 		);
 	}
 };

test/cli/samples/warn-import-export/dep.js

@@ -0,0 +1,6 @@
+import 'unresolvedExternal';
+import 'otherUnresolvedExternal';
+
+export const foo = 1;
+export const bar = 2;
+export const baz = 3;

test/cli/samples/warn-import-export/main.js

@@ -1,4 +1,10 @@
 import unused from 'external';
+import * as dep from './dep.js';
 import alsoUnused from './dep.js';
-export const foo = 1;
+import 'unresolvedExternal';
+
+export const missing = dep.missing;
 export default 42;
+
+export * from './dep.js';
+export * from './dep2.js';

test/cli/samples/warn-multiple/_config.js

@@ -3,24 +3,32 @@ const { assertStderrIncludes } = require('../../../utils.js');
 module.exports = {
 	description: 'aggregates warnings of different types',
 	command: 'rollup -c',
-	stderr: stderr =>
+	stderr: stderr => {
 		assertStderrIncludes(
 			stderr,
 			'(!) Missing shims for Node.js built-ins\n' +
-				"Creating a browser bundle that depends on 'url', 'assert' and 'path'. You might need to include https://www.npmjs.com/package/rollup-plugin-node-builtins\n" +
-				'(!) Import of non-existent export\n' +
+				"Creating a browser bundle that depends on 'url', 'assert' and 'path'. You might need to include https://www.npmjs.com/package/rollup-plugin-node-builtins\n"
+		);
+		assertStderrIncludes(
+			stderr,
+			'(!) Import of non-existent export\n' +
 				'main.js\n' +
 				"4: import assert from 'assert';\n" +
 				"5: import path from 'path';\n" +
 				"6: import {doesNotExist} from './dep.js';\n" +
 				'           ^\n' +
 				'7: \n' +
-				'8: export {url, assert, path};\n' +
-				"(!) Module level directives cause errors when bundled, 'use stuff' was ignored.\n" +
+				'8: export {url, assert, path};\n'
+		);
+		assertStderrIncludes(
+			stderr,
+
+			"(!) Module level directives cause errors when bundled, 'use stuff' was ignored.\n" +
 				'main.js: (1:0)\n' +
 				"1: 'use stuff';\n" +
 				'   ^\n' +
 				'2: \n' +
-				"3: import url from 'url';"
-		)
+				"3: import url from 'url';\n"
+		);
+	}
 };