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`.
This commit is contained in:
David Sherret 2024-04-14 20:07:04 -04:00 committed by GitHub
parent 8f1a92f3c3
commit 1835b4f061
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 33 additions and 55 deletions

View file

@ -1628,6 +1628,7 @@ mod tests {
config.tree.inject_config_file(config_file).await; config.tree.inject_config_file(config_file).await;
} }
StateSnapshot { StateSnapshot {
project_version: 0,
documents, documents,
assets: Default::default(), assets: Default::default(),
cache_metadata: cache::CacheMetadata::new(Arc::new( cache_metadata: cache::CacheMetadata::new(Arc::new(

View file

@ -851,7 +851,6 @@ pub struct Documents {
redirect_resolver: Arc<RedirectResolver>, redirect_resolver: Arc<RedirectResolver>,
/// If --unstable-sloppy-imports is enabled. /// If --unstable-sloppy-imports is enabled.
unstable_sloppy_imports: bool, unstable_sloppy_imports: bool,
project_version: usize,
} }
impl Documents { impl Documents {
@ -878,7 +877,6 @@ impl Documents {
has_injected_types_node_package: false, has_injected_types_node_package: false,
redirect_resolver: Arc::new(RedirectResolver::new(cache)), redirect_resolver: Arc::new(RedirectResolver::new(cache)),
unstable_sloppy_imports: false, unstable_sloppy_imports: false,
project_version: 0,
} }
} }
@ -890,14 +888,6 @@ impl Documents {
.flat_map(|value| value.get_type().or_else(|| value.get_code())) .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 /// "Open" a document from the perspective of the editor, meaning that
/// requests for information from the document will come from the in-memory /// requests for information from the document will come from the in-memory
/// representation received from the language server client, versus reading /// representation received from the language server client, versus reading
@ -925,7 +915,6 @@ impl Documents {
self.file_system_docs.set_dirty(true); self.file_system_docs.set_dirty(true);
self.open_docs.insert(specifier, document.clone()); self.open_docs.insert(specifier, document.clone());
self.increment_project_version();
self.dirty = true; self.dirty = true;
document document
} }
@ -957,7 +946,6 @@ impl Documents {
self.get_npm_resolver(), self.get_npm_resolver(),
)?; )?;
self.open_docs.insert(doc.specifier().clone(), doc.clone()); self.open_docs.insert(doc.specifier().clone(), doc.clone());
self.increment_project_version();
Ok(doc) Ok(doc)
} }
@ -985,7 +973,6 @@ impl Documents {
.docs .docs
.insert(specifier.clone(), document); .insert(specifier.clone(), document);
self.increment_project_version();
self.dirty = true; self.dirty = true;
} }
Ok(()) Ok(())

View file

@ -176,6 +176,7 @@ pub struct StateNpmSnapshot {
/// Snapshot of the state used by TSC. /// Snapshot of the state used by TSC.
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub struct StateSnapshot { pub struct StateSnapshot {
pub project_version: usize,
pub assets: AssetsSnapshot, pub assets: AssetsSnapshot,
pub cache_metadata: cache::CacheMetadata, pub cache_metadata: cache::CacheMetadata,
pub config: Arc<ConfigSnapshot>, pub config: Arc<ConfigSnapshot>,
@ -254,6 +255,7 @@ pub struct Inner {
maybe_testing_server: Option<testing::TestServer>, maybe_testing_server: Option<testing::TestServer>,
/// Services used for dealing with npm related functionality. /// Services used for dealing with npm related functionality.
npm: LspNpmServices, npm: LspNpmServices,
project_version: usize,
/// A collection of measurements which instrument that performance of the LSP. /// A collection of measurements which instrument that performance of the LSP.
performance: Arc<Performance>, performance: Arc<Performance>,
/// A memoized version of fixable diagnostic codes retrieved from TypeScript. /// A memoized version of fixable diagnostic codes retrieved from TypeScript.
@ -537,6 +539,7 @@ impl Inner {
initial_cwd: initial_cwd.clone(), initial_cwd: initial_cwd.clone(),
jsr_search_api, jsr_search_api,
maybe_global_cache_path: None, maybe_global_cache_path: None,
project_version: 0,
task_queue: Default::default(), task_queue: Default::default(),
maybe_testing_server: None, maybe_testing_server: None,
module_registries, module_registries,
@ -669,6 +672,7 @@ impl Inner {
} }
}); });
Arc::new(StateSnapshot { Arc::new(StateSnapshot {
project_version: self.project_version,
assets: self.assets.snapshot(), assets: self.assets.snapshot(),
cache_metadata: self.cache_metadata.clone(), cache_metadata: self.cache_metadata.clone(),
config: self.config.snapshot(), config: self.config.snapshot(),
@ -1190,15 +1194,7 @@ impl Inner {
// a @types/node package and now's a good time to do that anyway // a @types/node package and now's a good time to do that anyway
self.refresh_npm_specifiers().await; self.refresh_npm_specifiers().await;
self self.project_changed(&[], true).await;
.ts_server
.project_changed(
self.snapshot(),
&[],
self.documents.project_version(),
true,
)
.await;
} }
fn shutdown(&self) -> LspResult<()> { fn shutdown(&self) -> LspResult<()> {
@ -1233,15 +1229,8 @@ impl Inner {
params.text_document.language_id.parse().unwrap(), params.text_document.language_id.parse().unwrap(),
params.text_document.text.into(), params.text_document.text.into(),
); );
let version = self.documents.project_version();
self self
.ts_server .project_changed(&[(document.specifier(), ChangeKind::Opened)], false)
.project_changed(
self.snapshot(),
&[(document.specifier(), ChangeKind::Opened)],
version,
false,
)
.await; .await;
self.performance.measure(mark); self.performance.measure(mark);
@ -1260,13 +1249,9 @@ impl Inner {
) { ) {
Ok(document) => { Ok(document) => {
if document.is_diagnosable() { if document.is_diagnosable() {
let version = self.documents.project_version();
self self
.ts_server
.project_changed( .project_changed(
self.snapshot(),
&[(document.specifier(), ChangeKind::Modified)], &[(document.specifier(), ChangeKind::Modified)],
version,
false, false,
) )
.await; .await;
@ -1320,15 +1305,8 @@ impl Inner {
if let Err(err) = self.documents.close(&specifier) { if let Err(err) = self.documents.close(&specifier) {
error!("{:#}", err); error!("{:#}", err);
} }
let version = self.documents.project_version();
self self
.ts_server .project_changed(&[(&specifier, ChangeKind::Closed)], false)
.project_changed(
self.snapshot(),
&[(&specifier, ChangeKind::Closed)],
version,
false,
)
.await; .await;
self.performance.measure(mark); self.performance.measure(mark);
} }
@ -3015,6 +2993,18 @@ impl Inner {
Ok(maybe_symbol_information) 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) { fn send_diagnostics_update(&self) {
let snapshot = DiagnosticServerUpdateMessage { let snapshot = DiagnosticServerUpdateMessage {
snapshot: self.snapshot(), snapshot: self.snapshot(),

View file

@ -306,7 +306,6 @@ impl TsServer {
&self, &self,
snapshot: Arc<StateSnapshot>, snapshot: Arc<StateSnapshot>,
modified_scripts: &[(&ModuleSpecifier, ChangeKind)], modified_scripts: &[(&ModuleSpecifier, ChangeKind)],
new_project_version: String,
config_changed: bool, config_changed: bool,
) { ) {
let modified_scripts = modified_scripts let modified_scripts = modified_scripts
@ -315,7 +314,9 @@ impl TsServer {
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let req = TscRequest { let req = TscRequest {
method: "$projectChanged", method: "$projectChanged",
args: json!([modified_scripts, new_project_version, config_changed,]), args: json!(
[modified_scripts, snapshot.project_version, config_changed,]
),
}; };
self self
.request::<()>(snapshot, req) .request::<()>(snapshot, req)
@ -340,7 +341,7 @@ impl TsServer {
.into_iter() .into_iter()
.map(|s| self.specifier_map.denormalize(&s)) .map(|s| self.specifier_map.denormalize(&s))
.collect::<Vec<String>>(), .collect::<Vec<String>>(),
snapshot.documents.project_version() snapshot.project_version,
]), ]),
}; };
let raw_diagnostics = self.request_with_cancellation::<HashMap<String, Vec<crate::tsc::Diagnostic>>>(snapshot, req, token).await?; let raw_diagnostics = self.request_with_cancellation::<HashMap<String, Vec<crate::tsc::Diagnostic>>>(snapshot, req, token).await?;
@ -4153,12 +4154,12 @@ fn op_ts_config(state: &mut OpState) -> serde_json::Value {
r r
} }
#[op2] #[op2(fast)]
#[string] #[number]
fn op_project_version(state: &mut OpState) -> String { fn op_project_version(state: &mut OpState) -> usize {
let state: &mut State = state.borrow_mut::<State>(); let state: &mut State = state.borrow_mut::<State>();
let mark = state.performance.mark("tsc.op.op_project_version"); 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); state.performance.measure(mark);
r r
} }
@ -4253,6 +4254,7 @@ deno_core::extension!(deno_tsc,
state = |state, options| { state = |state, options| {
state.put(State::new( state.put(State::new(
Arc::new(StateSnapshot { Arc::new(StateSnapshot {
project_version: 0,
assets: Default::default(), assets: Default::default(),
cache_metadata: CacheMetadata::new(options.cache.clone()), cache_metadata: CacheMetadata::new(options.cache.clone()),
config: Default::default(), config: Default::default(),
@ -4723,6 +4725,7 @@ mod tests {
) )
.await; .await;
StateSnapshot { StateSnapshot {
project_version: 0,
documents, documents,
assets: Default::default(), assets: Default::default(),
cache_metadata: CacheMetadata::new(cache), cache_metadata: CacheMetadata::new(cache),
@ -5178,10 +5181,8 @@ mod tests {
) )
.unwrap(); .unwrap();
let snapshot = { let snapshot = {
let mut documents = snapshot.documents.clone();
documents.increment_project_version();
Arc::new(StateSnapshot { Arc::new(StateSnapshot {
documents, project_version: snapshot.project_version + 1,
..snapshot.as_ref().clone() ..snapshot.as_ref().clone()
}) })
}; };
@ -5189,7 +5190,6 @@ mod tests {
.project_changed( .project_changed(
snapshot.clone(), snapshot.clone(),
&[(&specifier_dep, ChangeKind::Opened)], &[(&specifier_dep, ChangeKind::Opened)],
snapshot.documents.project_version(),
false, false,
) )
.await; .await;

View file

@ -165,7 +165,7 @@ delete Object.prototype.__proto__;
/** @type {ts.CompilerOptions | null} */ /** @type {ts.CompilerOptions | null} */
let tsConfigCache = null; let tsConfigCache = null;
/** @type {string | null} */ /** @type {number | null} */
let projectVersionCache = null; let projectVersionCache = null;
const ChangeKind = { const ChangeKind = {
@ -1039,7 +1039,7 @@ delete Object.prototype.__proto__;
case "$projectChanged": { case "$projectChanged": {
/** @type {[string, number][]} */ /** @type {[string, number][]} */
const changedScripts = args[0]; const changedScripts = args[0];
/** @type {string} */ /** @type {number} */
const newProjectVersion = args[1]; const newProjectVersion = args[1];
/** @type {boolean} */ /** @type {boolean} */
const configChanged = args[2]; const configChanged = args[2];