diff --git a/src/tools/lsp/client.test.ts b/src/tools/lsp/client.test.ts index 8c805d144a..db4f543770 100644 --- a/src/tools/lsp/client.test.ts +++ b/src/tools/lsp/client.test.ts @@ -1,6 +1,7 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs" -import { join } from "node:path" +import { join, resolve } from "node:path" import { tmpdir } from "node:os" +import { pathToFileURL } from "node:url" import { describe, it, expect, spyOn, mock, beforeEach, afterEach } from "bun:test" @@ -257,4 +258,214 @@ describe("LSPClient", () => { } }) }) + + describe("diagnostics", () => { + it("polls diagnosticsStore when pull fails and push arrives during poll window", async () => { + // #given + const dir = mkdtempSync(join(tmpdir(), "lsp-diagnostics-test-")) + const filePath = join(dir, "test.ts") + writeFileSync(filePath, "const a = 1\n") + + const originalSetTimeout = globalThis.setTimeout + globalThis.setTimeout = ((fn: (...args: unknown[]) => void, _ms?: number) => { + fn() + return 0 as unknown as ReturnType + }) as typeof setTimeout + + const server: ResolvedServer = { + id: "typescript", + command: ["typescript-language-server", "--stdio"], + extensions: [".ts"], + priority: 0, + } + + const client = new LSPClient(dir, server) + + // Mock sendRequest to throw (pull fails) + const sendRequestSpy = spyOn( + client as unknown as { sendRequest: (m: string, p?: unknown) => Promise }, + "sendRequest" + ) + sendRequestSpy.mockImplementation(async () => { + throw new Error("pull not supported") + }) + + // Mock openFile to avoid actual file operations + const openFileSpy = spyOn(client, "openFile") + openFileSpy.mockImplementation(async () => {}) + + try { + // #when + const expectedDiagnostics = [{ range: { start: { line: 0, character: 0 }, end: { line: 0, character: 5 } }, message: "error" }] + const absPath = resolve(filePath) + const uri = pathToFileURL(absPath).href + + // Pre-populate store to simulate push notification that arrived before poll + ;(client as unknown as { diagnosticsStore: Map }).diagnosticsStore.set(uri, expectedDiagnostics) + + const result = await client.diagnostics(filePath) + + // #then + expect(result.items).toEqual(expectedDiagnostics) + } finally { + globalThis.setTimeout = originalSetTimeout + sendRequestSpy.mockRestore() + openFileSpy.mockRestore() + rmSync(dir, { recursive: true, force: true }) + } + }) + + it("returns empty array when pull fails and no push arrives within timeout", async () => { + // #given + const dir = mkdtempSync(join(tmpdir(), "lsp-diagnostics-timeout-test-")) + const filePath = join(dir, "test.ts") + writeFileSync(filePath, "const a = 1\n") + + const originalSetTimeout = globalThis.setTimeout + globalThis.setTimeout = ((fn: (...args: unknown[]) => void, _ms?: number) => { + fn() + return 0 as unknown as ReturnType + }) as typeof setTimeout + + const dateNowSpy = spyOn(Date, "now") + let callCount = 0 + dateNowSpy.mockImplementation(() => { + callCount++ + return callCount <= 2 ? 0 : 10_000 + }) + + const server: ResolvedServer = { + id: "typescript", + command: ["typescript-language-server", "--stdio"], + extensions: [".ts"], + priority: 0, + } + + const client = new LSPClient(dir, server) + + // Mock sendRequest to throw (pull fails) + const sendRequestSpy = spyOn( + client as unknown as { sendRequest: (m: string, p?: unknown) => Promise }, + "sendRequest" + ) + sendRequestSpy.mockImplementation(async () => { + throw new Error("pull not supported") + }) + + // Mock openFile to avoid actual file operations + const openFileSpy = spyOn(client, "openFile") + openFileSpy.mockImplementation(async () => {}) + + try { + // #when + const result = await client.diagnostics(filePath) + + // #then + expect(result.items).toEqual([]) + } finally { + globalThis.setTimeout = originalSetTimeout + dateNowSpy.mockRestore() + sendRequestSpy.mockRestore() + openFileSpy.mockRestore() + rmSync(dir, { recursive: true, force: true }) + } + }) + + it("returns empty diagnostics immediately when push delivers clean file during poll", async () => { + // #given + const dir = mkdtempSync(join(tmpdir(), "lsp-diagnostics-empty-test-")) + const filePath = join(dir, "test.ts") + writeFileSync(filePath, "const a = 1\n") + + const originalSetTimeout = globalThis.setTimeout + globalThis.setTimeout = ((fn: (...args: unknown[]) => void, _ms?: number) => { + fn() + return 0 as unknown as ReturnType + }) as typeof setTimeout + + const server: ResolvedServer = { + id: "typescript", + command: ["typescript-language-server", "--stdio"], + extensions: [".ts"], + priority: 0, + } + + const client = new LSPClient(dir, server) + + const sendRequestSpy = spyOn( + client as unknown as { sendRequest: (m: string, p?: unknown) => Promise }, + "sendRequest" + ) + sendRequestSpy.mockImplementation(async () => { + throw new Error("pull not supported") + }) + + const openFileSpy = spyOn(client, "openFile") + openFileSpy.mockImplementation(async () => {}) + + try { + // #when + const absPath = resolve(filePath) + const uri = pathToFileURL(absPath).href + ;(client as unknown as { diagnosticsStore: Map }).diagnosticsStore.set(uri, []) + + const result = await client.diagnostics(filePath) + + // #then + expect(result.items).toEqual([]) + } finally { + globalThis.setTimeout = originalSetTimeout + sendRequestSpy.mockRestore() + openFileSpy.mockRestore() + rmSync(dir, { recursive: true, force: true }) + } + }) + + it("returns pull result immediately without polling when pull succeeds", async () => { + // #given + const dir = mkdtempSync(join(tmpdir(), "lsp-diagnostics-pull-success-test-")) + const filePath = join(dir, "test.ts") + writeFileSync(filePath, "const a = 1\n") + + const server: ResolvedServer = { + id: "typescript", + command: ["typescript-language-server", "--stdio"], + extensions: [".ts"], + priority: 0, + } + + const client = new LSPClient(dir, server) + + const pullDiagnostics = [{ range: { start: { line: 0, character: 0 }, end: { line: 0, character: 5 } }, message: "pull error" }] + + // Mock sendRequest to return diagnostics (pull succeeds) + const sendRequestSpy = spyOn( + client as unknown as { sendRequest: (m: string, p?: unknown) => Promise }, + "sendRequest" + ) + sendRequestSpy.mockImplementation(async () => ({ + items: pullDiagnostics, + })) + + // Mock openFile to avoid actual file operations + const openFileSpy = spyOn(client, "openFile") + openFileSpy.mockImplementation(async () => {}) + + try { + // #when + const startTime = Date.now() + const result = await client.diagnostics(filePath) + const elapsed = Date.now() - startTime + + // #then + expect(result.items).toEqual(pullDiagnostics) + // Should return quickly without waiting for poll timeout + expect(elapsed).toBeLessThan(1000) + } finally { + sendRequestSpy.mockRestore() + openFileSpy.mockRestore() + rmSync(dir, { recursive: true, force: true }) + } + }) + }) }) diff --git a/src/tools/lsp/lsp-client-transport.test.ts b/src/tools/lsp/lsp-client-transport.test.ts new file mode 100644 index 0000000000..47af0ab45b --- /dev/null +++ b/src/tools/lsp/lsp-client-transport.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from "bun:test" + +import { LSPClientTransport } from "./lsp-client-transport" +import type { Diagnostic, ResolvedServer } from "./types" + +const TEST_SERVER: ResolvedServer = { + id: "test", + command: ["test"], + extensions: [".ts"], + priority: 0, +} + +class TestTransport extends LSPClientTransport { + public setDiagnostics(uri: string, diagnostics: Diagnostic[]): void { + this.diagnosticsStore.set(uri, diagnostics) + } + + public publishDiagnostics(params: { uri?: string; diagnostics?: Diagnostic[] }): void { + this.handlePublishDiagnostics(params) + } + + public waitFor(uri: string, timeoutMs: number): Promise { + return this.waitForPushDiagnostics(uri, timeoutMs) + } + + public getWaiterCount(uri: string): number { + return this.diagnosticsWaiters.get(uri)?.length ?? 0 + } +} + +describe("LSPClientTransport", () => { + describe("waitForPushDiagnostics", () => { + it("returns cached diagnostics immediately when already in store", async () => { + //#given + const transport = new TestTransport(process.cwd(), TEST_SERVER) + const uri = "file:///tmp/test.ts" + const cached: Diagnostic[] = [ + { + range: { start: { line: 0, character: 0 }, end: { line: 0, character: 1 } }, + message: "cached", + }, + ] + transport.setDiagnostics(uri, cached) + + //#when + const result = await transport.waitFor(uri, 100) + + //#then + expect(result).toEqual(cached) + expect(transport.getWaiterCount(uri)).toBe(0) + }) + + it("resolves when push diagnostics arrives and clears waiter", async () => { + //#given + const transport = new TestTransport(process.cwd(), TEST_SERVER) + const uri = "file:///tmp/test.ts" + const pushed: Diagnostic[] = [ + { + range: { start: { line: 0, character: 0 }, end: { line: 0, character: 5 } }, + message: "pushed", + }, + ] + + const waiting = transport.waitFor(uri, 1000) + + //#when + expect(transport.getWaiterCount(uri)).toBe(1) + transport.publishDiagnostics({ uri, diagnostics: pushed }) + const result = await waiting + + //#then + expect(result).toEqual(pushed) + expect(transport.getWaiterCount(uri)).toBe(0) + await expect(transport.waitFor(uri, 100)).resolves.toEqual(pushed) + }) + + it("resolves empty list on timeout and clears waiter", async () => { + //#given + const transport = new TestTransport(process.cwd(), TEST_SERVER) + const uri = "file:///tmp/test.ts" + + //#when + const result = await transport.waitFor(uri, 0) + + //#then + expect(result).toEqual([]) + expect(transport.getWaiterCount(uri)).toBe(0) + }) + }) +}) diff --git a/src/tools/lsp/lsp-client-transport.ts b/src/tools/lsp/lsp-client-transport.ts index d4590262bd..746959752c 100644 --- a/src/tools/lsp/lsp-client-transport.ts +++ b/src/tools/lsp/lsp-client-transport.ts @@ -14,6 +14,7 @@ export class LSPClientTransport { protected readonly stderrBuffer: string[] = [] protected processExited = false protected readonly diagnosticsStore = new Map() + protected readonly diagnosticsWaiters = new Map void>>() protected readonly REQUEST_TIMEOUT = 15000 constructor(protected root: string, protected server: ResolvedServer) {} @@ -67,9 +68,7 @@ export class LSPClientTransport { this.connection = createMessageConnection(new StreamMessageReader(nodeReadable), new StreamMessageWriter(nodeWritable)) this.connection.onNotification("textDocument/publishDiagnostics", (params: { uri?: string; diagnostics?: Diagnostic[] }) => { - if (params.uri) { - this.diagnosticsStore.set(params.uri, params.diagnostics ?? []) - } + this.handlePublishDiagnostics(params) }) this.connection.onRequest("workspace/configuration", (params: { items?: Array<{ section?: string }> }) => { @@ -114,6 +113,62 @@ export class LSPClientTransport { read() } + protected handlePublishDiagnostics(params: { uri?: string; diagnostics?: Diagnostic[] }): void { + const uri = params.uri + if (!uri) return + + const diagnostics = params.diagnostics ?? [] + this.diagnosticsStore.set(uri, diagnostics) + + const waiters = this.diagnosticsWaiters.get(uri) + if (!waiters) return + + this.diagnosticsWaiters.delete(uri) + for (const resolve of waiters) { + resolve(diagnostics) + } + } + + protected waitForPushDiagnostics(uri: string, timeoutMs: number): Promise { + const existing = this.diagnosticsStore.get(uri) + if (existing !== undefined) { + return Promise.resolve(existing) + } + + return new Promise((resolve) => { + let settled = false + let timeoutId: ReturnType | undefined + + const waiter = (diagnostics: Diagnostic[]) => { + if (settled) return + settled = true + if (timeoutId !== undefined) clearTimeout(timeoutId) + resolve(diagnostics) + } + + const existingWaiters = this.diagnosticsWaiters.get(uri) ?? [] + existingWaiters.push(waiter) + this.diagnosticsWaiters.set(uri, existingWaiters) + + timeoutId = setTimeout(() => { + if (settled) return + settled = true + + const waiters = this.diagnosticsWaiters.get(uri) + if (waiters) { + const remaining = waiters.filter((w) => w !== waiter) + if (remaining.length === 0) { + this.diagnosticsWaiters.delete(uri) + } else { + this.diagnosticsWaiters.set(uri, remaining) + } + } + + resolve([]) + }, timeoutMs) + }) + } + protected async sendRequest(method: string, params?: unknown): Promise { if (!this.connection) throw new Error("LSP client not started") @@ -190,5 +245,11 @@ export class LSPClientTransport { } this.processExited = true this.diagnosticsStore.clear() + for (const waiters of this.diagnosticsWaiters.values()) { + for (const resolve of waiters) { + resolve([]) + } + } + this.diagnosticsWaiters.clear() } } diff --git a/src/tools/lsp/lsp-client.ts b/src/tools/lsp/lsp-client.ts index 4785909cc4..c2a60dc334 100644 --- a/src/tools/lsp/lsp-client.ts +++ b/src/tools/lsp/lsp-client.ts @@ -94,7 +94,6 @@ export class LSPClient extends LSPClientConnection { const absPath = resolve(filePath) const uri = pathToFileURL(absPath).href await this.openFile(absPath) - await new Promise((r) => setTimeout(r, 500)) try { const result = await this.sendRequest<{ items?: Diagnostic[] }>("textDocument/diagnostic", { @@ -105,7 +104,7 @@ export class LSPClient extends LSPClientConnection { } } catch {} - return { items: this.diagnosticsStore.get(uri) ?? [] } + return { items: await this.waitForPushDiagnostics(uri, 3000) } } async prepareRename(filePath: string, line: number, character: number): Promise {