From 4d088c35ffaa530a3e6936899d13903d0a8c75b9 Mon Sep 17 00:00:00 2001 From: Vadim Lazuk Date: Sat, 7 Nov 2026 20:43:59 +0300 Subject: [PATCH] fix(lsp): leave documents open on a server out of its watched-file changes (#1995) Closes #1780. When a file open in the editor changed on disk, its language server got the change twice: a workspace/didChangeWatchedFiles notification, then the editor's reload as textDocument/didChange diffed against the pre-reload copy. Servers that re-read files on watched changes (csharp-ls) applied both and kept analysing stale content. Change events for documents open on that server instance are now left out (paths compared canonically); create and delete events are still delivered, servers without document sync still get every event, and other servers are unaffected. Co-authored-by: Inf1mum <8135225+Inf1mum@users.noreply.github.com> --- crates/ide-lsp/src/lib.rs | 37 +++++- crates/ide-lsp/tests/mock_server.rs | 194 +++++++++++++++++++++++++++- 2 files changed, 227 insertions(+), 4 deletions(-) diff --git a/crates/ide-lsp/src/lib.rs b/crates/ide-lsp/src/lib.rs index 4a7d00051..85e61a2fd 100644 --- a/crates/ide-lsp/src/lib.rs +++ b/crates/ide-lsp/src/lib.rs @@ -2334,6 +2334,13 @@ impl LspManager { /// into `.venv` or a `go.mod` rewritten by `go get` is picked up without a restart. Other /// servers hear of their languages' files under their root; without it jdtls's incremental /// build never sees them, so a launch runs stale classes. + /// + /// Creates and changes for documents synced on a server's current instance are left out. + /// Their text reaches the server through `didOpen`/`didChange`, including clean-buffer + /// reloads. A server that re-reads the file on the watched change (csharp-ls) would apply + /// the reload and then the `didChange` computed against the pre-reload copy, so it would + /// keep analysing content that diverges from both the disk and the editor (#1780). + /// Deletions still reach it: document sync does not describe workspace membership. pub fn watched_files_changed( &self, created: &[PathBuf], @@ -2343,19 +2350,43 @@ impl LspManager { ) { use file_watch::Change; let servers: Vec> = self.servers.lock().clone(); + let open: HashMap<(ide_core::ServerId, u64), HashSet> = { + let docs = self.docs.lock(); + let mut open: HashMap<_, HashSet<_>> = HashMap::new(); + for d in docs.values() { + for a in &d.servers { + if let Some(generation) = a.opened_gen { + open.entry((a.server.id, generation)) + .or_default() + .extend([d.path.clone(), d.canonical.clone()]); + } + } + } + open + }; + let canonical: HashMap<_, _> = + created.iter().chain(modified).filter_map(|p| dunce::canonicalize(p).ok().map(|c| (p, c))).collect(); for s in servers { let Some(run) = s.running() else { continue }; + let open = open.get(&(s.id, run.generation)).filter(|_| run.caps.read().sync != SyncKind::None); + let send = |p: &PathBuf, change: Change| { + change == Change::Deleted + || !open + .is_some_and(|paths| paths.contains(p) || canonical.get(p).is_some_and(|c| paths.contains(c))) + }; let all = [(created, Change::Created), (modified, Change::Changed), (removed, Change::Deleted)] .into_iter() .flat_map(|(paths, change)| paths.iter().map(move |p| (p, change))); let watchers = run.watchers.read(); let changes: Vec<(&PathBuf, Change)> = if watchers.is_empty() { let languages = s.languages(); - all.filter(|(p, _)| p.starts_with(&s.root) && languages.contains(&self.languages.detect(p, None))) - .collect() + all.filter(|(p, change)| { + p.starts_with(&s.root) && languages.contains(&self.languages.detect(p, None)) && send(p, *change) + }) + .collect() } else { all.chain(excluded_dirs.iter().map(|p| (p, Change::Created))) - .filter(|(p, change)| watchers.wants(&s.root, p, *change)) + .filter(|(p, change)| watchers.wants(&s.root, p, *change) && send(p, *change)) .collect() }; drop(watchers); diff --git a/crates/ide-lsp/tests/mock_server.rs b/crates/ide-lsp/tests/mock_server.rs index 052ba7975..2da972ad2 100644 --- a/crates/ide-lsp/tests/mock_server.rs +++ b/crates/ide-lsp/tests/mock_server.rs @@ -422,7 +422,8 @@ async fn files_changed_on_disk_reach_the_servers_of_their_language() { h.open("a.mk", "x"); h.wait_status("mock", ServerStatus::Ready).await; let s = h.launcher.state("mock"); - let (created, modified, removed) = (h.root.join("new.mk"), h.root.join("a.mk"), h.root.join("old.mk")); + // `a.mk` is open on the server: its changes reach it through document sync, not here. + let (created, modified, removed) = (h.root.join("new.mk"), h.root.join("changed.mk"), h.root.join("old.mk")); // Other languages and files outside the server's root are left out. let other = h.root.join("notes.txt"); let outside = tempfile::tempdir().unwrap().path().join("far.mk"); @@ -448,6 +449,197 @@ async fn files_changed_on_disk_reach_the_servers_of_their_language() { h.mgr.shutdown_all().await; } +/// An external change to a document open on the server's current instance reaches it only +/// through the reload's `didChange`: a watched-file change for it too would make a server +/// that re-reads the file on one (csharp-ls) apply the reload and then the diff computed +/// against the pre-reload copy, so it would keep analysing stale content (#1780). A closed +/// file's change is watched as before, and the file is watched again once its document closes. +#[tokio::test(flavor = "multi_thread")] +async fn documents_open_on_a_server_are_left_out_of_watched_file_changes() { + let h = harness(&[("mock", incremental_caps())]); + let (doc, path) = h.open("a.mk", "old"); + h.wait_status("mock", ServerStatus::Ready).await; + let s = h.launcher.state("mock"); + eventually(|| !s.notified("textDocument/didOpen").is_empty()).await; + + // The open document and a closed file changed outside the editor. + let closed = h.root.join("b.mk"); + h.mgr.watched_files_changed(&[], &[path.clone(), closed.clone()], &[], &[]); + eventually(|| !s.notified("workspace/didChangeWatchedFiles").is_empty()).await; + assert_eq!( + s.notified("workspace/didChangeWatchedFiles")[0]["changes"], + json!([{ "uri": uri_of(&closed), "type": 2 }]) + ); + + // The editor reloads the open buffer from disk; the server hears the new text. + h.mgr.did_change(doc, 2, &[change(rng(0, 0, 0, 3), "new")], &|| "new".to_owned()); + eventually(|| s.docs.lock().get(uri_of(&path).as_str()).map(String::as_str) == Some("new")).await; + + // Once the document closes, the file is watched again. + h.mgr.did_close(doc); + h.mgr.watched_files_changed(&[], std::slice::from_ref(&path), &[], &[]); + eventually(|| s.notified("workspace/didChangeWatchedFiles").len() == 2).await; + assert_eq!( + s.notified("workspace/didChangeWatchedFiles")[1]["changes"], + json!([{ "uri": uri_of(&path), "type": 2 }]) + ); + h.mgr.shutdown_all().await; +} + +async fn watch_all(s: &MockState) { + s.peer() + .request::<_, Value>( + "client/registerCapability", + json!({ "registrations": [{ "id": "all", "method": "workspace/didChangeWatchedFiles", + "registerOptions": { "watchers": [{ "globPattern": "**/*" }] } }] }), + None, + ) + .await + .unwrap(); +} + +#[tokio::test(flavor = "multi_thread")] +async fn open_document_watches_keep_deletions_and_renames() { + for registered in [false, true] { + let h = harness(&[("mock", incremental_caps())]); + let (_, path) = h.open("a.mk", "old"); + h.wait_status("mock", ServerStatus::Ready).await; + let s = h.launcher.state("mock"); + if registered { + watch_all(&s).await; + } + let closed = h.root.join("closed.mk"); + // An atomic replacement is reported as created and still uses document sync. + h.mgr.watched_files_changed(std::slice::from_ref(&path), &[path.clone(), closed.clone()], &[], &[]); + eventually(|| !s.notified("workspace/didChangeWatchedFiles").is_empty()).await; + assert_eq!( + s.notified("workspace/didChangeWatchedFiles")[0]["changes"], + json!([{ "uri": uri_of(&closed), "type": 2 }]) + ); + + let renamed = h.root.join("renamed.mk"); + std::fs::rename(&path, &renamed).unwrap(); + h.mgr.watched_files_changed(std::slice::from_ref(&renamed), &[], std::slice::from_ref(&path), &[]); + eventually(|| s.notified("workspace/didChangeWatchedFiles").len() == 2).await; + assert_eq!( + s.notified("workspace/didChangeWatchedFiles")[1]["changes"], + json!([ + { "uri": uri_of(&renamed), "type": 1 }, + { "uri": uri_of(&path), "type": 3 } + ]) + ); + h.mgr.shutdown_all().await; + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn open_document_watches_resolve_path_aliases() { + for registered in [false, true] { + let h = harness(&[("mock", incremental_caps())]); + let (_, path) = h.open("Mixed.mk", "old"); + h.wait_status("mock", ServerStatus::Ready).await; + let s = h.launcher.state("mock"); + if registered { + watch_all(&s).await; + } + let mut aliases = vec![path.clone()]; + let case_alias = h.root.join("mixed.mk"); + if dunce::canonicalize(&case_alias).ok() == Some(path.clone()) { + aliases.push(case_alias); + } + #[cfg(unix)] + { + let link = h.root.join("alias.mk"); + std::os::unix::fs::symlink(&path, &link).unwrap(); + aliases.push(link); + } + let closed = h.root.join("closed.mk"); + aliases.push(closed.clone()); + h.mgr.watched_files_changed(&[], &aliases, &[], &[]); + eventually(|| !s.notified("workspace/didChangeWatchedFiles").is_empty()).await; + assert_eq!( + s.notified("workspace/didChangeWatchedFiles")[0]["changes"], + json!([{ "uri": uri_of(&closed), "type": 2 }]) + ); + h.mgr.shutdown_all().await; + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn open_document_watches_are_scoped_to_the_attached_server() { + let h = harness(&[("mock", incremental_caps()), ("other", incremental_caps())]); + h.mgr.set_servers_hook(Box::new(|_, configs| configs.iter().filter(|c| c.name == "mock").cloned().collect())); + let (_, path) = h.open("a.mk", "old"); + h.wait_status("mock", ServerStatus::Ready).await; + h.mgr.set_servers_hook(Box::new(|_, configs| configs.iter().filter(|c| c.name == "other").cloned().collect())); + h.open("b.mk", "other"); + h.wait_status("other", ServerStatus::Ready).await; + let first = h.launcher.state("mock"); + let second = h.launcher.state("other"); + watch_all(&first).await; + watch_all(&second).await; + let closed = h.root.join("closed.mk"); + h.mgr.watched_files_changed(&[], &[path.clone(), closed.clone()], &[], &[]); + eventually(|| { + !first.notified("workspace/didChangeWatchedFiles").is_empty() + && !second.notified("workspace/didChangeWatchedFiles").is_empty() + }) + .await; + assert_eq!( + first.notified("workspace/didChangeWatchedFiles")[0]["changes"], + json!([{ "uri": uri_of(&closed), "type": 2 }]) + ); + assert_eq!( + second.notified("workspace/didChangeWatchedFiles")[0]["changes"], + json!([ + { "uri": uri_of(&path), "type": 2 }, { "uri": uri_of(&closed), "type": 2 } + ]) + ); + h.mgr.shutdown_all().await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn open_document_watches_resume_on_a_restart_without_open_documents() { + let h = harness(&[("mock", incremental_caps())]); + let (doc, path) = h.open("a.mk", "old"); + h.wait_status("mock", ServerStatus::Ready).await; + let s = h.launcher.state("mock"); + let id = h.mgr.servers()[0].id; + h.mgr.restart(id).await.unwrap(); + let closed = h.root.join("closed.mk"); + h.mgr.watched_files_changed(&[], &[path.clone(), closed.clone()], &[], &[]); + eventually(|| !s.notified("workspace/didChangeWatchedFiles").is_empty()).await; + assert_eq!( + s.notified("workspace/didChangeWatchedFiles")[0]["changes"], + json!([{ "uri": uri_of(&closed), "type": 2 }]) + ); + h.mgr.did_close(doc); + h.mgr.restart(id).await.unwrap(); + h.mgr.watched_files_changed(&[], std::slice::from_ref(&path), &[], &[]); + eventually(|| s.notified("workspace/didChangeWatchedFiles").len() == 2).await; + assert_eq!( + s.notified("workspace/didChangeWatchedFiles")[1]["changes"], + json!([{ "uri": uri_of(&path), "type": 2 }]) + ); + h.mgr.shutdown_all().await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn servers_without_document_sync_keep_open_document_watches() { + let h = harness(&[("mock", json!({ "textDocumentSync": 0 }))]); + let (_, path) = h.open("a.mk", "old"); + h.wait_status("mock", ServerStatus::Ready).await; + let s = h.launcher.state("mock"); + let closed = h.root.join("closed.mk"); + h.mgr.watched_files_changed(&[], &[path.clone(), closed.clone()], &[], &[]); + eventually(|| !s.notified("workspace/didChangeWatchedFiles").is_empty()).await; + assert_eq!( + s.notified("workspace/didChangeWatchedFiles")[0]["changes"], + json!([{ "uri": uri_of(&path), "type": 2 }, { "uri": uri_of(&closed), "type": 2 }]) + ); + h.mgr.shutdown_all().await; +} + /// A server that registers file watchers hears of what they match, inside excluded folders /// too (`uv add` into `.venv`, `go get` rewriting `go.mod`); folders outside the project /// that relative patterns name are reported for watching; unregistered, it falls back to