diff options
author | Kitson Kelly <me@kitsonkelly.com> | 2022-02-02 13:04:26 +1100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-02-02 13:04:26 +1100 |
commit | 26f5c223df26962f19ccc957d0507facc09c55c5 (patch) | |
tree | 2d12e824acefba3e35d8a117693a7b4a7f210cf3 /cli | |
parent | 8176a4d1663529fb8aeebf7734c4994fa1d583f4 (diff) |
fix(lsp): properly display x-deno-warning with redirects (#13554)
Fixes: #13472
Diffstat (limited to 'cli')
-rw-r--r-- | cli/lsp/cache.rs | 100 | ||||
-rw-r--r-- | cli/lsp/diagnostics.rs | 14 | ||||
-rw-r--r-- | cli/lsp/documents.rs | 42 | ||||
-rw-r--r-- | cli/lsp/language_server.rs | 17 | ||||
-rw-r--r-- | cli/tests/testdata/x_deno_warning_redirect.js (renamed from cli/tests/testdata/x_deno_warning.js) | 0 |
5 files changed, 135 insertions, 38 deletions
diff --git a/cli/lsp/cache.rs b/cli/lsp/cache.rs index b05dcbc38..d174907d5 100644 --- a/cli/lsp/cache.rs +++ b/cli/lsp/cache.rs @@ -5,19 +5,25 @@ use crate::cache::FetchCacher; use crate::config_file::ConfigFile; use crate::flags::Flags; use crate::graph_util::graph_valid; +use crate::http_cache; use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; use deno_core::anyhow::anyhow; use deno_core::error::AnyError; +use deno_core::parking_lot::Mutex; use deno_core::ModuleSpecifier; use deno_runtime::permissions::Permissions; use deno_runtime::tokio_util::create_basic_runtime; use import_map::ImportMap; +use std::collections::HashMap; +use std::fs; +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::thread; +use std::time::SystemTime; use tokio::sync::mpsc; use tokio::sync::oneshot; @@ -117,3 +123,97 @@ impl CacheServer { rx.await? } } + +/// Calculate a version for for a given path. +pub(crate) fn calculate_fs_version(path: &Path) -> Option<String> { + let metadata = fs::metadata(path).ok()?; + if let Ok(modified) = metadata.modified() { + if let Ok(n) = modified.duration_since(SystemTime::UNIX_EPOCH) { + Some(n.as_millis().to_string()) + } else { + Some("1".to_string()) + } + } else { + Some("1".to_string()) + } +} + +/// Populate the metadata map based on the supplied headers +fn parse_metadata( + headers: &HashMap<String, String>, +) -> HashMap<MetadataKey, String> { + let mut metadata = HashMap::new(); + if let Some(warning) = headers.get("x-deno-warning").cloned() { + metadata.insert(MetadataKey::Warning, warning); + } + metadata +} + +#[derive(Debug, PartialEq, Eq, Hash)] +pub(crate) enum MetadataKey { + /// Represent the `x-deno-warning` header associated with the document + Warning, +} + +#[derive(Debug, Clone)] +struct Metadata { + values: Arc<HashMap<MetadataKey, String>>, + version: Option<String>, +} + +#[derive(Debug, Default, Clone)] +pub(crate) struct CacheMetadata { + cache: http_cache::HttpCache, + metadata: Arc<Mutex<HashMap<ModuleSpecifier, Metadata>>>, +} + +impl CacheMetadata { + pub fn new(location: &Path) -> Self { + Self { + cache: http_cache::HttpCache::new(location), + metadata: Default::default(), + } + } + + /// Return the meta data associated with the specifier. Unlike the `get()` + /// method, redirects of the supplied specifier will not be followed. + pub fn get( + &self, + specifier: &ModuleSpecifier, + ) -> Option<Arc<HashMap<MetadataKey, String>>> { + if specifier.scheme() == "file" { + return None; + } + let version = self + .cache + .get_cache_filename(specifier) + .map(|ref path| calculate_fs_version(path)) + .flatten(); + let metadata = self.metadata.lock().get(specifier).cloned(); + if metadata.as_ref().map(|m| m.version.clone()).flatten() != version { + self.refresh(specifier).map(|m| m.values) + } else { + metadata.map(|m| m.values) + } + } + + fn refresh(&self, specifier: &ModuleSpecifier) -> Option<Metadata> { + if specifier.scheme() == "file" { + return None; + } + let cache_filename = self.cache.get_cache_filename(specifier)?; + let specifier_metadata = + http_cache::Metadata::read(&cache_filename).ok()?; + let values = Arc::new(parse_metadata(&specifier_metadata.headers)); + let version = calculate_fs_version(&cache_filename); + let mut metadata_map = self.metadata.lock(); + let metadata = Metadata { values, version }; + metadata_map.insert(specifier.clone(), metadata.clone()); + Some(metadata) + } + + pub fn set_location(&mut self, location: &Path) { + self.cache = http_cache::HttpCache::new(location); + self.metadata.lock().clear(); + } +} diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 62b89e526..4f7e50725 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1,6 +1,7 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. use super::analysis; +use super::cache; use super::client::Client; use super::config::ConfigSnapshot; use super::documents; @@ -571,6 +572,7 @@ fn resolution_error_as_code( fn diagnose_dependency( diagnostics: &mut Vec<lsp::Diagnostic>, documents: &Documents, + cache_metadata: &cache::CacheMetadata, resolved: &deno_graph::Resolved, is_dynamic: bool, maybe_assert_type: Option<&str>, @@ -579,8 +581,10 @@ fn diagnose_dependency( Resolved::Ok { specifier, range, .. } => { - if let Some(doc) = documents.get(specifier) { - if let Some(message) = doc.maybe_warning() { + if let Some(metadata) = cache_metadata.get(specifier) { + if let Some(message) = + metadata.get(&cache::MetadataKey::Warning).cloned() + { diagnostics.push(lsp::Diagnostic { range: documents::to_lsp_range(range), severity: Some(lsp::DiagnosticSeverity::WARNING), @@ -588,8 +592,10 @@ fn diagnose_dependency( source: Some("deno".to_string()), message, ..Default::default() - }) + }); } + } + if let Some(doc) = documents.get(specifier) { if doc.media_type() == MediaType::Json { match maybe_assert_type { // The module has the correct assertion type, no diagnostic @@ -667,6 +673,7 @@ async fn generate_deps_diagnostics( diagnose_dependency( &mut diagnostics, &snapshot.documents, + &snapshot.cache_metadata, &dependency.maybe_code, dependency.is_dynamic, dependency.maybe_assert_type.as_deref(), @@ -674,6 +681,7 @@ async fn generate_deps_diagnostics( diagnose_dependency( &mut diagnostics, &snapshot.documents, + &snapshot.cache_metadata, &dependency.maybe_type, dependency.is_dynamic, dependency.maybe_assert_type.as_deref(), diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 19753389d..c9157a86a 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1,5 +1,6 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. +use super::cache::calculate_fs_version; use super::text::LineIndex; use super::tsc; use super::tsc::AssetDocument; @@ -226,7 +227,6 @@ struct DocumentInner { maybe_module: Option<Result<deno_graph::Module, deno_graph::ModuleGraphError>>, maybe_navigation_tree: Option<Arc<tsc::NavigationTree>>, - maybe_warning: Option<String>, specifier: ModuleSpecifier, text_info: SourceTextInfo, } @@ -242,9 +242,6 @@ impl Document { content: Arc<String>, maybe_resolver: Option<&dyn deno_graph::source::Resolver>, ) -> Self { - let maybe_warning = maybe_headers - .map(|h| h.get("x-deno-warning").cloned()) - .flatten(); let parser = SourceParser::default(); // we only ever do `Document::new` on on disk resources that are supposed to // be diagnosable, unlike `Document::open`, so it is safe to unconditionally @@ -272,7 +269,6 @@ impl Document { maybe_lsp_version: None, maybe_module, maybe_navigation_tree: None, - maybe_warning, text_info, specifier, })) @@ -314,7 +310,6 @@ impl Document { maybe_lsp_version: Some(version), maybe_module, maybe_navigation_tree: None, - maybe_warning: None, text_info: source, specifier, })) @@ -496,10 +491,6 @@ impl Document { self.0.maybe_navigation_tree.clone() } - pub fn maybe_warning(&self) -> Option<String> { - self.0.maybe_warning.clone() - } - pub fn dependencies(&self) -> Vec<(String, deno_graph::Dependency)> { self .0 @@ -692,9 +683,11 @@ impl FileSystemDocuments { ) } else { let cache_filename = cache.get_cache_filename(specifier)?; - let metadata = http_cache::Metadata::read(&cache_filename).ok()?; - let maybe_content_type = metadata.headers.get("content-type").cloned(); - let maybe_headers = Some(&metadata.headers); + let specifier_metadata = + http_cache::Metadata::read(&cache_filename).ok()?; + let maybe_content_type = + specifier_metadata.headers.get("content-type").cloned(); + let maybe_headers = Some(&specifier_metadata.headers); let (_, maybe_charset) = map_content_type(specifier, maybe_content_type); let content = Arc::new(get_source_from_bytes(bytes, maybe_charset).ok()?); Document::new( @@ -711,19 +704,6 @@ impl FileSystemDocuments { } } -fn calculate_fs_version(path: &Path) -> Option<String> { - let metadata = fs::metadata(path).ok()?; - if let Ok(modified) = metadata.modified() { - if let Ok(n) = modified.duration_since(SystemTime::UNIX_EPOCH) { - Some(n.as_millis().to_string()) - } else { - Some("1".to_string()) - } - } else { - Some("1".to_string()) - } -} - fn get_document_path( cache: &HttpCache, specifier: &ModuleSpecifier, @@ -906,8 +886,8 @@ impl Documents { } /// Return a document for the specifier. - pub fn get(&self, specifier: &ModuleSpecifier) -> Option<Document> { - let specifier = self.specifier_resolver.resolve(specifier)?; + pub fn get(&self, original_specifier: &ModuleSpecifier) -> Option<Document> { + let specifier = self.specifier_resolver.resolve(original_specifier)?; if let Some(document) = self.open_docs.get(&specifier) { Some(document.clone()) } else { @@ -1013,10 +993,10 @@ impl Documents { } /// Update the location of the on disk cache for the document store. - pub fn set_location(&mut self, location: PathBuf) { + pub fn set_location(&mut self, location: &Path) { // TODO update resolved dependencies? - self.cache = HttpCache::new(&location); - self.specifier_resolver = Arc::new(SpecifierResolver::new(&location)); + self.cache = HttpCache::new(location); + self.specifier_resolver = Arc::new(SpecifierResolver::new(location)); self.dirty = true; } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 426710b83..a26bafa3f 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -28,7 +28,7 @@ use super::analysis::fix_ts_import_changes; use super::analysis::ts_changes_to_edit; use super::analysis::CodeActionCollection; use super::analysis::CodeActionData; -use super::cache::CacheServer; +use super::cache; use super::capabilities; use super::client::Client; use super::code_lens; @@ -79,6 +79,7 @@ pub struct LanguageServer(Arc<tokio::sync::Mutex<Inner>>); #[derive(Debug, Default)] pub(crate) struct StateSnapshot { pub assets: AssetsSnapshot, + pub cache_metadata: cache::CacheMetadata, pub documents: Documents, pub root_uri: Option<Url>, } @@ -88,6 +89,9 @@ pub(crate) struct Inner { /// Cached versions of "fixed" assets that can either be inlined in Rust or /// are part of the TypeScript snapshot and have to be fetched out. assets: Assets, + /// A representation of metadata associated with specifiers in the DENO_DIR + /// which is used by the language server + cache_metadata: cache::CacheMetadata, /// The LSP client that this LSP server is connected to. pub(crate) client: Client, /// Configuration information. @@ -104,7 +108,7 @@ pub(crate) struct Inner { /// options. maybe_cache_path: Option<PathBuf>, /// A lazily created "server" for handling cache requests. - maybe_cache_server: Option<CacheServer>, + maybe_cache_server: Option<cache::CacheServer>, /// An optional configuration file which has been specified in the client /// options. maybe_config_file: Option<ConfigFile>, @@ -147,6 +151,7 @@ impl Inner { .expect("could not create module registries"); let location = dir.root.join(CACHE_PATH); let documents = Documents::new(&location); + let cache_metadata = cache::CacheMetadata::new(&location); let performance = Arc::new(Performance::default()); let ts_server = Arc::new(TsServer::new(performance.clone())); let config = Config::new(); @@ -159,6 +164,7 @@ impl Inner { Self { assets, + cache_metadata, client, config, diagnostics_server, @@ -390,6 +396,7 @@ impl Inner { pub(crate) fn snapshot(&self) -> Arc<StateSnapshot> { Arc::new(StateSnapshot { assets: self.assets.snapshot(), + cache_metadata: self.cache_metadata.clone(), documents: self.documents.clone(), root_uri: self.root_uri.clone(), }) @@ -447,7 +454,9 @@ impl Inner { }, )?; self.module_registries_location = module_registries_location; - self.documents.set_location(dir.root.join(CACHE_PATH)); + let location = dir.root.join(CACHE_PATH); + self.documents.set_location(&location); + self.cache_metadata.set_location(&location); self.maybe_cache_path = maybe_cache_path; } Ok(()) @@ -2695,7 +2704,7 @@ impl Inner { if self.maybe_cache_server.is_none() { self.maybe_cache_server = Some( - CacheServer::new( + cache::CacheServer::new( self.maybe_cache_path.clone(), self.maybe_import_map.clone(), self.maybe_config_file.clone(), diff --git a/cli/tests/testdata/x_deno_warning.js b/cli/tests/testdata/x_deno_warning_redirect.js index 34b950566..34b950566 100644 --- a/cli/tests/testdata/x_deno_warning.js +++ b/cli/tests/testdata/x_deno_warning_redirect.js |