fix(lsp): don't normalize urls in cache command params (#22182)

cli/lsp/language_server.rs

@@ -284,7 +284,8 @@ impl LanguageServer {
   /// in the Deno cache, including any of their dependencies.
   pub async fn cache_request(
     &self,
-    params: Option<Value>,
+    specifiers: Vec<ModuleSpecifier>,
+    referrer: ModuleSpecifier,
   ) -> LspResult<Option<Value>> {
     async fn create_graph_for_caching(
       cli_options: CliOptions,
@@ -330,12 +331,10 @@ impl LanguageServer {
       Ok(())
     }
 
-    match params.map(serde_json::from_value) {
-      Some(Ok(params)) => {
     // do as much as possible in a read, then do a write outside
     let maybe_prepare_cache_result = {
       let inner = self.0.read().await; // ensure dropped
-          match inner.prepare_cache(params) {
+      match inner.prepare_cache(specifiers, referrer) {
         Ok(maybe_cache_result) => maybe_cache_result,
         Err(err) => {
           self
@@ -371,10 +370,6 @@ impl LanguageServer {
     }
     Ok(Some(json!(true)))
   }
-      Some(Err(err)) => Err(LspError::invalid_params(err.to_string())),
-      None => Err(LspError::invalid_params("Missing parameters")),
-    }
-  }
 
   /// This request is only used by the lsp integration tests to
   /// coordinate the tests receiving the latest diagnostics.
@@ -396,12 +391,6 @@ impl LanguageServer {
     Ok(Some(self.0.read().await.get_performance()))
   }
 
-  pub async fn reload_import_registries_request(
-    &self,
-  ) -> LspResult<Option<Value>> {
-    self.0.write().await.reload_import_registries().await
-  }
-
   pub async fn task_definitions(&self) -> LspResult<Vec<TaskDefinition>> {
     self.0.read().await.task_definitions()
   }
@@ -1081,24 +1070,18 @@ impl Inner {
               compiler_options_obj.get("jsxImportSource")
             {
               if let Some(jsx_import_source) = jsx_import_source.as_str() {
-                let cache_params = lsp_custom::CacheParams {
+                let specifiers = vec![Url::parse(&format!(
-                  referrer: TextDocumentIdentifier {
-                    uri: config_file.specifier.clone(),
-                  },
-                  uris: vec![TextDocumentIdentifier {
-                    uri: Url::parse(&format!(
                   "data:application/typescript;base64,{}",
-                      base64::engine::general_purpose::STANDARD.encode(
+                  base64::engine::general_purpose::STANDARD.encode(format!(
-                        format!("import '{jsx_import_source}/jsx-runtime';")
+                    "import '{jsx_import_source}/jsx-runtime';"
-                      )
                   ))
-                    .unwrap(),
+                ))
-                  }],
+                .unwrap()];
-                };
+                let referrer = config_file.specifier.clone();
                 self.task_queue.queue_task(Box::new(|ls: LanguageServer| {
                   spawn(async move {
                     if let Err(err) =
-                      ls.cache_request(Some(json!(cache_params))).await
+                      ls.cache_request(specifiers, referrer).await
                     {
                       lsp_warn!("{}", err);
                     }
@@ -3222,24 +3205,13 @@ impl tower_lsp::LanguageServer for LanguageServer {
   ) -> LspResult<Option<Value>> {
     if params.command == "deno.cache" {
       let mut arguments = params.arguments.into_iter();
-      let uris = serde_json::to_value(arguments.next()).unwrap();
+      let specifiers = serde_json::to_value(arguments.next()).unwrap();
-      let uris: Vec<Url> = serde_json::from_value(uris)
+      let specifiers: Vec<Url> = serde_json::from_value(specifiers)
         .map_err(|err| LspError::invalid_params(err.to_string()))?;
       let referrer = serde_json::to_value(arguments.next()).unwrap();
       let referrer: Url = serde_json::from_value(referrer)
         .map_err(|err| LspError::invalid_params(err.to_string()))?;
-      self
+      self.cache_request(specifiers, referrer).await
-        .cache_request(Some(
-          serde_json::to_value(lsp_custom::CacheParams {
-            referrer: TextDocumentIdentifier { uri: referrer },
-            uris: uris
-              .into_iter()
-              .map(|uri| TextDocumentIdentifier { uri })
-              .collect(),
-          })
-          .expect("well formed json"),
-        ))
-        .await
     } else if params.command == "deno.reloadImportRegistries" {
       self.0.write().await.reload_import_registries().await
     } else {
@@ -3421,7 +3393,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
 
   async fn did_save(&self, params: DidSaveTextDocumentParams) {
     let uri = &params.text_document.uri;
-    {
+    let specifier = {
       let mut inner = self.0.write().await;
       let specifier = inner.url_map.normalize_url(uri, LspUrlKind::File);
       inner.documents.save(&specifier);
@@ -3438,18 +3410,10 @@ impl tower_lsp::LanguageServer for LanguageServer {
         Ok(path) if is_importable_ext(&path) => {}
         _ => return,
       }
-    }
+      specifier
-    if let Err(err) = self
+    };
-      .cache_request(Some(
+    if let Err(err) = self.cache_request(vec![], specifier.clone()).await {
-        serde_json::to_value(lsp_custom::CacheParams {
+      lsp_warn!("Failed to cache \"{}\" on save: {}", &specifier, err);
-          referrer: TextDocumentIdentifier { uri: uri.clone() },
-          uris: vec![TextDocumentIdentifier { uri: uri.clone() }],
-        })
-        .unwrap(),
-      ))
-      .await
-    {
-      lsp_warn!("Failed to cache \"{}\" on save: {}", uri.to_string(), err);
     }
   }
 
@@ -3693,22 +3657,17 @@ struct PrepareCacheResult {
 impl Inner {
   fn prepare_cache(
     &self,
-    params: lsp_custom::CacheParams,
+    specifiers: Vec<ModuleSpecifier>,
+    referrer: ModuleSpecifier,
   ) -> Result<Option<PrepareCacheResult>, AnyError> {
-    let referrer = self
+    let mark = self
-      .url_map
+      .performance
-      .normalize_url(&params.referrer.uri, LspUrlKind::File);
+      .mark_with_args("lsp.cache", (&specifiers, &referrer));
-    let mark = self.performance.mark_with_args("lsp.cache", &params);
+    let roots = if !specifiers.is_empty() {
-    let roots = if !params.uris.is_empty() {
+      specifiers
-      params
-        .uris
-        .iter()
-        .map(|t| self.url_map.normalize_url(&t.uri, LspUrlKind::File))
-        .collect()
     } else {
       vec![referrer]
     };
-
     let workspace_settings = self.config.workspace_settings();
     let mut cli_options = CliOptions::new(
       Flags {

cli/lsp/lsp_custom.rs

@@ -4,26 +4,12 @@ use deno_core::serde::Deserialize;
 use deno_core::serde::Serialize;
 use tower_lsp::lsp_types as lsp;
 
-pub const CACHE_REQUEST: &str = "deno/cache";
 pub const PERFORMANCE_REQUEST: &str = "deno/performance";
 pub const TASK_REQUEST: &str = "deno/taskDefinitions";
-pub const RELOAD_IMPORT_REGISTRIES_REQUEST: &str =
-  "deno/reloadImportRegistries";
 pub const VIRTUAL_TEXT_DOCUMENT: &str = "deno/virtualTextDocument";
 pub const LATEST_DIAGNOSTIC_BATCH_INDEX: &str =
   "deno/internalLatestDiagnosticBatchIndex";
 
-#[derive(Debug, Deserialize, Serialize)]
-#[serde(rename_all = "camelCase")]
-pub struct CacheParams {
-  /// The document currently open in the editor.  If there are no `uris`
-  /// supplied, the referrer will be cached.
-  pub referrer: lsp::TextDocumentIdentifier,
-  /// Any documents that have been specifically asked to be cached via the
-  /// command.
-  pub uris: Vec<lsp::TextDocumentIdentifier>,
-}
-
 #[derive(Debug, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
 pub struct TaskDefinition {

cli/lsp/mod.rs

@@ -48,20 +48,10 @@ pub async fn start() -> Result<(), AnyError> {
       token.clone(),
     )
   })
-  // TODO(nayeemrmn): The extension has replaced this with the `deno.cache`
-  // command as of vscode_deno 3.21.0 / 2023.09.05. Remove this eventually.
-  .custom_method(lsp_custom::CACHE_REQUEST, LanguageServer::cache_request)
   .custom_method(
     lsp_custom::PERFORMANCE_REQUEST,
     LanguageServer::performance_request,
   )
-  // TODO(nayeemrmn): The extension has replaced this with the
-  // `deno.reloadImportRegistries` command as of vscode_deno
-  // 3.26.0 / 2023.10.10. Remove this eventually.
-  .custom_method(
-    lsp_custom::RELOAD_IMPORT_REGISTRIES_REQUEST,
-    LanguageServer::reload_import_registries_request,
-  )
   .custom_method(lsp_custom::TASK_REQUEST, LanguageServer::task_definitions)
   // TODO(nayeemrmn): Rename this to `deno/taskDefinitions` in vscode_deno and
   // remove this alias.

cli/tests/integration/lsp_tests.rs

@@ -4945,6 +4945,73 @@ fn lsp_cache_on_save() {
   client.shutdown();
 }
 
+// Regression test for https://github.com/denoland/deno/issues/22122.
+#[test]
+fn lsp_cache_then_definition() {
+  let context = TestContextBuilder::new()
+    .use_http_server()
+    .use_temp_cwd()
+    .build();
+  let temp_dir = context.temp_dir();
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  client.did_open(json!({
+    "textDocument": {
+      "uri": temp_dir.uri().join("file.ts").unwrap(),
+      "languageId": "typescript",
+      "version": 1,
+      "text": r#"import "http://localhost:4545/run/002_hello.ts";"#,
+    },
+  }));
+  // Prior to the fix, this would cause a faulty memoization that maps the
+  // URL "http://localhost:4545/run/002_hello.ts" to itself, preventing it from
+  // being reverse-mapped to "deno:/http/localhost%3A4545/run/002_hello.ts" on
+  // "textDocument/definition" request.
+  client.write_request(
+    "workspace/executeCommand",
+    json!({
+      "command": "deno.cache",
+      "arguments": [
+        ["http://localhost:4545/run/002_hello.ts"],
+        temp_dir.uri().join("file.ts").unwrap(),
+      ],
+    }),
+  );
+  let res = client.write_request(
+    "textDocument/definition",
+    json!({
+      "textDocument": { "uri": temp_dir.uri().join("file.ts").unwrap() },
+      "position": { "line": 0, "character": 8 },
+    }),
+  );
+  assert_eq!(
+    res,
+    json!([{
+      "targetUri": "deno:/http/localhost%3A4545/run/002_hello.ts",
+      "targetRange": {
+        "start": {
+          "line": 0,
+          "character": 0,
+        },
+        "end": {
+          "line": 1,
+          "character": 0,
+        },
+      },
+      "targetSelectionRange": {
+        "start": {
+          "line": 0,
+          "character": 0,
+        },
+        "end": {
+          "line": 1,
+          "character": 0,
+        },
+      },
+    }]),
+  );
+}
+
 #[test]
 fn lsp_code_actions_imports() {
   let context = TestContextBuilder::new().use_temp_cwd().build();