fix(lsp): await publishDiagnostics instead of a single event-loop turn
getDiagnostics used to sync the document then wait one macrotask (setTimeout 0) before reading the cached snapshot. tsserver/pyright on a large file publish asynchronously and often hadn't fired yet, so the call returned a stale (or empty) snapshot right after an edit — exactly when the model asks for diagnostics to verify its change. Now: clear the stale URI snapshot, sync, then race the next publishDiagnostics for that URI against a 1500ms timeout via a per-URI waiter map woken by the publish handler. A slow server gets a real chance to compute fresh diagnostics; on timeout we fall through to whatever's cached (possibly empty). _resetForTests clears the waiter map too. Verified: typecheck clean, 250 tests pass.
This commit is contained in:
@@ -287,6 +287,28 @@ export interface DiagnosticsResult {
|
||||
* returns the latest snapshot here. Forces a document sync first so the snapshot is current. */
|
||||
const diagnosticsByUri = new Map<string, Diagnostic[]>();
|
||||
|
||||
// Per-URI resolvers waiting on the next publishDiagnostics notification. getDiagnostics arms one
|
||||
// for the file it just synced, then races it against a timeout — so a slow server (tsserver on a
|
||||
// large file) still gets a chance to publish the fresh snapshot rather than the caller reading a
|
||||
// stale one after a single event-loop turn. Resolved and cleared by the publishDiagnostics handler.
|
||||
const diagWaiters = new Map<string, () => void>();
|
||||
|
||||
/** Wait for the next publishDiagnostics for `uri`, or give up after `timeoutMs`. Resolves true if
|
||||
* a publish arrived, false on timeout. The waiter is removed either way. */
|
||||
function waitForDiagnostics(uri: string, timeoutMs: number): Promise<boolean> {
|
||||
return new Promise((resolve) => {
|
||||
const timer = setTimeout(() => {
|
||||
diagWaiters.delete(uri);
|
||||
resolve(false);
|
||||
}, timeoutMs);
|
||||
diagWaiters.set(uri, () => {
|
||||
clearTimeout(timer);
|
||||
diagWaiters.delete(uri);
|
||||
resolve(true);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
const SEVERITY_MAP: Record<number, Severity> = {
|
||||
1: "error",
|
||||
2: "warning",
|
||||
@@ -303,12 +325,18 @@ export async function getDiagnostics(filePath: string, cwd: string): Promise<Dia
|
||||
(handle as unknown as { __diagWired?: boolean }).__diagWired = true;
|
||||
handle.connection.onNotification("textDocument/publishDiagnostics", (params: { uri: string; diagnostics: Diagnostic[] }) => {
|
||||
diagnosticsByUri.set(params.uri, params.diagnostics);
|
||||
// Wake a getDiagnostics call waiting on this URI, if any.
|
||||
diagWaiters.get(params.uri)?.();
|
||||
});
|
||||
}
|
||||
// Clear any stale snapshot for this URI before syncing so a timeout fallthrough can't return
|
||||
// diagnostics from before the edit. The server publishes asynchronously after didChange; race
|
||||
// its next publish against a short timeout so a slow server (tsserver on a large file) still
|
||||
// gets a chance to compute fresh diagnostics rather than us reading a stale snapshot after one
|
||||
// event-loop turn. Fall through to whatever's cached on timeout (possibly empty).
|
||||
diagnosticsByUri.delete(uri);
|
||||
await syncDocument(handle, absPath, cwd);
|
||||
// Give the server a beat to publish after the sync, then read the latest snapshot. A real LSP
|
||||
// server publishes asynchronously; we await one event-loop turn rather than polling on a timer.
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
await waitForDiagnostics(uri, 1500);
|
||||
const diags = diagnosticsByUri.get(uri) ?? [];
|
||||
return {
|
||||
diagnostics: diags.map((d) => ({
|
||||
@@ -345,4 +373,5 @@ export async function shutdownAll(): Promise<void> {
|
||||
export function _resetForTests(): void {
|
||||
handles.clear();
|
||||
diagnosticsByUri.clear();
|
||||
diagWaiters.clear();
|
||||
}
|
||||
Reference in New Issue
Block a user