refactor: improve tsc diagnostics (#7420)

cli/dts/lib.deno.unstable.d.ts

@@ -188,12 +188,10 @@ declare namespace Deno {
 
   /** The log category for a diagnostic message. */
   export enum DiagnosticCategory {
-    Log = 0,
+    Warning = 0,
-    Debug = 1,
+    Error = 1,
-    Info = 2,
+    Suggestion = 2,
-    Error = 3,
+    Message = 3,
-    Warning = 4,
-    Suggestion = 5,
   }
 
   export interface DiagnosticMessageChain {
@@ -203,37 +201,33 @@ declare namespace Deno {
     next?: DiagnosticMessageChain[];
   }
 
-  export interface DiagnosticItem {
+  export interface Diagnostic {
     /** A string message summarizing the diagnostic. */
-    message: string;
+    messageText?: string;
     /** An ordered array of further diagnostics. */
     messageChain?: DiagnosticMessageChain;
     /** Information related to the diagnostic. This is present when there is a
      * suggestion or other additional diagnostic information */
-    relatedInformation?: DiagnosticItem[];
+    relatedInformation?: Diagnostic[];
     /** The text of the source line related to the diagnostic. */
     sourceLine?: string;
-    /** The line number that is related to the diagnostic. */
+    source?: string;
-    lineNumber?: number;
+    /** The start position of the error. Zero based index. */
-    /** The name of the script resource related to the diagnostic. */
+    start?: {
-    scriptResourceName?: string;
+      line: number;
-    /** The start position related to the diagnostic. */
+      character: number;
-    startPosition?: number;
+    };
-    /** The end position related to the diagnostic. */
+    /** The end position of the error.  Zero based index. */
-    endPosition?: number;
+    end?: {
+      line: number;
+      character: number;
+    };
+    /** The filename of the resource related to the diagnostic message. */
+    fileName?: string;
     /** The category of the diagnostic. */
     category: DiagnosticCategory;
     /** A number identifier. */
     code: number;
-    /** The the start column of the sourceLine related to the diagnostic. */
-    startColumn?: number;
-    /** The end column of the sourceLine related to the diagnostic. */
-    endColumn?: number;
-  }
-
-  export interface Diagnostic {
-    /** An array of diagnostic items. */
-    items: DiagnosticItem[];
   }
 
   /** **UNSTABLE**: new API, yet to be vetted.
@@ -247,9 +241,9 @@ declare namespace Deno {
    * console.log(Deno.formatDiagnostics(diagnostics));  // User friendly output of diagnostics
    * ```
    *
-   * @param items An array of diagnostic items to format
+   * @param diagnostics An array of diagnostic items to format
    */
-  export function formatDiagnostics(items: DiagnosticItem[]): string;
+  export function formatDiagnostics(diagnostics: Diagnostic[]): string;
 
   /** **UNSTABLE**: new API, yet to be vetted.
    *
@@ -530,7 +524,7 @@ declare namespace Deno {
     rootName: string,
     sources?: Record<string, string>,
     options?: CompilerOptions,
-  ): Promise<[DiagnosticItem[] | undefined, Record<string, string>]>;
+  ): Promise<[Diagnostic[] | undefined, Record<string, string>]>;
 
   /** **UNSTABLE**: new API, yet to be vetted.
    *
@@ -573,7 +567,7 @@ declare namespace Deno {
     rootName: string,
     sources?: Record<string, string>,
     options?: CompilerOptions,
-  ): Promise<[DiagnosticItem[] | undefined, string]>;
+  ): Promise<[Diagnostic[] | undefined, string]>;
 
   /** **UNSTABLE**: Should not have same name as `window.location` type. */
   interface Location {

cli/ops/errors.rs

@@ -1,6 +1,6 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 
-use crate::diagnostics::Diagnostic;
+use crate::diagnostics::Diagnostics;
 use crate::source_maps::get_orig_position;
 use crate::source_maps::CachedMaps;
 use deno_core::ErrBox;
@@ -52,6 +52,6 @@ fn op_format_diagnostic(
   args: Value,
   _zero_copy: &mut [ZeroCopyBuf],
 ) -> Result<Value, ErrBox> {
-  let diagnostic = serde_json::from_value::<Diagnostic>(args)?;
+  let diagnostic: Diagnostics = serde_json::from_value(args)?;
   Ok(json!(diagnostic.to_string()))
 }

cli/rt/40_diagnostics.js

@@ -6,19 +6,15 @@
 
 ((window) => {
   const DiagnosticCategory = {
-    0: "Log",
+    0: "Warning",
-    1: "Debug",
+    1: "Error",
-    2: "Info",
+    2: "Suggestion",
-    3: "Error",
+    3: "Message",
-    4: "Warning",
-    5: "Suggestion",
 
-    Log: 0,
+    Warning: 0,
-    Debug: 1,
+    Error: 1,
-    Info: 2,
+    Suggestion: 2,
-    Error: 3,
+    Message: 3,
-    Warning: 4,
-    Suggestion: 5,
   };
 
   window.__bootstrap.diagnostics = {

cli/rt/40_error_stack.js

@@ -8,8 +8,8 @@
   const internals = window.__bootstrap.internals;
   const dispatchJson = window.__bootstrap.dispatchJson;
 
-  function opFormatDiagnostics(items) {
+  function opFormatDiagnostics(diagnostics) {
-    return dispatchJson.sendSync("op_format_diagnostic", { items });
+    return dispatchJson.sendSync("op_format_diagnostic", diagnostics);
   }
 
   function opApplySourceMap(location) {

cli/tests/unit/format_error_test.ts

@@ -2,27 +2,33 @@
 import { assert, unitTest } from "./test_util.ts";
 
 unitTest(function formatDiagnosticBasic() {
-  const fixture: Deno.DiagnosticItem[] = [
+  const fixture: Deno.Diagnostic[] = [
     {
-      message: "Example error",
+      start: {
-      category: Deno.DiagnosticCategory.Error,
+        line: 0,
-      sourceLine: "abcdefghijklmnopqrstuv",
+        character: 0,
-      lineNumber: 1000,
+      },
-      scriptResourceName: "foo.ts",
+      end: {
-      startColumn: 1,
+        line: 0,
-      endColumn: 2,
+        character: 7,
-      code: 4000,
+      },
+      fileName: "test.ts",
+      messageText:
+        "Cannot find name 'console'. Do you need to change your target library? Try changing the `lib` compiler option to include 'dom'.",
+      sourceLine: `console.log("a");`,
+      category: 1,
+      code: 2584,
     },
   ];
   const out = Deno.formatDiagnostics(fixture);
-  assert(out.includes("Example error"));
+  assert(out.includes("Cannot find name"));
-  assert(out.includes("foo.ts"));
+  assert(out.includes("test.ts"));
 });
 
 unitTest(function formatDiagnosticError() {
   let thrown = false;
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  const bad = ([{ hello: 123 }] as any) as Deno.DiagnosticItem[];
+  const bad = ([{ hello: 123 }] as any) as Deno.Diagnostic[];
   try {
     Deno.formatDiagnostics(bad);
   } catch (e) {

cli/tsc.rs

@@ -1,8 +1,7 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 
 use crate::colors;
-use crate::diagnostics::Diagnostic;
+use crate::diagnostics::Diagnostics;
-use crate::diagnostics::DiagnosticItem;
 use crate::disk_cache::DiskCache;
 use crate::file_fetcher::SourceFile;
 use crate::file_fetcher::SourceFileFetcher;
@@ -396,7 +395,7 @@ struct EmittedSource {
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct BundleResponse {
-  diagnostics: Diagnostic,
+  diagnostics: Diagnostics,
   bundle_output: Option<String>,
   stats: Option<Vec<Stat>>,
 }
@@ -404,7 +403,7 @@ struct BundleResponse {
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct CompileResponse {
-  diagnostics: Diagnostic,
+  diagnostics: Diagnostics,
   emit_map: HashMap<String, EmittedSource>,
   build_info: Option<String>,
   stats: Option<Vec<Stat>>,
@@ -425,14 +424,14 @@ struct TranspileTsOptions {
 #[serde(rename_all = "camelCase")]
 #[allow(unused)]
 struct RuntimeBundleResponse {
-  diagnostics: Vec<DiagnosticItem>,
+  diagnostics: Diagnostics,
   output: String,
 }
 
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct RuntimeCompileResponse {
-  diagnostics: Vec<DiagnosticItem>,
+  diagnostics: Diagnostics,
   emit_map: HashMap<String, EmittedSource>,
 }
 
@@ -647,7 +646,7 @@ impl TsCompiler {
 
     let compile_response: CompileResponse = serde_json::from_str(&json_str)?;
 
-    if !compile_response.diagnostics.items.is_empty() {
+    if !compile_response.diagnostics.0.is_empty() {
       return Err(ErrBox::error(compile_response.diagnostics.to_string()));
     }
 
@@ -769,7 +768,7 @@ impl TsCompiler {
 
     maybe_log_stats(bundle_response.stats);
 
-    if !bundle_response.diagnostics.items.is_empty() {
+    if !bundle_response.diagnostics.0.is_empty() {
       return Err(ErrBox::error(bundle_response.diagnostics.to_string()));
     }
 
@@ -1287,7 +1286,7 @@ pub async fn runtime_compile(
 
   let response: RuntimeCompileResponse = serde_json::from_str(&json_str)?;
 
-  if response.diagnostics.is_empty() && sources.is_none() {
+  if response.diagnostics.0.is_empty() && sources.is_none() {
     compiler.cache_emitted_files(response.emit_map)?;
   }
 

cli/tsc/99_main_compiler.js

@@ -24,262 +24,62 @@ delete Object.prototype.__proto__;
   const errorStack = window.__bootstrap.errorStack;
   const errors = window.__bootstrap.errors.errors;
 
-  function opNow() {
+  /**
-    const res = dispatchJson.sendSync("op_now");
+   * @param {import("../dts/typescript").DiagnosticRelatedInformation} diagnostic
-    return res.seconds * 1e3 + res.subsecNanos / 1e6;
+   */
-  }
+  function fromRelatedInformation({
-
+    start,
-  const DiagnosticCategory = {
+    length,
-    0: "Log",
+    file,
-    1: "Debug",
+    messageText: msgText,
-    2: "Info",
+    ...ri
-    3: "Error",
+  }) {
-    4: "Warning",
+    let messageText;
-    5: "Suggestion",
+    let messageChain;
-
+    if (typeof msgText === "object") {
-    Log: 0,
+      messageChain = msgText;
-    Debug: 1,
+    } else {
-    Info: 2,
+      messageText = msgText;
-    Error: 3,
-    Warning: 4,
-    Suggestion: 5,
-  };
-
-  const unstableDenoGlobalProperties = [
-    "CompilerOptions",
-    "DatagramConn",
-    "Diagnostic",
-    "DiagnosticCategory",
-    "DiagnosticItem",
-    "DiagnosticMessageChain",
-    "EnvPermissionDescriptor",
-    "HrtimePermissionDescriptor",
-    "HttpClient",
-    "LinuxSignal",
-    "Location",
-    "MacOSSignal",
-    "NetPermissionDescriptor",
-    "PermissionDescriptor",
-    "PermissionName",
-    "PermissionState",
-    "PermissionStatus",
-    "Permissions",
-    "PluginPermissionDescriptor",
-    "ReadPermissionDescriptor",
-    "RunPermissionDescriptor",
-    "ShutdownMode",
-    "Signal",
-    "SignalStream",
-    "StartTlsOptions",
-    "SymlinkOptions",
-    "TranspileOnlyResult",
-    "UnixConnectOptions",
-    "UnixListenOptions",
-    "WritePermissionDescriptor",
-    "applySourceMap",
-    "bundle",
-    "compile",
-    "connect",
-    "consoleSize",
-    "createHttpClient",
-    "fdatasync",
-    "fdatasyncSync",
-    "formatDiagnostics",
-    "futime",
-    "futimeSync",
-    "fstat",
-    "fstatSync",
-    "fsync",
-    "fsyncSync",
-    "ftruncate",
-    "ftruncateSync",
-    "hostname",
-    "kill",
-    "link",
-    "linkSync",
-    "listen",
-    "listenDatagram",
-    "loadavg",
-    "mainModule",
-    "openPlugin",
-    "osRelease",
-    "permissions",
-    "ppid",
-    "setRaw",
-    "shutdown",
-    "signal",
-    "signals",
-    "startTls",
-    "symlink",
-    "symlinkSync",
-    "transpileOnly",
-    "umask",
-    "utime",
-    "utimeSync",
-  ];
-
-  function transformMessageText(messageText, code) {
-    switch (code) {
-      case 2339: {
-        const property = messageText
-          .replace(/^Property '/, "")
-          .replace(/' does not exist on type 'typeof Deno'\./, "");
-
-        if (
-          messageText.endsWith("on type 'typeof Deno'.") &&
-          unstableDenoGlobalProperties.includes(property)
-        ) {
-          return `${messageText} 'Deno.${property}' is an unstable API. Did you forget to run with the '--unstable' flag?`;
-        }
-        break;
-      }
-      case 2551: {
-        const suggestionMessagePattern = / Did you mean '(.+)'\?$/;
-        const property = messageText
-          .replace(/^Property '/, "")
-          .replace(/' does not exist on type 'typeof Deno'\./, "")
-          .replace(suggestionMessagePattern, "");
-        const suggestion = messageText.match(suggestionMessagePattern);
-        const replacedMessageText = messageText.replace(
-          suggestionMessagePattern,
-          "",
-        );
-        if (suggestion && unstableDenoGlobalProperties.includes(property)) {
-          const suggestedProperty = suggestion[1];
-          return `${replacedMessageText} 'Deno.${property}' is an unstable API. Did you forget to run with the '--unstable' flag, or did you mean '${suggestedProperty}'?`;
-        }
-        break;
-      }
     }
-
+    if (start !== undefined && length !== undefined && file) {
-    return messageText;
+      const startPos = file.getLineAndCharacterOfPosition(start);
-  }
+      const sourceLine = file.getFullText().split("\n")[startPos.line];
-
+      const fileName = file.fileName;
-  function fromDiagnosticCategory(category) {
-    switch (category) {
-      case ts.DiagnosticCategory.Error:
-        return DiagnosticCategory.Error;
-      case ts.DiagnosticCategory.Message:
-        return DiagnosticCategory.Info;
-      case ts.DiagnosticCategory.Suggestion:
-        return DiagnosticCategory.Suggestion;
-      case ts.DiagnosticCategory.Warning:
-        return DiagnosticCategory.Warning;
-      default:
-        throw new Error(
-          `Unexpected DiagnosticCategory: "${category}"/"${
-            ts.DiagnosticCategory[category]
-          }"`,
-        );
-    }
-  }
-
-  function getSourceInformation(sourceFile, start, length) {
-    const scriptResourceName = sourceFile.fileName;
-    const {
-      line: lineNumber,
-      character: startColumn,
-    } = sourceFile.getLineAndCharacterOfPosition(start);
-    const endPosition = sourceFile.getLineAndCharacterOfPosition(
-      start + length,
-    );
-    const endColumn = lineNumber === endPosition.line
-      ? endPosition.character
-      : startColumn;
-    const lastLineInFile = sourceFile.getLineAndCharacterOfPosition(
-      sourceFile.text.length,
-    ).line;
-    const lineStart = sourceFile.getPositionOfLineAndCharacter(lineNumber, 0);
-    const lineEnd = lineNumber < lastLineInFile
-      ? sourceFile.getPositionOfLineAndCharacter(lineNumber + 1, 0)
-      : sourceFile.text.length;
-    const sourceLine = sourceFile.text
-      .slice(lineStart, lineEnd)
-      .replace(/\s+$/g, "")
-      .replace("\t", " ");
-    return {
-      sourceLine,
-      lineNumber,
-      scriptResourceName,
-      startColumn,
-      endColumn,
-    };
-  }
-
-  function fromDiagnosticMessageChain(messageChain) {
-    if (!messageChain) {
-      return undefined;
-    }
-
-    return messageChain.map(({ messageText, code, category, next }) => {
-      const message = transformMessageText(messageText, code);
       return {
-        message,
+        start: startPos,
-        code,
+        end: file.getLineAndCharacterOfPosition(start + length),
-        category: fromDiagnosticCategory(category),
+        fileName,
-        next: fromDiagnosticMessageChain(next),
+        messageChain,
+        messageText,
+        sourceLine,
+        ...ri,
       };
+    } else {
+      return {
+        messageChain,
+        messageText,
+        ...ri,
+      };
+    }
+  }
+
+  /**
+   * @param {import("../dts/typescript").Diagnostic[]} diagnostics 
+   */
+  function fromTypeScriptDiagnostic(diagnostics) {
+    return diagnostics.map(({ relatedInformation: ri, source, ...diag }) => {
+      const value = fromRelatedInformation(diag);
+      value.relatedInformation = ri
+        ? ri.map(fromRelatedInformation)
+        : undefined;
+      value.source = source;
+      return value;
     });
   }
 
-  function parseDiagnostic(item) {
+  function opNow() {
-    const {
+    const res = dispatchJson.sendSync("op_now");
-      messageText,
+    return res.seconds * 1e3 + res.subsecNanos / 1e6;
-      category: sourceCategory,
-      code,
-      file,
-      start: startPosition,
-      length,
-    } = item;
-    const sourceInfo = file && startPosition && length
-      ? getSourceInformation(file, startPosition, length)
-      : undefined;
-    const endPosition = startPosition && length
-      ? startPosition + length
-      : undefined;
-    const category = fromDiagnosticCategory(sourceCategory);
-
-    let message;
-    let messageChain;
-    if (typeof messageText === "string") {
-      message = transformMessageText(messageText, code);
-    } else {
-      message = transformMessageText(messageText.messageText, messageText.code);
-      messageChain = fromDiagnosticMessageChain([messageText])[0];
-    }
-
-    const base = {
-      message,
-      messageChain,
-      code,
-      category,
-      startPosition,
-      endPosition,
-    };
-
-    return sourceInfo ? { ...base, ...sourceInfo } : base;
-  }
-
-  function parseRelatedInformation(relatedInformation) {
-    const result = [];
-    for (const item of relatedInformation) {
-      result.push(parseDiagnostic(item));
-    }
-    return result;
-  }
-
-  function fromTypeScriptDiagnostic(diagnostics) {
-    const items = [];
-    for (const sourceDiagnostic of diagnostics) {
-      const item = parseDiagnostic(sourceDiagnostic);
-      if (sourceDiagnostic.relatedInformation) {
-        item.relatedInformation = parseRelatedInformation(
-          sourceDiagnostic.relatedInformation,
-        );
-      }
-      items.push(item);
-    }
-    return { items };
   }
 
   // We really don't want to depend on JSON dispatch during snapshotting, so
@@ -1353,7 +1153,7 @@ delete Object.prototype.__proto__;
     });
 
     const maybeDiagnostics = diagnostics.length
-      ? fromTypeScriptDiagnostic(diagnostics).items
+      ? fromTypeScriptDiagnostic(diagnostics)
       : [];
 
     return {
@@ -1413,7 +1213,7 @@ delete Object.prototype.__proto__;
     });
 
     const maybeDiagnostics = diagnostics.length
-      ? fromTypeScriptDiagnostic(diagnostics).items
+      ? fromTypeScriptDiagnostic(diagnostics)
       : [];
 
     return {