Keep diagnostic data structured in DiagnosticError (#358)

packages/adl/compiler/binder.ts

@@ -1,4 +1,4 @@
-import { DiagnosticError, formatDiagnostic } from "./diagnostics.js";
+import { createDiagnostic, Diagnostic, DiagnosticError, formatDiagnostic } from "./diagnostics.js";
 import { visitChildren } from "./parser.js";
 import { ADLSourceFile, Program } from "./program.js";
 import {
@@ -157,7 +157,7 @@ export function createBinder(): Binder {
 
   function reportDuplicateSymbols(symbols: SymbolTable) {
     let reported = new Set<Sym>();
-    let messages = new Array<string>();
+    let diagnostics = new Array<Diagnostic>();
 
     for (const symbol of currentFile.symbols.duplicates) {
       report(symbol);
@@ -174,7 +174,7 @@ export function createBinder(): Binder {
       }
     }
 
-    if (messages.length > 0) {
+    if (diagnostics.length > 0) {
       // TODO: We're now reporting all duplicates up to the binding of the first file
       // that introduced one, but still bailing the compilation rather than
       // recovering and reporting other issues including the possibility of more
@@ -183,15 +183,14 @@ export function createBinder(): Binder {
       // That said, decorators are entered into the global symbol table before
       // any source file is bound and therefore this will include all duplicate
       // decorator implementations.
-
-      throw new DiagnosticError(messages.join("\n"));
+      throw new DiagnosticError(diagnostics);
     }
 
     function report(symbol: Sym) {
       if (!reported.has(symbol)) {
         reported.add(symbol);
-        const message = formatDiagnostic("Duplicate name: " + symbol.name, symbol);
-        messages.push(message);
+        const diagnostic = createDiagnostic("Duplicate name: " + symbol.name, symbol);
+        diagnostics.push(diagnostic);
       }
     }
   }

packages/adl/compiler/cli.ts

@@ -8,7 +8,7 @@ import { readFileSync } from "fs";
 import { compile } from "../compiler/program.js";
 import { spawnSync } from "child_process";
 import { CompilerOptions } from "../compiler/options.js";
-import { DiagnosticError, dumpError } from "./diagnostics.js";
+import { DiagnosticError, dumpError, logDiagnostics } from "./diagnostics.js";
 
 const adlVersion = getVersion();
 
@@ -77,7 +77,7 @@ async function compileInput(compilerOptions: CompilerOptions): Promise<boolean>
     return true;
   } catch (err) {
     if (err instanceof DiagnosticError) {
-      console.error(err.message);
+      logDiagnostics(err.diagnostics, console.error);
       if (args.debug) {
         console.error(`Stack trace:\n\n${err.stack}`);
       }

packages/adl/compiler/diagnostics.ts

@@ -1,30 +1,37 @@
 import { createSourceFile } from "./scanner.js";
 import { Message, Node, SourceLocation, SyntaxKind, Type, Sym } from "./types.js";
 
+export interface Diagnostic extends SourceLocation {
+  readonly message: string;
+  readonly code?: number;
+  readonly severity: "warning" | "error";
+}
+
 /**
  * Represents an error in the code input that is fatal and bails the compilation.
  *
  * This isn't meant to be kept long term, but we currently do this on all errors.
  */
 export class DiagnosticError extends Error {
-  constructor(message: string) {
-    super(message);
+  constructor(public readonly diagnostics: readonly Diagnostic[]) {
+    super("Code diagnostics. See diagnostics array.");
   }
 }
 
 /**
  * Represents a failure with multiple errors.
  */
-export class ChainedError extends Error {
-  readonly innerErrors: readonly Error[];
+export class AggregateError extends Error {
+  readonly errors: readonly Error[];
 
-  constructor(message: string, ...innerErrors: (Error | undefined)[]) {
-    super(message);
-    this.innerErrors = innerErrors.filter(isNotUndefined);
+  constructor(...errors: (Error | undefined)[]) {
+    super("Multiple errors. See errors array.");
+    this.errors = errors.filter(isNotUndefined);
   }
 }
 
 export type DiagnosticTarget = Node | Type | Sym | SourceLocation;
+export type WriteLine = (text?: string) => void;
 
 export type ErrorHandler = (
   message: Message | string,
@@ -39,18 +46,14 @@ export function throwDiagnostic(
   target: DiagnosticTarget,
   ...args: Array<string | number>
 ): never {
-  throw new DiagnosticError(formatDiagnostic(message, target, ...args));
+  throw new DiagnosticError([createDiagnostic(message, target, ...args)]);
 }
 
-/**
- * Format a diagnostic into <file>:<line> - ADL<code> <category>: <text>.
- * Take extra care to preserve all info in thrown Error if this fails.
- */
-export function formatDiagnostic(
+export function createDiagnostic(
   message: Message | string,
   target: DiagnosticTarget,
   ...args: Array<string | number>
-) {
+): Diagnostic {
   let location: SourceLocation;
   let locationError: Error | undefined;
 
@@ -67,23 +70,40 @@ export function formatDiagnostic(
 
   if (typeof message === "string") {
     // Temporarily allow ad-hoc strings as error messages.
-    message = { code: -1, text: message, category: "error" };
+    message = { text: message, severity: "error" };
   }
 
-  const [msg, formatError] = format(message.text, ...args);
-  const code = message.code < 0 ? "" : ` ADL${message.code}`;
-  const pos = location.file.getLineAndCharacterOfPosition(location.pos);
-  const diagnostic = `${location.file.path}:${pos.line + 1}:${pos.character + 1} - ${
-    message.category
-  }${code}: ${msg}`;
+  const [formattedMessage, formatError] = format(message.text, ...args);
+  const diagnostic = {
+    code: message.code,
+    severity: message.severity,
+    ...location,
+    message: formattedMessage,
+  };
 
   if (locationError || formatError) {
-    throw new ChainedError(diagnostic, locationError, formatError);
+    throw new AggregateError(new DiagnosticError([diagnostic]), locationError, formatError);
   }
 
   return diagnostic;
 }
 
+export function logDiagnostics(diagnostics: readonly Diagnostic[], writeLine: WriteLine) {
+  for (const diagnostic of diagnostics) {
+    writeLine(formatDiagnostic(diagnostic));
+  }
+}
+
+export function formatDiagnostic(diagnostic: Diagnostic) {
+  const code = diagnostic.code ? ` ADL${diagnostic.code}` : "";
+  const pos = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.pos);
+  const line = pos.line + 1;
+  const col = pos.character + 1;
+  const severity = diagnostic.severity;
+  const path = diagnostic.file.path;
+  return `${path}:${line}:${col} - ${severity}${code}: ${diagnostic.message}`;
+}
+
 export function getSourceLocation(target: DiagnosticTarget): SourceLocation {
   if ("file" in target) {
     return target;
@@ -119,14 +139,17 @@ function getSourceLocationOfNode(node: Node): SourceLocation {
   };
 }
 
-export function dumpError(error: Error, writeLine: (s?: string) => void) {
-  writeLine("");
-  writeLine(error.stack);
-
-  if (error instanceof ChainedError) {
-    for (const inner of error.innerErrors) {
+export function dumpError(error: Error, writeLine: WriteLine) {
+  if (error instanceof DiagnosticError) {
+    logDiagnostics(error.diagnostics, writeLine);
+    writeLine(error.stack);
+  } else if (error instanceof AggregateError) {
+    for (const inner of error.errors) {
       dumpError(inner, writeLine);
     }
+  } else {
+    writeLine("");
+    writeLine(error.stack);
   }
 }
 

packages/adl/compiler/messages.ts

@@ -2,30 +2,30 @@ import { Message } from "./types";
 
 export const messages: { [key: string]: Message } = {
   // Scanner errors
-  DigitExpected: { code: 1100, category: "error", text: "Digit expected (0-9)" },
-  HexDigitExpected: { code: 1101, category: "error", text: "Hex Digit expected (0-F,0-f)" },
-  BinaryDigitExpected: { code: 1102, category: "error", text: "Binary Digit expected (0,1)" },
+  DigitExpected: { code: 1100, severity: "error", text: "Digit expected (0-9)" },
+  HexDigitExpected: { code: 1101, severity: "error", text: "Hex Digit expected (0-F,0-f)" },
+  BinaryDigitExpected: { code: 1102, severity: "error", text: "Binary Digit expected (0,1)" },
   UnexpectedEndOfFile: {
     code: 1103,
-    category: "error",
+    severity: "error",
     text: "Unexpected end of file while searching for '{0}'",
   },
-  InvalidEscapeSequence: { code: 1104, category: "error", text: "Invalid escape sequence" },
+  InvalidEscapeSequence: { code: 1104, severity: "error", text: "Invalid escape sequence" },
   NoNewLineAtStartOfTripleQuotedString: {
     code: 1105,
-    category: "error",
+    severity: "error",
     text: "String content in triple quotes must begin on a new line",
   },
   NoNewLineAtEndOfTripleQuotedString: {
     code: 1106,
-    category: "error",
+    severity: "error",
     text: "Closing triple quotes must begin on a new line",
   },
   InconsistentTripleQuoteIndentation: {
     code: 1107,
-    category: "error",
+    severity: "error",
     text:
       "All lines in triple-quoted string lines must have the same indentation as closing triple quotes",
   },
-  UnexpectedToken: { code: 1108, category: "error", text: "Unexpected token: '{0}'" },
+  UnexpectedToken: { code: 1108, severity: "error", text: "Unexpected token: '{0}'" },
 };

packages/adl/compiler/types.ts

@@ -404,7 +404,7 @@ export interface SourceLocation extends TextRange {
 }
 
 export interface Message {
-  code: number;
+  code?: number;
   text: string;
-  category: "error" | "warning";
+  severity: "error" | "warning";
 }