Repository: denoland/deno
Pull request: fix(lsp): encode url parts before parsing as uri

Fixes https://github.com/denoland/vscode_deno/issues/1157.
Fixes #25395 (which already no longer panics).

Changed files:
filename: cli/lsp/diagnostics.rs
patch: @@ -12,6 +12,7 @@ use super::language_server::StateSnapshot;
 use super::performance::Performance;
 use super::tsc;
 use super::tsc::TsServer;
+use super::urls::uri_parse_unencoded;
 use super::urls::url_to_uri;
 use super::urls::LspUrlMap;
 
@@ -53,11 +54,9 @@ use deno_semver::package::PackageReq;
 use import_map::ImportMap;
 use import_map::ImportMapError;
 use log::error;
-use lsp_types::Uri;
 use std::collections::HashMap;
 use std::collections::HashSet;
 use std::path::PathBuf;
-use std::str::FromStr;
 use std::sync::atomic::AtomicUsize;
 use std::sync::Arc;
 use std::thread;
@@ -738,7 +737,7 @@ fn to_lsp_related_information(
         if let (Some(file_name), Some(start), Some(end)) =
           (&ri.file_name, &ri.start, &ri.end)
         {
-          let uri = Uri::from_str(file_name).unwrap();
+          let uri = uri_parse_unencoded(file_name).unwrap();
           Some(lsp::DiagnosticRelatedInformation {
             location: lsp::Location {
               uri,

filename: cli/lsp/repl.rs
patch: @@ -1,7 +1,6 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
 use std::collections::HashMap;
-use std::str::FromStr;
 
 use deno_ast::LineAndColumnIndex;
 use deno_ast::ModuleSpecifier;
@@ -42,6 +41,7 @@ use super::config::LanguageWorkspaceSettings;
 use super::config::ObjectLiteralMethodSnippets;
 use super::config::TestingSettings;
 use super::config::WorkspaceSettings;
+use super::urls::uri_parse_unencoded;
 use super::urls::url_to_uri;
 
 #[derive(Debug)]
@@ -263,7 +263,8 @@ impl ReplLanguageServer {
   }
 
   fn get_document_uri(&self) -> Uri {
-    Uri::from_str(self.cwd_uri.join("$deno$repl.ts").unwrap().as_str()).unwrap()
+    uri_parse_unencoded(self.cwd_uri.join("$deno$repl.ts").unwrap().as_str())
+      .unwrap()
   }
 }
 

filename: cli/lsp/testing/execution.rs
patch: @@ -12,6 +12,7 @@ use crate::lsp::client::Client;
 use crate::lsp::client::TestingNotification;
 use crate::lsp::config;
 use crate::lsp::logging::lsp_log;
+use crate::lsp::urls::uri_parse_unencoded;
 use crate::lsp::urls::uri_to_url;
 use crate::lsp::urls::url_to_uri;
 use crate::tools::test;
@@ -32,12 +33,10 @@ use deno_core::ModuleSpecifier;
 use deno_runtime::deno_permissions::Permissions;
 use deno_runtime::tokio_util::create_and_run_current_thread;
 use indexmap::IndexMap;
-use lsp_types::Uri;
 use std::borrow::Cow;
 use std::collections::HashMap;
 use std::collections::HashSet;
 use std::num::NonZeroUsize;
-use std::str::FromStr;
 use std::sync::Arc;
 use std::time::Duration;
 use std::time::Instant;
@@ -536,7 +535,7 @@ impl LspTestDescription {
     &self,
     tests: &IndexMap<usize, LspTestDescription>,
   ) -> lsp_custom::TestIdentifier {
-    let uri = Uri::from_str(&self.location().file_name).unwrap();
+    let uri = uri_parse_unencoded(&self.location().file_name).unwrap();
     let static_id = self.static_id();
     let mut root_desc = self;
     while let Some(parent_id) = root_desc.parent_id() {

filename: cli/lsp/urls.rs
patch: @@ -55,6 +55,25 @@ const COMPONENT: &percent_encoding::AsciiSet = &percent_encoding::CONTROLS
   .add(b'+')
   .add(b',');
 
+/// Characters that may be left unencoded in a `Url` path but not valid in a
+/// `Uri` path.
+const URL_TO_URI_PATH: &percent_encoding::AsciiSet =
+  &percent_encoding::CONTROLS
+    .add(b'[')
+    .add(b']')
+    .add(b'^')
+    .add(b'|');
+
+/// Characters that may be left unencoded in a `Url` query but not valid in a
+/// `Uri` query.
+const URL_TO_URI_QUERY: &percent_encoding::AsciiSet =
+  &URL_TO_URI_PATH.add(b'\\').add(b'`').add(b'{').add(b'}');
+
+/// Characters that may be left unencoded in a `Url` fragment but not valid in
+/// a `Uri` fragment.
+const URL_TO_URI_FRAGMENT: &percent_encoding::AsciiSet =
+  &URL_TO_URI_PATH.add(b'#').add(b'\\').add(b'{').add(b'}');
+
 fn hash_data_specifier(specifier: &ModuleSpecifier) -> String {
   let mut file_name_str = specifier.path().to_string();
   if let Some(query) = specifier.query() {
@@ -122,8 +141,33 @@ impl LspUrlMapInner {
   }
 }
 
+pub fn uri_parse_unencoded(s: &str) -> Result<Uri, AnyError> {
+  url_to_uri(&Url::parse(s)?)
+}
+
 pub fn url_to_uri(url: &Url) -> Result<Uri, AnyError> {
-  Ok(Uri::from_str(url.as_str()).inspect_err(|err| {
+  let components = deno_core::url::quirks::internal_components(url);
+  let mut input = String::with_capacity(url.as_str().len());
+  input.push_str(&url.as_str()[..components.path_start as usize]);
+  input.push_str(
+    &percent_encoding::utf8_percent_encode(url.path(), URL_TO_URI_PATH)
+      .to_string(),
+  );
+  if let Some(query) = url.query() {
+    input.push('?');
+    input.push_str(
+      &percent_encoding::utf8_percent_encode(query, URL_TO_URI_QUERY)
+        .to_string(),
+    );
+  }
+  if let Some(fragment) = url.fragment() {
+    input.push('#');
+    input.push_str(
+      &percent_encoding::utf8_percent_encode(fragment, URL_TO_URI_FRAGMENT)
+        .to_string(),
+    );
+  }
+  Ok(Uri::from_str(&input).inspect_err(|err| {
     lsp_warn!("Could not convert URL \"{url}\" to URI: {err}")
   })?)
 }
@@ -189,7 +233,7 @@ impl LspUrlMap {
         } else {
           to_deno_uri(specifier)
         };
-        let uri = Uri::from_str(&uri_str)?;
+        let uri = uri_parse_unencoded(&uri_str)?;
         inner.put(specifier.clone(), uri.clone());
         uri
       };

filename: tests/integration/lsp_tests.rs
patch: @@ -9722,6 +9722,51 @@ fn lsp_lockfile_redirect_resolution() {
   client.shutdown();
 }
 
+// Regression test for https://github.com/denoland/vscode_deno/issues/1157.
+#[test]
+fn lsp_diagnostics_brackets_in_file_name() {
+  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": {
+      "uri": "file:///a/%5Bfile%5D.ts",
+      "languageId": "typescript",
+      "version": 1,
+      "text": "/** @deprecated */\nexport const a = \"a\";\n\na;\n",
+    },
+  }));
+  assert_eq!(
+    json!(diagnostics.all()),
+    json!([
+      {
+        "range": {
+          "start": { "line": 3, "character": 0 },
+          "end": { "line": 3, "character": 1 },
+        },
+        "severity": 4,
+        "code": 6385,
+        "source": "deno-ts",
+        "message": "'a' is deprecated.",
+        "relatedInformation": [
+          {
+            "location": {
+              "uri": "file:///a/%5Bfile%5D.ts",
+              "range": {
+                "start": { "line": 0, "character": 4 },
+                "end": { "line": 0, "character": 16 },
+              },
+            },
+            "message": "The declaration was marked as deprecated here.",
+          },
+        ],
+        "tags": [2],
+      },
+    ]),
+  );
+  client.shutdown();
+}
+
 #[test]
 fn lsp_diagnostics_deprecated() {
   let context = TestContextBuilder::new().use_temp_cwd().build();