Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add locations to logs, warnings and error messages #5424

Merged
merged 1 commit into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 13 additions & 13 deletions cli/run/batchWarnings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
3 changes: 2 additions & 1 deletion src/utils/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
Expand Down
42 changes: 39 additions & 3 deletions src/utils/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
19 changes: 2 additions & 17 deletions src/utils/options/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions test/cli/samples/filter-logs/_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
`
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
});
15 changes: 8 additions & 7 deletions test/cli/samples/log-side-effects/_config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const { sep } = require('node:path');
const { assertIncludes } = require('../../../utils.js');

module.exports = defineTest({
Expand All @@ -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');
Expand Down
19 changes: 10 additions & 9 deletions test/cli/samples/logs/_config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const assert = require('node:assert');
const { sep } = require('node:path');

const BOLD = '\u001B[1m';
const BLUE = '\u001B[34m';
Expand All @@ -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}
`
Expand Down
4 changes: 2 additions & 2 deletions test/cli/samples/validate/_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"); /*
Expand Down
8 changes: 4 additions & 4 deletions test/cli/samples/warn-multiple/_config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const { sep } = require('node:path');
const { assertIncludes } = require('../../../utils.js');

module.exports = defineTest({
Expand Down Expand Up @@ -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' +
Expand Down
15 changes: 8 additions & 7 deletions test/cli/samples/warn-plugin-loc/_config.js
Original file line number Diff line number Diff line change
@@ -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'
);
}
});
20 changes: 12 additions & 8 deletions test/cli/samples/warn-plugin/_config.js
Original file line number Diff line number Diff line change
@@ -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'
);
}
Expand Down
2 changes: 1 addition & 1 deletion test/cli/samples/watch/bundle-error/_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
10 changes: 5 additions & 5 deletions test/cli/samples/watch/filter-logs/_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`
);
}
Expand Down