fix(lsp): Pass diagnostic codes to TSC as numbers (#23720)

Fixes the `Debug Failure` errors described in
https://github.com/denoland/deno/issues/23643#issuecomment-2094552765 .

The issue here was that we were passing diagnostic codes as strings but
TSC expects the codes to be numbers. This resulted in some quick fixes
not working (as illustrated by the test added here which fails before
this PR).

The first commit is the actual fix. The rest are just test related.

cli/lsp/language_server.rs

@@ -1550,8 +1550,14 @@ impl Inner {
         match diagnostic.source.as_deref() {
           Some("deno-ts") => {
             let code = match diagnostic.code.as_ref().unwrap() {
-              NumberOrString::String(code) => code.to_string(),
-              NumberOrString::Number(code) => code.to_string(),
+              NumberOrString::String(code) => match code.parse() {
+                Ok(c) => c,
+                Err(e) => {
+                  lsp_warn!("Invalid diagnostic code {code}: {e}");
+                  continue;
+                }
+              },
+              NumberOrString::Number(code) => *code,
             };
             let codes = vec![code];
             let actions = self

cli/lsp/tsc.rs

@@ -511,7 +511,7 @@ impl TsServer {
     snapshot: Arc<StateSnapshot>,
     specifier: ModuleSpecifier,
     range: Range<u32>,
-    codes: Vec<String>,
+    codes: Vec<i32>,
     format_code_settings: FormatCodeSettings,
     preferences: UserPreferences,
   ) -> Vec<CodeFixAction> {
@@ -4817,7 +4817,7 @@ pub enum TscRequest {
       String,
       u32,
       u32,
-      Vec<String>,
+      Vec<i32>,
       FormatCodeSettings,
       UserPreferences,
     )>,

tests/integration/jupyter_tests.rs

@@ -5,6 +5,7 @@ use std::sync::Arc;
 use std::time::Duration;
 
 use bytes::Bytes;
+use test_util::assertions::assert_json_subset;
 use test_util::DenoChild;
 use test_util::TestContext;
 use test_util::TestContextBuilder;
@@ -488,31 +489,6 @@ async fn setup() -> (TestContext, JupyterClient, JupyterServerProcess) {
   (context, client, process)
 }
 
-/// Asserts that the actual value is equal to the expected value, but
-/// only for the keys present in the expected value.
-/// In other words, `assert_eq_subset(json!({"a": 1, "b": 2}), json!({"a": 1}))` would pass.
-#[track_caller]
-fn assert_eq_subset(actual: Value, expected: Value) {
-  match (actual, expected) {
-    (Value::Object(actual), Value::Object(expected)) => {
-      for (k, v) in expected.iter() {
-        let Some(actual_v) = actual.get(k) else {
-          panic!("Key {k:?} not found in actual value ({actual:#?})");
-        };
-        assert_eq_subset(actual_v.clone(), v.clone());
-      }
-    }
-    (Value::Array(actual), Value::Array(expected)) => {
-      for (i, v) in expected.iter().enumerate() {
-        assert_eq_subset(actual[i].clone(), v.clone());
-      }
-    }
-    (actual, expected) => {
-      assert_eq!(actual, expected);
-    }
-  }
-}
-
 #[tokio::test]
 async fn jupyter_heartbeat_echoes() -> Result<()> {
   let (_ctx, client, _process) = setup().await;
@@ -531,7 +507,7 @@ async fn jupyter_kernel_info() -> Result<()> {
     .await?;
   let msg = client.recv(Control).await?;
   assert_eq!(msg.header.msg_type, "kernel_info_reply");
-  assert_eq_subset(
+  assert_json_subset(
     msg.content,
     json!({
       "status": "ok",
@@ -568,7 +544,7 @@ async fn jupyter_execute_request() -> Result<()> {
     .await?;
   let reply = client.recv(Shell).await?;
   assert_eq!(reply.header.msg_type, "execute_reply");
-  assert_eq_subset(
+  assert_json_subset(
     reply.content,
     json!({
       "status": "ok",
@@ -602,7 +578,7 @@ async fn jupyter_execute_request() -> Result<()> {
     })
     .expect("execution_state idle not found");
   assert_eq!(execution_idle.parent_header, request.header.to_json());
-  assert_eq_subset(
+  assert_json_subset(
     execution_idle.content.clone(),
     json!({
       "execution_state": "idle",
@@ -615,7 +591,7 @@ async fn jupyter_execute_request() -> Result<()> {
     .expect("stream not found");
   assert_eq!(execution_result.header.msg_type, "stream");
   assert_eq!(execution_result.parent_header, request.header.to_json());
-  assert_eq_subset(
+  assert_json_subset(
     execution_result.content.clone(),
     json!({
       "name": "stdout",
@@ -643,7 +619,7 @@ async fn jupyter_store_history_false() -> Result<()> {
 
   let reply = client.recv(Shell).await?;
   assert_eq!(reply.header.msg_type, "execute_reply");
-  assert_eq_subset(
+  assert_json_subset(
     reply.content,
     json!({
       "status": "ok",

tests/integration/lsp_tests.rs

@@ -9,6 +9,7 @@ use deno_core::url::Url;
 use pretty_assertions::assert_eq;
 use std::fs;
 use test_util::assert_starts_with;
+use test_util::assertions::assert_json_subset;
 use test_util::deno_cmd_with_deno_dir;
 use test_util::env_vars_for_npm_tests;
 use test_util::lsp::LspClient;
@@ -16,6 +17,65 @@ use test_util::testdata_path;
 use test_util::TestContextBuilder;
 use tower_lsp::lsp_types as lsp;
 
+/// Helper to get the `lsp::Range` of the `n`th occurrence of
+/// `text` in `src`. `n` is zero-based, like most indexes.
+fn range_of_nth(
+  n: usize,
+  text: impl AsRef<str>,
+  src: impl AsRef<str>,
+) -> lsp::Range {
+  let text = text.as_ref();
+
+  let src = src.as_ref();
+
+  let start = src
+    .match_indices(text)
+    .nth(n)
+    .map(|(i, _)| i)
+    .unwrap_or_else(|| panic!("couldn't find text {text} in source {src}"));
+  let end = start + text.len();
+  let mut line = 0;
+  let mut col = 0;
+  let mut byte_idx = 0;
+
+  let pos = |line, col| lsp::Position {
+    line,
+    character: col,
+  };
+
+  let mut start_pos = None;
+  let mut end_pos = None;
+  for c in src.chars() {
+    if byte_idx == start {
+      start_pos = Some(pos(line, col));
+    }
+    if byte_idx == end {
+      end_pos = Some(pos(line, col));
+      break;
+    }
+    if c == '\n' {
+      line += 1;
+      col = 0;
+    } else {
+      col += c.len_utf16() as u32;
+    }
+    byte_idx += c.len_utf8();
+  }
+  if start_pos.is_some() && end_pos.is_none() {
+    // range extends to end of string
+    end_pos = Some(pos(line, col));
+  }
+
+  let (start, end) = (start_pos.unwrap(), end_pos.unwrap());
+  lsp::Range { start, end }
+}
+
+/// Helper to get the `lsp::Range` of the first occurrence of
+/// `text` in `src`. Equivalent to `range_of_nth(0, text, src)`.
+fn range_of(text: impl AsRef<str>, src: impl AsRef<str>) -> lsp::Range {
+  range_of_nth(0, text, src)
+}
+
 #[test]
 fn lsp_startup_shutdown() {
   let context = TestContextBuilder::new().use_temp_cwd().build();
@@ -12548,3 +12608,72 @@ fn lsp_cjs_internal_types_default_export() {
   // previously, diagnostic about `add` not being callable
   assert_eq!(json!(diagnostics.all()), json!([]));
 }
+
+#[test]
+fn lsp_ts_code_fix_any_param() {
+  let context = TestContextBuilder::new().use_temp_cwd().build();
+  let temp_dir = context.temp_dir();
+
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+
+  let src = "export function foo(param) { console.log(param); }";
+
+  let param_def = range_of("param", src);
+
+  let main_url = temp_dir.path().join("main.ts").uri_file();
+  let diagnostics = client.did_open(json!({
+    "textDocument": {
+      "uri": main_url,
+      "languageId": "typescript",
+      "version": 1,
+      "text": src,
+    }
+  }));
+  // make sure the "implicit any type" diagnostic is there for "param"
+  assert_json_subset(
+    json!(diagnostics.all()),
+    json!([{
+      "range": param_def,
+      "code": 7006,
+      "message": "Parameter 'param' implicitly has an 'any' type."
+    }]),
+  );
+
+  // response is array of fixes
+  let response = client.write_request(
+    "textDocument/codeAction",
+    json!({
+      "textDocument": {
+        "uri": main_url,
+      },
+      "range": lsp::Range {
+        start: param_def.end,
+        ..param_def
+      },
+      "context": {
+        "diagnostics": diagnostics.all(),
+      }
+    }),
+  );
+  let fixes = response.as_array().unwrap();
+
+  // we're looking for the quick fix that pertains to our diagnostic,
+  // specifically the "Infer parameter types from usage" fix
+  for fix in fixes {
+    let Some(diags) = fix.get("diagnostics") else {
+      continue;
+    };
+    let Some(fix_title) = fix.get("title").and_then(|s| s.as_str()) else {
+      continue;
+    };
+    if diags == &json!(diagnostics.all())
+      && fix_title == "Infer parameter types from usage"
+    {
+      // found it!
+      return;
+    }
+  }
+
+  panic!("failed to find 'Infer parameter types from usage' fix in fixes: {fixes:#?}");
+}

tests/util/server/src/assertions.rs

@@ -108,3 +108,44 @@ pub fn assert_wildcard_match_with_logger(
     }
   }
 }
+
+/// Asserts that the actual `serde_json::Value` is equal to the expected `serde_json::Value`, but
+/// only for the keys present in the expected value.
+///
+/// # Example
+///
+/// ```
+/// # use serde_json::json;
+/// # use test_server::assertions::assert_json_subset;
+/// assert_json_subset(json!({"a": 1, "b": 2}), json!({"a": 1}));
+///
+/// // Arrays are compared element by element
+/// assert_json_subset(json!([{ "a": 1, "b": 2 }, {}]), json!([{"a": 1}, {}]));
+/// ```
+#[track_caller]
+pub fn assert_json_subset(
+  actual: serde_json::Value,
+  expected: serde_json::Value,
+) {
+  match (actual, expected) {
+    (
+      serde_json::Value::Object(actual),
+      serde_json::Value::Object(expected),
+    ) => {
+      for (k, v) in expected.iter() {
+        let Some(actual_v) = actual.get(k) else {
+          panic!("Key {k:?} not found in actual value ({actual:#?})");
+        };
+        assert_json_subset(actual_v.clone(), v.clone());
+      }
+    }
+    (serde_json::Value::Array(actual), serde_json::Value::Array(expected)) => {
+      for (i, v) in expected.iter().enumerate() {
+        assert_json_subset(actual[i].clone(), v.clone());
+      }
+    }
+    (actual, expected) => {
+      assert_eq!(actual, expected);
+    }
+  }
+}