Repository: denoland/deno
Pull request: fix(lsp): normalize all uris originating from client

Fixes #29008.
Fixes #28815. Tentative. There's a good chance all issues for non-vscode editors and LSP >= 2.2.7 are linked to this but I don't know.

Changed files:
filename: cli/lsp/documents.rs
patch: @@ -60,6 +60,7 @@ use super::testing::TestModule;
 use super::text::LineIndex;
 use super::tsc::ChangeKind;
 use super::tsc::NavigationTree;
+use super::urls::normalize_uri;
 use super::urls::uri_is_file_like;
 use super::urls::uri_to_file_path;
 use super::urls::uri_to_url;
@@ -503,15 +504,16 @@ impl Documents {
     text: Arc<str>,
     notebook_uri: Option<Arc<Uri>>,
   ) -> Arc<OpenDocument> {
+    let uri = normalize_uri(&uri);
     self.server.remove(&uri);
     let doc = Arc::new(OpenDocument::new(
-      uri.clone(),
+      uri.as_ref().clone(),
       version,
       language_id,
       text,
       notebook_uri,
     ));
-    self.open.insert(uri, doc.clone());
+    self.open.insert(uri.into_owned(), doc.clone());
     if !doc.uri.scheme().is_some_and(|s| s.eq_lowercase("file")) {
       let url = uri_to_url(&doc.uri);
       if url.scheme() == "file" {
@@ -527,7 +529,8 @@ impl Documents {
     version: i32,
     changes: Vec<lsp::TextDocumentContentChangeEvent>,
   ) -> Result<Arc<OpenDocument>, AnyError> {
-    let Some((uri, doc)) = self.open.shift_remove_entry(uri) else {
+    let uri = normalize_uri(uri);
+    let Some((uri, doc)) = self.open.shift_remove_entry(uri.as_ref()) else {
       return Err(
         JsErrorBox::new(
           "NotFound",
@@ -545,8 +548,11 @@ impl Documents {
   }
 
   fn close(&mut self, uri: &Uri) -> Result<Arc<OpenDocument>, AnyError> {
-    self.file_like_uris_by_url.retain(|_, u| u.as_ref() != uri);
-    let doc = self.open.shift_remove(uri).ok_or_else(|| {
+    let uri = normalize_uri(uri);
+    self
+      .file_like_uris_by_url
+      .retain(|_, u| u.as_ref() != uri.as_ref());
+    let doc = self.open.shift_remove(uri.as_ref()).ok_or_else(|| {
       JsErrorBox::new(
         "NotFound",
         format!(
@@ -563,7 +569,7 @@ impl Documents {
     uri: Uri,
     cells: Vec<lsp::TextDocumentItem>,
   ) -> Vec<Arc<OpenDocument>> {
-    let uri = Arc::new(uri);
+    let uri = Arc::new(normalize_uri(&uri).into_owned());
     let mut documents = Vec::with_capacity(cells.len());
     for cell in cells {
       let language_id = cell.language_id.parse().unwrap_or_else(|err| {
@@ -598,7 +604,7 @@ impl Documents {
     structure: Option<lsp::NotebookDocumentCellChangeStructure>,
     content: Option<Vec<lsp::NotebookDocumentChangeTextContent>>,
   ) -> Vec<(Arc<OpenDocument>, ChangeKind)> {
-    let uri = Arc::new(uri.clone());
+    let uri = Arc::new(normalize_uri(uri).into_owned());
     let mut documents_with_change_kinds = Vec::new();
     if let Some(structure) = structure {
       if let Some(cells) = self.cells_by_notebook_uri.get_mut(&uri) {
@@ -610,7 +616,7 @@ impl Documents {
             .cells
             .into_iter()
             .flatten()
-            .map(|c| Arc::new(c.document)),
+            .map(|c| Arc::new(normalize_uri(&c.document).into_owned())),
         );
       }
       for closed in structure.did_close.into_iter().flatten() {
@@ -663,7 +669,9 @@ impl Documents {
   }
 
   pub fn close_notebook(&mut self, uri: &Uri) -> Vec<Arc<OpenDocument>> {
-    let Some(cell_uris) = self.cells_by_notebook_uri.remove(uri) else {
+    let uri = normalize_uri(uri);
+    let Some(cell_uris) = self.cells_by_notebook_uri.remove(uri.as_ref())
+    else {
       lsp_warn!(
         "The URI \"{}\" does not refer to an open notebook document.",
         uri.as_str(),
@@ -685,33 +693,35 @@ impl Documents {
   }
 
   pub fn get(&self, uri: &Uri) -> Option<Document> {
-    if let Some(doc) = self.open.get(uri) {
+    let uri = normalize_uri(uri);
+    if let Some(doc) = self.open.get(uri.as_ref()) {
       return Some(Document::Open(doc.clone()));
     }
-    if let Some(doc) = ASSET_DOCUMENTS.get(uri) {
+    if let Some(doc) = ASSET_DOCUMENTS.get(&uri) {
       return Some(Document::Server(doc.clone()));
     }
-    if let Some(doc) = self.server.get(uri) {
+    if let Some(doc) = self.server.get(&uri) {
       return Some(Document::Server(doc.clone()));
     }
-    let doc = if let Some(doc) = ServerDocument::load(uri) {
+    let doc = if let Some(doc) = ServerDocument::load(&uri) {
       doc
-    } else if let Some(data_url) = self.data_urls_by_uri.get(uri) {
-      ServerDocument::data_url(uri, data_url.value().clone())?
+    } else if let Some(data_url) = self.data_urls_by_uri.get(&uri) {
+      ServerDocument::data_url(&uri, data_url.value().clone())?
     } else {
       return None;
     };
     let doc = Arc::new(doc);
-    self.server.insert(uri.clone(), doc.clone());
+    self.server.insert(uri.into_owned(), doc.clone());
     Some(Document::Server(doc))
   }
 
   /// This will not create any server entries, only retrieve existing entries.
   pub fn inspect(&self, uri: &Uri) -> Option<Document> {
-    if let Some(doc) = self.open.get(uri) {
+    let uri = normalize_uri(uri);
+    if let Some(doc) = self.open.get(uri.as_ref()) {
       return Some(Document::Open(doc.clone()));
     }
-    if let Some(doc) = self.server.get(uri) {
+    if let Some(doc) = self.server.get(&uri) {
       return Some(Document::Server(doc.clone()));
     }
     None

filename: cli/lsp/language_server.rs
patch: @@ -662,7 +662,7 @@ impl Inner {
   ) -> LspResult<Arc<tsc::NavigationTree>> {
     let mark = self.performance.mark_with_args(
       "lsp.get_navigation_tree",
-      json!({ "uri": &module.specifier }),
+      json!({ "specifier": &module.specifier }),
     );
     let result = module
       .navigation_tree
@@ -759,8 +759,8 @@ impl Inner {
     let global_cache_url = maybe_cache.and_then(|cache_str| {
       if let Ok(url) = Url::from_file_path(cache_str) {
         Some(url)
-      } else if let Some(root_uri) = self.config.root_url() {
-        root_uri.join(cache_str).inspect_err(|err| lsp_warn!("Failed to resolve custom cache path: {err}")).ok()
+      } else if let Some(root_url) = self.config.root_url() {
+        root_url.join(cache_str).inspect_err(|err| lsp_warn!("Failed to resolve custom cache path: {err}")).ok()
       } else {
         lsp_warn!(
           "The configured cache path \"{cache_str}\" is not resolvable outside of a workspace.",
@@ -774,7 +774,7 @@ impl Inner {
     let maybe_root_path = self
       .config
       .root_url()
-      .and_then(|uri| url_to_file_path(uri).ok());
+      .and_then(|url| url_to_file_path(url).ok());
     let root_cert_store = get_root_cert_store(
       maybe_root_path,
       workspace_settings.certificate_stores.clone(),
@@ -1422,6 +1422,7 @@ impl Inner {
     };
     if document.is_diagnosable() {
       self.refresh_dep_info().await;
+      self.diagnostics_server.invalidate(&[&document.uri]);
       let changed_specifier = self
         .document_modules
         .primary_specifier(&Document::Open(document.clone()))
@@ -1430,9 +1431,6 @@ impl Inner {
       // `self.project_changed()` so its module entries will be dropped.
       drop(document);
       self.project_changed(changed_specifier, ProjectScopesChange::None);
-      self
-        .diagnostics_server
-        .invalidate(&[&params.text_document.uri]);
       self.send_diagnostics_update();
       self.send_testing_update();
     }
@@ -1459,7 +1457,7 @@ impl Inner {
             let specifier = self
               .document_modules
               .primary_specifier(&Document::Open((*d).clone()))?;
-            Some((specifier, ChangeKind::Closed))
+            Some((specifier, ChangeKind::Opened))
           })
           .collect::<Vec<_>>(),
         ProjectScopesChange::OpenNotebooks,
@@ -1568,24 +1566,28 @@ impl Inner {
       .collect::<Vec<_>>();
     if !diagnosable_documents.is_empty() {
       self.refresh_dep_info().await;
-      self.project_changed(
-        diagnosable_documents
-          .iter()
-          .flat_map(|d| {
-            let specifier = self
-              .document_modules
-              .primary_specifier(&Document::Open((*d).clone()))?;
-            Some((specifier, ChangeKind::Closed))
-          })
-          .collect::<Vec<_>>(),
-        ProjectScopesChange::OpenNotebooks,
-      );
       self.diagnostics_server.invalidate(
         &diagnosable_documents
           .iter()
           .map(|d| d.uri.as_ref())
           .collect::<Vec<_>>(),
       );
+      let changed_specifiers = diagnosable_documents
+        .iter()
+        .flat_map(|d| {
+          let specifier = self
+            .document_modules
+            .primary_specifier(&Document::Open((*d).clone()))?;
+          Some((specifier, ChangeKind::Closed))
+        })
+        .collect::<Vec<_>>();
+      // Invalidate the weak references of `documents` before calling
+      // `self.project_changed()` so their module entries will be dropped.
+      drop(documents);
+      self.project_changed(
+        changed_specifiers,
+        ProjectScopesChange::OpenNotebooks,
+      );
       self.send_diagnostics_update();
     }
   }

filename: cli/lsp/urls.rs
patch: @@ -1,5 +1,6 @@
 // Copyright 2018-2025 the Deno authors. MIT license.
 
+use std::borrow::Cow;
 use std::path::Component;
 use std::path::Path;
 use std::path::PathBuf;
@@ -80,6 +81,17 @@ pub fn uri_parse_unencoded(s: &str) -> Result<Uri, AnyError> {
   url_to_uri(&Url::parse(s)?)
 }
 
+pub fn normalize_uri(uri: &Uri) -> Cow<'_, Uri> {
+  if !uri.scheme().is_some_and(|s| s.eq_lowercase("file")) {
+    return Cow::Borrowed(uri);
+  }
+  let url = normalize_url(Url::parse(uri.as_str()).unwrap());
+  let Ok(normalized_uri) = url_to_uri(&url) else {
+    return Cow::Borrowed(uri);
+  };
+  Cow::Owned(normalized_uri)
+}
+
 pub fn url_to_uri(url: &Url) -> Result<Uri, AnyError> {
   let components = deno_core::url::quirks::internal_components(url);
   let mut input = String::with_capacity(url.as_str().len());

filename: tests/integration/lsp_tests.rs
patch: @@ -11610,6 +11610,74 @@ fn lsp_untitled_file_diagnostics() {
   client.shutdown();
 }
 
+#[test]
+#[timeout(300_000)]
+fn lsp_non_normalized_uri_diagnostics_and_completions() {
+  let context = TestContextBuilder::new().use_temp_cwd().build();
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  let diagnostics = client.did_open(json!({
+    "textDocument": {
+      // Drive letters are not uppercase in our normalized URI representation.
+      "uri": "file:///C:/file.ts",
+      "languageId": "typescript",
+      "version": 1,
+      "text": r#"
+        const foo: string = 1;
+        console.log(foo);
+      "#,
+    },
+  }));
+  assert_eq!(
+    json!(diagnostics.all_messages()),
+    json!([
+      {
+        // Accordingly, the drive letter returned here is lowercase.
+        // Spec-compliant language clients must deal with that. See:
+        // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#uri
+        "uri": "file:///c%3A/file.ts",
+        "diagnostics": [
+          {
+            "range": {
+              "start": { "line": 1, "character": 14 },
+              "end": { "line": 1, "character": 17 },
+            },
+            "severity": 1,
+            "code": 2322,
+            "source": "deno-ts",
+            "message": "Type 'number' is not assignable to type 'string'.",
+          },
+        ],
+        "version": 1,
+      },
+    ]),
+  );
+  let list = client.get_completion_list(
+    "file:///C:/file.ts",
+    (2, 23),
+    json!({ "triggerKind": 1 }),
+  );
+  assert!(!list.is_incomplete);
+  let item = list.items.iter().find(|i| i.label == "foo").unwrap();
+  assert_eq!(
+    json!(item),
+    json!({
+      "label": "foo",
+      "kind": 6,
+      "sortText": "11",
+      "data": {
+        "tsc": {
+          "uri": "file:///c%3A/file.ts",
+          "position": 55,
+          "name": "foo",
+          "useCodeSnippet": false,
+        },
+      },
+    }),
+  );
+  client.shutdown();
+}
+
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct PerformanceAverage {