fix(lsp): handle ts debug errors better (#8914)

Fixes #8864

cli/lsp/diagnostics.rs

@@ -244,20 +244,10 @@ pub async fn generate_ts_diagnostics(
     let version = doc_data.version;
     let current_version = diagnostic_collection.get_version(&file_id);
     if version != current_version {
-      // TODO(@kitsonk): consider refactoring to get all diagnostics in one shot
-      // for a file.
-      let req = tsc::RequestMethod::GetSemanticDiagnostics(specifier.clone());
-      let mut ts_diagnostics = ts_json_to_diagnostics(
+      let req = tsc::RequestMethod::GetDiagnostics(specifier.clone());
+      let ts_diagnostics = ts_json_to_diagnostics(
         ts_server.request(state_snapshot.clone(), req).await?,
       )?;
-      let req = tsc::RequestMethod::GetSuggestionDiagnostics(specifier.clone());
-      ts_diagnostics.append(&mut ts_json_to_diagnostics(
-        ts_server.request(state_snapshot.clone(), req).await?,
-      )?);
-      let req = tsc::RequestMethod::GetSyntacticDiagnostics(specifier.clone());
-      ts_diagnostics.append(&mut ts_json_to_diagnostics(
-        ts_server.request(state_snapshot.clone(), req).await?,
-      )?);
       diagnostics.push((file_id, version, ts_diagnostics));
     }
   }

cli/lsp/language_server.rs

@@ -154,12 +154,19 @@ impl LanguageServer {
       if enabled {
         let diagnostics = {
           let diagnostic_collection = self.diagnostics.read().unwrap().clone();
-          diagnostics::generate_ts_diagnostics(
+          match diagnostics::generate_ts_diagnostics(
             &self.ts_server,
             &diagnostic_collection,
             self.snapshot(),
           )
-          .await?
+          .await
+          {
+            Ok(diagnostics) => diagnostics,
+            Err(err) => {
+              error!("Error processing TypeScript diagnostics:\n{}", err);
+              vec![]
+            }
+          }
         };
         {
           let mut diagnostics_collection = self.diagnostics.write().unwrap();

cli/lsp/tsc.rs

@@ -1123,12 +1123,8 @@ pub enum RequestMethod {
   Configure(TsConfig),
   /// Retrieve the text of an assets that exists in memory in the isolate.
   GetAsset(ModuleSpecifier),
-  /// Return semantic diagnostics for given file.
-  GetSemanticDiagnostics(ModuleSpecifier),
-  /// Returns suggestion diagnostics for given file.
-  GetSuggestionDiagnostics(ModuleSpecifier),
-  /// Return syntactic diagnostics for a given file.
-  GetSyntacticDiagnostics(ModuleSpecifier),
+  /// Return diagnostics for given file.
+  GetDiagnostics(ModuleSpecifier),
   /// Return quick info at position (hover information).
   GetQuickInfo((ModuleSpecifier, u32)),
   /// Return document highlights at position.
@@ -1156,19 +1152,9 @@ impl RequestMethod {
         "method": "getAsset",
         "specifier": specifier,
       }),
-      RequestMethod::GetSemanticDiagnostics(specifier) => json!({
+      RequestMethod::GetDiagnostics(specifier) => json!({
         "id": id,
-        "method": "getSemanticDiagnostics",
-        "specifier": specifier,
-      }),
-      RequestMethod::GetSuggestionDiagnostics(specifier) => json!({
-        "id": id,
-        "method": "getSuggestionDiagnostics",
-        "specifier": specifier,
-      }),
-      RequestMethod::GetSyntacticDiagnostics(specifier) => json!({
-        "id": id,
-        "method": "getSyntacticDiagnostics",
+        "method": "getDiagnostics",
         "specifier": specifier,
       }),
       RequestMethod::GetQuickInfo((specifier, position)) => json!({
@@ -1369,7 +1355,7 @@ mod tests {
   }
 
   #[test]
-  fn test_get_semantic_diagnostics() {
+  fn test_get_diagnostics() {
     let (mut runtime, state_snapshot) = setup(
       false,
       json!({
@@ -1384,7 +1370,7 @@ mod tests {
     let result = request(
       &mut runtime,
       state_snapshot,
-      RequestMethod::GetSemanticDiagnostics(specifier),
+      RequestMethod::GetDiagnostics(specifier),
     );
     assert!(result.is_ok());
     let response = result.unwrap();
@@ -1437,7 +1423,7 @@ mod tests {
     let result = request(
       &mut runtime,
       state_snapshot,
-      RequestMethod::GetSemanticDiagnostics(specifier),
+      RequestMethod::GetDiagnostics(specifier),
     );
     assert!(result.is_ok());
     let response = result.unwrap();
@@ -1467,11 +1453,29 @@ mod tests {
     let result = request(
       &mut runtime,
       state_snapshot,
-      RequestMethod::GetSyntacticDiagnostics(specifier),
+      RequestMethod::GetDiagnostics(specifier),
     );
     assert!(result.is_ok());
     let response = result.unwrap();
-    assert_eq!(response, json!([]));
+    assert_eq!(
+      response,
+      json!([{
+        "start": {
+          "line": 1,
+          "character": 8
+        },
+        "end": {
+          "line": 1,
+          "character": 30
+        },
+        "fileName": "file:///a.ts",
+        "messageText": "\'A\' is declared but its value is never read.",
+        "sourceLine": "        import { A } from \".\";",
+        "category": 2,
+        "code": 6133,
+        "reportsUnnecessary": true,
+      }])
+    );
   }
 
   #[test]
@@ -1501,7 +1505,7 @@ mod tests {
     let result = request(
       &mut runtime,
       state_snapshot,
-      RequestMethod::GetSyntacticDiagnostics(specifier),
+      RequestMethod::GetDiagnostics(specifier),
     );
     assert!(result.is_ok());
     let response = result.unwrap();
@@ -1538,13 +1542,28 @@ mod tests {
     let result = request(
       &mut runtime,
       state_snapshot,
-      RequestMethod::GetSyntacticDiagnostics(specifier),
+      RequestMethod::GetDiagnostics(specifier),
     );
     assert!(result.is_ok());
     let response = result.unwrap();
     assert_eq!(
       response,
       json!([{
+        "start": {
+          "line": 1,
+          "character": 8
+        },
+        "end": {
+          "line": 6,
+          "character": 55,
+        },
+        "fileName": "file:///a.ts",
+        "messageText": "All imports in import declaration are unused.",
+        "sourceLine": "        import {",
+        "category": 2,
+        "code": 6192,
+        "reportsUnnecessary": true
+      }, {
         "start": {
           "line": 8,
           "character": 29
@@ -1562,6 +1581,30 @@ mod tests {
     );
   }
 
+  #[test]
+  fn test_no_debug_failure() {
+    let (mut runtime, state_snapshot) = setup(
+      false,
+      json!({
+        "target": "esnext",
+        "module": "esnext",
+        "lib": ["deno.ns", "deno.window"],
+        "noEmit": true,
+      }),
+      vec![("file:///a.ts", r#"const url = new URL("b.js", import."#, 1)],
+    );
+    let specifier = ModuleSpecifier::resolve_url("file:///a.ts")
+      .expect("could not resolve url");
+    let result = request(
+      &mut runtime,
+      state_snapshot,
+      RequestMethod::GetDiagnostics(specifier),
+    );
+    assert!(result.is_ok());
+    let response = result.unwrap();
+    assert_eq!(response, json!([]));
+  }
+
   #[test]
   fn test_request_asset() {
     let (mut runtime, state_snapshot) = setup(

cli/tests/integration_tests.rs

@@ -922,7 +922,7 @@ fn ts_reload() {
     .output()
     .expect("failed to spawn script");
   // check the output of the the bundle program.
-  assert!(std::str::from_utf8(&output.stdout)
+  assert!(std::str::from_utf8(&output.stderr)
     .unwrap()
     .trim()
     .contains("host.writeFile(\"deno://002_hello.js\")"));

cli/tsc/99_main_compiler.js

@@ -31,10 +31,22 @@ delete Object.prototype.__proto__;
       const stringifiedArgs = args.map((arg) =>
         typeof arg === "string" ? arg : JSON.stringify(arg)
       ).join(" ");
-      core.print(`DEBUG ${logSource} - ${stringifiedArgs}\n`);
+      // adding a non-zero integer value to the end of the debug string causes
+      // the message to be printed to stderr instead of stdout, which is better
+      // aligned to the behaviour of debug messages
+      core.print(`DEBUG ${logSource} - ${stringifiedArgs}\n`, 1);
     }
   }
 
+  function error(...args) {
+    const stringifiedArgs = args.map((arg) =>
+      typeof arg === "string" || arg instanceof Error
+        ? String(arg)
+        : JSON.stringify(arg)
+    ).join(" ");
+    core.print(`ERROR ${logSource} = ${stringifiedArgs}\n`, 1);
+  }
+
   class AssertionError extends Error {
     constructor(msg) {
       super(msg);
@@ -497,23 +509,18 @@ delete Object.prototype.__proto__;
         );
         return respond(id, sourceFile && sourceFile.text);
       }
-      case "getSemanticDiagnostics": {
-        const diagnostics = languageService.getSemanticDiagnostics(
-          request.specifier,
-        ).filter(({ code }) => !IGNORED_DIAGNOSTICS.includes(code));
+      case "getDiagnostics": {
+        try {
+          const diagnostics = [
+            ...languageService.getSemanticDiagnostics(request.specifier),
+            ...languageService.getSuggestionDiagnostics(request.specifier),
+            ...languageService.getSyntacticDiagnostics(request.specifier),
+          ].filter(({ code }) => !IGNORED_DIAGNOSTICS.includes(code));
           return respond(id, fromTypeScriptDiagnostic(diagnostics));
+        } catch (e) {
+          error(e);
+          return respond(id, []);
         }
-      case "getSuggestionDiagnostics": {
-        const diagnostics = languageService.getSuggestionDiagnostics(
-          request.specifier,
-        ).filter(({ code }) => !IGNORED_DIAGNOSTICS.includes(code));
-        return respond(id, fromTypeScriptDiagnostic(diagnostics));
-      }
-      case "getSyntacticDiagnostics": {
-        const diagnostics = languageService.getSyntacticDiagnostics(
-          request.specifier,
-        ).filter(({ code }) => !IGNORED_DIAGNOSTICS.includes(code));
-        return respond(id, fromTypeScriptDiagnostic(diagnostics));
       }
       case "getQuickInfo": {
         return respond(

cli/tsc/compiler.d.ts

@@ -36,16 +36,14 @@ declare global {
     // deno-lint-ignore no-explicit-any
     jsonOpSync<T>(name: string, params: T): any;
     ops(): void;
-    print(msg: string): void;
+    print(msg: string, code?: number): void;
     registerErrorClass(name: string, Ctor: typeof Error): void;
   }
 
   type LanguageServerRequest =
     | ConfigureRequest
     | GetAsset
-    | GetSyntacticDiagnosticsRequest
-    | GetSemanticDiagnosticsRequest
-    | GetSuggestionDiagnosticsRequest
+    | GetDiagnosticsRequest
     | GetQuickInfoRequest
     | GetDocumentHighlightsRequest
     | GetReferencesRequest
@@ -69,18 +67,8 @@ declare global {
     specifier: string;
   }
 
-  interface GetSyntacticDiagnosticsRequest extends BaseLanguageServerRequest {
-    method: "getSyntacticDiagnostics";
-    specifier: string;
-  }
-
-  interface GetSemanticDiagnosticsRequest extends BaseLanguageServerRequest {
-    method: "getSemanticDiagnostics";
-    specifier: string;
-  }
-
-  interface GetSuggestionDiagnosticsRequest extends BaseLanguageServerRequest {
-    method: "getSuggestionDiagnostics";
+  interface GetDiagnosticsRequest extends BaseLanguageServerRequest {
+    method: "getDiagnostics";
     specifier: string;
   }