feat(schema-compiler): Pretty print compile errors grouped by files

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

@@ -5,4 +5,8 @@ export class CompileError extends Error {
   ) {
     super(`Compile errors:\n${messages}`);
   }
+
+  public get plainMessage(): string | undefined {
+    return this.plainMessages;
+  }
 }

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

@@ -1001,7 +1001,7 @@ export class CubeValidator {
     const result = cube.isView ? viewSchema.validate(cube, options) : cubeSchema.validate(cube, options);
 
     if (result.error != null) {
-      errorReporter.error(formatErrorMessage(result.error), result.error);
+      errorReporter.error(formatErrorMessage(result.error));
     } else {
       this.validCubes.set(cube.name, true);
     }

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

@@ -631,7 +631,7 @@ export class DataSchemaCompiler {
     return files.map((file, index) => {
       errorsReport.inFile(file);
       if (!res[index]) { // This should not happen in theory but just to be safe
-        errorsReport.error(`No transpilation result received for the file ${file.fileName}.`);
+        errorsReport.error('No transpilation result received for the file.');
         return undefined;
       }
       errorsReport.addErrors(res[index].errors, file.fileName);
@@ -658,7 +658,7 @@ export class DataSchemaCompiler {
     return files.map((file, index) => {
       errorsReport.inFile(file);
       if (!res[index]) { // This should not happen in theory but just to be safe
-        errorsReport.error(`No transpilation result received for the file ${file.fileName}.`);
+        errorsReport.error('No transpilation result received for the file.');
         return undefined;
       }
       errorsReport.addErrors(res[index].errors, file.fileName);
@@ -737,7 +737,7 @@ export class DataSchemaCompiler {
         const err = e as SyntaxErrorInterface;
         const line = file.content.split('\n')[(err.loc?.start?.line || 1) - 1];
         const spaces = Array(err.loc?.start.column).fill(' ').join('');
-        errorsReport.error(`Syntax error during '${file.fileName}' parsing: ${err.message}:\n${line}\n${spaces}^`);
+        errorsReport.error(`Syntax error during parsing: ${err.message}:\n${line}\n${spaces}^`, file.fileName);
       } else {
         errorsReport.error(e);
       }

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

@@ -20,7 +20,8 @@ export interface CompilerErrorInterface {
 export interface SyntaxErrorInterface {
   message: string;
   plainMessage?: string
-  loc: SourceLocation | null | undefined,
+  loc?: SourceLocation | null | undefined,
+  fileName?: string;
 }
 
 interface File {
@@ -32,6 +33,8 @@ export interface ErrorReporterOptions {
   logger: (msg: string) => void
 }
 
+const NO_FILE_SPECIFIED = '_No-file-specified';
+
 export class ErrorReporter {
   protected warnings: SyntaxErrorInterface[] = [];
 
@@ -56,13 +59,15 @@ export class ErrorReporter {
     this.file = file;
   }
 
-  public warning(e: SyntaxErrorInterface) {
+  public warning(e: SyntaxErrorInterface, fileName?: string) {
+    const targetFileName = fileName || e.fileName || this.file?.fileName;
+
     if (this.file && e.loc) {
       const codeFrame = codeFrameColumns(this.file.content, e.loc, {
         message: e.message,
         highlightCode: true,
       });
-      const plainMessage = `Warning: ${e.message} in ${this.file.fileName}`;
+      const plainMessage = `Warning: ${e.message}`;
       const message = `${plainMessage}\n${codeFrame}`;
 
       if (this.rootReporter().warnings.find(m => (m.message || m) === message)) {
@@ -73,27 +78,41 @@ export class ErrorReporter {
         message,
         plainMessage,
         loc: e.loc,
+        fileName: targetFileName,
       });
 
-      this.options.logger(message);
+      if (targetFileName) {
+        this.options.logger(`${targetFileName}:\n${message}`);
+      } else {
+        this.options.logger(message);
+      }
     } else {
       if (this.rootReporter().warnings.find(m => (m.message || m) === e.message)) {
         return;
       }
 
-      this.rootReporter().warnings.push(e);
+      this.rootReporter().warnings.push({
+        ...e,
+        fileName: targetFileName,
+      });
 
-      this.options.logger(e.message);
+      if (targetFileName) {
+        this.options.logger(`${targetFileName}:\n${e.message}`);
+      } else {
+        this.options.logger(e.message);
+      }
     }
   }
 
-  public syntaxError(e: SyntaxErrorInterface) {
+  public syntaxError(e: SyntaxErrorInterface, fileName?: string) {
+    const targetFileName = fileName || e.fileName || this.file?.fileName;
+
     if (this.file && e.loc) {
       const codeFrame = codeFrameColumns(this.file.content, e.loc, {
         message: e.message,
         highlightCode: true,
       });
-      const plainMessage = `Syntax Error: ${e.message} in ${this.file.fileName}`;
+      const plainMessage = `Syntax Error: ${e.message}`;
       const message = `${plainMessage}\n${codeFrame}`;
 
       if (this.rootReporter().errors.find(m => (m.message || m) === message)) {
@@ -103,44 +122,98 @@ export class ErrorReporter {
       this.rootReporter().errors.push({
         message,
         plainMessage,
+        fileName: targetFileName,
       });
     } else {
       if (this.rootReporter().errors.find(m => (m.message || m) === e.message)) {
         return;
       }
 
-      this.rootReporter().errors.push(e);
+      this.rootReporter().errors.push({
+        ...e,
+        fileName: targetFileName,
+      });
     }
   }
 
   public error(e: any, fileName?: any, lineNumber?: any, position?: any) {
+    const targetFileName = fileName || this.file?.fileName;
     const message = `${this.context.length ? `${this.context.join(' -> ')}: ` : ''}${e.message ? e.message : (e.stack || e)}`;
     if (this.rootReporter().errors.find(m => (m.message || m) === message)) {
       return;
     }
 
-    if (fileName) {
-      this.rootReporter().errors.push({
-        message, fileName, lineNumber, position
-      });
-    } else {
-      this.rootReporter().errors.push({
-        message,
-      });
-    }
+    this.rootReporter().errors.push({
+      message,
+      fileName: targetFileName,
+      lineNumber,
+      position
+    });
   }
 
   public inContext(context: string) {
     return new ErrorReporter(this, this.context.concat(context));
   }
 
+  private groupErrors(): Map<string, CompilerErrorInterface[]> {
+    const { errors } = this.rootReporter();
+
+    const errorsByFile = new Map<string, CompilerErrorInterface[]>();
+
+    for (const error of errors) {
+      const key = error.fileName || NO_FILE_SPECIFIED;
+      if (!errorsByFile.has(key)) {
+        errorsByFile.set(key, []);
+      }
+      errorsByFile.get(key)!.push(error);
+    }
+
+    return errorsByFile;
+  }
+
   public throwIfAny() {
-    if (this.rootReporter().errors.length) {
-      throw new CompileError(
-        this.rootReporter().errors.map((e) => e.message).join('\n'),
-        this.rootReporter().errors.map((e) => e.plainMessage).join('\n')
-      );
+    const { errors } = this.rootReporter();
+
+    if (errors.length === 0) {
+      return;
     }
+
+    const errorsByFile = this.groupErrors();
+
+    // Build formatted report
+    const messageParts: string[] = [];
+    const plainMessageParts: string[] = [];
+
+    const sortedFiles = Array.from(errorsByFile.keys()).sort();
+
+    for (const fileName of sortedFiles) {
+      const fileErrors = errorsByFile.get(fileName)!;
+      const reportFileName = fileName === NO_FILE_SPECIFIED ? '' : `${fileName} `;
+
+      messageParts.push(`${reportFileName}Errors:`);
+
+      const plainMessagesForFile: string[] = [];
+      for (const error of fileErrors) {
+        messageParts.push(error.message);
+        if (error.plainMessage) {
+          plainMessagesForFile.push(error.plainMessage);
+        }
+      }
+
+      if (plainMessagesForFile.length > 0) {
+        plainMessageParts.push(`${reportFileName}Errors:`);
+        plainMessageParts.push(...plainMessagesForFile);
+        plainMessageParts.push('');
+      }
+
+      // Add blank line between file groups
+      messageParts.push('');
+    }
+
+    throw new CompileError(
+      messageParts.join('\n'),
+      plainMessageParts.join('\n')
+    );
   }
 
   public getErrors() {

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

@@ -0,0 +1,132 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`ErrorReporter should deduplicate identical errors and warnings 1`] = `
+Object {
+  "errors": Array [
+    Object {
+      "fileName": "test.js",
+      "message": "Syntax Error: Duplicate error
+> 1 | test content
+    | ^^^ Duplicate error",
+      "plainMessage": "Syntax Error: Duplicate error",
+    },
+  ],
+  "warnings": Array [
+    Object {
+      "fileName": "test.js",
+      "message": "Duplicate warning",
+    },
+  ],
+}
+`;
+
+exports[`ErrorReporter should group and format errors and warnings from different files: grouped-errors-message 1`] = `
+"Compile errors:
+Errors:
+Generic error
+
+config/database.js Errors:
+Connection failed
+
+schema/custom.js Errors:
+Parse error
+
+schema/orders.js Errors:
+Missing required field
+
+schema/products.js Errors:
+Validation error
+
+schema/users.js Errors:
+Syntax Error: Invalid measure definition
+  2 |   sql: \`SELECT * FROM users\`,
+  3 |   measures: {
+> 4 |     count: {
+    |    ^^^^^ Invalid measure definition
+  5 |       type: 'count'
+  6 |     }
+  7 |   }
+"
+`;
+
+exports[`ErrorReporter should group and format errors and warnings from different files: grouped-errors-plain-message 1`] = `
+"schema/users.js Errors:
+Syntax Error: Invalid measure definition
+"
+`;
+
+exports[`ErrorReporter should group and format errors and warnings from different files: warning-logs 1`] = `
+Array [
+  "schema/users.js:
+Warning: Deprecated syntax
+  1 | cube('Users', {
+> 2 |   sql: \`SELECT * FROM users\`,
+    |  ^^^ Deprecated syntax
+  3 |   measures: {
+  4 |     count: {
+  5 |       type: 'count'",
+  "schema/orders.js:
+Consider adding indexes",
+  "schema/analytics.js:
+Performance warning",
+]
+`;
+
+exports[`ErrorReporter should handle addErrors and addWarnings 1`] = `
+Object {
+  "errors": Array [
+    Object {
+      "fileName": "batch.js",
+      "lineNumber": undefined,
+      "message": "Error 1",
+      "position": undefined,
+    },
+    Object {
+      "fileName": "batch.js",
+      "lineNumber": undefined,
+      "message": "Error 2",
+      "position": undefined,
+    },
+    Object {
+      "fileName": "batch.js",
+      "lineNumber": undefined,
+      "message": "Error 3",
+      "position": undefined,
+    },
+  ],
+  "warnings": Array [
+    Object {
+      "fileName": undefined,
+      "message": "Warning 1",
+    },
+    Object {
+      "fileName": undefined,
+      "message": "Warning 2",
+    },
+  ],
+}
+`;
+
+exports[`ErrorReporter should handle errors without fileName at the end 1`] = `
+"Compile errors:
+Errors:
+Generic error without file
+
+fileA.js Errors:
+Error in file A
+
+fileB.js Errors:
+Error in file B
+"
+`;
+
+exports[`ErrorReporter should handle inContext correctly 1`] = `
+Array [
+  Object {
+    "fileName": undefined,
+    "lineNumber": undefined,
+    "message": "Processing Users cube: Test error",
+    "position": undefined,
+  },
+]
+`;