diff --git a/crates/ide-core/src/event.rs b/crates/ide-core/src/event.rs index 9db7dfb0..bebd8422 100644 --- a/crates/ide-core/src/event.rs +++ b/crates/ide-core/src/event.rs @@ -67,6 +67,11 @@ pub enum IdeEvent { status: ServerStatus, message: Option, }, + /// A stopped language server left the project's list (disabled in settings, or a + /// fallback that a preferred server took over from again). + LanguageServerRemoved { + server: ServerId, + }, /// A language server asked clients to re-request one kind of data for its /// open documents (`workspace/inlayHint/refresh`, …), e.g. after indexing. LspRefresh { diff --git a/crates/ide-lsp/src/lib.rs b/crates/ide-lsp/src/lib.rs index d313d417..bde52c93 100644 --- a/crates/ide-lsp/src/lib.rs +++ b/crates/ide-lsp/src/lib.rs @@ -1931,7 +1931,7 @@ impl LspManager { tracing::info!("{}: shadowed by an enabled server, stopping", s.name); self.stop_instance(s, true).await; } - self.servers.lock().retain(|s| !detached.iter().any(|d| d.id == s.id)); + self.forget(&detached); } /// Stops and forgets the servers that none of their languages uses any more. @@ -1949,10 +1949,18 @@ impl LspManager { self.stop_instance(s, true).await; } let gone = |s: &Arc| unlisted.iter().any(|u| u.id == s.id); - self.servers.lock().retain(|s| !gone(s)); for d in self.docs.lock().values_mut() { d.servers.retain(|a| !gone(&a.server)); } + self.forget(&unlisted); + } + + /// Drops stopped servers from the list and tells the UI they are gone. + fn forget(&self, gone: &[Arc]) { + self.servers.lock().retain(|s| !gone.iter().any(|g| g.id == s.id)); + for s in gone { + self.sink.emit(ide_core::IdeEvent::LanguageServerRemoved { server: s.id }); + } } pub fn set_apply_edit_hook(&self, hook: Box bool + Send + Sync>) { diff --git a/crates/ide-lsp/tests/mock_server.rs b/crates/ide-lsp/tests/mock_server.rs index 4f0da19c..d782be7c 100644 --- a/crates/ide-lsp/tests/mock_server.rs +++ b/crates/ide-lsp/tests/mock_server.rs @@ -1,7 +1,7 @@ //! Protocol tests against in-memory mock language servers. use ide_core::event::ServerStatus; -use ide_core::{DocId, FileEdit, IdeEvent, LanguageId, Pos, Range, Severity, WorkspaceEdit}; +use ide_core::{DocId, FileEdit, IdeEvent, LanguageId, Pos, Range, ServerId, Severity, WorkspaceEdit}; use ide_jsonrpc::{Handler, NoopHandler, Peer}; use ide_language::{LanguageConfig, LanguageRegistry, LanguageServerConfig}; use ide_lsp::types::*; @@ -9,7 +9,7 @@ use ide_lsp::{LaunchError, Launcher, LspManager}; use ide_text::AppliedChange; use parking_lot::Mutex; use serde_json::{Value, json}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -275,6 +275,22 @@ impl Harness { .collect() } + /// Whether every instance of `name` was reported as removed from the server list. + fn removed(&self, name: &str) -> bool { + let events = self.events.lock(); + let ids: HashSet = events + .iter() + .filter_map(|e| match e { + IdeEvent::LanguageServer { server, name: n, .. } if n == name => Some(*server), + _ => None, + }) + .collect(); + !ids.is_empty() + && ids.iter().all(|id| { + events.iter().any(|e| matches!(e, IdeEvent::LanguageServerRemoved { server } if server == id)) + }) + } + async fn wait_status(&self, name: &str, status: ServerStatus) { eventually(|| self.statuses(name).last() == Some(&status)).await; } @@ -847,6 +863,8 @@ async fn settings_changes_reach_servers_without_restarts() { h.mgr.reconfigure().await; assert_eq!(h.mgr.servers().iter().map(|s| s.name.as_str()).collect::>(), ["mock"]); assert_eq!(h.statuses("extra").last(), Some(&ServerStatus::Stopped)); + assert!(h.removed("extra")); + assert!(!h.removed("mock")); disabled.lock().clear(); h.mgr.reconfigure().await; h.wait_status("extra", ServerStatus::Ready).await; @@ -899,6 +917,7 @@ async fn a_fallback_leaves_when_the_preferred_server_is_enabled_again() { h.wait_status("alt", ServerStatus::Ready).await; eventually(|| h.launcher.state("alt").docs.lock().contains_key(&uri)).await; assert_eq!(h.mgr.servers().iter().map(|s| s.name.as_str()).collect::>(), ["alt"]); + assert!(h.removed("main")); // Enabled again: the fallback closes the document and stops. disabled.lock().clear(); @@ -907,6 +926,7 @@ async fn a_fallback_leaves_when_the_preferred_server_is_enabled_again() { eventually(|| h.launcher.state("main").docs.lock().contains_key(&uri)).await; assert!(!h.launcher.state("alt").notified("textDocument/didClose").is_empty()); assert_eq!(h.statuses("alt").last(), Some(&ServerStatus::Stopped)); + assert!(h.removed("alt")); assert_eq!(h.mgr.servers().iter().map(|s| s.name.as_str()).collect::>(), ["main"]); assert!(h.mgr.hover(doc, Pos::new(0, 0), Default::default()).await.is_ok()); assert_eq!(h.launcher.state("alt").requested("textDocument/hover").len(), 0); diff --git a/src/bindings.ts b/src/bindings.ts index 5ca5aa18..acde503a 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -3088,6 +3088,11 @@ export type IdeEvent = { type: "indexState"; indexing: boolean; files: number; symbols: number } | /** Git status / branch information should be refreshed. */ { type: "vcsChanged"; root: IpcPath } | { type: "languageServer"; server: ServerId; name: string; status: ServerStatus; message: string | null } | +/** + * A stopped language server left the project's list (disabled in settings, or a + * fallback that a preferred server took over from again). + */ +{ type: "languageServerRemoved"; server: ServerId } | /** * A language server asked clients to re-request one kind of data for its * open documents (`workspace/inlayHint/refresh`, …), e.g. after indexing. diff --git a/src/features/code-insight/servers.test.ts b/src/features/code-insight/servers.test.ts index 4ad01d45..e23f5c20 100644 --- a/src/features/code-insight/servers.test.ts +++ b/src/features/code-insight/servers.test.ts @@ -39,6 +39,36 @@ describe("server status", () => { await new Promise((r) => setTimeout(r, 0)); expect(useServers.getState().servers.get(id)).toMatchObject({ status: "missing", message: "install jdtls" }); }); + + it("drops a server the backend removed, even if an older snapshot still lists it", async () => { + useProject.setState({ project: { name: "p", root: "/p", isGit: false, settings: {} } as never }); + dispose = installServerEvents(); + const info = (id: number, name: string): ServerInfo => ({ + id: id as ServerId, + name, + languages: ["python"], + root: "/p", + status: "ready", + pid: null, + }); + pending.splice(0).forEach((resolve) => resolve([info(11, "basedpyright"), info(12, "pyright")])); + await vi.waitFor(() => expect(useServers.getState().servers.size).toBe(2)); + expect(serversSummary([...useServers.getState().servers.values()])?.text).toBe("basedpyright +1"); + + // The fallback stops and leaves the list; a snapshot taken before that answers late. + const status = (id: number, name: string, status: ServerInfo["status"]) => + emitLocalIdeEvent({ type: "languageServer", server: id as ServerId, name, status, message: null }); + status(13, "ty", "starting"); + status(12, "pyright", "stopped"); + emitLocalIdeEvent({ type: "languageServerRemoved", server: 12 as ServerId }); + expect(useServers.getState().servers.has(12 as ServerId)).toBe(false); + pending.splice(0).forEach((resolve) => resolve([info(11, "basedpyright"), info(12, "pyright"), info(13, "ty")])); + await vi.waitFor(() => expect(useServers.getState().servers.get(13 as ServerId)?.languages).toEqual(["python"])); + expect([...useServers.getState().servers.keys()]).toEqual([11, 13]); + + emitLocalIdeEvent({ type: "languageServerRemoved", server: 13 as ServerId }); + expect(serversSummary([...useServers.getState().servers.values()])?.text).toBe("basedpyright"); + }); }); describe("serversSummary", () => { @@ -54,4 +84,10 @@ describe("serversSummary", () => { expect(serversSummary([entry(1, "vtsls", "missing"), entry(2, "typescript-language-server", "ready")])?.tone).toBe("ok"); expect(serversSummary([entry(1, "vtsls", "missing")])?.tone).toBe("error"); }); + it("doesn't count stopped servers next to running ones", () => { + const text = (...servers: ReturnType[]) => serversSummary(servers)?.text; + expect(text(entry(1, "basedpyright", "ready"), entry(2, "pyright", "stopped"))).toBe("basedpyright"); + expect(text(entry(1, "basedpyright", "ready"), entry(2, "ty", "starting"))).toBe("basedpyright +1"); + expect(serversSummary([entry(2, "pyright", "stopped")])).toMatchObject({ text: "pyright", tone: "idle" }); + }); }); diff --git a/src/features/code-insight/servers.ts b/src/features/code-insight/servers.ts index 4cbf940a..1da19ec0 100644 --- a/src/features/code-insight/servers.ts +++ b/src/features/code-insight/servers.ts @@ -1,4 +1,4 @@ -/** Language server status: `lsp_servers` snapshot + `languageServer` events. */ +/** Language server status: `lsp_servers` snapshot + `languageServer` / `languageServerRemoved` events. */ import { getAction } from "@/platform/actions"; import { create } from "zustand"; import type { ServerId, ServerInfo, ServerStatus } from "@/bindings"; @@ -26,6 +26,8 @@ const notifiedMissing = new Set(); /** Event count at each server's latest event, so a snapshot taken earlier doesn't undo it. */ let eventSeq = 0; const lastEvent = new Map(); +/** Event count when each server left the list, so an older snapshot doesn't bring it back. */ +const removedAt = new Map(); function notifyMissing(e: ServerEntry) { if (notifiedMissing.has(e.name)) return; @@ -66,6 +68,7 @@ export async function refreshServers() { const prev = useServers.getState().servers; const map = new Map(); for (const s of list) { + if ((removedAt.get(s.id) ?? 0) > since) continue; const p = prev.get(s.id); const newer = p && (lastEvent.get(s.id) ?? 0) > since; map.set(s.id, { @@ -104,6 +107,16 @@ export function installServerEvents(): () => void { upsertServer({ id: e.server, name: e.name, status: e.status, message: e.message }); if (!known) void refreshServers(); }); + const offRemoved = onIdeEvent("languageServerRemoved", (e) => { + removedAt.set(e.server, ++eventSeq); + lastEvent.delete(e.server); + if (!useServers.getState().servers.has(e.server)) return; + useServers.setState((s) => { + const servers = new Map(s.servers); + servers.delete(e.server); + return { servers }; + }); + }); let root = useProject.getState().project?.root ?? null; void refreshServers(); const unsub = useProject.subscribe((s) => { @@ -115,6 +128,7 @@ export function installServerEvents(): () => void { }); return () => { off(); + offRemoved(); unsub(); }; } @@ -148,7 +162,9 @@ export function serversSummary(servers: readonly ServerEntry[]): { text: string; ); const busy = servers.filter((s) => s.status === "starting" || s.status === "busy"); const tone = bad.length ? "error" : busy.length ? "busy" : servers.some((s) => s.status === "ready") ? "ok" : "idle"; - const names = [...new Set(servers.map((s) => s.name))]; + // A stopped server (one a preferred server took over from) isn't worth naming next to running ones. + const active = servers.filter((s) => s.status !== "stopped"); + const names = [...new Set((active.length ? active : servers).map((s) => s.name))]; const text = names.length === 1 ? names[0]! : `${names[0]} +${names.length - 1}`; return { text, tone }; }