Skip to content

Commit 0c5c21c

Browse files
KSDaemonKSDaemon
authored
feat(schema-compiler): Pretty print compile errors grouped by files (#10025)
* Refactor ErrorReporter to pretty print errors * add tests * fix filename passed to errorReporter * update snapshots
1 parent a8e791c commit 0c5c21c

File tree

6 files changed

+445
-27
lines changed

6 files changed

+445
-27
lines changed

‎packages/cubejs-schema-compiler/src/compiler/CompileError.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,8 @@ export class CompileError extends Error {
55
) {
66
super(`Compile errors:\n${messages}`);
77
}
8+
9+
public get plainMessage(): string | undefined {
10+
return this.plainMessages;
11+
}
812
}

‎packages/cubejs-schema-compiler/src/compiler/CubeValidator.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1001,7 +1001,7 @@ export class CubeValidator {
10011001
const result = cube.isView ? viewSchema.validate(cube, options) : cubeSchema.validate(cube, options);
10021002

10031003
if (result.error != null) {
1004-
errorReporter.error(formatErrorMessage(result.error), result.error);
1004+
errorReporter.error(formatErrorMessage(result.error));
10051005
} else {
10061006
this.validCubes.set(cube.name, true);
10071007
}

‎packages/cubejs-schema-compiler/src/compiler/DataSchemaCompiler.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ export class DataSchemaCompiler {
631631
return files.map((file, index) => {
632632
errorsReport.inFile(file);
633633
if (!res[index]) { // This should not happen in theory but just to be safe
634-
errorsReport.error(`No transpilation result received for the file ${file.fileName}.`);
634+
errorsReport.error('No transpilation result received for the file.');
635635
return undefined;
636636
}
637637
errorsReport.addErrors(res[index].errors, file.fileName);
@@ -658,7 +658,7 @@ export class DataSchemaCompiler {
658658
return files.map((file, index) => {
659659
errorsReport.inFile(file);
660660
if (!res[index]) { // This should not happen in theory but just to be safe
661-
errorsReport.error(`No transpilation result received for the file ${file.fileName}.`);
661+
errorsReport.error('No transpilation result received for the file.');
662662
return undefined;
663663
}
664664
errorsReport.addErrors(res[index].errors, file.fileName);
@@ -737,7 +737,7 @@ export class DataSchemaCompiler {
737737
const err = e as SyntaxErrorInterface;
738738
const line = file.content.split('\n')[(err.loc?.start?.line || 1) - 1];
739739
const spaces = Array(err.loc?.start.column).fill(' ').join('');
740-
errorsReport.error(`Syntax error during '${file.fileName}' parsing: ${err.message}:\n${line}\n${spaces}^`);
740+
errorsReport.error(`Syntax error during parsing: ${err.message}:\n${line}\n${spaces}^`, file.fileName);
741741
} else {
742742
errorsReport.error(e);
743743
}

‎packages/cubejs-schema-compiler/src/compiler/ErrorReporter.ts‎

Lines changed: 96 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ export interface CompilerErrorInterface {
2020
export interface SyntaxErrorInterface {
2121
message: string;
2222
plainMessage?: string
23-
loc: SourceLocation | null | undefined,
23+
loc?: SourceLocation | null | undefined,
24+
fileName?: string;
2425
}
2526

2627
interface File {
@@ -32,6 +33,8 @@ export interface ErrorReporterOptions {
3233
logger: (msg: string) => void
3334
}
3435

36+
const NO_FILE_SPECIFIED = '_No-file-specified';
37+
3538
export class ErrorReporter {
3639
protected warnings: SyntaxErrorInterface[] = [];
3740

@@ -56,13 +59,15 @@ export class ErrorReporter {
5659
this.file = file;
5760
}
5861

59-
public warning(e: SyntaxErrorInterface) {
62+
public warning(e: SyntaxErrorInterface, fileName?: string) {
63+
const targetFileName = fileName || e.fileName || this.file?.fileName;
64+
6065
if (this.file && e.loc) {
6166
const codeFrame = codeFrameColumns(this.file.content, e.loc, {
6267
message: e.message,
6368
highlightCode: true,
6469
});
65-
const plainMessage = `Warning: ${e.message} in ${this.file.fileName}`;
70+
const plainMessage = `Warning: ${e.message}`;
6671
const message = `${plainMessage}\n${codeFrame}`;
6772

6873
if (this.rootReporter().warnings.find(m => (m.message || m) === message)) {
@@ -73,27 +78,41 @@ export class ErrorReporter {
7378
message,
7479
plainMessage,
7580
loc: e.loc,
81+
fileName: targetFileName,
7682
});
7783

78-
this.options.logger(message);
84+
if (targetFileName) {
85+
this.options.logger(`${targetFileName}:\n${message}`);
86+
} else {
87+
this.options.logger(message);
88+
}
7989
} else {
8090
if (this.rootReporter().warnings.find(m => (m.message || m) === e.message)) {
8191
return;
8292
}
8393

84-
this.rootReporter().warnings.push(e);
94+
this.rootReporter().warnings.push({
95+
...e,
96+
fileName: targetFileName,
97+
});
8598

86-
this.options.logger(e.message);
99+
if (targetFileName) {
100+
this.options.logger(`${targetFileName}:\n${e.message}`);
101+
} else {
102+
this.options.logger(e.message);
103+
}
87104
}
88105
}
89106

90-
public syntaxError(e: SyntaxErrorInterface) {
107+
public syntaxError(e: SyntaxErrorInterface, fileName?: string) {
108+
const targetFileName = fileName || e.fileName || this.file?.fileName;
109+
91110
if (this.file && e.loc) {
92111
const codeFrame = codeFrameColumns(this.file.content, e.loc, {
93112
message: e.message,
94113
highlightCode: true,
95114
});
96-
const plainMessage = `Syntax Error: ${e.message} in ${this.file.fileName}`;
115+
const plainMessage = `Syntax Error: ${e.message}`;
97116
const message = `${plainMessage}\n${codeFrame}`;
98117

99118
if (this.rootReporter().errors.find(m => (m.message || m) === message)) {
@@ -103,44 +122,98 @@ export class ErrorReporter {
103122
this.rootReporter().errors.push({
104123
message,
105124
plainMessage,
125+
fileName: targetFileName,
106126
});
107127
} else {
108128
if (this.rootReporter().errors.find(m => (m.message || m) === e.message)) {
109129
return;
110130
}
111131

112-
this.rootReporter().errors.push(e);
132+
this.rootReporter().errors.push({
133+
...e,
134+
fileName: targetFileName,
135+
});
113136
}
114137
}
115138

116139
public error(e: any, fileName?: any, lineNumber?: any, position?: any) {
140+
const targetFileName = fileName || this.file?.fileName;
117141
const message = `${this.context.length ? `${this.context.join(' -> ')}: ` : ''}${e.message ? e.message : (e.stack || e)}`;
118142
if (this.rootReporter().errors.find(m => (m.message || m) === message)) {
119143
return;
120144
}
121145

122-
if (fileName) {
123-
this.rootReporter().errors.push({
124-
message, fileName, lineNumber, position
125-
});
126-
} else {
127-
this.rootReporter().errors.push({
128-
message,
129-
});
130-
}
146+
this.rootReporter().errors.push({
147+
message,
148+
fileName: targetFileName,
149+
lineNumber,
150+
position
151+
});
131152
}
132153

133154
public inContext(context: string) {
134155
return new ErrorReporter(this, this.context.concat(context));
135156
}
136157

158+
private groupErrors(): Map<string, CompilerErrorInterface[]> {
159+
const { errors } = this.rootReporter();
160+
161+
const errorsByFile = new Map<string, CompilerErrorInterface[]>();
162+
163+
for (const error of errors) {
164+
const key = error.fileName || NO_FILE_SPECIFIED;
165+
if (!errorsByFile.has(key)) {
166+
errorsByFile.set(key, []);
167+
}
168+
errorsByFile.get(key)!.push(error);
169+
}
170+
171+
return errorsByFile;
172+
}
173+
137174
public throwIfAny() {
138-
if (this.rootReporter().errors.length) {
139-
throw new CompileError(
140-
this.rootReporter().errors.map((e) => e.message).join('\n'),
141-
this.rootReporter().errors.map((e) => e.plainMessage).join('\n')
142-
);
175+
const { errors } = this.rootReporter();
176+
177+
if (errors.length === 0) {
178+
return;
143179
}
180+
181+
const errorsByFile = this.groupErrors();
182+
183+
// Build formatted report
184+
const messageParts: string[] = [];
185+
const plainMessageParts: string[] = [];
186+
187+
const sortedFiles = Array.from(errorsByFile.keys()).sort();
188+
189+
for (const fileName of sortedFiles) {
190+
const fileErrors = errorsByFile.get(fileName)!;
191+
const reportFileName = fileName === NO_FILE_SPECIFIED ? '' : `${fileName} `;
192+
193+
messageParts.push(`${reportFileName}Errors:`);
194+
195+
const plainMessagesForFile: string[] = [];
196+
for (const error of fileErrors) {
197+
messageParts.push(error.message);
198+
if (error.plainMessage) {
199+
plainMessagesForFile.push(error.plainMessage);
200+
}
201+
}
202+
203+
if (plainMessagesForFile.length > 0) {
204+
plainMessageParts.push(`${reportFileName}Errors:`);
205+
plainMessageParts.push(...plainMessagesForFile);
206+
plainMessageParts.push('');
207+
}
208+
209+
// Add blank line between file groups
210+
messageParts.push('');
211+
}
212+
213+
throw new CompileError(
214+
messageParts.join('\n'),
215+
plainMessageParts.join('\n')
216+
);
144217
}
145218

146219
public getErrors() {

‎packages/cubejs-schema-compiler/test/unit/__snapshots__/error-reporter.test.ts.snap‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`ErrorReporter should deduplicate identical errors and warnings 1`] = `
4+
Object {
5+
"errors": Array [
6+
Object {
7+
"fileName": "test.js",
8+
"message": "Syntax Error: Duplicate error
9+
> 1 | test content
10+
 | ^^^ Duplicate error",
11+
"plainMessage": "Syntax Error: Duplicate error",
12+
},
13+
],
14+
"warnings": Array [
15+
Object {
16+
"fileName": "test.js",
17+
"message": "Duplicate warning",
18+
},
19+
],
20+
}
21+
`;
22+
23+
exports[`ErrorReporter should group and format errors and warnings from different files: grouped-errors-message 1`] = `
24+
"Compile errors:
25+
Errors:
26+
Generic error
27+
28+
config/database.js Errors:
29+
Connection failed
30+
31+
schema/custom.js Errors:
32+
Parse error
33+
34+
schema/orders.js Errors:
35+
Missing required field
36+
37+
schema/products.js Errors:
38+
Validation error
39+
40+
schema/users.js Errors:
41+
Syntax Error: Invalid measure definition
42+
  2 | sql: \`SELECT * FROM users\`,
43+
 3 | measures: {
44+
> 4 | count: {
45+
 | ^^^^^ Invalid measure definition
46+
 5 | type: 'count'
47+
 6 | }
48+
 7 | }
49+
"
50+
`;
51+
52+
exports[`ErrorReporter should group and format errors and warnings from different files: grouped-errors-plain-message 1`] = `
53+
"schema/users.js Errors:
54+
Syntax Error: Invalid measure definition
55+
"
56+
`;
57+
58+
exports[`ErrorReporter should group and format errors and warnings from different files: warning-logs 1`] = `
59+
Array [
60+
"schema/users.js:
61+
Warning: Deprecated syntax
62+
  1 | cube('Users', {
63+
> 2 | sql: \`SELECT * FROM users\`,
64+
 | ^^^ Deprecated syntax
65+
 3 | measures: {
66+
 4 | count: {
67+
 5 | type: 'count'",
68+
"schema/orders.js:
69+
Consider adding indexes",
70+
"schema/analytics.js:
71+
Performance warning",
72+
]
73+
`;
74+
75+
exports[`ErrorReporter should handle addErrors and addWarnings 1`] = `
76+
Object {
77+
"errors": Array [
78+
Object {
79+
"fileName": "batch.js",
80+
"lineNumber": undefined,
81+
"message": "Error 1",
82+
"position": undefined,
83+
},
84+
Object {
85+
"fileName": "batch.js",
86+
"lineNumber": undefined,
87+
"message": "Error 2",
88+
"position": undefined,
89+
},
90+
Object {
91+
"fileName": "batch.js",
92+
"lineNumber": undefined,
93+
"message": "Error 3",
94+
"position": undefined,
95+
},
96+
],
97+
"warnings": Array [
98+
Object {
99+
"fileName": undefined,
100+
"message": "Warning 1",
101+
},
102+
Object {
103+
"fileName": undefined,
104+
"message": "Warning 2",
105+
},
106+
],
107+
}
108+
`;
109+
110+
exports[`ErrorReporter should handle errors without fileName at the end 1`] = `
111+
"Compile errors:
112+
Errors:
113+
Generic error without file
114+
115+
fileA.js Errors:
116+
Error in file A
117+
118+
fileB.js Errors:
119+
Error in file B
120+
"
121+
`;
122+
123+
exports[`ErrorReporter should handle inContext correctly 1`] = `
124+
Array [
125+
Object {
126+
"fileName": undefined,
127+
"lineNumber": undefined,
128+
"message": "Processing Users cube: Test error",
129+
"position": undefined,
130+
},
131+
]
132+
`;

0 commit comments

Comments
 (0)