From 1835b4f06189560eb17a103b8db05e011a7cd799 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sun, 14 Apr 2024 20:07:04 -0400 Subject: [PATCH] fix(lsp): ensure project version is incremented when config changes (#23366) I'm running into a node resolution bug in the lsp only and while tracking it down I noticed this one. Fixed by moving the project version out of `Documents`. --- cli/lsp/diagnostics.rs | 1 + cli/lsp/documents.rs | 13 ---------- cli/lsp/language_server.rs | 48 +++++++++++++++---------------------- cli/lsp/tsc.rs | 22 ++++++++--------- cli/tsc/99_main_compiler.js | 4 ++-- 5 files changed, 33 insertions(+), 55 deletions(-) diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 71c34bdbeb..431cddeb65 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1628,6 +1628,7 @@ mod tests { config.tree.inject_config_file(config_file).await; } StateSnapshot { + project_version: 0, documents, assets: Default::default(), cache_metadata: cache::CacheMetadata::new(Arc::new( diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 8100b0732b..1dd18f5997 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -851,7 +851,6 @@ pub struct Documents { redirect_resolver: Arc, /// If --unstable-sloppy-imports is enabled. unstable_sloppy_imports: bool, - project_version: usize, } impl Documents { @@ -878,7 +877,6 @@ impl Documents { has_injected_types_node_package: false, redirect_resolver: Arc::new(RedirectResolver::new(cache)), unstable_sloppy_imports: false, - project_version: 0, } } @@ -890,14 +888,6 @@ impl Documents { .flat_map(|value| value.get_type().or_else(|| value.get_code())) } - pub fn project_version(&self) -> String { - self.project_version.to_string() - } - - pub fn increment_project_version(&mut self) { - self.project_version += 1; - } - /// "Open" a document from the perspective of the editor, meaning that /// requests for information from the document will come from the in-memory /// representation received from the language server client, versus reading @@ -925,7 +915,6 @@ impl Documents { self.file_system_docs.set_dirty(true); self.open_docs.insert(specifier, document.clone()); - self.increment_project_version(); self.dirty = true; document } @@ -957,7 +946,6 @@ impl Documents { self.get_npm_resolver(), )?; self.open_docs.insert(doc.specifier().clone(), doc.clone()); - self.increment_project_version(); Ok(doc) } @@ -985,7 +973,6 @@ impl Documents { .docs .insert(specifier.clone(), document); - self.increment_project_version(); self.dirty = true; } Ok(()) diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 342c32358d..73847807b9 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -176,6 +176,7 @@ pub struct StateNpmSnapshot { /// Snapshot of the state used by TSC. #[derive(Clone, Debug)] pub struct StateSnapshot { + pub project_version: usize, pub assets: AssetsSnapshot, pub cache_metadata: cache::CacheMetadata, pub config: Arc, @@ -254,6 +255,7 @@ pub struct Inner { maybe_testing_server: Option, /// Services used for dealing with npm related functionality. npm: LspNpmServices, + project_version: usize, /// A collection of measurements which instrument that performance of the LSP. performance: Arc, /// A memoized version of fixable diagnostic codes retrieved from TypeScript. @@ -537,6 +539,7 @@ impl Inner { initial_cwd: initial_cwd.clone(), jsr_search_api, maybe_global_cache_path: None, + project_version: 0, task_queue: Default::default(), maybe_testing_server: None, module_registries, @@ -669,6 +672,7 @@ impl Inner { } }); Arc::new(StateSnapshot { + project_version: self.project_version, assets: self.assets.snapshot(), cache_metadata: self.cache_metadata.clone(), config: self.config.snapshot(), @@ -1190,15 +1194,7 @@ impl Inner { // a @types/node package and now's a good time to do that anyway self.refresh_npm_specifiers().await; - self - .ts_server - .project_changed( - self.snapshot(), - &[], - self.documents.project_version(), - true, - ) - .await; + self.project_changed(&[], true).await; } fn shutdown(&self) -> LspResult<()> { @@ -1233,15 +1229,8 @@ impl Inner { params.text_document.language_id.parse().unwrap(), params.text_document.text.into(), ); - let version = self.documents.project_version(); self - .ts_server - .project_changed( - self.snapshot(), - &[(document.specifier(), ChangeKind::Opened)], - version, - false, - ) + .project_changed(&[(document.specifier(), ChangeKind::Opened)], false) .await; self.performance.measure(mark); @@ -1260,13 +1249,9 @@ impl Inner { ) { Ok(document) => { if document.is_diagnosable() { - let version = self.documents.project_version(); self - .ts_server .project_changed( - self.snapshot(), &[(document.specifier(), ChangeKind::Modified)], - version, false, ) .await; @@ -1320,15 +1305,8 @@ impl Inner { if let Err(err) = self.documents.close(&specifier) { error!("{:#}", err); } - let version = self.documents.project_version(); self - .ts_server - .project_changed( - self.snapshot(), - &[(&specifier, ChangeKind::Closed)], - version, - false, - ) + .project_changed(&[(&specifier, ChangeKind::Closed)], false) .await; self.performance.measure(mark); } @@ -3015,6 +2993,18 @@ impl Inner { Ok(maybe_symbol_information) } + async fn project_changed( + &mut self, + modified_scripts: &[(&ModuleSpecifier, ChangeKind)], + config_changed: bool, + ) { + self.project_version += 1; // increment before getting the snapshot + self + .ts_server + .project_changed(self.snapshot(), modified_scripts, config_changed) + .await; + } + fn send_diagnostics_update(&self) { let snapshot = DiagnosticServerUpdateMessage { snapshot: self.snapshot(), diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index ac4871dfcd..2e0726a795 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -306,7 +306,6 @@ impl TsServer { &self, snapshot: Arc, modified_scripts: &[(&ModuleSpecifier, ChangeKind)], - new_project_version: String, config_changed: bool, ) { let modified_scripts = modified_scripts @@ -315,7 +314,9 @@ impl TsServer { .collect::>(); let req = TscRequest { method: "$projectChanged", - args: json!([modified_scripts, new_project_version, config_changed,]), + args: json!( + [modified_scripts, snapshot.project_version, config_changed,] + ), }; self .request::<()>(snapshot, req) @@ -340,7 +341,7 @@ impl TsServer { .into_iter() .map(|s| self.specifier_map.denormalize(&s)) .collect::>(), - snapshot.documents.project_version() + snapshot.project_version, ]), }; let raw_diagnostics = self.request_with_cancellation::>>(snapshot, req, token).await?; @@ -4153,12 +4154,12 @@ fn op_ts_config(state: &mut OpState) -> serde_json::Value { r } -#[op2] -#[string] -fn op_project_version(state: &mut OpState) -> String { +#[op2(fast)] +#[number] +fn op_project_version(state: &mut OpState) -> usize { let state: &mut State = state.borrow_mut::(); let mark = state.performance.mark("tsc.op.op_project_version"); - let r = state.state_snapshot.documents.project_version(); + let r = state.state_snapshot.project_version; state.performance.measure(mark); r } @@ -4253,6 +4254,7 @@ deno_core::extension!(deno_tsc, state = |state, options| { state.put(State::new( Arc::new(StateSnapshot { + project_version: 0, assets: Default::default(), cache_metadata: CacheMetadata::new(options.cache.clone()), config: Default::default(), @@ -4723,6 +4725,7 @@ mod tests { ) .await; StateSnapshot { + project_version: 0, documents, assets: Default::default(), cache_metadata: CacheMetadata::new(cache), @@ -5178,10 +5181,8 @@ mod tests { ) .unwrap(); let snapshot = { - let mut documents = snapshot.documents.clone(); - documents.increment_project_version(); Arc::new(StateSnapshot { - documents, + project_version: snapshot.project_version + 1, ..snapshot.as_ref().clone() }) }; @@ -5189,7 +5190,6 @@ mod tests { .project_changed( snapshot.clone(), &[(&specifier_dep, ChangeKind::Opened)], - snapshot.documents.project_version(), false, ) .await; diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js index 274fc1ccab..be99628148 100644 --- a/cli/tsc/99_main_compiler.js +++ b/cli/tsc/99_main_compiler.js @@ -165,7 +165,7 @@ delete Object.prototype.__proto__; /** @type {ts.CompilerOptions | null} */ let tsConfigCache = null; - /** @type {string | null} */ + /** @type {number | null} */ let projectVersionCache = null; const ChangeKind = { @@ -1039,7 +1039,7 @@ delete Object.prototype.__proto__; case "$projectChanged": { /** @type {[string, number][]} */ const changedScripts = args[0]; - /** @type {string} */ + /** @type {number} */ const newProjectVersion = args[1]; /** @type {boolean} */ const configChanged = args[2];