From 6865b45dadb2408c0bd13b7ef305680fa52e5d17 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 2 Aug 2023 15:09:58 -0400 Subject: [PATCH 1/2] feat(unstable/lsp): support navigating to deno_modules folder --- cli/cache/http_cache/local.rs | 421 ++++++++++++++++++++++++++++++---- cli/cache/http_cache/mod.rs | 1 + cli/cache/mod.rs | 1 + cli/lsp/client.rs | 7 +- cli/lsp/diagnostics.rs | 38 ++- cli/lsp/documents.rs | 9 +- cli/lsp/language_server.rs | 21 +- cli/lsp/tsc.rs | 11 +- cli/lsp/urls.rs | 33 ++- 9 files changed, 470 insertions(+), 72 deletions(-) diff --git a/cli/cache/http_cache/local.rs b/cli/cache/http_cache/local.rs index 016118c3fe7975..4603545577828e 100644 --- a/cli/cache/http_cache/local.rs +++ b/cli/cache/http_cache/local.rs @@ -11,12 +11,9 @@ use std::time::SystemTime; use deno_ast::MediaType; use deno_core::error::AnyError; use deno_core::parking_lot::RwLock; -use deno_core::serde_json; use deno_core::url::Url; use indexmap::IndexMap; use once_cell::sync::Lazy; -use serde::Deserialize; -use serde::Serialize; use crate::cache::CACHE_PERM; use crate::http_util::HeadersMap; @@ -31,6 +28,129 @@ use super::CachedUrlMetadata; use super::HttpCache; use super::HttpCacheItemKey; +/// A deno_modules http cache for the lsp that provides functionality +/// for doing a reverse mapping. +#[derive(Debug)] +pub struct LocalLspHttpCache { + cache: LocalHttpCache, +} + +impl LocalLspHttpCache { + pub fn new(path: PathBuf, global_cache: Arc) -> Self { + assert!(path.is_absolute()); + let manifest = LocalCacheManifest::new_for_lsp(path.join("manifest.json")); + Self { + cache: LocalHttpCache { + path, + manifest, + global_cache, + }, + } + } + + pub fn get_file_url(&self, url: &Url) -> Option { + { + let data = self.cache.manifest.data.read(); + if let Some(data) = data.get(url) { + if let Some(path) = &data.path { + return Url::from_file_path(self.cache.path.join(path)).ok(); + } + } + } + match self.cache.get_cache_filepath(url, &Default::default()) { + Ok(path) if path.exists() => Url::from_file_path(path).ok(), + _ => None, + } + } + + pub fn get_remote_url(&self, path: &Path) -> Option { + let Ok(path) = path.strip_prefix(&self.cache.path) else { + return None; // not in this directory + }; + let has_hashed_component = path + .components() + .any(|p| p.as_os_str().to_string_lossy().starts_with('#')); + if has_hashed_component { + // check in the manifest + { + let data = self.cache.manifest.data.read(); + if let Some(url) = data.get_reverse_mapping(path) { + return Some(url); + } + } + None + } else { + // we can work backwards from the path to the url + let mut parts = Vec::new(); + for (i, part) in path.components().enumerate() { + let part = part.as_os_str().to_string_lossy(); + if i == 0 { + let mut result = String::new(); + let part = if let Some(part) = part.strip_prefix("http_") { + result.push_str("http://"); + part + } else { + result.push_str("https://"); + &part + }; + if let Some((domain, port)) = part.rsplit_once('_') { + result.push_str(&format!("{}:{}", domain, port)); + } else { + result.push_str(part); + } + parts.push(result); + } else { + parts.push(part.to_string()); + } + } + Url::parse(&parts.join("/")).ok() + } + } +} + +impl HttpCache for LocalLspHttpCache { + fn cache_item_key<'a>( + &self, + url: &'a Url, + ) -> Result, AnyError> { + self.cache.cache_item_key(url) + } + + fn contains(&self, url: &Url) -> bool { + self.cache.contains(url) + } + + fn set( + &self, + url: &Url, + headers: HeadersMap, + content: &[u8], + ) -> Result<(), AnyError> { + self.cache.set(url, headers, content) + } + + fn read_modified_time( + &self, + key: &HttpCacheItemKey, + ) -> Result, AnyError> { + self.cache.read_modified_time(key) + } + + fn read_file_bytes( + &self, + key: &HttpCacheItemKey, + ) -> Result>, AnyError> { + self.cache.read_file_bytes(key) + } + + fn read_metadata( + &self, + key: &HttpCacheItemKey, + ) -> Result, AnyError> { + self.cache.read_metadata(key) + } +} + #[derive(Debug)] pub struct LocalHttpCache { path: PathBuf, @@ -181,7 +301,7 @@ impl HttpCache for LocalHttpCache { } } -struct LocalCacheSubPath { +pub(super) struct LocalCacheSubPath { pub has_hash: bool, pub parts: Vec, } @@ -194,6 +314,14 @@ impl LocalCacheSubPath { } path } + + pub fn as_relative_path(&self) -> PathBuf { + let mut path = PathBuf::with_capacity(self.parts.len()); + for part in &self.parts { + path.push(part); + } + path + } } fn url_to_local_sub_path( @@ -335,48 +463,28 @@ fn url_to_local_sub_path( Ok(LocalCacheSubPath { has_hash, parts }) } -#[derive(Debug, Default, Clone)] -struct LocalCacheManifestData { - serialized: SerializedLocalCacheManifestData, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -struct SerializedLocalCacheManifestDataModule { - #[serde(skip_serializing_if = "Option::is_none")] - pub path: Option, - #[serde( - default = "IndexMap::new", - skip_serializing_if = "IndexMap::is_empty" - )] - pub headers: IndexMap, -} - -#[derive(Debug, Default, Clone, Serialize, Deserialize)] -struct SerializedLocalCacheManifestData { - pub modules: IndexMap, -} - #[derive(Debug)] struct LocalCacheManifest { file_path: PathBuf, - data: RwLock, + data: RwLock, } impl LocalCacheManifest { pub fn new(file_path: PathBuf) -> Self { - let serialized: SerializedLocalCacheManifestData = - std::fs::read(&file_path) - .ok() - .and_then(|data| match serde_json::from_slice(&data) { - Ok(data) => Some(data), - Err(err) => { - log::debug!("Failed deserializing local cache manifest: {:#}", err); - None - } - }) - .unwrap_or_default(); + Self::new_internal(file_path, false) + } + + pub fn new_for_lsp(file_path: PathBuf) -> Self { + Self::new_internal(file_path, true) + } + + fn new_internal(file_path: PathBuf, use_reverse_mapping: bool) -> Self { + let text = std::fs::read_to_string(&file_path).ok(); Self { - data: RwLock::new(LocalCacheManifestData { serialized }), + data: RwLock::new(manifest::LocalCacheManifestData::new( + text.as_deref(), + use_reverse_mapping, + )), file_path, } } @@ -419,9 +527,9 @@ impl LocalCacheManifest { let mut data = self.data.write(); let is_empty = headers_subset.is_empty() && !sub_path.has_hash; let has_changed = if is_empty { - data.serialized.modules.remove(&url).is_some() + data.remove(&url, &sub_path) } else { - let new_data = SerializedLocalCacheManifestDataModule { + let new_data = manifest::SerializedLocalCacheManifestDataModule { path: if headers_subset.contains_key("location") { None } else { @@ -429,10 +537,10 @@ impl LocalCacheManifest { }, headers: headers_subset, }; - if data.serialized.modules.get(&url) == Some(&new_data) { + if data.get(&url) == Some(&new_data) { false } else { - data.serialized.modules.insert(url.clone(), new_data); + data.insert(url.clone(), &sub_path, new_data); true } }; @@ -440,11 +548,8 @@ impl LocalCacheManifest { if has_changed { // don't bother ensuring the directory here because it will // eventually be created by files being added to the cache - let result = atomic_write_file( - &self.file_path, - serde_json::to_string_pretty(&data.serialized).unwrap(), - CACHE_PERM, - ); + let result = + atomic_write_file(&self.file_path, data.as_json(), CACHE_PERM); if let Err(err) = result { log::debug!("Failed saving local cache manifest: {:#}", err); } @@ -453,7 +558,7 @@ impl LocalCacheManifest { pub fn get_metadata(&self, url: &Url) -> Option { let data = self.data.read(); - match data.serialized.modules.get(url) { + match data.get(url) { Some(module) => { let headers = module .headers @@ -500,6 +605,126 @@ impl LocalCacheManifest { } } +// This is in a separate module in order to enforce keeping +// the internal implementation private. +mod manifest { + use std::collections::HashMap; + use std::path::Path; + use std::path::PathBuf; + + use deno_core::serde_json; + use deno_core::url::Url; + use indexmap::IndexMap; + use serde::Deserialize; + use serde::Serialize; + + use super::LocalCacheSubPath; + + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] + pub struct SerializedLocalCacheManifestDataModule { + #[serde(skip_serializing_if = "Option::is_none")] + pub path: Option, + #[serde( + default = "IndexMap::new", + skip_serializing_if = "IndexMap::is_empty" + )] + pub headers: IndexMap, + } + + #[derive(Debug, Default, Clone, Serialize, Deserialize)] + struct SerializedLocalCacheManifestData { + pub modules: IndexMap, + } + + #[derive(Debug, Default, Clone)] + pub(super) struct LocalCacheManifestData { + serialized: SerializedLocalCacheManifestData, + // reverse mapping used in the lsp + reverse_mapping: Option>, + } + + impl LocalCacheManifestData { + pub fn new(maybe_text: Option<&str>, use_reverse_mapping: bool) -> Self { + let serialized: SerializedLocalCacheManifestData = maybe_text + .and_then(|text| match serde_json::from_str(text) { + Ok(data) => Some(data), + Err(err) => { + log::debug!("Failed deserializing local cache manifest: {:#}", err); + None + } + }) + .unwrap_or_default(); + let reverse_mapping = if use_reverse_mapping { + Some( + serialized + .modules + .iter() + .filter_map(|(url, module)| { + module.path.as_ref().map(|path| { + let path = if cfg!(windows) { + PathBuf::from(path.split('/').collect::>().join("\\")) + } else { + PathBuf::from(path) + }; + (path, url.clone()) + }) + }) + .collect::>(), + ) + } else { + None + }; + Self { + serialized, + reverse_mapping, + } + } + + pub fn get( + &self, + url: &Url, + ) -> Option<&SerializedLocalCacheManifestDataModule> { + self.serialized.modules.get(url) + } + + pub fn get_reverse_mapping(&self, path: &Path) -> Option { + debug_assert!(self.reverse_mapping.is_some()); // only call this if you're in the lsp + self + .reverse_mapping + .as_ref() + .and_then(|mapping| mapping.get(path)) + .cloned() + } + + pub fn insert( + &mut self, + url: Url, + sub_path: &LocalCacheSubPath, + new_data: SerializedLocalCacheManifestDataModule, + ) { + if let Some(reverse_mapping) = &mut self.reverse_mapping { + reverse_mapping.insert(sub_path.as_relative_path(), url.clone()); + } + self.serialized.modules.insert(url, new_data); + } + + pub fn remove(&mut self, url: &Url, sub_path: &LocalCacheSubPath) -> bool { + if self.serialized.modules.remove(url).is_some() { + if let Some(reverse_mapping) = &mut self.reverse_mapping { + reverse_mapping.remove(&sub_path.as_relative_path()); + } + true + } else { + false + } + } + + pub fn as_json(&self) -> String { + serde_json::to_string_pretty(&self.serialized).unwrap() + } + } +} + #[cfg(test)] mod test { use super::*; @@ -869,4 +1094,104 @@ mod test { ); } } + + #[test] + fn test_lsp_local_cache() { + let temp_dir = TempDir::new(); + let global_cache_path = temp_dir.path().join("global"); + let local_cache_path = temp_dir.path().join("local"); + let global_cache = + Arc::new(GlobalHttpCache::new(global_cache_path.to_path_buf())); + let local_cache = LocalLspHttpCache::new( + local_cache_path.to_path_buf(), + global_cache.clone(), + ); + + // mapped url + { + let url = Url::parse("https://deno.land/x/mod.ts").unwrap(); + let content = "export const test = 5;"; + global_cache + .set( + &url, + HashMap::from([( + "content-type".to_string(), + "application/typescript".to_string(), + )]), + content.as_bytes(), + ) + .unwrap(); + let key = local_cache.cache_item_key(&url).unwrap(); + assert_eq!( + String::from_utf8(local_cache.read_file_bytes(&key).unwrap().unwrap()) + .unwrap(), + content + ); + + // check getting the file url works + let file_url = local_cache.get_file_url(&url); + let expected = local_cache_path.uri().join("deno.land/x/mod.ts").unwrap(); + assert_eq!(file_url, Some(expected)); + + // get the reverse mapping + let mapping = local_cache.get_remote_url( + local_cache_path + .join("deno.land") + .join("x") + .join("mod.ts") + .as_path(), + ); + assert_eq!(mapping.as_ref(), Some(&url)); + } + + // now try a file with a different content-type header + { + let url = + Url::parse("https://deno.land/x/different_content_type.ts").unwrap(); + let content = "export const test = 5;"; + global_cache + .set( + &url, + HashMap::from([( + "content-type".to_string(), + "application/javascript".to_string(), + )]), + content.as_bytes(), + ) + .unwrap(); + let key = local_cache.cache_item_key(&url).unwrap(); + assert_eq!( + String::from_utf8(local_cache.read_file_bytes(&key).unwrap().unwrap()) + .unwrap(), + content + ); + + let file_url = local_cache.get_file_url(&url).unwrap(); + let path = file_url.to_file_path().unwrap(); + assert!(path.exists()); + let mapping = local_cache.get_remote_url(&path); + assert_eq!(mapping.as_ref(), Some(&url)); + } + + // try an http specifier that can't be mapped to the file system + { + let url = Url::parse("http://deno.land/INVALID/Module.ts?dev").unwrap(); + let content = "export const test = 5;"; + global_cache + .set(&url, HashMap::new(), content.as_bytes()) + .unwrap(); + let key = local_cache.cache_item_key(&url).unwrap(); + assert_eq!( + String::from_utf8(local_cache.read_file_bytes(&key).unwrap().unwrap()) + .unwrap(), + content + ); + + let file_url = local_cache.get_file_url(&url).unwrap(); + let path = file_url.to_file_path().unwrap(); + assert!(path.exists()); + let mapping = local_cache.get_remote_url(&path); + assert_eq!(mapping.as_ref(), Some(&url)); + } + } } diff --git a/cli/cache/http_cache/mod.rs b/cli/cache/http_cache/mod.rs index eb5c38bbdd4a4e..8d09b0995fa5f0 100644 --- a/cli/cache/http_cache/mod.rs +++ b/cli/cache/http_cache/mod.rs @@ -16,6 +16,7 @@ mod local; pub use global::url_to_filename; pub use global::GlobalHttpCache; pub use local::LocalHttpCache; +pub use local::LocalLspHttpCache; /// Cached metadata about a url. #[derive(Debug, Serialize, Deserialize, PartialEq, Eq)] diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs index 7903a9665e6b96..2427133d1254f1 100644 --- a/cli/cache/mod.rs +++ b/cli/cache/mod.rs @@ -38,6 +38,7 @@ pub use http_cache::CachedUrlMetadata; pub use http_cache::GlobalHttpCache; pub use http_cache::HttpCache; pub use http_cache::LocalHttpCache; +pub use http_cache::LocalLspHttpCache; pub use incremental::IncrementalCache; pub use node::NodeAnalysisCache; pub use parsed_source::ParsedSourceCache; diff --git a/cli/lsp/client.rs b/cli/lsp/client.rs index 5f1a7fcef26466..13347a77245c21 100644 --- a/cli/lsp/client.rs +++ b/cli/lsp/client.rs @@ -154,11 +154,14 @@ impl OutsideLockClient { pub async fn publish_diagnostics( &self, - uri: lsp::Url, + uri: LspClientUrl, diags: Vec, version: Option, ) { - self.0.publish_diagnostics(uri, diags, version).await; + self + .0 + .publish_diagnostics(uri.into_url(), diags, version) + .await; } } diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 7f65c09484906c..88c4c91cb6a776 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -12,6 +12,8 @@ use super::language_server::StateSnapshot; use super::performance::Performance; use super::tsc; use super::tsc::TsServer; +use super::urls::LspClientUrl; +use super::urls::LspUrlMap; use crate::args::LintOptions; use crate::graph_util; @@ -54,6 +56,7 @@ pub struct DiagnosticServerUpdateMessage { pub snapshot: Arc, pub config: Arc, pub lint_options: LintOptions, + pub url_map: LspUrlMap, } struct DiagnosticRecord { @@ -107,6 +110,7 @@ impl DiagnosticsPublisher { &self, source: DiagnosticSource, diagnostics: DiagnosticVec, + url_map: &LspUrlMap, token: &CancellationToken, ) -> usize { let mut diagnostics_by_specifier = @@ -141,7 +145,9 @@ impl DiagnosticsPublisher { .client .when_outside_lsp_lock() .publish_diagnostics( - record.specifier, + url_map + .normalize_specifier(&record.specifier) + .unwrap_or(LspClientUrl::new(record.specifier)), all_specifier_diagnostics, version, ) @@ -169,7 +175,9 @@ impl DiagnosticsPublisher { .client .when_outside_lsp_lock() .publish_diagnostics( - specifier.clone(), + url_map + .normalize_specifier(specifier) + .unwrap_or_else(|_| LspClientUrl::new(specifier.clone())), Vec::new(), removed_value.version, ) @@ -366,9 +374,11 @@ impl DiagnosticsServer { snapshot, config, lint_options, + url_map, }, batch_index, } = message; + let url_map = Arc::new(url_map); // cancel the previous run token.cancel(); @@ -383,6 +393,7 @@ impl DiagnosticsServer { let ts_diagnostics_store = ts_diagnostics_store.clone(); let snapshot = snapshot.clone(); let config = config.clone(); + let url_map = url_map.clone(); async move { if let Some(previous_handle) = previous_ts_handle { // Wait on the previous run to complete in order to prevent @@ -419,7 +430,12 @@ impl DiagnosticsServer { if !token.is_cancelled() { ts_diagnostics_store.update(&diagnostics); messages_len = diagnostics_publisher - .publish(DiagnosticSource::Ts, diagnostics, &token) + .publish( + DiagnosticSource::Ts, + diagnostics, + &url_map, + &token, + ) .await; if !token.is_cancelled() { @@ -447,6 +463,7 @@ impl DiagnosticsServer { let token = token.clone(); let snapshot = snapshot.clone(); let config = config.clone(); + let url_map = url_map.clone(); async move { if let Some(previous_handle) = previous_deps_handle { previous_handle.await; @@ -463,7 +480,12 @@ impl DiagnosticsServer { let mut messages_len = 0; if !token.is_cancelled() { messages_len = diagnostics_publisher - .publish(DiagnosticSource::Deno, diagnostics, &token) + .publish( + DiagnosticSource::Deno, + diagnostics, + &url_map, + &token, + ) .await; if !token.is_cancelled() { @@ -491,6 +513,7 @@ impl DiagnosticsServer { let token = token.clone(); let snapshot = snapshot.clone(); let config = config.clone(); + let url_map = url_map.clone(); async move { if let Some(previous_handle) = previous_lint_handle { previous_handle.await; @@ -514,7 +537,12 @@ impl DiagnosticsServer { let mut messages_len = 0; if !token.is_cancelled() { messages_len = diagnostics_publisher - .publish(DiagnosticSource::Lint, diagnostics, &token) + .publish( + DiagnosticSource::Lint, + diagnostics, + &url_map, + &token, + ) .await; if !token.is_cancelled() { diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index c39c81a4198dc7..dbaecb95c0ed8d 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -969,6 +969,13 @@ impl Documents { } } + pub fn resolve_redirected( + &self, + specifier: &ModuleSpecifier, + ) -> Option { + self.specifier_resolver.resolve(specifier) + } + /// Return `true` if the specifier can be resolved to a document. pub fn exists(&self, specifier: &ModuleSpecifier) -> bool { let specifier = self.specifier_resolver.resolve(specifier); @@ -1498,7 +1505,7 @@ impl Documents { self.resolve_dependency(specifier, maybe_node_resolver) } else { let media_type = doc.media_type(); - Some((specifier.clone(), media_type)) + Some((doc.specifier().clone(), media_type)) } } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index a6f686ed2d367b..7b863ff0165d3e 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -88,7 +88,7 @@ use crate::cache::DenoDir; use crate::cache::FastInsecureHasher; use crate::cache::GlobalHttpCache; use crate::cache::HttpCache; -use crate::cache::LocalHttpCache; +use crate::cache::LocalLspHttpCache; use crate::factory::CliFactory; use crate::file_fetcher::FileFetcher; use crate::graph_util; @@ -204,7 +204,7 @@ pub struct Inner { /// An abstraction that handles interactions with TypeScript. pub ts_server: Arc, /// A map of specifiers and URLs used to translate over the LSP. - pub url_map: Arc, + pub url_map: urls::LspUrlMap, } impl LanguageServer { @@ -905,16 +905,18 @@ impl Inner { self.module_registries_location = module_registries_location; // update the cache path let global_cache = Arc::new(GlobalHttpCache::new(dir.deps_folder_path())); - let cache: Arc = - match self.config.maybe_deno_modules_dir_path() { - Some(local_path) => { - Arc::new(LocalHttpCache::new(local_path, global_cache)) - } - None => global_cache, - }; + let maybe_local_cache = + self.config.maybe_deno_modules_dir_path().map(|local_path| { + Arc::new(LocalLspHttpCache::new(local_path, global_cache.clone())) + }); + let cache: Arc = maybe_local_cache + .clone() + .map(|c| c as Arc) + .unwrap_or(global_cache); self.deps_http_cache = cache.clone(); self.documents.set_cache(cache.clone()); self.cache_metadata.set_cache(cache); + self.url_map.set_cache(maybe_local_cache); self.maybe_global_cache_path = new_cache_path; Ok(()) } @@ -2946,6 +2948,7 @@ impl Inner { snapshot: self.snapshot(), config: self.config.snapshot(), lint_options: self.lint_options.clone(), + url_map: self.url_map.clone(), }; if let Err(err) = self.diagnostics_server.update(snapshot) { error!("Cannot update diagnostics: {}", err); diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index b26fa57bcbe759..6c0095b29c6f6d 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -3147,6 +3147,7 @@ fn op_resolve( let state = state.borrow_mut::(); let mark = state.performance.mark("op_resolve", Some(&args)); let referrer = state.normalize_specifier(&args.base)?; + eprintln!("RESOLVING from {}: {:?}", args.base, args.specifiers); let result = match state.get_asset_or_document(&referrer) { Some(referrer_doc) => { let resolved = state.state_snapshot.documents.resolve( @@ -3214,9 +3215,13 @@ fn op_script_names(state: &mut OpState) -> Vec { .filter_map(|dep| dep.get_type().or_else(|| dep.get_code())), ); for specifier in specifiers { - if seen.insert(specifier.as_str()) && documents.exists(specifier) { - // only include dependencies we know to exist otherwise typescript will error - result.push(specifier.to_string()); + if seen.insert(specifier.as_str()) { + if let Some(specifier) = documents.resolve_redirected(specifier) { + if documents.exists(&specifier) { + // only include dependencies we know to exist otherwise typescript will error + result.push(specifier.to_string()); + } + } } } } diff --git a/cli/lsp/urls.rs b/cli/lsp/urls.rs index ee9684f6483548..8b16292e9bdfcd 100644 --- a/cli/lsp/urls.rs +++ b/cli/lsp/urls.rs @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use crate::cache::LocalLspHttpCache; use crate::file_fetcher::map_content_type; use data_url::DataUrl; @@ -12,6 +13,7 @@ use deno_core::url::Url; use deno_core::ModuleSpecifier; use once_cell::sync::Lazy; use std::collections::HashMap; +use std::sync::Arc; /// Used in situations where a default URL needs to be used where otherwise a /// panic is undesired. @@ -119,17 +121,31 @@ pub enum LspUrlKind { /// A bi-directional map of URLs sent to the LSP client and internal module /// specifiers. We need to map internal specifiers into `deno:` schema URLs /// to allow the Deno language server to manage these as virtual documents. -#[derive(Debug, Default)] -pub struct LspUrlMap(Mutex); +#[derive(Debug, Default, Clone)] +pub struct LspUrlMap { + local_http_cache: Option>, + inner: Arc>, +} impl LspUrlMap { + pub fn set_cache(&mut self, http_cache: Option>) { + self.local_http_cache = http_cache; + } + /// Normalize a specifier that is used internally within Deno (or tsc) to a /// URL that can be handled as a "virtual" document by an LSP client. pub fn normalize_specifier( &self, specifier: &ModuleSpecifier, ) -> Result { - let mut inner = self.0.lock(); + if let Some(cache) = &self.local_http_cache { + if matches!(specifier.scheme(), "http" | "https") { + if let Some(file_url) = cache.get_file_url(specifier) { + return Ok(LspClientUrl(file_url)); + } + } + } + let mut inner = self.inner.lock(); if let Some(url) = inner.get_url(specifier).cloned() { Ok(url) } else { @@ -183,7 +199,16 @@ impl LspUrlMap { /// so we need to force it to in the mapping and nee to explicitly state whether /// this is a file or directory url. pub fn normalize_url(&self, url: &Url, kind: LspUrlKind) -> ModuleSpecifier { - let mut inner = self.0.lock(); + if let Some(cache) = &self.local_http_cache { + if url.scheme() == "file" { + if let Ok(path) = url.to_file_path() { + if let Some(remote_url) = cache.get_remote_url(&path) { + return remote_url; + } + } + } + } + let mut inner = self.inner.lock(); if let Some(specifier) = inner.get_specifier(url).cloned() { specifier } else { From c420de017d0d0426a129cc95361f1b45c80a6262 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 2 Aug 2023 15:55:37 -0400 Subject: [PATCH 2/2] Update lsp test. --- cli/cache/http_cache/local.rs | 5 +- cli/lsp/tsc.rs | 3 +- cli/tests/integration/lsp_tests.rs | 116 +++++++++++++++++++++++++++-- test_util/src/fs.rs | 6 +- 4 files changed, 119 insertions(+), 11 deletions(-) diff --git a/cli/cache/http_cache/local.rs b/cli/cache/http_cache/local.rs index 4603545577828e..833c6e9357a4ca 100644 --- a/cli/cache/http_cache/local.rs +++ b/cli/cache/http_cache/local.rs @@ -1130,7 +1130,10 @@ mod test { // check getting the file url works let file_url = local_cache.get_file_url(&url); - let expected = local_cache_path.uri().join("deno.land/x/mod.ts").unwrap(); + let expected = local_cache_path + .uri_dir() + .join("deno.land/x/mod.ts") + .unwrap(); assert_eq!(file_url, Some(expected)); // get the reverse mapping diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 6c0095b29c6f6d..3538665130778d 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -3147,7 +3147,6 @@ fn op_resolve( let state = state.borrow_mut::(); let mark = state.performance.mark("op_resolve", Some(&args)); let referrer = state.normalize_specifier(&args.base)?; - eprintln!("RESOLVING from {}: {:?}", args.base, args.specifiers); let result = match state.get_asset_or_document(&referrer) { Some(referrer_doc) => { let resolved = state.state_snapshot.documents.resolve( @@ -3217,8 +3216,8 @@ fn op_script_names(state: &mut OpState) -> Vec { for specifier in specifiers { if seen.insert(specifier.as_str()) { if let Some(specifier) = documents.resolve_redirected(specifier) { + // only include dependencies we know to exist otherwise typescript will error if documents.exists(&specifier) { - // only include dependencies we know to exist otherwise typescript will error result.push(specifier.to_string()); } } diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index a073cf30df1efc..28591d6312c90a 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -573,7 +573,7 @@ fn lsp_import_map_config_file_auto_discovered_symlink() { client.did_change_watched_files(json!({ "changes": [{ // the client will give a watched file changed event for the symlink's target - "uri": temp_dir.path().join("subdir/deno.json").canonicalize().uri(), + "uri": temp_dir.path().join("subdir/deno.json").canonicalize().uri_file(), "type": 2 }] })); @@ -601,7 +601,7 @@ fn lsp_import_map_config_file_auto_discovered_symlink() { client.did_change_watched_files(json!({ "changes": [{ // now still say that the target path has changed - "uri": temp_dir.path().join("subdir/deno.json").canonicalize().uri(), + "uri": temp_dir.path().join("subdir/deno.json").canonicalize().uri_file(), "type": 2 }] })); @@ -8777,13 +8777,13 @@ fn lsp_deno_modules_dir() { let mut client = context.new_lsp_command().build(); client.initialize_default(); - let file_uri = temp_dir.uri().join("file.ts").unwrap(); + let local_file_uri = temp_dir.uri().join("file.ts").unwrap(); client.did_open(json!({ "textDocument": { - "uri": file_uri, + "uri": local_file_uri, "languageId": "typescript", "version": 1, - "text": "import { returnsHi } from 'http://localhost:4545/subdir/mod1.ts'; console.log(returnsHi());", + "text": "import { returnsHi } from 'http://localhost:4545/subdir/mod1.ts';\nconst test: string = returnsHi();\nconsole.log(test);", } })); let cache = |client: &mut LspClient| { @@ -8791,7 +8791,7 @@ fn lsp_deno_modules_dir() { "deno/cache", json!({ "referrer": { - "uri": file_uri, + "uri": local_file_uri, }, "uris": [ { @@ -8850,11 +8850,113 @@ fn lsp_deno_modules_dir() { assert_eq!(diagnostics.all().len(), 0, "{:#?}", diagnostics); // cached // no caching necessary because it was already cached. It should exist now - assert!(temp_dir .path() .join("deno_modules/http_localhost_4545/subdir/mod1.ts") .exists()); + // the declaration should be found in the deno_modules directory + let res = client.write_request( + "textDocument/references", + json!({ + "textDocument": { + "uri": local_file_uri, + }, + "position": { "line": 0, "character": 9 }, // returnsHi + "context": { + "includeDeclaration": false + } + }), + ); + + // ensure that it's using the deno_modules directory + let references = res.as_array().unwrap(); + assert_eq!(references.len(), 2, "references: {:#?}", references); + let uri = references[1] + .as_object() + .unwrap() + .get("uri") + .unwrap() + .as_str() + .unwrap(); + let file_path = temp_dir + .path() + .join("deno_modules/http_localhost_4545/subdir/mod1.ts"); + let remote_file_uri = file_path.uri_file(); + assert_eq!(uri, remote_file_uri.as_str()); + + let file_text = file_path.read_to_string(); + let diagnostics = client.did_open(json!({ + "textDocument": { + "uri": remote_file_uri, + "languageId": "typescript", + "version": 1, + "text": file_text, + } + })); + assert_eq!(diagnostics.all(), Vec::new()); + + client.write_notification( + "textDocument/didChange", + json!({ + "textDocument": { + "uri": remote_file_uri, + "version": 2 + }, + "contentChanges": [ + { + "range": { + "start": { "line": 0, "character": 0 }, + "end": { "line": 17, "character": 0 }, + }, + "text": "export function returnsHi(): number { return new Date(); }" + } + ] + }), + ); + + let diagnostics = client.read_diagnostics(); + + assert_eq!( + json!( + diagnostics + .messages_with_file_and_source(remote_file_uri.as_str(), "deno-ts") + .diagnostics + ), + json!([ + { + "range": { + "start": { "line": 0, "character": 38 }, + "end": { "line": 0, "character": 44 } + }, + "severity": 1, + "code": 2322, + "source": "deno-ts", + "message": "Type 'Date' is not assignable to type 'number'." + } + ]), + ); + + assert_eq!( + json!( + diagnostics + .messages_with_file_and_source(local_file_uri.as_str(), "deno-ts") + .diagnostics + ), + json!([ + { + "range": { + "start": { "line": 1, "character": 6 }, + "end": { "line": 1, "character": 10 } + }, + "severity": 1, + "code": 2322, + "source": "deno-ts", + "message": "Type 'number' is not assignable to type 'string'." + } + ]), + ); + assert_eq!(diagnostics.all().len(), 2); + client.shutdown(); } diff --git a/test_util/src/fs.rs b/test_util/src/fs.rs index 57ab8ee101758b..b2f0eceac52dc4 100644 --- a/test_util/src/fs.rs +++ b/test_util/src/fs.rs @@ -47,10 +47,14 @@ impl PathRef { PathRef(self.as_path().parent().unwrap().to_path_buf()) } - pub fn uri(&self) -> Url { + pub fn uri_dir(&self) -> Url { Url::from_directory_path(self.as_path()).unwrap() } + pub fn uri_file(&self) -> Url { + Url::from_file_path(self.as_path()).unwrap() + } + pub fn as_path(&self) -> &Path { self.0.as_path() }