refactor(lsp): add LspClientUrl (#18376)

This will make it a bit harder to accidentally use a client url in the
wrong place. I don't fully understand why we do this mapping, but this
will help prevent bugs like #18373

Closes #18374

cli/lsp/client.rs

@@ -17,6 +17,7 @@ use super::config::SpecifierSettings;
 use super::config::SETTINGS_SECTION;
 use super::lsp_custom;
 use super::testing::lsp_custom as testing_lsp_custom;
+use super::urls::LspClientUrl;
 
 #[derive(Debug)]
 pub enum TestingNotification {
@@ -98,18 +99,23 @@ impl OutsideLockClient {
 
   pub async fn specifier_configurations(
     &self,
-    specifiers: Vec<lsp::Url>,
+    specifiers: Vec<LspClientUrl>,
   ) -> Result<Vec<Result<SpecifierSettings, AnyError>>, AnyError> {
-    self.0.specifier_configurations(specifiers).await
+    self
+      .0
+      .specifier_configurations(
+        specifiers.into_iter().map(|s| s.into_url()).collect(),
+      )
+      .await
   }
 
   pub async fn specifier_configuration(
     &self,
-    specifier: &lsp::Url,
+    specifier: &LspClientUrl,
   ) -> Result<SpecifierSettings, AnyError> {
     let values = self
       .0
-      .specifier_configurations(vec![specifier.clone()])
+      .specifier_configurations(vec![specifier.as_url().clone()])
       .await?;
     if let Some(value) = values.into_iter().next() {
       value.map_err(|err| {

cli/lsp/language_server.rs

@@ -59,6 +59,7 @@ use super::tsc::Assets;
 use super::tsc::AssetsSnapshot;
 use super::tsc::TsServer;
 use super::urls;
+use super::urls::LspClientUrl;
 use crate::args::get_root_cert_store;
 use crate::args::package_json;
 use crate::args::resolve_import_map_from_specifier;
@@ -338,7 +339,10 @@ impl LanguageServer {
             match &ls.config.workspace_folders {
               Some(entry) => {
                 for (specifier, folder) in entry {
-                  specifiers.insert(specifier.clone(), folder.uri.clone());
+                  specifiers.insert(
+                    specifier.clone(),
+                    LspClientUrl::new(folder.uri.clone()),
+                  );
                 }
               }
               None => {
@@ -2868,9 +2872,10 @@ impl tower_lsp::LanguageServer for LanguageServer {
     let (client, client_uri, specifier, had_specifier_settings) = {
       let mut inner = self.0.write().await;
       let client = inner.client.clone();
-      let client_uri = params.text_document.uri.clone();
+      let client_uri = LspClientUrl::new(params.text_document.uri.clone());
-      let specifier =
+      let specifier = inner
-        inner.url_map.normalize_url(&client_uri, LspUrlKind::File);
+        .url_map
+        .normalize_url(client_uri.as_url(), LspUrlKind::File);
       let document = inner.did_open(&specifier, params).await;
       let has_specifier_settings =
         inner.config.has_specifier_settings(&specifier);

cli/lsp/tsc.rs

@@ -14,6 +14,7 @@ use super::refactor::EXTRACT_TYPE;
 use super::semantic_tokens;
 use super::semantic_tokens::SemanticTokensBuilder;
 use super::text::LineIndex;
+use super::urls::LspClientUrl;
 use super::urls::LspUrlMap;
 use super::urls::INVALID_SPECIFIER;
 
@@ -36,7 +37,6 @@ use deno_core::serde::Serialize;
 use deno_core::serde_json;
 use deno_core::serde_json::json;
 use deno_core::serde_json::Value;
-use deno_core::url::Url;
 use deno_core::JsRuntime;
 use deno_core::ModuleSpecifier;
 use deno_core::OpState;
@@ -842,7 +842,7 @@ impl DocumentSpan {
       };
     let link = lsp::LocationLink {
       origin_selection_range,
-      target_uri,
+      target_uri: target_uri.into_url(),
       target_range,
       target_selection_range,
     };
@@ -864,7 +864,8 @@ impl DocumentSpan {
     let mut target = language_server
       .url_map
       .normalize_specifier(&specifier)
-      .ok()?;
+      .ok()?
+      .into_url();
     target.set_fragment(Some(&format!(
       "L{},{}",
       range.start.line + 1,
@@ -915,7 +916,10 @@ impl NavigateToItem {
       .normalize_specifier(&specifier)
       .ok()?;
     let range = self.text_span.to_range(line_index);
-    let location = lsp::Location { uri, range };
+    let location = lsp::Location {
+      uri: uri.into_url(),
+      range,
+    };
 
     let mut tags: Option<Vec<lsp::SymbolTag>> = None;
     let kind_modifiers = parse_kind_modifier(&self.kind_modifiers);
@@ -1160,9 +1164,11 @@ impl ImplementationLocation {
     let uri = language_server
       .url_map
       .normalize_specifier(&specifier)
-      .unwrap_or_else(|_| ModuleSpecifier::parse("deno://invalid").unwrap());
+      .unwrap_or_else(|_| {
+        LspClientUrl::new(ModuleSpecifier::parse("deno://invalid").unwrap())
+      });
     lsp::Location {
-      uri,
+      uri: uri.into_url(),
       range: self.document_span.text_span.to_range(line_index),
     }
   }
@@ -1196,8 +1202,10 @@ impl RenameLocations {
     new_name: &str,
     language_server: &language_server::Inner,
   ) -> Result<lsp::WorkspaceEdit, AnyError> {
-    let mut text_document_edit_map: HashMap<Url, lsp::TextDocumentEdit> =
+    let mut text_document_edit_map: HashMap<
-      HashMap::new();
+      LspClientUrl,
+      lsp::TextDocumentEdit,
+    > = HashMap::new();
     for location in self.locations.iter() {
       let specifier = normalize_specifier(&location.document_span.file_name)?;
       let uri = language_server.url_map.normalize_specifier(&specifier)?;
@@ -1209,7 +1217,7 @@ impl RenameLocations {
           uri.clone(),
           lsp::TextDocumentEdit {
             text_document: lsp::OptionalVersionedTextDocumentIdentifier {
-              uri: uri.clone(),
+              uri: uri.as_url().clone(),
               version: asset_or_doc.document_lsp_version(),
             },
             edits:
@@ -1698,9 +1706,9 @@ impl ReferenceEntry {
       .unwrap_or_else(|_| INVALID_SPECIFIER.clone());
     let uri = url_map
       .normalize_specifier(&specifier)
-      .unwrap_or_else(|_| INVALID_SPECIFIER.clone());
+      .unwrap_or_else(|_| LspClientUrl::new(INVALID_SPECIFIER.clone()));
     lsp::Location {
-      uri,
+      uri: uri.into_url(),
       range: self.document_span.text_span.to_range(line_index),
     }
   }
@@ -1748,11 +1756,11 @@ impl CallHierarchyItem {
     let uri = language_server
       .url_map
       .normalize_specifier(&target_specifier)
-      .unwrap_or_else(|_| INVALID_SPECIFIER.clone());
+      .unwrap_or_else(|_| LspClientUrl::new(INVALID_SPECIFIER.clone()));
 
     let use_file_name = self.is_source_file_item();
-    let maybe_file_path = if uri.scheme() == "file" {
+    let maybe_file_path = if uri.as_url().scheme() == "file" {
-      specifier_to_file_path(&uri).ok()
+      specifier_to_file_path(uri.as_url()).ok()
     } else {
       None
     };
@@ -1760,7 +1768,7 @@ impl CallHierarchyItem {
       if let Some(file_path) = maybe_file_path.as_ref() {
         file_path.file_name().unwrap().to_string_lossy().to_string()
       } else {
-        uri.to_string()
+        uri.as_str().to_string()
       }
     } else {
       self.name.clone()
@@ -1796,7 +1804,7 @@ impl CallHierarchyItem {
     lsp::CallHierarchyItem {
       name,
       tags,
-      uri,
+      uri: uri.into_url(),
       detail: Some(detail),
       kind: self.kind.clone().into(),
       range: self.span.to_range(line_index.clone()),

cli/lsp/urls.rs

@@ -59,19 +59,50 @@ fn hash_data_specifier(specifier: &ModuleSpecifier) -> String {
   crate::util::checksum::gen(&[file_name_str.as_bytes()])
 }
 
+/// This exists to make it a little bit harder to accidentally use a `Url`
+/// in the wrong place where a client url should be used.
+#[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
+pub struct LspClientUrl(Url);
+
+impl LspClientUrl {
+  pub fn new(url: Url) -> Self {
+    Self(url)
+  }
+
+  pub fn as_url(&self) -> &Url {
+    &self.0
+  }
+
+  pub fn into_url(self) -> Url {
+    self.0
+  }
+
+  pub fn as_str(&self) -> &str {
+    self.0.as_str()
+  }
+}
+
+impl std::fmt::Display for LspClientUrl {
+  fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+    self.0.fmt(f)
+  }
+}
+
 #[derive(Debug, Default)]
 struct LspUrlMapInner {
-  specifier_to_url: HashMap<ModuleSpecifier, Url>,
+  specifier_to_url: HashMap<ModuleSpecifier, LspClientUrl>,
   url_to_specifier: HashMap<Url, ModuleSpecifier>,
 }
 
 impl LspUrlMapInner {
-  fn put(&mut self, specifier: ModuleSpecifier, url: Url) {
+  fn put(&mut self, specifier: ModuleSpecifier, url: LspClientUrl) {
-    self.specifier_to_url.insert(specifier.clone(), url.clone());
+    self
-    self.url_to_specifier.insert(url, specifier);
+      .url_to_specifier
+      .insert(url.as_url().clone(), specifier.clone());
+    self.specifier_to_url.insert(specifier, url);
   }
 
-  fn get_url(&self, specifier: &ModuleSpecifier) -> Option<&Url> {
+  fn get_url(&self, specifier: &ModuleSpecifier) -> Option<&LspClientUrl> {
     self.specifier_to_url.get(specifier)
   }
 
@@ -98,13 +129,13 @@ impl LspUrlMap {
   pub fn normalize_specifier(
     &self,
     specifier: &ModuleSpecifier,
-  ) -> Result<Url, AnyError> {
+  ) -> Result<LspClientUrl, AnyError> {
     let mut inner = self.0.lock();
     if let Some(url) = inner.get_url(specifier).cloned() {
       Ok(url)
     } else {
       let url = if specifier.scheme() == "file" {
-        specifier.clone()
+        LspClientUrl(specifier.clone())
       } else {
         let specifier_str = if specifier.scheme() == "asset" {
           format!("deno:/asset{}", specifier.path())
@@ -136,7 +167,7 @@ impl LspUrlMap {
           path.push_str(&parts.join("/"));
           format!("deno:/{path}")
         };
-        let url = Url::parse(&specifier_str)?;
+        let url = LspClientUrl(Url::parse(&specifier_str)?);
         inner.put(specifier.clone(), url.clone());
         url
       };
@@ -165,7 +196,7 @@ impl LspUrlMap {
       } else {
         url.clone()
       };
-      inner.put(specifier.clone(), url.clone());
+      inner.put(specifier.clone(), LspClientUrl(url.clone()));
       specifier
     }
   }
@@ -195,9 +226,10 @@ mod tests {
       .expect("could not handle specifier");
     let expected_url =
       Url::parse("deno:/https/deno.land/x/pkg%401.0.0/mod.ts").unwrap();
-    assert_eq!(actual_url, expected_url);
+    assert_eq!(actual_url.as_url(), &expected_url);
 
-    let actual_specifier = map.normalize_url(&actual_url, LspUrlKind::File);
+    let actual_specifier =
+      map.normalize_url(actual_url.as_url(), LspUrlKind::File);
     assert_eq!(actual_specifier, fixture);
   }
 
@@ -210,9 +242,10 @@ mod tests {
       .normalize_specifier(&fixture)
       .expect("could not handle specifier");
     let expected_url = Url::parse("deno:/https/cdn.skypack.dev/-/postcss%40v8.2.9-E4SktPp9c0AtxrJHp8iV/dist%3Des2020%2Cmode%3Dtypes/lib/postcss.d.ts").unwrap();
-    assert_eq!(actual_url, expected_url);
+    assert_eq!(actual_url.as_url(), &expected_url);
 
-    let actual_specifier = map.normalize_url(&actual_url, LspUrlKind::File);
+    let actual_specifier =
+      map.normalize_url(actual_url.as_url(), LspUrlKind::File);
     assert_eq!(actual_specifier, fixture);
   }
 
@@ -224,9 +257,10 @@ mod tests {
       .normalize_specifier(&fixture)
       .expect("could not handle specifier");
     let expected_url = Url::parse("deno:/c21c7fc382b2b0553dc0864aa81a3acacfb7b3d1285ab5ae76da6abec213fb37/data_url.ts").unwrap();
-    assert_eq!(actual_url, expected_url);
+    assert_eq!(actual_url.as_url(), &expected_url);
 
-    let actual_specifier = map.normalize_url(&actual_url, LspUrlKind::File);
+    let actual_specifier =
+      map.normalize_url(actual_url.as_url(), LspUrlKind::File);
     assert_eq!(actual_specifier, fixture);
   }