diff --git a/source/language_service/src/protocol.rs b/source/language_service/src/protocol.rs index 3b524425c9a..f3c80fe12de 100644 --- a/source/language_service/src/protocol.rs +++ b/source/language_service/src/protocol.rs @@ -63,7 +63,7 @@ pub struct DocumentStatusDiagnostic { #[derive(Debug)] pub struct DiagnosticUpdate { pub uri: String, - pub version: Option, + pub version: Option, // No version if not open pub errors: Vec, } diff --git a/source/npm/qsharp/src/language-service/diagnosticsPublisher.ts b/source/npm/qsharp/src/language-service/diagnosticsPublisher.ts new file mode 100644 index 00000000000..1236664a139 --- /dev/null +++ b/source/npm/qsharp/src/language-service/diagnosticsPublisher.ts @@ -0,0 +1,120 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import type { VSDiagnostic } from "../../lib/web/qsc_wasm.js"; + +/** + * Both configuration for DiagnosticsPublisher and hooks so that different editors + * (and tests) can implement key operations differently. + */ +export interface DiagnosticsPublisherImpl { + /** Called when diagnostics are ready to be displayed to the user. */ + publish: (uri: string, diagnostics: VSDiagnostic[]) => void; + /** Called to schedule deferred work. */ + schedule: (callback: () => void, delayMs: number) => () => void; + /** How long after the last edit to wait before publishing. */ + delayMs: number; + /** Upper bound on how long sustained typing can withhold a publish. */ + maxDelayMs: number; +} + +interface PendingDiagnostics { + uri: string; + diagnostics: VSDiagnostic[]; +} + +/** + * Withholds diagnostics for the document the user is currently typing in, so squiggles + * don't churn on every character of a half-written token. + * + * The goal is to have a small delay when the user is idle but a cap on the maximum delay + * before squiggles are drawn. + * + * Only the active file is affected - everything else (e.g. closed files) still publishes + * ASAP. + */ +export class DiagnosticsPublisher { + // Active file (i.e. subject to delays) + private activeUri: string | undefined; + // Deferred diagnostics for `activeUri` + private pending: PendingDiagnostics | undefined; + private cancelIdle: (() => void) | undefined; + private cancelCap: (() => void) | undefined; + + constructor(private readonly impl: DiagnosticsPublisherImpl) {} + + /// Callback for when a document is edited + onEdit(uri: string | undefined) { + if (uri !== this.activeUri) { + // A pending entry belongs to the document the user just left, which will get no + // further edits to end its burst. + this.publishPending(); + this.endBurst(); + this.activeUri = uri; + } + + // Indicates a non-QDK document (which is still interesting, since it changes the activeUri) + if (uri === undefined) { + return; + } + + this.cancelIdle?.(); + this.cancelIdle = this.impl.schedule(() => { + this.cancelIdle = undefined; + this.endBurst(); + this.publishPending(); + }, this.impl.delayMs); + + // Deliberately not restarted per edit - that is what makes it a cap rather than a + // second debounce, so a long typing run still refreshes. + if (!this.cancelCap) { + this.cancelCap = this.impl.schedule(() => { + this.cancelCap = undefined; + this.publishPending(); + }, this.impl.maxDelayMs); + } + } + + onDiagnosticsUpdate(uri: string, diagnostics: VSDiagnostic[]) { + // Nothing is waiting on this result: either it's for another document, or the + // typing already stopped and the burst ended. + if (uri !== this.activeUri || !this.isBursting) { + this.impl.publish(uri, diagnostics); + return; + } + + // Clearing the last error is the one result worth showing mid-token. + if (diagnostics.length === 0) { + this.pending = undefined; + this.impl.publish(uri, diagnostics); + return; + } + + this.pending = { uri, diagnostics }; + } + + publishPending() { + const pending = this.pending; + this.pending = undefined; + if (pending) { + this.impl.publish(pending.uri, pending.diagnostics); + } + } + + dispose() { + this.pending = undefined; + this.endBurst(); + } + + /** A burst runs from an edit until `delayMs` of quiet. */ + private get isBursting() { + return this.cancelIdle !== undefined; + } + + private endBurst() { + this.cancelIdle?.(); + this.cancelIdle = undefined; + this.cancelCap?.(); + this.cancelCap = undefined; + } +} diff --git a/source/npm/qsharp/src/language-service/language-service.ts b/source/npm/qsharp/src/language-service/language-service.ts index 802d2190273..2de87c0e194 100644 --- a/source/npm/qsharp/src/language-service/language-service.ts +++ b/source/npm/qsharp/src/language-service/language-service.ts @@ -290,7 +290,7 @@ export class QSharpLanguageService implements ILanguageService { Event; event.detail = { uri, - version: version ?? 0, + version: version ?? 0, // No version if not open diagnostics, }; this.eventHandler.dispatchEvent(event); diff --git a/source/npm/qsharp/src/main.ts b/source/npm/qsharp/src/main.ts index f6a0dd7e1fa..8f67222683e 100644 --- a/source/npm/qsharp/src/main.ts +++ b/source/npm/qsharp/src/main.ts @@ -213,6 +213,7 @@ export * as utils from "./utils.js"; export { log } from "./log.js"; export { QscEventTarget } from "./compiler/events.js"; export { QdkDiagnostics } from "./diagnostics.js"; +export { DiagnosticsPublisher } from "./language-service/diagnosticsPublisher.js"; export { default as samples } from "./samples.generated.js"; export { default as openqasm_samples } from "./openqasm-samples.generated.js"; export { @@ -233,6 +234,7 @@ export type { LanguageServiceEvent, LanguageServiceTestCallablesEvent, } from "./language-service/language-service.js"; +export type { DiagnosticsPublisherImpl } from "./language-service/diagnosticsPublisher.js"; export type { ProjectLoader } from "./project.js"; export type { CircuitGroup as CircuitData } from "./data-structures/circuit.js"; export type { LogLevel } from "./log.js"; diff --git a/source/npm/qsharp/test/diagnosticsPublisher.test.mjs b/source/npm/qsharp/test/diagnosticsPublisher.test.mjs new file mode 100644 index 00000000000..3c76629cdf8 --- /dev/null +++ b/source/npm/qsharp/test/diagnosticsPublisher.test.mjs @@ -0,0 +1,280 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// @ts-check + +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { DiagnosticsPublisher } from "../dist/language-service/diagnosticsPublisher.js"; + +/** + * @typedef {import("../dist/../lib/web/qsc_wasm.js").VSDiagnostic} VSDiagnostic + */ + +const idleDelayMs = 300; +const maxDelayMs = 1500; + +/** + * Only `code` is ever asserted on; the rest is filler to satisfy the shape. + * @param {string} code + * @returns {VSDiagnostic[]} + */ +function errors(code) { + return [ + { + range: { + start: { line: 0, character: 0 }, + end: { line: 0, character: 1 }, + }, + message: code, + severity: "error", + code, + }, + ]; +} + +const anError = errors("Qsc.Parse"); + +/** + * Time never advances on its own here: `schedule` only records the callback, and tests + * fire it explicitly. That keeps every case free of real delays and races. + */ +function createHarness() { + /** @type {{ uri: string; diagnostics: VSDiagnostic[] }[]} */ + const published = []; + /** @type {{ callback: () => void; delayMs: number; cancelled: boolean; fired: boolean }[]} */ + const timers = []; + + const publisher = new DiagnosticsPublisher({ + publish: (uri, diagnostics) => published.push({ uri, diagnostics }), + schedule: (callback, delayMs) => { + const timer = { callback, delayMs, cancelled: false, fired: false }; + timers.push(timer); + return () => { + timer.cancelled = true; + }; + }, + delayMs: idleDelayMs, + maxDelayMs, + }); + + /** @param {number} delayMs */ + function getLiveTimersWithDelay(delayMs) { + return timers.filter( + (t) => t.delayMs === delayMs && !t.cancelled && !t.fired, + ); + } + + /** @param {number} delayMs */ + function fire(delayMs) { + const pending = getLiveTimersWithDelay(delayMs); + assert.equal(pending.length, 1, `expected one live ${delayMs}ms timer`); + pending[0].fired = true; + pending[0].callback(); + } + + return { publisher, published, timers, getLiveTimersWithDelay, fire }; +} + +test("a document that is not being edited publishes immediately", () => { + const { publisher, published } = createHarness(); + publisher.onEdit("file:///a.qs"); + + publisher.onDiagnosticsUpdate("file:///b.qs", anError); + + assert.deepEqual(published, [{ uri: "file:///b.qs", diagnostics: anError }]); +}); + +test("a burst with no edits publishes every uri immediately", () => { + const { publisher, published, timers } = createHarness(); + + publisher.onDiagnosticsUpdate("file:///a.qs", anError); + publisher.onDiagnosticsUpdate("file:///b.qs", anError); + publisher.onDiagnosticsUpdate("file:///c.qs", []); + + assert.deepEqual( + published.map((p) => p.uri), + ["file:///a.qs", "file:///b.qs", "file:///c.qs"], + ); + assert.equal(timers.length, 0); +}); + +test("errors for the document being edited are withheld", () => { + const { publisher, published, getLiveTimersWithDelay } = createHarness(); + publisher.onEdit("file:///a.qs"); + + publisher.onDiagnosticsUpdate("file:///a.qs", anError); + + assert.deepEqual(published, []); + assert.equal(getLiveTimersWithDelay(idleDelayMs).length, 1); + assert.equal(getLiveTimersWithDelay(maxDelayMs).length, 1); +}); + +test("a result that arrives after the typing stopped publishes immediately", () => { + const { publisher, published, fire } = createHarness(); + publisher.onEdit("file:///a.qs"); + + // Stands in for a compilation slower than the wait: the burst ends before it finishes. + fire(idleDelayMs); + publisher.onDiagnosticsUpdate("file:///a.qs", anError); + + assert.deepEqual(published, [{ uri: "file:///a.qs", diagnostics: anError }]); +}); + +test("the wait restarts on each edit", () => { + const { publisher, timers, getLiveTimersWithDelay } = createHarness(); + + publisher.onEdit("file:///a.qs"); + publisher.onEdit("file:///a.qs"); + publisher.onEdit("file:///a.qs"); + + assert.equal(timers.filter((t) => t.delayMs === idleDelayMs).length, 3); + assert.equal(getLiveTimersWithDelay(idleDelayMs).length, 1); +}); + +test("the cap is scheduled once per burst, not per edit", () => { + const { publisher, timers } = createHarness(); + + publisher.onEdit("file:///a.qs"); + publisher.onEdit("file:///a.qs"); + publisher.onEdit("file:///a.qs"); + + assert.equal(timers.filter((t) => t.delayMs === maxDelayMs).length, 1); +}); + +test("only the latest diagnostics for the edited document are published", () => { + const { publisher, published, fire } = createHarness(); + publisher.onEdit("file:///a.qs"); + + publisher.onDiagnosticsUpdate("file:///a.qs", errors("first")); + publisher.onDiagnosticsUpdate("file:///a.qs", errors("second")); + publisher.onDiagnosticsUpdate("file:///a.qs", errors("third")); + + fire(idleDelayMs); + + assert.deepEqual(published, [ + { uri: "file:///a.qs", diagnostics: errors("third") }, + ]); +}); + +test("clearing all errors publishes immediately and drops the pending entry", () => { + const { publisher, published, getLiveTimersWithDelay, fire } = + createHarness(); + publisher.onEdit("file:///a.qs"); + publisher.onDiagnosticsUpdate("file:///a.qs", anError); + + publisher.onDiagnosticsUpdate("file:///a.qs", []); + + assert.deepEqual(published, [{ uri: "file:///a.qs", diagnostics: [] }]); + + // The burst belongs to the typing, not to the pending entry, so it keeps running. + assert.equal(getLiveTimersWithDelay(idleDelayMs).length, 1); + assert.equal(getLiveTimersWithDelay(maxDelayMs).length, 1); + + fire(idleDelayMs); + + assert.equal(published.length, 1); +}); + +test("editing another document flushes the pending entry", () => { + const { publisher, published } = createHarness(); + publisher.onEdit("file:///a.qs"); + publisher.onDiagnosticsUpdate("file:///a.qs", anError); + + publisher.onEdit("file:///b.qs"); + + assert.deepEqual(published, [{ uri: "file:///a.qs", diagnostics: anError }]); + + // The first document is no longer the one being edited. + publisher.onDiagnosticsUpdate("file:///a.qs", errors("later")); + + assert.deepEqual(published, [ + { uri: "file:///a.qs", diagnostics: anError }, + { uri: "file:///a.qs", diagnostics: errors("later") }, + ]); +}); + +test("editing a document we don't publish for flushes and stops debouncing", () => { + const { publisher, published, getLiveTimersWithDelay } = createHarness(); + publisher.onEdit("file:///a.qs"); + publisher.onDiagnosticsUpdate("file:///a.qs", anError); + + publisher.onEdit(undefined); + + assert.deepEqual(published, [{ uri: "file:///a.qs", diagnostics: anError }]); + assert.equal(getLiveTimersWithDelay(idleDelayMs).length, 0); + assert.equal(getLiveTimersWithDelay(maxDelayMs).length, 0); + + publisher.onDiagnosticsUpdate("file:///a.qs", anError); + + assert.deepEqual(published, [ + { uri: "file:///a.qs", diagnostics: anError }, + { uri: "file:///a.qs", diagnostics: anError }, + ]); +}); + +test("the idle timer ends the burst", () => { + const { publisher, published, getLiveTimersWithDelay, fire } = + createHarness(); + publisher.onEdit("file:///a.qs"); + publisher.onDiagnosticsUpdate("file:///a.qs", anError); + + fire(idleDelayMs); + + assert.deepEqual(published, [{ uri: "file:///a.qs", diagnostics: anError }]); + assert.equal(getLiveTimersWithDelay(maxDelayMs).length, 0); + + publisher.onDiagnosticsUpdate("file:///a.qs", errors("later")); + + assert.deepEqual(published, [ + { uri: "file:///a.qs", diagnostics: anError }, + { uri: "file:///a.qs", diagnostics: errors("later") }, + ]); +}); + +test("the cap publishes without ending the burst", () => { + const { publisher, published, getLiveTimersWithDelay, fire } = + createHarness(); + publisher.onEdit("file:///a.qs"); + publisher.onDiagnosticsUpdate("file:///a.qs", anError); + + fire(maxDelayMs); + + assert.deepEqual(published, [{ uri: "file:///a.qs", diagnostics: anError }]); + + // Typing hasn't stopped, so the next result is still withheld. + assert.equal(getLiveTimersWithDelay(idleDelayMs).length, 1); + publisher.onDiagnosticsUpdate("file:///a.qs", errors("later")); + + assert.equal(published.length, 1); +}); + +test("a cap-driven publish starts a fresh cap on the next edit", () => { + const { publisher, published, timers, fire } = createHarness(); + publisher.onEdit("file:///a.qs"); + publisher.onDiagnosticsUpdate("file:///a.qs", anError); + fire(maxDelayMs); + + publisher.onEdit("file:///a.qs"); + publisher.onDiagnosticsUpdate("file:///a.qs", errors("later")); + + assert.equal(timers.filter((t) => t.delayMs === maxDelayMs).length, 2); + + fire(maxDelayMs); + + assert.deepEqual(published, [ + { uri: "file:///a.qs", diagnostics: anError }, + { uri: "file:///a.qs", diagnostics: errors("later") }, + ]); +}); + +test("dispose cancels without publishing", () => { + const { publisher, published, timers } = createHarness(); + publisher.onEdit("file:///a.qs"); + publisher.onDiagnosticsUpdate("file:///a.qs", anError); + + publisher.dispose(); + + assert.deepEqual(published, []); + assert.ok(timers.every((t) => t.cancelled)); +}); diff --git a/source/vscode/src/language-service/diagnostics.ts b/source/vscode/src/language-service/diagnostics.ts index cbd0e9ac1d0..bc222bdeeea 100644 --- a/source/vscode/src/language-service/diagnostics.ts +++ b/source/vscode/src/language-service/diagnostics.ts @@ -2,12 +2,19 @@ // Licensed under the MIT License. import { + DiagnosticsPublisher, ILanguageService, VSDiagnostic, qsharpLibraryUriScheme, } from "qsharp-lang"; import * as vscode from "vscode"; -import { qsharpLanguageId, toVsCodeDiagnostic } from "../common"; +import { isQdkDocument, qsharpLanguageId, toVsCodeDiagnostic } from "../common"; + +/** How long after the last edit to wait before refreshing squiggles. */ +const idleDelayMs = 300; + +/** Upper bound on how long sustained typing can withhold a refresh. */ +const maxDelayMs = 1500; export function startLanguageServiceDiagnostics( languageService: ILanguageService, @@ -15,6 +22,20 @@ export function startLanguageServiceDiagnostics( const diagCollection = vscode.languages.createDiagnosticCollection(qsharpLanguageId); + const publisher = new DiagnosticsPublisher({ + publish: (uri, diagnostics) => + diagCollection.set( + vscode.Uri.parse(uri), + diagnostics.map((d) => toVsCodeDiagnostic(d)), + ), + schedule: (callback, delayMs) => { + const handle = setTimeout(callback, delayMs); + return () => clearTimeout(handle); + }, + delayMs: idleDelayMs, + maxDelayMs, + }); + async function onDiagnostics(evt: { detail: { uri: string; @@ -30,20 +51,40 @@ export function startLanguageServiceDiagnostics( return; } - diagCollection.set( - uri, - diagnostics.diagnostics.map((d) => toVsCodeDiagnostic(d)), - ); + publisher.onDiagnosticsUpdate(diagnostics.uri, diagnostics.diagnostics); } languageService.addEventListener("diagnostics", onDiagnostics); + // Feed edit events to the DiagnosticsPublisher + const diagnosticsPublisherEditTracker = + vscode.workspace.onDidChangeTextDocument((evt) => { + const uri = evt.document.uri; + + if (uri.scheme === "output") { + // NB: this fires for output window changes, so it's very important not to cause + // output window changes in response (i.e. to avoid a cycle). + return; + } + + // Dirty-state and encoding changes raise this event too, with no edit behind them. + if (evt.contentChanges.length === 0) { + return; + } + + publisher.onEdit( + isQdkDocument(evt.document) ? uri.toString() : undefined, + ); + }); + return [ { dispose: () => { languageService.removeEventListener("diagnostics", onDiagnostics); + publisher.dispose(); }, }, + diagnosticsPublisherEditTracker, diagCollection, ]; }