Add locations to logs, warnings and error messages (#5424)

rollup · Mar 12, 2024 · 341d987 · 341d987
1 parent bbcdac3
commit 341d987
Show file tree

Hide file tree

Showing 120 changed files with 754 additions and 635 deletions.
diff --git a/cli/run/batchWarnings.ts b/cli/run/batchWarnings.ts
@@ -195,22 +195,18 @@ const deferredHandlers: {
 	PLUGIN_WARNING(warnings) {
 		const nestedByPlugin = nest(warnings, 'plugin');
 
-		for (const { key: plugin, items } of nestedByPlugin) {
+		for (const { items } of nestedByPlugin) {
 			const nestedByMessage = nest(items, 'message');
 
 			let lastUrl = '';
 
 			for (const { key: message, items } of nestedByMessage) {
-				title(`Plugin ${plugin}: ${message}`);
+				title(message);
 				for (const warning of items) {
 					if (warning.url && warning.url !== lastUrl) info((lastUrl = warning.url));
 
-					const id = warning.id || warning.loc?.file;
-					if (id) {
-						let loc = relativeId(id);
-						if (warning.loc) {
-							loc += `: (${warning.loc.line}:${warning.loc.column})`;
-						}
+					const loc = formatLocation(warning);
+					if (loc) {
 						stderr(bold(loc));
 					}
 					if (warning.frame) info(warning.frame);
@@ -273,11 +269,9 @@ function defaultBody(log: RollupLog): void {
 		info(getRollupUrl(log.url));
 	}
 
-	const id = log.loc?.file || log.id;
-	if (id) {
-		const loc = log.loc ? `${relativeId(id)} (${log.loc.line}:${log.loc.column})` : relativeId(id);
-
-		stderr(bold(relativeId(loc)));
+	const loc = formatLocation(log);
+	if (loc) {
+		stderr(bold(loc));
 	}
 
 	if (log.frame) info(log.frame);
@@ -340,3 +334,9 @@ function generateLogFilter(command: Record<string, any>) {
 	}
 	return getLogFilter(filters);
 }
+
+function formatLocation(log: RollupLog): string | null {
+	const id = log.loc?.file || log.id;
+	if (!id) return null;
+	return log.loc ? `${id}:${log.loc.line}:${log.loc.column}` : id;
+}
diff --git a/src/utils/logger.ts b/src/utils/logger.ts
@@ -11,7 +11,7 @@ import { getSortedValidatedPlugins } from './PluginDriver';
 import { EMPTY_SET } from './blank';
 import { doNothing } from './doNothing';
 import { LOGLEVEL_DEBUG, LOGLEVEL_INFO, LOGLEVEL_WARN, logLevelPriority } from './logging';
-import { error } from './logs';
+import { augmentLogMessage, error } from './logs';
 import { normalizeLog } from './options/options';
 
 export function getLogger(
@@ -23,6 +23,7 @@ export function getLogger(
 	plugins = getSortedValidatedPlugins('onLog', plugins);
 	const minimalPriority = logLevelPriority[logLevel];
 	const logger = (level: LogLevel, log: RollupLog, skipped: ReadonlySet<Plugin> = EMPTY_SET) => {
+		augmentLogMessage(log);
 		const logPriority = logLevelPriority[level];
 		if (logPriority < minimalPriority) {
 			return;

diff --git a/src/utils/logs.ts b/src/utils/logs.ts
@@ -37,8 +37,12 @@ export function error(base: Error | RollupLog): never {
 }
 
 export function getRollupError(base: RollupLog): Error & RollupLog {
+	augmentLogMessage(base);
 	const errorInstance = Object.assign(new Error(base.message), base);
-	Object.defineProperty(errorInstance, 'name', { value: 'RollupError', writable: true });
+	Object.defineProperty(errorInstance, 'name', {
+		value: 'RollupError',
+		writable: true
+	});
 	return errorInstance;
 }
 
@@ -67,6 +71,32 @@ export function augmentCodeLocation(
 	}
 }
 
+const symbolAugmented = Symbol('augmented');
+
+interface AugmentedRollupLog extends RollupLog {
+	[symbolAugmented]?: boolean;
+}
+
+export function augmentLogMessage(log: AugmentedRollupLog): void {
+	// Make sure to only augment the log message once
+	if (!(log.plugin || log.loc) || log[symbolAugmented]) {
+		return;
+	}
+	log[symbolAugmented] = true;
+	let prefix = '';
+
+	if (log.plugin) {
+		prefix += `[plugin ${log.plugin}] `;
+	}
+	const id = log.id || log.loc?.file;
+	if (id) {
+		const position = log.loc ? ` (${log.loc.line}:${log.loc.column})` : '';
+		prefix += `${relativeId(id)}${position}: `;
+	}
+
+	log.message = prefix + log.message;
+}
+
 // Error codes should be sorted alphabetically while errors should be sorted by
 // error code below
 const ADDON_ERROR = 'ADDON_ERROR',
@@ -323,7 +353,10 @@ export function logConstVariableReassignError() {
 }
 
 export function logDuplicateArgumentNameError(name: string): RollupLog {
-	return { code: DUPLICATE_ARGUMENT_NAME, message: `Duplicate argument name "${name}"` };
+	return {
+		code: DUPLICATE_ARGUMENT_NAME,
+		message: `Duplicate argument name "${name}"`
+	};
 }
 
 export function logDuplicateExportError(name: string): RollupLog {
@@ -838,7 +871,10 @@ export function logParseError(message: string, pos?: number): RollupLog {
 }
 
 export function logRedeclarationError(name: string): RollupLog {
-	return { code: REDECLARATION_ERROR, message: `Identifier "${name}" has already been declared` };
+	return {
+		code: REDECLARATION_ERROR,
+		message: `Identifier "${name}" has already been declared`
+	};
 }
 
 export function logModuleParseError(error: Error, moduleId: string): RollupLog {

diff --git a/src/utils/options/options.ts b/src/utils/options/options.ts
@@ -19,7 +19,6 @@ import { EMPTY_ARRAY } from '../blank';
 import { LOGLEVEL_DEBUG, LOGLEVEL_ERROR, LOGLEVEL_WARN, logLevelPriority } from '../logging';
 import { error, logInvalidOption, logUnknownOption } from '../logs';
 import { printQuotedStringList } from '../printStringList';
-import relativeId from '../relativeId';
 
 export interface GenericConfigObject {
 	[key: string]: unknown;
@@ -60,7 +59,7 @@ const getDefaultOnLog = (printLog: LogHandler, onwarn?: WarningHandlerWithDefaul
 
 const addLogToString = (log: RollupLog): RollupLog => {
 	Object.defineProperty(log, 'toString', {
-		value: () => getExtendedLogMessage(log),
+		value: () => log.message,
 		writable: true
 	});
 	return log;
@@ -73,21 +72,7 @@ export const normalizeLog = (log: RollupLog | string | (() => RollupLog | string
 			? normalizeLog(log())
 			: log;
 
-const getExtendedLogMessage = (log: RollupLog): string => {
-	let prefix = '';
-
-	if (log.plugin) {
-		prefix += `(${log.plugin} plugin) `;
-	}
-	if (log.loc) {
-		prefix += `${relativeId(log.loc.file!)} (${log.loc.line}:${log.loc.column}) `;
-	}
-
-	return prefix + log.message;
-};
-
-const defaultPrintLog: LogHandler = (level, log) => {
-	const message = getExtendedLogMessage(log);
+const defaultPrintLog: LogHandler = (level, { message }) => {
 	switch (level) {
 		case LOGLEVEL_WARN: {
 			return console.warn(message);

diff --git a/test/cli/samples/filter-logs/_config.js b/test/cli/samples/filter-logs/_config.js
@@ -14,11 +14,11 @@ module.exports = defineTest({
 			stderr,
 			`
 main.js → stdout...
-first
-second
-third
-fourth
-fifth
+[plugin test] first
+[plugin test] second
+[plugin test] third
+[plugin test] fourth
+[plugin test] fifth
 `
 		);
 	}

diff --git a/test/cli/samples/handles-uncaught-errors-under-watch/_config.js b/test/cli/samples/handles-uncaught-errors-under-watch/_config.js
@@ -5,6 +5,6 @@ module.exports = defineTest({
 	command: 'rollup --config rollup.config.js -w',
 	error: () => true,
 	stderr(stderr) {
-		assertIncludes(stderr, 'Uncaught RollupError: LOL');
+		assertIncludes(stderr, 'Uncaught RollupError: [plugin test] LOL');
 	}
 });
diff --git a/test/cli/samples/log-side-effects/_config.js b/test/cli/samples/log-side-effects/_config.js
@@ -1,3 +1,4 @@
+const { sep } = require('node:path');
 const { assertIncludes } = require('../../../utils.js');
 
 module.exports = defineTest({
@@ -6,27 +7,27 @@ module.exports = defineTest({
 	env: { FORCE_COLOR: undefined, NO_COLOR: true },
 	stderr: stderr =>
 		assertIncludes(
-			stderr,
-			`First side effect in dep-mapped.js is at (2:26)
+			stderr.replaceAll(__dirname + sep, 'CWD/'),
+			`dep-mapped.js (1:0): First side effect in dep-mapped.js is at (2:26)
 1: const removed = true;
 2: const alsoRemoved = true; console.log('mapped effect');
                              ^
-dep-mapped.js (1:0)
+CWD/dep-mapped.js:1:0
 1: console.log('mapped effect');
    ^
-First side effect in dep-long-line.js is at (1:126)
+dep-long-line.js (1:126): First side effect in dep-long-line.js is at (1:126)
 1: /* This side effect is deeply hidden inside a very long line, beyond the 120-character limit that we impose for truncation */ console.lo...
                                                                                                                                  ^
-dep-long-line.js (1:126)
+CWD/dep-long-line.js:1:126
 1: /* This side effect is deeply hidden inside a very long line, beyond the 120-character limit that we impose for truncation */ console.lo...
                                                                                                                                  ^
-First side effect in main.js is at (3:0)
+main.js (3:0): First side effect in main.js is at (3:0)
 1: import './dep-mapped';
 2: import './dep-long-line';
 3: console.log('main effect');
    ^
 4: console.log('other effect');
-main.js (3:0)
+CWD/main.js:3:0
 1: import './dep-mapped';
 2: import './dep-long-line';
 3: console.log('main effect');

diff --git a/test/cli/samples/logs/_config.js b/test/cli/samples/logs/_config.js
@@ -1,4 +1,5 @@
 const assert = require('node:assert');
+const { sep } = require('node:path');
 
 const BOLD = '\u001B[1m';
 const BLUE = '\u001B[34m';
@@ -14,23 +15,23 @@ module.exports = defineTest({
 	env: { FORCE_COLOR: '1', TERM: 'xterm' },
 	stderr(stderr) {
 		assert.strictEqual(
-			stderr,
+			stderr.replaceAll(__dirname + sep, 'CWD/'),
 			`${CYAN}
 ${BOLD}main.js${REGULAR} → ${BOLD}stdout${REGULAR}...${NOCOLOR}
-${BOLD}${CYAN}simple-info${NOCOLOR}${REGULAR}
-${BOLD}${CYAN}complex-info${NOCOLOR}${REGULAR}
+${BOLD}${CYAN}[plugin test] simple-info${NOCOLOR}${REGULAR}
+${BOLD}${CYAN}[plugin test] complex-info${NOCOLOR}${REGULAR}
 ${GRAY}https://rollupjs.org/https://my-url.net${NOCOLOR}
-${BOLD}${BLUE}simple-debug${NOCOLOR}${REGULAR}
-${BOLD}${BLUE}complex-debug${NOCOLOR}${REGULAR}
+${BOLD}${BLUE}[plugin test] simple-debug${NOCOLOR}${REGULAR}
+${BOLD}${BLUE}[plugin test] complex-debug${NOCOLOR}${REGULAR}
 ${GRAY}https://rollupjs.org/https://my-url.net${NOCOLOR}
-${BOLD}${CYAN}transform-info${NOCOLOR}${REGULAR}
+${BOLD}${CYAN}[plugin test] main.js (1:12): transform-info${NOCOLOR}${REGULAR}
 ${GRAY}https://rollupjs.org/https://my-url.net${NOCOLOR}
-${BOLD}main.js (1:12)${REGULAR}
+${BOLD}CWD/main.js:1:12${REGULAR}
 ${GRAY}1: assert.ok( true );
                ^${NOCOLOR}
-${BOLD}${BLUE}transform-debug${NOCOLOR}${REGULAR}
+${BOLD}${BLUE}[plugin test] main.js (1:13): transform-debug${NOCOLOR}${REGULAR}
 ${GRAY}https://rollupjs.org/https://my-url.net${NOCOLOR}
-${BOLD}main.js (1:13)${REGULAR}
+${BOLD}CWD/main.js:1:13${REGULAR}
 ${GRAY}1: assert.ok( true );
                 ^${NOCOLOR}
 `

diff --git a/test/cli/samples/validate/_config.js b/test/cli/samples/validate/_config.js
@@ -8,8 +8,8 @@ module.exports = defineTest({
 	stderr: stderr =>
 		assertIncludes(
 			stderr,
-			`(!) Chunk "out.js" is not valid JavaScript: Unterminated block comment.
-out.js (3:20)
+			`(!) out.js (3:20): Chunk "out.js" is not valid JavaScript: Unterminated block comment.
+out.js:3:20
 1: console.log(2 );
 2: 
 3: console.log("end"); /*

diff --git a/test/cli/samples/warn-multiple/_config.js b/test/cli/samples/warn-multiple/_config.js
@@ -1,3 +1,4 @@
+const { sep } = require('node:path');
 const { assertIncludes } = require('../../../utils.js');
 
 module.exports = defineTest({
@@ -31,10 +32,9 @@ module.exports = defineTest({
 				'8: export {url, assert, path};'
 		);
 		assertIncludes(
-			stderr,
-
-			'(!) Module level directives cause errors when bundled, "use stuff" in "main.js" was ignored.\n' +
-				'main.js (1:0)\n' +
+			stderr.replaceAll(__dirname + sep, 'CWD/'),
+			'(!) main.js (1:0): Module level directives cause errors when bundled, "use stuff" in "main.js" was ignored.\n' +
+				'CWD/main.js:1:0\n' +
 				"1: 'use stuff';\n" +
 				'   ^\n' +
 				'2: \n' +

diff --git a/test/cli/samples/warn-plugin-loc/_config.js b/test/cli/samples/warn-plugin-loc/_config.js
@@ -1,17 +1,18 @@
+const { sep } = require('node:path');
 const { assertIncludes } = require('../../../utils.js');
 
 module.exports = defineTest({
 	description: 'correctly adds locations to plugin warnings',
 	command: 'rollup -c',
 	stderr: stderr => {
 		assertIncludes(
-			stderr,
-			'(!) Plugin test: Warning with file and id\n' +
-				'file1: (1:2)\n' +
-				'(!) Plugin test: Warning with file\n' +
-				'file2: (2:3)\n' +
-				'(!) Plugin test: Warning with id\n' +
-				'file-id3: (3:4)\n'
+			stderr.replaceAll(__dirname + sep, 'CWD/'),
+			'(!) [plugin test] file1 (1:2): Warning with file and id\n' +
+				'CWD/file1:1:2\n' +
+				'(!) [plugin test] file2 (2:3): Warning with file\n' +
+				'CWD/file2:2:3\n' +
+				'(!) [plugin test] file-id3 (3:4): Warning with id\n' +
+				'CWD/file-id3:3:4\n'
 		);
 	}
 });
diff --git a/test/cli/samples/warn-plugin/_config.js b/test/cli/samples/warn-plugin/_config.js
@@ -1,20 +1,24 @@
+const { sep } = require('node:path');
 const { assertIncludes } = require('../../../utils.js');
 
 module.exports = defineTest({
 	description: 'displays warnings from plugins',
 	command: 'rollup -c',
 	env: { FORCE_COLOR: undefined, NO_COLOR: true },
 	stderr: stderr => {
-		assertIncludes(stderr, 'Fifth\nother.js\n');
 		assertIncludes(
-			stderr,
-			'(!) Plugin test-plugin: First\n' +
-				'(!) Plugin test-plugin: Second\n' +
+			stderr.replaceAll(__dirname + sep, 'CWD/'),
+			'[plugin second-plugin] other.js: Fifth\n' + 'CWD/other.js\n'
+		);
+		assertIncludes(
+			stderr.replaceAll(__dirname + sep, 'CWD/'),
+			'(!) [plugin test-plugin] First\n' +
+				'(!) [plugin test-plugin] Second\n' +
 				'https://information\n' +
-				'(!) Plugin second-plugin: Third\n' +
-				'other.js\n' +
-				'(!) Plugin second-plugin: Fourth\n' +
-				'other.js: (1:2)\n' +
+				'(!) [plugin second-plugin] other.js: Third\n' +
+				'CWD/other.js\n' +
+				'(!) [plugin second-plugin] other.js (1:2): Fourth\n' +
+				'CWD/other.js:1:2\n' +
 				'custom frame'
 		);
 	}

diff --git a/test/cli/samples/watch/bundle-error/_config.js b/test/cli/samples/watch/bundle-error/_config.js
@@ -16,7 +16,7 @@ module.exports = defineTest({
 		setTimeout(() => unlinkSync(mainFile), 300);
 	},
 	abortOnStderr(data) {
-		if (data.includes('[!] RollupError: Expression expected')) {
+		if (data.includes('[!] RollupError: main.js (1:0): Expression expected')) {
 			setTimeout(() => atomicWriteFileSync(mainFile, 'export default 42;'), 500);
 			return false;
 		}

diff --git a/test/cli/samples/watch/filter-logs/_config.js b/test/cli/samples/watch/filter-logs/_config.js
@@ -19,11 +19,11 @@ module.exports = defineTest({
 			stderr,
 			`
 bundles main.js → _actual.js...
-first
-second
-third
-fourth
-fifth
+[plugin test] first
+[plugin test] second
+[plugin test] third
+[plugin test] fourth
+[plugin test] fifth
 created _actual.js`
 		);
 	}