From b1347dd3311f3cc5f1793aa14e8d98b97cc805dd Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Thu, 29 Oct 2026 20:49:01 +0800 Subject: [PATCH] fix(lsp): keep background usage counts from delaying navigation (#282) * fix(lsp): keep background usage counts from delaying navigation * fix(lsp): retry incomplete counts and preserve newer hints --- src/editor/lsp/codeLens.test.ts | 192 ++++++++++++++++++++++- src/editor/lsp/codeLens.ts | 65 +++++--- src/editor/lsp/hint.test.ts | 62 ++++++++ src/editor/lsp/hint.ts | 27 +++- src/editor/lsp/navigation.ts | 9 +- src/editor/lsp/navigationPending.test.ts | 62 ++++++++ src/platform/locales/text/zh.json | 1 + 7 files changed, 394 insertions(+), 24 deletions(-) create mode 100644 src/editor/lsp/hint.test.ts create mode 100644 src/editor/lsp/navigationPending.test.ts diff --git a/src/editor/lsp/codeLens.test.ts b/src/editor/lsp/codeLens.test.ts index d9f450437..69b5be68b 100644 --- a/src/editor/lsp/codeLens.test.ts +++ b/src/editor/lsp/codeLens.test.ts @@ -1,6 +1,72 @@ -import { describe, expect, it } from "vitest"; -import type { OutlineItem } from "@/platform/ipc"; -import { lensTargets } from "./codeLens"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { EditorState } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import type { OutlineItem, References } from "@/platform/ipc"; +import { _addEditor, _removeEditor, codeLenses, documentSynced } from "@/editor/api"; + +const ipc = vi.hoisted(() => ({ + docOutline: vi.fn(), + lspReferences: vi.fn(), + cancelRequest: vi.fn<(id: number) => Promise>(async () => {}), +})); +vi.mock("@/platform/ipc", async (orig) => { + const real = await orig(); + return { ...real, isTauri: () => true, commands: { ...real.commands, ...ipc } }; +}); +const executeAction = vi.hoisted(() => vi.fn(async () => true)); +vi.mock("@/platform/actions", () => ({ executeAction })); + +const { lensTargets, usagesLensExtension } = await import("./codeLens"); +const { lspDoc } = await import("./lsp"); +const { useProject } = await import("@/platform/project"); +const { emitLocalIdeEvent } = await import("@/platform/ipc"); + +const ctx = { doc: 7, path: "/p/a.rs", language: "rust", uiMode: "Rust", readOnly: false }; +const views: EditorView[] = []; +const ok = (data: T) => ({ status: "ok" as const, data }); +const refs = (n: number, incomplete: References["incomplete"] = null) => + ok({ + locations: Array.from({ length: n }, () => ({ path: "/p/b.rs", range: item("x", "function", 0).range })), + incomplete, + }); + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((r) => (resolve = r)); + return { promise, resolve }; +} + +function editor(count = 1, sync?: () => Promise) { + const view = new EditorView({ + state: EditorState.create({ + doc: Array.from({ length: count }, (_, i) => `fn f${i}() {}`).join("\n"), + extensions: [lspDoc.of(ctx), usagesLensExtension(), ...(sync ? [documentSynced.of(sync)] : [])], + }), + parent: document.body, + }); + views.push(view); + return view; +} + +const titles = (view: EditorView) => view.state.facet(codeLenses).map((l) => l.title); +const settle = () => vi.advanceTimersByTimeAsync(1000); + +beforeEach(() => { + vi.useFakeTimers(); + ipc.docOutline.mockReset().mockResolvedValue(ok([item("f0", "function", 0)])); + ipc.lspReferences.mockReset().mockResolvedValue(refs(2)); + ipc.cancelRequest.mockClear(); + executeAction.mockClear(); + useProject.setState({ + settings: { editor: { inlay: { codeVisionUsages: true, codeVisionAuthor: false } } } as never, + }); +}); + +afterEach(() => { + views.splice(0).forEach((v) => v.destroy()); + useProject.setState({ settings: null }); + vi.useRealTimers(); +}); const item = (name: string, kind: OutlineItem["kind"], line: number, children: OutlineItem[] = []): OutlineItem => { const r = { start: { line, col: 0 }, end: { line, col: 1 } }; @@ -18,3 +84,123 @@ describe("usages lens targets", () => { expect(lensTargets(items).map((t) => t.name)).toEqual(["Cart", "count"]); }); }); + +describe("background usage counts", () => { + it("uses the server's existing scope and leaves full searches to the explicit action", async () => { + const view = editor(); + await settle(); + expect(ipc.lspReferences).toHaveBeenCalledWith(expect.any(Number), 7, { line: 0, col: 0 }, false, false); + expect(titles(view)).toEqual(["2 usages"]); + view.state.facet(codeLenses)[0]!.run!(view, new MouseEvent("click")); + expect(executeAction).toHaveBeenCalledWith("find.showUsages"); + }); + + it("allows a full search even when the server's current scope has no usages", async () => { + ipc.lspReferences.mockResolvedValue(refs(0)); + const view = editor(); + await settle(); + expect(titles(view)).toEqual(["no usages"]); + view.state.facet(codeLenses)[0]!.run!(view, new MouseEvent("click")); + expect(executeAction).toHaveBeenCalledWith("find.showUsages"); + }); + + it("does not cache an incomplete count while indexing, and retries on server refresh", async () => { + ipc.lspReferences.mockResolvedValueOnce(refs(0, "indexing")); + const view = editor(); + await settle(); + expect(titles(view)).toEqual([]); + emitLocalIdeEvent({ type: "lspRefresh", kind: "codeLens", server: 1 }); + await settle(); + expect(ipc.lspReferences).toHaveBeenCalledTimes(2); + expect(titles(view)).toEqual(["2 usages"]); + }); + + it("cancels obsolete requests immediately on edits and ignores their late replies", async () => { + const old = deferred>(); + ipc.lspReferences.mockReturnValueOnce(old.promise); + const view = editor(); + await settle(); + const request = ipc.lspReferences.mock.calls[0]![0]; + view.dispatch({ changes: { from: view.state.doc.length, insert: " " } }); + expect(ipc.cancelRequest).toHaveBeenCalledWith(request); + await settle(); + expect(titles(view)).toEqual(["2 usages"]); + old.resolve(refs(99)); + await settle(); + expect(titles(view)).toEqual(["2 usages"]); + }); + + it("retries incomplete counts when indexing ends without a code-lens refresh", async () => { + ipc.lspReferences.mockResolvedValueOnce(refs(0, "indexing")); + const view = editor(); + emitLocalIdeEvent({ type: "languageServer", server: 1, name: "rust-analyzer", status: "busy", message: null }); + await settle(); + expect(titles(view)).toEqual([]); + await settle(); + emitLocalIdeEvent({ type: "languageServer", server: 1, name: "rust-analyzer", status: "ready", message: null }); + await settle(); + expect(ipc.lspReferences).toHaveBeenCalledTimes(2); + expect(titles(view)).toEqual(["2 usages"]); + }); + + it("does not repeat complete counts when the server finishes other work", async () => { + const view = editor(); + emitLocalIdeEvent({ type: "languageServer", server: 1, name: "rust-analyzer", status: "busy", message: null }); + await settle(); + await settle(); + emitLocalIdeEvent({ type: "languageServer", server: 1, name: "rust-analyzer", status: "ready", message: null }); + await settle(); + expect(ipc.lspReferences).toHaveBeenCalledTimes(1); + expect(titles(view)).toEqual(["2 usages"]); + }); + + it("cancels all in-flight requests on destroy without starting the next batch", async () => { + const pending = deferred>(); + ipc.docOutline.mockResolvedValue(ok(Array.from({ length: 8 }, (_, i) => item(`f${i}`, "function", i)))); + ipc.lspReferences.mockReturnValue(pending.promise); + const view = editor(8); + await settle(); + expect(ipc.lspReferences).toHaveBeenCalledTimes(4); + const requests = ipc.lspReferences.mock.calls.map(([id]) => id); + view.destroy(); + views.splice(views.indexOf(view), 1); + expect(ipc.cancelRequest.mock.calls.map(([id]) => id)).toEqual(requests); + pending.resolve(refs(1)); + await settle(); + expect(ipc.lspReferences).toHaveBeenCalledTimes(4); + }); + + it("cancels pending counts when the feature is disabled", async () => { + const pending = deferred>(); + ipc.lspReferences.mockReturnValue(pending.promise); + const view = editor(); + await settle(); + useProject.setState({ + settings: { editor: { inlay: { codeVisionUsages: false, codeVisionAuthor: false } } } as never, + }); + expect(ipc.cancelRequest).toHaveBeenCalledWith(ipc.lspReferences.mock.calls[0]![0]); + pending.resolve(refs(2)); + await settle(); + expect(titles(view)).toEqual([]); + }); + + it("does not send obsolete requests after waiting for document synchronization", async () => { + const synced = deferred(); + const view = editor(1, () => synced.promise); + const handle = { view, ctx }; + _addEditor(handle); + try { + await settle(); + expect(ipc.docOutline).not.toHaveBeenCalled(); + view.destroy(); + views.splice(views.indexOf(view), 1); + synced.resolve(); + await settle(); + expect(ipc.docOutline).not.toHaveBeenCalled(); + expect(ipc.lspReferences).not.toHaveBeenCalled(); + expect(ipc.cancelRequest).not.toHaveBeenCalled(); + } finally { + _removeEditor(handle); + } + }); +}); diff --git a/src/editor/lsp/codeLens.ts b/src/editor/lsp/codeLens.ts index 132672435..8f7d136e7 100644 --- a/src/editor/lsp/codeLens.ts +++ b/src/editor/lsp/codeLens.ts @@ -9,11 +9,11 @@ import { EditorSelection, MapMode, StateEffect, StateField, type Extension, type import { EditorView, ViewPlugin, type ViewUpdate } from "@codemirror/view"; import { codeLenses, type CodeLens } from "@/editor/api"; import { executeAction } from "@/platform/actions"; -import { commands, nextRequestId, type OutlineItem, type SymbolKind } from "@/platform/ipc"; +import { commands, nextRequestId, type OutlineItem, type RequestId, type SymbolKind } from "@/platform/ipc"; import { useProject } from "@/platform/project"; import { debounce, docOf, isUnavailable, lspRequest } from "./lsp"; import { posToOffset } from "./positions"; -import { featureBackoff, onLspRefresh } from "./availability"; +import { featureBackoff, onLspRefresh, onServerSettled } from "./availability"; import { editorPrefs } from "@/platform/editorSettings"; import { authorLabel, authorSummary, authorTooltip, blameFor, onBlameInvalidated } from "./authorLens"; import { tx } from "@/platform/i18n"; @@ -106,14 +106,17 @@ function openUsages(view: EditorView, offset: number) { } class UsagesLensPlugin { - private refresh = debounce(() => void this.run(), 900); + private schedule = debounce(() => void this.run(), 900); /** `${line}:${col}:${name}` → count, valid for `cacheDoc`. */ private cache = new Map(); private cacheDoc: Text | null = null; private outline: { doc: Text; targets: LensTarget[] } | null = null; private generation = 0; + private pending = new Set(); + private incomplete = false; private unsubscribe: () => void; private offReady: () => void; + private offSettled: () => void; private offBlame: () => void; constructor(private view: EditorView) { @@ -126,6 +129,9 @@ class UsagesLensPlugin { this.cache.clear(); this.refresh(); }); + this.offSettled = onServerSettled(() => { + if (this.incomplete) this.refresh(); + }); this.offBlame = onBlameInvalidated(() => this.refresh()); } @@ -133,12 +139,23 @@ class UsagesLensPlugin { if (u.docChanged || u.viewportChanged) this.refresh(); } + private cancel() { + this.generation++; + for (const id of this.pending) void commands.cancelRequest(id).catch(() => {}); + this.pending.clear(); + } + + private refresh() { + this.cancel(); + this.schedule(); + } + private async run() { const view = this.view; if (!view.dom.isConnected) return; const doc = docOf(view); if (!doc) return; - const gen = ++this.generation; + const gen = this.generation; const usages = usagesEnabled() && !usagesBackoff.active(doc.doc); const authors = authorEnabled() && !doc.virtual; if (!usages && !authors) { @@ -146,6 +163,8 @@ class UsagesLensPlugin { return; } const text = view.state.doc; + const wanted = () => gen === this.generation && view.state.doc === text; + this.incomplete = false; // Blame runs beside the reference requests; it never holds up the counts. const blame = authors ? blameFor(doc.path, text) : Promise.resolve(null); if (this.cacheDoc !== text) { @@ -154,8 +173,8 @@ class UsagesLensPlugin { } try { if (this.outline?.doc !== text) { - const items = await lspRequest(() => commands.docOutline(doc.doc)); - if (gen !== this.generation || view.state.doc !== text) return; + const items = await lspRequest(() => commands.docOutline(doc.doc), wanted); + if (!wanted()) return; this.outline = { doc: text, targets: lensTargets(items) }; } const { from, to } = view.viewport; @@ -170,21 +189,30 @@ class UsagesLensPlugin { for (let i = 0; i < todo.length && !unavailable; i += CONCURRENCY) { await Promise.all( todo.slice(i, i + CONCURRENCY).map(async (t) => { + const request = nextRequestId(); try { - const { locations: refs } = await lspRequest(() => - commands.lspReferences(nextRequestId(), doc.doc, { line: t.line, col: t.col }, false, true), - ); - this.cache.set(key(t), refs.length); + const { locations: refs, incomplete } = await lspRequest(() => { + this.pending.add(request); + // Opening candidate files for each count triggers expensive server analysis. + // Only an explicit Find Usages should widen the server's scope. + return commands.lspReferences(request, doc.doc, { line: t.line, col: t.col }, false, false); + }, wanted); + if (wanted()) { + if (incomplete) this.incomplete = true; + else this.cache.set(key(t), refs.length); + } } catch (e) { - if (isUnavailable(e)) unavailable = true; + if (wanted() && isUnavailable(e)) unavailable = true; + } finally { + this.pending.delete(request); } }), ); - if (gen !== this.generation || view.state.doc !== text) return; + if (!wanted()) return; } if (unavailable) usagesBackoff.mark(doc.doc); const lines = await blame; - if (gen !== this.generation || view.state.doc !== text) return; + if (!wanted()) return; const lenses: CodeLens[] = []; for (const t of visible) { const offset = posToOffset(text, { line: t.line, col: t.col }); @@ -196,23 +224,24 @@ class UsagesLensPlugin { lenses.push({ pos: offset, title: usagesLabel(n), - tooltip: n ? tx`Show usages of ${t.name}` : undefined, + tooltip: tx`Show usages of ${t.name}`, order: 10, - run: n ? (v) => openUsages(v, Math.min(offset, v.state.doc.length)) : undefined, + run: (v) => openUsages(v, Math.min(offset, v.state.doc.length)), }); } if (!sameLenses(view.state.field(lensField), lenses)) view.dispatch({ effects: setLenses.of(lenses) }); } catch (e) { - if (isUnavailable(e)) usagesBackoff.mark(doc.doc); + if (wanted() && isUnavailable(e)) usagesBackoff.mark(doc.doc); } } destroy() { - this.refresh.cancel(); + this.schedule.cancel(); this.unsubscribe(); this.offReady(); + this.offSettled(); this.offBlame(); - this.generation++; + this.cancel(); } } diff --git a/src/editor/lsp/hint.test.ts b/src/editor/lsp/hint.test.ts new file mode 100644 index 000000000..fedff076c --- /dev/null +++ b/src/editor/lsp/hint.test.ts @@ -0,0 +1,62 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { EditorState } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import { showHint, showPendingHint } from "./hint"; + +let view: EditorView; +const hint = () => document.querySelector(".cm-lsp-hint")?.textContent ?? null; + +beforeEach(() => { + vi.useFakeTimers(); + view = new EditorView({ state: EditorState.create({ doc: "foo()" }), parent: document.body }); +}); +afterEach(() => { + view.destroy(); + vi.useRealTimers(); +}); + +describe("pending hints", () => { + it("does not flash for fast requests", () => { + const done = showPendingHint(view, "Waiting…"); + vi.advanceTimersByTime(100); + done(); + vi.advanceTimersByTime(1000); + expect(hint()).toBeNull(); + }); + + it("shows a slow request and removes its hint on completion", () => { + const done = showPendingHint(view, "Waiting…"); + vi.advanceTimersByTime(250); + expect(hint()).toBe("Waiting…"); + done(); + expect(hint()).toBeNull(); + }); + + it("does not dismiss a newer hint when an old request finishes", () => { + const done = showPendingHint(view, "Waiting…"); + vi.advanceTimersByTime(250); + showHint(view, "New error", { tone: "error" }); + done(); + expect(hint()).toBe("New error"); + }); + + it("does not overwrite a newer hint while waiting to show progress", () => { + const done = showPendingHint(view, "Waiting…"); + vi.advanceTimersByTime(100); + showHint(view, "New error", { tone: "error" }); + vi.advanceTimersByTime(150); + expect(hint()).toBe("New error"); + done(); + expect(hint()).toBe("New error"); + }); + + it.each(["edit", "move", "destroy"])("does not show stale progress after %s", (change) => { + const done = showPendingHint(view, "Waiting…"); + if (change === "edit") view.dispatch({ changes: { from: 0, insert: "x" } }); + if (change === "move") view.dispatch({ selection: { anchor: 3 } }); + if (change === "destroy") view.destroy(); + vi.advanceTimersByTime(250); + done(); + expect(hint()).toBeNull(); + }); +}); diff --git a/src/editor/lsp/hint.ts b/src/editor/lsp/hint.ts index d103f78ad..8db8656a1 100644 --- a/src/editor/lsp/hint.ts +++ b/src/editor/lsp/hint.ts @@ -67,11 +67,36 @@ export function showHint( timers.set( view, setTimeout(() => { - if (view.state.field(hintField, false)) view.dispatch({ effects: setHint.of(null) }); + if (view.dom.isConnected && view.state.field(hintField, false)) view.dispatch({ effects: setHint.of(null) }); }, opts.timeout ?? 4000), ); } +/** Show progress only for slow requests; the disposer removes only its own hint. */ +export function showPendingHint(view: EditorView, content: string): () => void { + const { doc, selection } = view.state; + const previous = view.state.field(hintField, false); + let shown: Hint | null = null; + const timer = setTimeout(() => { + if ( + !view.dom.isConnected || + view.state.doc !== doc || + !view.state.selection.eq(selection) || + view.state.field(hintField, false) !== previous + ) + return; + showHint(view, content, { timeout: 30_000 }); + shown = view.state.field(hintField); + }, 250); + return () => { + clearTimeout(timer); + if (shown && view.state.field(hintField, false) === shown) { + clearTimeout(timers.get(view)); + if (view.dom.isConnected) hideHint(view); + } + }; +} + export function hideHint(view: EditorView): boolean { if (!view.state.field(hintField, false)) return false; view.dispatch({ effects: setHint.of(null) }); diff --git a/src/editor/lsp/navigation.ts b/src/editor/lsp/navigation.ts index cede48117..33052b67b 100644 --- a/src/editor/lsp/navigation.ts +++ b/src/editor/lsp/navigation.ts @@ -46,7 +46,7 @@ import { localHighlights, type DeclarationTarget, } from "./syntaxFallback"; -import { showHint } from "./hint"; +import { showHint, showPendingHint } from "./hint"; import { caretAnchor, openPopup, type ListPopupItem } from "./popupStore"; import { identifierAt, readManyFileLines } from "./files"; import { documentLinks, followReference, knownLinks, linkReference, referenceAt, textReference } from "./links"; @@ -166,7 +166,12 @@ export async function gotoTargets(view: EditorView, kind: GotoKind, at?: number) const declaration = kind === "definition" || kind === "declaration"; let serverDown = false; try { - locs = dedupeLocations(await gotoAfterEdits(view, () => lspRequest(() => commands.lspGoto(doc.doc, pos, kind)))); + const done = showPendingHint(view, tx`Finding navigation target…`); + try { + locs = dedupeLocations(await gotoAfterEdits(view, () => lspRequest(() => commands.lspGoto(doc.doc, pos, kind)))); + } finally { + done(); + } } catch (e) { if (isCancelled(e) || !declaration) { reportLspError(view, e, true); diff --git a/src/editor/lsp/navigationPending.test.ts b/src/editor/lsp/navigationPending.test.ts new file mode 100644 index 000000000..50589aca2 --- /dev/null +++ b/src/editor/lsp/navigationPending.test.ts @@ -0,0 +1,62 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { EditorState } from "@codemirror/state"; +import { EditorView } from "@codemirror/view"; +import type { commands } from "@/platform/ipc"; + +const lspGoto = vi.hoisted(() => vi.fn()); +vi.mock("@/platform/ipc", async (orig) => { + const real = await orig(); + return { ...real, isTauri: () => true, commands: { ...real.commands, lspGoto } }; +}); + +const { gotoTargets } = await import("./navigation"); +const { lspDoc } = await import("./lsp"); +const { provideEditorService } = await import("@/workbench/contributions"); +const openFile = vi.fn(async () => {}); +provideEditorService({ openFile } as never); + +let view: EditorView; +beforeEach(() => { + vi.useFakeTimers(); + openFile.mockClear(); + view = new EditorView({ + state: EditorState.create({ + doc: "run()", + extensions: [lspDoc.of({ doc: 1, path: "/p/main.rs", language: "rust", uiMode: "Rust", readOnly: false })], + }), + parent: document.body, + }); +}); +afterEach(() => { + view.destroy(); + vi.useRealTimers(); +}); + +describe("slow navigation feedback", () => { + it("shows progress while finding a definition, then opens the target and clears the hint", async () => { + let resolve!: (result: Awaited>) => void; + lspGoto.mockImplementationOnce(() => new Promise((r) => (resolve = r))); + const pending = gotoTargets(view, "definition"); + await vi.advanceTimersByTimeAsync(250); + expect(document.querySelector(".cm-lsp-hint")?.textContent).toBe("Finding navigation target…"); + expect(openFile).not.toHaveBeenCalled(); + resolve({ + status: "ok", + data: [{ path: "/p/lib.rs", range: { start: { line: 3, col: 4 }, end: { line: 3, col: 7 } }, uri: null }], + }); + await pending; + expect(openFile).toHaveBeenCalledWith("/p/lib.rs", { selection: { line: 3, col: 4 }, focus: true }); + expect(document.querySelector(".cm-lsp-hint")).toBeNull(); + }); + + it("replaces progress with the server error", async () => { + let resolve!: (result: Awaited>) => void; + lspGoto.mockImplementationOnce(() => new Promise((r) => (resolve = r))); + const pending = gotoTargets(view, "implementation"); + await vi.advanceTimersByTimeAsync(250); + resolve({ status: "error", error: { kind: "protocol", message: "Server timed out" } }); + await pending; + expect(document.querySelector(".cm-lsp-hint")?.textContent).toBe("Server timed out"); + expect(openFile).not.toHaveBeenCalled(); + }); +}); diff --git a/src/platform/locales/text/zh.json b/src/platform/locales/text/zh.json index d9b6ffed2..e506b0f7b 100644 --- a/src/platform/locales/text/zh.json +++ b/src/platform/locales/text/zh.json @@ -1033,6 +1033,7 @@ "Log filters": "日志筛选器", "Long tasks": "长任务", "Looking for references…": "正在查找引用…", + "Finding navigation target…": "正在查找跳转目标…", "Looks for TODO and FIXME in added lines.": "在新增的行中查找 TODO 和 FIXME。", "Macros": "宏", "Main Menu": "主菜单",