diff --git a/src/ui/lsp/client.rs b/src/ui/lsp/client.rs index 922cf189..f409a7e0 100644 --- a/src/ui/lsp/client.rs +++ b/src/ui/lsp/client.rs @@ -16,6 +16,7 @@ use std::path::Path; use std::process::{Child, Command, Stdio}; use std::sync::atomic::{AtomicI64, Ordering}; use std::sync::{Arc, Mutex, Weak}; +use std::time::Duration; use anyhow::{Context as _, anyhow}; use serde_json::Value; @@ -23,7 +24,18 @@ use serde_json::Value; use super::rpc::{self, FrameReader, Incoming, RequestId, ResponseError}; type Reply = smol::channel::Sender>; -type Pending = Arc>>; +/// Requests waiting for an answer. `None` once the server's output has +/// closed: nothing will ever be answered again, and a request made after +/// that fails at once instead of waiting forever. +type Pending = Arc>>>; + +/// How long an ordinary request waits for its answer. Everything the editor +/// asks is interactive — someone is waiting on a completion, a jump, a +/// rename — and a server that has not answered in this long is stuck. +pub(crate) const REQUEST_TIMEOUT: Duration = Duration::from_secs(10); + +/// `initialize` may index before answering; give it longer. +const INITIALIZE_TIMEOUT: Duration = Duration::from_secs(60); /// Something the server did that nobody asked for. #[derive(Debug)] @@ -121,7 +133,7 @@ impl LspClient { } })?; - let pending: Pending = Arc::default(); + let pending: Pending = Arc::new(Mutex::new(Some(HashMap::new()))); let (events_tx, events) = smol::channel::unbounded(); { let pending = pending.clone(); @@ -145,7 +157,11 @@ impl LspClient { }; let event = match message { Incoming::Response { id, result } => { - let reply = pending.lock().unwrap().remove(&id); + let reply = pending + .lock() + .unwrap() + .as_mut() + .and_then(|waiting| waiting.remove(&id)); if let Some(reply) = reply { let _ = reply.try_send(result); } @@ -167,7 +183,7 @@ impl LspClient { // Every request still waiting will never be answered: // dropping the senders fails them rather than leaving // them hanging. - pending.lock().unwrap().clear(); + pending.lock().unwrap().take(); let _ = events_tx.send_blocking(ServerEvent::Exited); })?; } @@ -211,7 +227,17 @@ impl LspClient { method: &str, params: Value, ) -> impl std::future::Future> + use<> { - self.request_inner(method, params, true) + self.request_inner(method, params, true, REQUEST_TIMEOUT) + } + + /// [`Self::request_raw`] with its own time limit. + pub(crate) fn request_raw_within( + self: &Arc, + method: &str, + params: Value, + timeout: Duration, + ) -> impl std::future::Future> + use<> { + self.request_inner(method, params, true, timeout) } /// `initialize`, which is the one request that goes past the gate. @@ -219,7 +245,7 @@ impl LspClient { self: &Arc, params: Value, ) -> impl std::future::Future> + use<> { - self.request_inner("initialize", params, false) + self.request_inner("initialize", params, false, INITIALIZE_TIMEOUT) } fn request_inner( @@ -227,15 +253,27 @@ impl LspClient { method: &str, params: Value, gated: bool, + timeout: Duration, ) -> impl std::future::Future> + use<> { let id = self.next_id.fetch_add(1, Ordering::Relaxed); let (tx, rx) = smol::channel::bounded(1); - self.pending.lock().unwrap().insert(id, tx); - self.send(rpc::encode(&rpc::request(id, method, params)), gated); + // Checked and registered under one lock: the reader closes the map + // under the same lock, so a request either makes it in before the + // server went away (and is failed with the rest) or sees it gone. + let registered = match self.pending.lock().unwrap().as_mut() { + Some(waiting) => { + waiting.insert(id, tx); + true + } + None => false, + }; + if registered { + self.send(rpc::encode(&rpc::request(id, method, params)), gated); + } let guard = CancelOnDrop { client: Arc::downgrade(self), id, - done: false, + done: !registered, }; let method = method.to_owned(); async move { @@ -243,7 +281,19 @@ impl LspClient { // capturing only `guard.done` would drop the guard — and cancel // the request — before it was ever sent an answer. let mut guard = guard; - let answer = rx.recv().await; + if !registered { + return Err(anyhow!("{method}: the language server has exited")); + } + let answer = smol::future::or(async { Some(rx.recv().await) }, async { + smol::Timer::after(timeout).await; + None + }) + .await; + // A timeout leaves `done` unset, so dropping the guard tells the + // server to stop working on it. + let Some(answer) = answer else { + return Err(anyhow!("{method}: no answer in {}s", timeout.as_secs_f32())); + }; guard.done = true; match answer { Ok(Ok(value)) => Ok(value), @@ -332,7 +382,13 @@ impl Drop for CancelOnDrop { let Some(client) = self.client.upgrade() else { return; }; - if client.pending.lock().unwrap().remove(&self.id).is_some() { + let removed = client + .pending + .lock() + .unwrap() + .as_mut() + .and_then(|waiting| waiting.remove(&self.id)); + if removed.is_some() { client.notify_raw("$/cancelRequest", serde_json::json!({ "id": self.id })); } } @@ -477,6 +533,35 @@ mod tests { )); } + #[test] + fn a_request_after_the_server_exited_fails_at_once() { + let (client, events, server) = connect(); + client.open_gate(); + drop(server); + assert!(matches!( + smol::block_on(events.recv()).unwrap(), + ServerEvent::Exited + )); + // No timeout involved: a limit of an hour would hang the test. + let late = + client.request_raw_within("textDocument/hover", Value::Null, Duration::from_secs(3600)); + let err = smol::block_on(late).unwrap_err(); + assert!(format!("{err:#}").contains("exited"), "{err:#}"); + } + + #[test] + fn a_request_nobody_answers_times_out_and_is_cancelled() { + let (client, _events, mut server) = connect(); + client.open_gate(); + let asked = client.request_raw_within("slow", Value::Null, Duration::from_millis(50)); + let sent = server.next(); + let err = smol::block_on(asked).unwrap_err(); + assert!(format!("{err:#}").contains("no answer"), "{err:#}"); + let cancel = server.next(); + assert_eq!(cancel["method"], "$/cancelRequest"); + assert_eq!(cancel["params"]["id"], sent["id"]); + } + #[test] fn dropping_an_unanswered_request_cancels_it() { let (client, _events, mut server) = connect(); diff --git a/src/ui/lsp/editor.rs b/src/ui/lsp/editor.rs index 26d5a355..3d501322 100644 --- a/src/ui/lsp/editor.rs +++ b/src/ui/lsp/editor.rs @@ -3,6 +3,7 @@ //! definition, format, rename), and the status bar's word on the server. use std::path::PathBuf; +use std::rc::Rc; use gpui::prelude::*; use gpui::{AnyElement, Context, Entity, WeakEntity, Window, div, px}; @@ -49,6 +50,14 @@ impl Tty7App { } else { Vec::new() }; + let app = cx.entity().downgrade(); + LspStore::register_window( + cx.entity_id(), + Rc::new(move |cx: &mut gpui::App| { + app.update(cx, |app, cx| app.lsp_sync_buffers(cx)).is_ok() + }), + cx, + ); LspStore::sync_window(cx.entity_id(), buffers, cx); } @@ -84,6 +93,7 @@ impl Tty7App { let text = f.input.read(cx).text().clone(); let doc = LspStore::context( &f.path, + f.input.entity_id(), Freshen::Text(&text), Some(window.window_handle()), cx, @@ -269,12 +279,16 @@ impl Tty7App { let text = f.input.read(cx).text().clone(); let Some(doc) = LspStore::context( &f.path, + f.input.entity_id(), Freshen::Text(&text), Some(window.window_handle()), cx, ) else { return; }; + // Every open buffer as the server sees it now; the answer is applied + // only if none of those it touches has changed by then. + let baseline = LspStore::snapshot(cx); let old_name = text .word_range(offset) .map(|r| text.slice(r).to_string()) @@ -293,7 +307,9 @@ impl Tty7App { cx.spawn_in(window, async move |_, cx| { let result = request.await; let _ = cx.update(|window, cx| match result { - Ok(Some(edit)) => super::apply_workspace_edit(edit, encoding, cx), + Ok(Some(edit)) => { + super::apply_workspace_edit(edit, encoding, Some(baseline), cx); + } Ok(None) => {} Err(e) => { let context = t_fmt(L10nKey::LspRenameFailed, &[("name", &old_name)]); diff --git a/src/ui/lsp/locations.rs b/src/ui/lsp/locations.rs index b931cdd3..1b391ad5 100644 --- a/src/ui/lsp/locations.rs +++ b/src/ui/lsp/locations.rs @@ -158,6 +158,7 @@ impl Tty7App { }; let Some(doc) = LspStore::context( &f.path, + f.input.entity_id(), Freshen::Text(&text), Some(window.window_handle()), cx, @@ -206,7 +207,8 @@ impl Tty7App { let Some(f) = self.editor_active_local_file() else { return; }; - let Some(doc) = LspStore::context(&f.path, Freshen::Skip, None, cx) else { + let Some(doc) = LspStore::context(&f.path, f.input.entity_id(), Freshen::Skip, None, cx) + else { return; }; if doc.caps.workspace_symbol_provider.is_none() { @@ -215,6 +217,7 @@ impl Tty7App { let app = cx.entity().downgrade(); let handle = window.window_handle(); let path = f.path.clone(); + let requester = f.input.entity_id(); let live: crate::ui::search::LiveQuery = std::rc::Rc::new(move |query: &str, cx: &mut App| { let (app, path, query) = (app.clone(), path.clone(), query.to_owned()); @@ -223,7 +226,7 @@ impl Tty7App { cx.defer(move |cx| { let _ = handle.update(cx, |_, window, cx| { let _ = app.update(cx, |app, cx| { - app.lsp_workspace_query(&path, query, window, cx) + app.lsp_workspace_query(&path, requester, query, window, cx) }); }); }); @@ -238,6 +241,7 @@ impl Tty7App { fn lsp_workspace_query( &mut self, path: &Path, + requester: gpui::EntityId, query: String, window: &mut Window, cx: &mut Context, @@ -252,7 +256,7 @@ impl Tty7App { } return; } - let Some(doc) = LspStore::context(path, Freshen::Skip, None, cx) else { + let Some(doc) = LspStore::context(path, requester, Freshen::Skip, None, cx) else { return; }; let client = doc.client.clone(); diff --git a/src/ui/lsp/mod.rs b/src/ui/lsp/mod.rs index 81dbbc05..f848647b 100644 --- a/src/ui/lsp/mod.rs +++ b/src/ui/lsp/mod.rs @@ -170,9 +170,16 @@ pub(crate) struct LspStore { docs: HashMap, missing: HashMap<&'static str, Instant>, next_generation: u64, + /// Each window editor's way to offer its buffers again, for when the + /// setting is turned back on. Returns `false` once the window is gone. + windows: HashMap, + /// `editor_lsp` as last seen, to tell a turn-on from any other change. + enabled: bool, _subscriptions: Vec, } +pub(crate) type Resync = Rc bool>; + impl Global for LspStore {} fn try_store_mut(cx: &mut App) -> Option<&mut LspStore> { @@ -199,15 +206,17 @@ impl LspStore { } let quit = cx.on_app_quit(|cx| LspStore::on_quit(cx)); let config = cx.observe_global::(|cx| { - if !cx.global::().editor_lsp - && cx - .try_global::() - .is_some_and(|s| !s.docs.is_empty()) - { - cx.update_global::(|store, cx| store.close_all(cx)); + let enabled = cx.global::().editor_lsp; + if cx.try_global::().is_none() { + return; } + cx.update_global::(|store, cx| store.set_enabled(enabled, cx)); }); + let enabled = cx + .try_global::() + .is_some_and(|c| c.editor_lsp); cx.set_global(LspStore { + enabled, _subscriptions: vec![quit, config], ..Default::default() }); @@ -220,6 +229,48 @@ impl LspStore { // ---- The editor's side ---- + /// Remembers how a window's editor offers its buffers, so turning the + /// setting back on can ask every window for them again. + pub(crate) fn register_window(app_id: EntityId, resync: Resync, cx: &mut App) { + Self::update(cx, |store, _| { + store.windows.entry(app_id).or_insert(resync); + }); + } + + /// The `editor_lsp` setting, as it changes. Off closes every document + /// and lets the servers go; on asks each window for its open buffers, + /// which starts them again — deferred, since the setting is changed from + /// inside some window's own update. + fn set_enabled(&mut self, enabled: bool, cx: &mut App) { + let was = std::mem::replace(&mut self.enabled, enabled); + if !enabled { + if !self.docs.is_empty() || !self.servers.is_empty() { + self.close_all(cx); + } + return; + } + if was { + return; + } + let windows: Vec<(EntityId, Resync)> = self + .windows + .iter() + .map(|(id, r)| (*id, r.clone())) + .collect(); + cx.defer(move |cx| { + let gone: Vec = windows + .into_iter() + .filter(|(_, resync)| !resync(cx)) + .map(|(id, _)| id) + .collect(); + if let Some(store) = try_store_mut(cx) { + for id in gone { + store.windows.remove(&id); + } + } + }); + } + /// Brings the documents one window's editor owns in line with the /// buffers it has: opens the new ones, closes the ones that went away or /// moved, and leaves the rest alone. Cheap when nothing changed. @@ -373,34 +424,57 @@ impl LspStore { /// The server behind `path`, with any edit still waiting sent first (see /// [`Freshen`]) so the request that follows sees the text the caller sees. + /// + /// Only the buffer that owns the document may ask (`requester` is its + /// `InputState`'s id). The same file open in a second window is a second + /// buffer whose text can differ from the owner's; letting it send that + /// text would swap the document under the owner, and letting it ask + /// against the owner's text would answer with positions for a text it + /// does not hold. So a non-owner is refused, quietly: that buffer simply + /// has no language features until the owner closes and it takes over. pub(crate) fn context( path: &Path, + requester: EntityId, sync: Freshen<'_>, window: Option, cx: &mut App, ) -> Option { cx.try_global::()?; Self::update(cx, |store, _| { - match sync { - Freshen::Text(text) => store.flush_text(path, text), - Freshen::Skip => {} - } - let doc = store.docs.get_mut(path)?; - if window.is_some() { - doc.window = window; - } - let server = store.servers.get(&doc.server)?; - if server.phase != Phase::Running { - return None; - } - Some(DocContext { - client: server.client.clone()?, - uri: doc.uri.clone(), - encoding: server.encoding, - caps: server.caps.clone(), - diagnostics: doc.diagnostics.clone(), - root: doc.server.root.clone(), - }) + store.context_for(path, requester, sync, window) + }) + } + + fn context_for( + &mut self, + path: &Path, + requester: EntityId, + sync: Freshen<'_>, + window: Option, + ) -> Option { + // Checked before anything is sent: a non-owner must never sync. + if self.docs.get(path)?.owner_id != requester { + return None; + } + match sync { + Freshen::Text(text) => self.flush_text(path, text), + Freshen::Skip => {} + } + let doc = self.docs.get_mut(path)?; + if window.is_some() { + doc.window = window; + } + let server = self.servers.get(&doc.server)?; + if server.phase != Phase::Running { + return None; + } + Some(DocContext { + client: server.client.clone()?, + uri: doc.uri.clone(), + encoding: server.encoding, + caps: server.caps.clone(), + diagnostics: doc.diagnostics.clone(), + root: doc.server.root.clone(), }) } @@ -849,8 +923,23 @@ impl LspStore { match serde_json::from_value::(params) { Ok(params) => { - apply_workspace_edit(params.edit, encoding, cx); - Ok(json!({ "applied": true })) + // Measured against what this server last + // heard: a buffer typed in since would take + // the edit at the wrong places. + let per_file = workspace_edit_files(params.edit); + match self.stale_target(&per_file, None, cx) { + Some(path) => Ok(json!({ + "applied": false, + "failureReason": format!( + "{} changed since the server last saw it", + path.display() + ), + })), + None => { + spawn_apply(per_file, encoding, cx); + Ok(json!({ "applied": true })) + } + } } Err(e) => { Ok(json!({ "applied": false, "failureReason": e.to_string() })) @@ -1018,13 +1107,10 @@ impl LspStore { let key = key.clone(); cx.spawn(async move |cx| { let executor = cx.background_executor().clone(); - let answered = client.request_raw("shutdown", Value::Null); - let timeout = executor.timer(Duration::from_secs(2)); - smol::future::or(async { answered.await.map(|_| ()).ok() }, async { - timeout.await; - None - }) - .await; + // Two seconds to say it is done; it is told to exit either way. + let _ = client + .request_raw_within("shutdown", Value::Null, Duration::from_secs(2)) + .await; client.notify_raw("exit", Value::Null); for _ in 0..20 { if client.reap_if_exited() { @@ -1189,12 +1275,105 @@ fn initialize_params(root: &Path, spec: &ServerSpec) -> Value { /// does, straight to disk otherwise. Run on a later turn of the event loop, /// never inside the update that asked for it — a buffer's own window may be /// the one being updated. +/// +/// `baseline` is what every open document held when the request that +/// produced the edit went out ([`LspStore::snapshot`]). If any buffer the +/// edit touches holds something else now — typed in, opened or closed since +/// — nothing at all is applied: half a rename is worse than none. Without a +/// baseline, the text the server last heard is the measure. Returns whether +/// the edit was applied. pub(crate) fn apply_workspace_edit( edit: lsp_types::WorkspaceEdit, encoding: Encoding, + baseline: Option>, cx: &mut App, -) { +) -> bool { let per_file = workspace_edit_files(edit); + let stale = cx + .try_global::() + .and_then(|store| store.stale_target(&per_file, baseline.as_ref(), cx)); + if let Some(path) = stale { + log::info!( + "lsp: not applying an edit: {} changed since it was asked for", + path.display() + ); + return false; + } + spawn_apply(per_file, encoding, cx); + true +} + +/// Whether every text in `now` is the one in `baseline`, for the files an +/// edit touches: the first that is not. A file open in only one of the two +/// was opened or closed in between, which counts as changed. +pub(crate) fn first_stale( + targets: &[PathBuf], + now: &HashMap, + baseline: &HashMap, +) -> Option { + targets + .iter() + .find(|path| match (now.get(*path), baseline.get(*path)) { + (Some(now), Some(then)) => now != then, + (None, None) => false, + _ => true, + }) + .cloned() +} + +impl LspStore { + /// What every open document's buffer holds right now — the baseline a + /// request that answers with edits is checked against. + pub(crate) fn snapshot(cx: &App) -> HashMap { + let Some(store) = cx.try_global::() else { + return HashMap::new(); + }; + store + .docs + .iter() + .filter_map(|(path, doc)| { + Some((path.clone(), doc.owner.upgrade()?.read(cx).text().clone())) + }) + .collect() + } + + fn stale_target( + &self, + per_file: &[(Uri, Vec)], + baseline: Option<&HashMap>, + cx: &App, + ) -> Option { + let targets: Vec = per_file + .iter() + .filter_map(|(u, _)| uri_to_path(u)) + .collect(); + let open = |path: &PathBuf| { + let doc = self.docs.get(path)?; + Some((doc, doc.owner.upgrade()?)) + }; + let now: HashMap = targets + .iter() + .filter_map(|p| Some((p.clone(), open(p)?.1.read(cx).text().clone()))) + .collect(); + let server_heard: HashMap; + let baseline = match baseline { + Some(baseline) => baseline, + None => { + server_heard = targets + .iter() + .filter_map(|p| { + let (doc, _) = open(p)?; + Some((p.clone(), doc.sent.clone().filter(|_| !doc.dirty)?)) + }) + .collect(); + &server_heard + } + }; + first_stale(&targets, &now, baseline) + } +} + +fn spawn_apply(per_file: Vec<(Uri, Vec)>, encoding: Encoding, cx: &mut App) { cx.spawn(async move |cx| { for (uri, edits) in per_file { let Some(path) = uri_to_path(&uri) else { diff --git a/src/ui/lsp/providers.rs b/src/ui/lsp/providers.rs index ed8aa4c0..fe19bf72 100644 --- a/src/ui/lsp/providers.rs +++ b/src/ui/lsp/providers.rs @@ -34,7 +34,13 @@ use crate::ui::app::Tty7App; const MAX_COMPLETIONS: usize = 300; pub(crate) struct LspProvider { + /// Every open buffer's text when the code-action menu was last filled: + /// an action picked from it is applied only if what it edits is + /// unchanged since. + actions_baseline: std::cell::RefCell>>, path: PathBuf, + /// The buffer this provider serves — always the document's owner. + input: gpui::EntityId, app: WeakEntity, } @@ -45,7 +51,9 @@ pub(super) fn install( cx: &mut App, ) { let provider = Rc::new(LspProvider { + actions_baseline: Default::default(), path: path.to_path_buf(), + input: input.entity_id(), app, }); input.update(cx, |state, cx| { @@ -69,7 +77,13 @@ pub(super) fn uninstall(input: &Entity, cx: &mut App) { impl LspProvider { fn context(&self, sync: Freshen<'_>, window: &Window, cx: &mut App) -> Option { - LspStore::context(&self.path, sync, Some(window.window_handle()), cx) + LspStore::context( + &self.path, + self.input, + sync, + Some(window.window_handle()), + cx, + ) } } @@ -143,7 +157,7 @@ impl CompletionProvider for LspProvider { ) -> Task> { // Resolving asks nothing of the text, which the caller may be in the // middle of changing: nothing is sent first. - let Some(doc) = LspStore::context(&self.path, Freshen::Skip, None, cx) else { + let Some(doc) = LspStore::context(&self.path, self.input, Freshen::Skip, None, cx) else { return Task::ready(Ok(item)); }; let resolvable = doc @@ -367,7 +381,7 @@ impl DefinitionProvider for LspProvider { return false; }; let position = location.target_selection_range.start; - let encoding = LspStore::context(&self.path, Freshen::Skip, None, cx) + let encoding = LspStore::context(&self.path, self.input, Freshen::Skip, None, cx) .map(|d| d.encoding) .unwrap_or_default(); let app = self.app.clone(); @@ -440,6 +454,7 @@ impl CodeActionProvider for LspProvider { if doc.caps.code_action_provider.is_none() { return not_available(); } + *self.actions_baseline.borrow_mut() = Some(LspStore::snapshot(cx)); let lsp_range = convert::offsets_to_lsp_range(&text, range, doc.encoding); // The diagnostics the range touches, which is what turns a server's // quick fixes on. @@ -501,6 +516,14 @@ impl CodeActionProvider for LspProvider { Some(lsp_types::CodeActionProviderCapability::Options(o)) if o.resolve_provider == Some(true) ); + // Taken when the menu was filled, which is also when an action that + // came with its edit was computed. Without one there is nothing to + // vouch for the open buffers, and an edit to any of them is refused. + let baseline = self + .actions_baseline + .borrow_mut() + .take() + .unwrap_or_default(); let client = doc.client.clone(); let encoding = doc.encoding; window.spawn(cx, async move |cx| { @@ -513,7 +536,13 @@ impl CodeActionProvider for LspProvider { action }; if let Some(edit) = action.edit { - cx.update(|_, cx| super::apply_workspace_edit(edit, encoding, cx))?; + let applied = cx.update(|_, cx| { + super::apply_workspace_edit(edit, encoding, Some(baseline), cx) + })?; + if !applied { + // Its command would act on the same stale ground. + return Ok(()); + } } if let Some(command) = action.command { // Whatever this changes comes back as `workspace/applyEdit`. diff --git a/src/ui/lsp/signature.rs b/src/ui/lsp/signature.rs index 4319f69f..9034d110 100644 --- a/src/ui/lsp/signature.rs +++ b/src/ui/lsp/signature.rs @@ -113,7 +113,8 @@ impl Tty7App { }; // Looked at without sending the edit: most keystrokes stop here, and // the server hears about them on the usual debounce. - let Some(doc) = LspStore::context(&f.path, Freshen::Skip, None, cx) else { + let Some(doc) = LspStore::context(&f.path, f.input.entity_id(), Freshen::Skip, None, cx) + else { return; }; let Some(options) = doc.caps.signature_help_provider.as_ref() else { @@ -127,7 +128,9 @@ impl Tty7App { if !is_trigger && !visible { return; } - let Some(doc) = LspStore::context(&f.path, Freshen::Text(&text), None, cx) else { + let Some(doc) = + LspStore::context(&f.path, f.input.entity_id(), Freshen::Text(&text), None, cx) + else { return; }; let context = lsp_types::SignatureHelpContext { diff --git a/src/ui/lsp/tests.rs b/src/ui/lsp/tests.rs index abad6237..4e5ac966 100644 --- a/src/ui/lsp/tests.rs +++ b/src/ui/lsp/tests.rs @@ -260,3 +260,283 @@ fn lsp_smoke_a_real_server_publishes_diagnostics_and_shuts_down() { client.kill(); assert!(exited, "clangd exits on `exit`"); } + +/// A store with one running server on the far end of two pipes, and one +/// document owned by `owner`. Returns the path and what the server reads. +mod harness { + use std::io::Read as _; + + use gpui::{Entity, TestAppContext, WindowHandle}; + use gpui_component::input::InputState; + + use super::super::*; + use crate::ui::lsp::rpc::FrameReader; + + pub(super) struct FakeServer { + frames: FrameReader, + from_client: std::io::PipeReader, + _to_client: std::io::PipeWriter, + } + + impl FakeServer { + pub(super) fn next(&mut self) -> Value { + let mut chunk = [0u8; 4096]; + loop { + if let Some(body) = self.frames.next_frame() { + return serde_json::from_slice(&body).unwrap(); + } + let n = self.from_client.read(&mut chunk).unwrap(); + assert!(n > 0, "the client closed its end"); + self.frames.push(&chunk[..n]); + } + } + } + + pub(super) struct Setup { + pub(super) window: WindowHandle, + pub(super) owner: Entity, + pub(super) other: Entity, + pub(super) path: PathBuf, + pub(super) server: FakeServer, + } + + pub(super) fn setup(cx: &mut TestAppContext) -> Setup { + cx.update(|cx| { + gpui_component::init(cx); + cx.set_global(crate::core::config::Config::default()); + }); + let window = cx.add_window(|window, cx| { + InputState::new(window, cx) + .code_editor("rust") + .multi_line(true) + .default_value("fn foo() {}\n") + }); + let owner = window.root(cx).unwrap(); + let other = window + .update(cx, |_, window, cx| { + cx.new(|cx| { + InputState::new(window, cx) + .code_editor("rust") + .multi_line(true) + .default_value("fn foo() { /* the other window's edit */ }\n") + }) + }) + .unwrap(); + let path = PathBuf::from("/tmp/tty7-lsp-test/src/lib.rs"); + let (client_reads, server_writes) = std::io::pipe().unwrap(); + let (server_reads, client_writes) = std::io::pipe().unwrap(); + let (client, _events) = + LspClient::from_streams("fake", client_reads, client_writes).unwrap(); + client.open_gate(); + let key = ServerKey { + name: "rust-analyzer", + root: PathBuf::from("/tmp/tty7-lsp-test"), + }; + cx.update(|cx| { + LspStore::ensure(cx); + let text = owner.read(cx).text().clone(); + let store = cx.global_mut::(); + store.servers.insert( + key.clone(), + Server { + spec: servers::spec_for_language("rust").unwrap(), + label: "fake".into(), + generation: 1, + phase: Phase::Running, + client: Some(client), + caps: Rc::new(lsp_types::ServerCapabilities::default()), + encoding: Encoding::Utf16, + crashes: Vec::new(), + idle_seq: 0, + _tasks: Vec::new(), + }, + ); + store.docs.insert( + path.clone(), + Doc { + uri: path_to_uri(&path).unwrap(), + language_id: "rust", + server: key, + version: 0, + owner: owner.downgrade(), + owner_id: owner.entity_id(), + app_id: owner.entity_id(), + app: WeakEntity::new_invalid(), + window: Some(window.into()), + sent: Some(text), + dirty: false, + change_seq: 0, + symbols_seq: 0, + diagnostics: Vec::new(), + problems: (0, 0), + }, + ); + }); + Setup { + window, + owner, + other, + path, + server: FakeServer { + frames: FrameReader::default(), + from_client: server_reads, + _to_client: server_writes, + }, + } + } +} + +/// The same file open in a second window is a second buffer, maybe with +/// other text. Its requests are refused before anything is sent, so the +/// server's copy stays the owner's. +#[gpui::test] +fn a_buffer_that_does_not_own_the_document_never_syncs(cx: &mut gpui::TestAppContext) { + let mut s = harness::setup(cx); + cx.update(|cx| { + let other_text = s.other.read(cx).text().clone(); + let refused = LspStore::context( + &s.path, + s.other.entity_id(), + Freshen::Text(&other_text), + None, + cx, + ); + assert!(refused.is_none(), "a non-owner gets no server"); + + let owner_text = gpui_component::Rope::from("fn foo() { owner(); }\n"); + let allowed = LspStore::context( + &s.path, + s.owner.entity_id(), + Freshen::Text(&owner_text), + None, + cx, + ); + assert!(allowed.is_some()); + }); + // The first thing the server hears is the owner's change: the other + // buffer's text never went out. + let change = s.server.next(); + assert_eq!(change["method"], "textDocument/didChange"); + assert_eq!( + change["params"]["contentChanges"][0]["text"], + "fn foo() { owner(); }\n" + ); +} + +#[test] +fn an_edit_is_stale_when_a_file_it_touches_changed_opened_or_closed() { + let rope = gpui_component::Rope::from; + let (a, b, c) = ( + PathBuf::from("/a"), + PathBuf::from("/b"), + PathBuf::from("/c"), + ); + let baseline = HashMap::from([(a.clone(), rope("x")), (b.clone(), rope("y"))]); + let targets = vec![a.clone(), c.clone()]; + // `a` unchanged, `c` on disk then and now. + let now = HashMap::from([(a.clone(), rope("x")), (b.clone(), rope("typed"))]); + assert_eq!( + first_stale(&targets, &now, &baseline), + None, + "only targets count" + ); + // `a` typed in. + let now = HashMap::from([(a.clone(), rope("x!"))]); + assert_eq!(first_stale(&targets, &now, &baseline), Some(a.clone())); + // `c` opened since. + let now = HashMap::from([(a.clone(), rope("x")), (c.clone(), rope("z"))]); + assert_eq!(first_stale(&targets, &now, &baseline), Some(c.clone())); + // `a` closed since. + assert_eq!(first_stale(&targets, &HashMap::new(), &baseline), Some(a)); +} + +/// A rename or code action's edit, answered against a text the buffer has +/// since moved on from, is dropped whole; answered against the text it +/// still holds, it is applied. +#[gpui::test] +fn a_workspace_edit_for_a_changed_buffer_is_not_applied(cx: &mut gpui::TestAppContext) { + let s = harness::setup(cx); + let rename = |cx: &mut gpui::App, baseline| { + let mut changes = HashMap::new(); + changes.insert( + path_to_uri(&s.path).unwrap(), + vec![TextEdit { + range: Range::new(Position::new(0, 3), Position::new(0, 6)), + new_text: "bar".into(), + }], + ); + let edit = WorkspaceEdit { + changes: Some(changes), + ..Default::default() + }; + apply_workspace_edit(edit, Encoding::Utf16, Some(baseline), cx) + }; + + let baseline = cx.update(|cx| LspStore::snapshot(cx)); + // Typed in after the request went out. + s.window + .update(cx, |state, window, cx| { + state.set_value("fn foo() { typed }\n", window, cx) + }) + .unwrap(); + assert!(!cx.update(|cx| rename(cx, baseline))); + cx.run_until_parked(); + assert_eq!( + cx.update(|cx| s.owner.read(cx).text().to_string()), + "fn foo() { typed }\n" + ); + + let baseline = cx.update(|cx| LspStore::snapshot(cx)); + assert!(cx.update(|cx| rename(cx, baseline))); + cx.run_until_parked(); + assert_eq!( + cx.update(|cx| s.owner.read(cx).text().to_string()), + "fn bar() { typed }\n" + ); +} + +/// Turning the setting back on asks every window for its open files again, +/// which is what starts their servers. +#[gpui::test] +fn turning_lsp_back_on_offers_every_window_again(cx: &mut gpui::TestAppContext) { + use std::cell::Cell; + cx.update(|cx| cx.set_global(crate::core::config::Config::default())); + let asked = Rc::new(Cell::new(0)); + let window_gone = Rc::new(Cell::new(false)); + cx.update(|cx| { + let asked = asked.clone(); + LspStore::register_window( + EntityId::from(1u64), + Rc::new(move |_| { + asked.set(asked.get() + 1); + true + }), + cx, + ); + let gone = window_gone.clone(); + LspStore::register_window( + EntityId::from(2u64), + Rc::new(move |_| { + gone.set(true); + false + }), + cx, + ); + }); + let set = |cx: &mut gpui::TestAppContext, on: bool| { + cx.update(|cx| { + cx.update_global::(|c, _| c.editor_lsp = on) + }); + cx.run_until_parked(); + }; + set(cx, false); + assert_eq!(asked.get(), 0, "turning it off asks nobody"); + set(cx, true); + assert_eq!(asked.get(), 1); + assert!(window_gone.get()); + // A closed window is forgotten; one still open is asked on every turn-on. + set(cx, false); + set(cx, true); + assert_eq!(asked.get(), 2); + assert_eq!(cx.update(|cx| cx.global::().windows.len()), 1); +} diff --git a/src/ui/settings/editor.rs b/src/ui/settings/editor.rs index 2764c340..bcb49057 100644 --- a/src/ui/settings/editor.rs +++ b/src/ui/settings/editor.rs @@ -61,8 +61,9 @@ impl Tty7App { ) } - /// Off closes every document's server (`LspStore` watches the config); - /// on lets the next frame open them again. + /// `LspStore` watches the config: off closes every document and lets + /// its server go; on asks every window for the files it has open, which + /// starts their servers again. pub(crate) fn set_editor_lsp(&mut self, on: bool, cx: &mut Context) { self.update_config(cx, |cfg| cfg.editor_lsp = on); cx.notify();