diff options
author | Nayeem Rahman <nayeemrmn99@gmail.com> | 2023-09-24 17:59:42 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-09-24 17:59:42 +0100 |
commit | 33f84321b29f97c5757f019a72228c1c9631852e (patch) | |
tree | 97b5d9cb5987010c674fb6743429b2ef14fad633 | |
parent | cb9ab9c3ac53ed642c6c648f4931056551a378b7 (diff) |
refactor(lsp): implement "deno.cacheOnSave" server-side (#20632)
-rw-r--r-- | cli/lsp/config.rs | 7 | ||||
-rw-r--r-- | cli/lsp/diagnostics.rs | 71 | ||||
-rw-r--r-- | cli/lsp/language_server.rs | 42 | ||||
-rw-r--r-- | cli/lsp/repl.rs | 1 | ||||
-rw-r--r-- | cli/tests/integration/lsp_tests.rs | 68 | ||||
-rw-r--r-- | test_util/src/lsp.rs | 12 |
6 files changed, 190 insertions, 11 deletions
diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index 1a83c00eb..52ee362b8 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -315,6 +315,11 @@ pub struct WorkspaceSettings { #[serde(default, deserialize_with = "empty_string_none")] pub cache: Option<String>, + /// Cache local modules and their dependencies on `textDocument/didSave` + /// notifications corresponding to them. + #[serde(default)] + pub cache_on_save: bool, + /// Override the default stores used to validate certificates. This overrides /// the environment variable `DENO_TLS_CA_STORE` if present. pub certificate_stores: Option<Vec<String>>, @@ -379,6 +384,7 @@ impl Default for WorkspaceSettings { disable_paths: vec![], enable_paths: None, cache: None, + cache_on_save: false, certificate_stores: None, config: None, import_map: None, @@ -1192,6 +1198,7 @@ mod tests { disable_paths: vec![], enable_paths: None, cache: None, + cache_on_save: false, certificate_stores: None, config: None, import_map: None, diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index fb998f880..f5a0af455 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -48,6 +48,7 @@ use std::sync::Arc; use std::thread; use tokio::sync::mpsc; use tokio::sync::Mutex; +use tokio::sync::RwLock; use tokio::time::Duration; use tokio_util::sync::CancellationToken; use tower_lsp::lsp_types as lsp; @@ -95,14 +96,16 @@ type DiagnosticsBySource = HashMap<DiagnosticSource, VersionedDiagnostics>; #[derive(Debug)] struct DiagnosticsPublisher { client: Client, + state: Arc<RwLock<DiagnosticsState>>, diagnostics_by_specifier: Mutex<HashMap<ModuleSpecifier, DiagnosticsBySource>>, } impl DiagnosticsPublisher { - pub fn new(client: Client) -> Self { + pub fn new(client: Client, state: Arc<RwLock<DiagnosticsState>>) -> Self { Self { client, + state, diagnostics_by_specifier: Default::default(), } } @@ -116,6 +119,7 @@ impl DiagnosticsPublisher { ) -> usize { let mut diagnostics_by_specifier = self.diagnostics_by_specifier.lock().await; + let mut state = self.state.write().await; let mut seen_specifiers = HashSet::with_capacity(diagnostics.len()); let mut messages_sent = 0; @@ -142,6 +146,7 @@ impl DiagnosticsPublisher { .cloned() .collect::<Vec<_>>(); + state.update(&record.specifier, version, &all_specifier_diagnostics); self .client .when_outside_lsp_lock() @@ -172,6 +177,7 @@ impl DiagnosticsPublisher { specifiers_to_remove.push(specifier.clone()); if let Some(removed_value) = maybe_removed_value { // clear out any diagnostics for this specifier + state.update(specifier, removed_value.version, &[]); self .client .when_outside_lsp_lock() @@ -290,6 +296,60 @@ struct ChannelUpdateMessage { batch_index: Option<usize>, } +#[derive(Clone, Debug)] +struct SpecifierState { + has_no_cache_diagnostic: bool, +} + +#[derive(Clone, Debug, Default)] +pub struct DiagnosticsState { + specifiers: HashMap<ModuleSpecifier, (Option<i32>, SpecifierState)>, +} + +impl DiagnosticsState { + fn update( + &mut self, + specifier: &ModuleSpecifier, + version: Option<i32>, + diagnostics: &[lsp::Diagnostic], + ) { + let current_version = self.specifiers.get(specifier).and_then(|s| s.0); + let is_new = match (version, current_version) { + (Some(arg), Some(existing)) => arg >= existing, + _ => true, + }; + if is_new { + self.specifiers.insert( + specifier.clone(), + ( + version, + SpecifierState { + has_no_cache_diagnostic: diagnostics.iter().any(|d| { + d.code + == Some(lsp::NumberOrString::String("no-cache".to_string())) + || d.code + == Some(lsp::NumberOrString::String( + "no-cache-npm".to_string(), + )) + }), + }, + ), + ); + } + } + + pub fn clear(&mut self, specifier: &ModuleSpecifier) { + self.specifiers.remove(specifier); + } + + pub fn has_no_cache_diagnostic(&self, specifier: &ModuleSpecifier) -> bool { + self + .specifiers + .get(specifier) + .map_or(false, |s| s.1.has_no_cache_diagnostic) + } +} + #[derive(Debug)] pub struct DiagnosticsServer { channel: Option<mpsc::UnboundedSender<ChannelMessage>>, @@ -298,6 +358,7 @@ pub struct DiagnosticsServer { performance: Arc<Performance>, ts_server: Arc<TsServer>, batch_counter: DiagnosticBatchCounter, + state: Arc<RwLock<DiagnosticsState>>, } impl DiagnosticsServer { @@ -313,9 +374,14 @@ impl DiagnosticsServer { performance, ts_server, batch_counter: Default::default(), + state: Default::default(), } } + pub fn state(&self) -> Arc<RwLock<DiagnosticsState>> { + self.state.clone() + } + pub fn get_ts_diagnostics( &self, specifier: &ModuleSpecifier, @@ -340,6 +406,7 @@ impl DiagnosticsServer { let (tx, mut rx) = mpsc::unbounded_channel::<ChannelMessage>(); self.channel = Some(tx); let client = self.client.clone(); + let state = self.state.clone(); let performance = self.performance.clone(); let ts_diagnostics_store = self.ts_diagnostics.clone(); let ts_server = self.ts_server.clone(); @@ -353,7 +420,7 @@ impl DiagnosticsServer { let mut lint_handle: Option<JoinHandle<()>> = None; let mut deps_handle: Option<JoinHandle<()>> = None; let diagnostics_publisher = - Arc::new(DiagnosticsPublisher::new(client.clone())); + Arc::new(DiagnosticsPublisher::new(client.clone(), state.clone())); loop { match rx.recv().await { diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index be3d7f5af..1750b23c5 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -108,6 +108,7 @@ use crate::npm::NpmResolution; use crate::tools::fmt::format_file; use crate::tools::fmt::format_parsed_source; use crate::util::fs::remove_dir_all_if_exists; +use crate::util::path::is_importable_ext; use crate::util::path::specifier_to_file_path; use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; @@ -1446,6 +1447,12 @@ impl Inner { async fn did_close(&mut self, params: DidCloseTextDocumentParams) { let mark = self.performance.mark("did_close", Some(¶ms)); + self + .diagnostics_server + .state() + .write() + .await + .clear(¶ms.text_document.uri); if params.text_document.uri.scheme() == "deno" { // we can ignore virtual text documents closing, as they don't need to // be tracked in memory, as they are static assets that won't change @@ -3267,9 +3274,38 @@ impl tower_lsp::LanguageServer for LanguageServer { self.0.write().await.did_change(params).await } - async fn did_save(&self, _params: DidSaveTextDocumentParams) { - // We don't need to do anything on save at the moment, but if this isn't - // implemented, lspower complains about it not being implemented. + async fn did_save(&self, params: DidSaveTextDocumentParams) { + let uri = ¶ms.text_document.uri; + let Ok(path) = specifier_to_file_path(uri) else { + return; + }; + if !is_importable_ext(&path) { + return; + } + let diagnostics_state = { + let inner = self.0.read().await; + if !inner.config.workspace_settings().cache_on_save + || !inner.config.specifier_enabled(uri) + { + return; + } + inner.diagnostics_server.state() + }; + if !diagnostics_state.read().await.has_no_cache_diagnostic(uri) { + return; + } + if let Err(err) = self + .cache_request(Some( + serde_json::to_value(lsp_custom::CacheParams { + referrer: TextDocumentIdentifier { uri: uri.clone() }, + uris: vec![TextDocumentIdentifier { uri: uri.clone() }], + }) + .unwrap(), + )) + .await + { + lsp_warn!("Failed to cache \"{}\" on save: {}", uri.to_string(), err); + } } async fn did_close(&self, params: DidCloseTextDocumentParams) { diff --git a/cli/lsp/repl.rs b/cli/lsp/repl.rs index 5e1bb66de..2512d2073 100644 --- a/cli/lsp/repl.rs +++ b/cli/lsp/repl.rs @@ -292,6 +292,7 @@ pub fn get_repl_workspace_settings() -> WorkspaceSettings { config: None, certificate_stores: None, cache: None, + cache_on_save: false, import_map: None, code_lens: Default::default(), internal_debug: false, diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 567af5399..44a5df700 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -4456,6 +4456,74 @@ fn lsp_code_actions_deno_cache_npm() { } #[test] +fn lsp_cache_on_save() { + let context = TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .build(); + let temp_dir = context.temp_dir(); + temp_dir.write( + "file.ts", + r#" + import { printHello } from "http://localhost:4545/subdir/print_hello.ts"; + printHello(); + "#, + ); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + client.write_notification( + "workspace/didChangeConfiguration", + json!({ + "settings": {} + }), + ); + let settings = json!({ + "deno": { + "enable": true, + "cacheOnSave": true, + }, + }); + // one for the workspace + client.handle_configuration_request(&settings); + // one for the specifier + client.handle_configuration_request(&settings); + + let diagnostics = client.did_open(json!({ + "textDocument": { + "uri": temp_dir.uri().join("file.ts").unwrap(), + "languageId": "typescript", + "version": 1, + "text": temp_dir.read_to_string("file.ts"), + } + })); + assert_eq!( + diagnostics.messages_with_source("deno"), + serde_json::from_value(json!({ + "uri": temp_dir.uri().join("file.ts").unwrap(), + "diagnostics": [{ + "range": { + "start": { "line": 1, "character": 33 }, + "end": { "line": 1, "character": 78 } + }, + "severity": 1, + "code": "no-cache", + "source": "deno", + "message": "Uncached or missing remote URL: http://localhost:4545/subdir/print_hello.ts", + "data": { "specifier": "http://localhost:4545/subdir/print_hello.ts" } + }], + "version": 1 + })) + .unwrap() + ); + client.did_save(json!({ + "textDocument": { "uri": temp_dir.uri().join("file.ts").unwrap() }, + })); + assert_eq!(client.read_diagnostics().all(), vec![]); + + client.shutdown(); +} + +#[test] fn lsp_code_actions_imports() { let context = TestContextBuilder::new().use_temp_cwd().build(); let mut client = context.new_lsp_command().build(); diff --git a/test_util/src/lsp.rs b/test_util/src/lsp.rs index 72d27f6d0..d953c86e7 100644 --- a/test_util/src/lsp.rs +++ b/test_util/src/lsp.rs @@ -748,6 +748,10 @@ impl LspClient { self.write_response(id, result); } + pub fn did_save(&mut self, params: Value) { + self.write_notification("textDocument/didSave", params); + } + pub fn did_change_watched_files(&mut self, params: Value) { self.write_notification("workspace/didChangeWatchedFiles", params); } @@ -760,11 +764,7 @@ impl LspClient { /// Reads the latest diagnostics. It's assumed that pub fn read_diagnostics(&mut self) -> CollectedDiagnostics { - // ask the server what the latest diagnostic batch index is - let latest_diagnostic_batch_index = - self.get_latest_diagnostic_batch_index(); - - // now wait for three (deno, lint, and typescript diagnostics) batch + // wait for three (deno, lint, and typescript diagnostics) batch // notification messages for that index let mut read = 0; let mut total_messages_len = 0; @@ -773,7 +773,7 @@ impl LspClient { self.read_notification::<DiagnosticBatchNotificationParams>(); assert_eq!(method, "deno/internalTestDiagnosticBatch"); let response = response.unwrap(); - if response.batch_index == latest_diagnostic_batch_index { + if response.batch_index == self.get_latest_diagnostic_batch_index() { read += 1; total_messages_len += response.messages_len; } |