diff --git a/language-server/src/build-server.ts b/language-server/src/build-server.ts index 59e6417..3531cbe 100644 --- a/language-server/src/build-server.ts +++ b/language-server/src/build-server.ts @@ -26,7 +26,11 @@ import type { Connection } from "vscode-languageserver"; export type LanguageServerSettings = { }; -export const buildServer = (connection: Connection): Server => { +export type BuildServerOptions = { + diagnosticsDelay?: number; +}; + +export const buildServer = (connection: Connection, options: BuildServerOptions = {}): Server => { const server = new Server(connection); const workspace = new Workspace(server); @@ -44,7 +48,7 @@ export const buildServer = (connection: Connection): Server => { new SchemaRegistrationDiagnosticsProvider(registry, jsonSchema), new SchemaValidationDiagnosticsProvider(jsonSchema), new DeprecatedDiagnosticsProvider(server, jsonSchema) - ]); + ], options.diagnosticsDelay); new Formatting(server, jsonDocuments); new Hover(server, jsonDocuments, jsonSchema); diff --git a/language-server/src/evaluation/LspEvaluationPlugin.ts b/language-server/src/evaluation/LspEvaluationPlugin.ts index 75c429b..b0ae066 100644 --- a/language-server/src/evaluation/LspEvaluationPlugin.ts +++ b/language-server/src/evaluation/LspEvaluationPlugin.ts @@ -1,7 +1,6 @@ import { getKeyword } from "@hyperjump/json-schema/experimental"; import * as Instance from "@hyperjump/json-schema/instance/experimental"; import * as JsonPointer from "@hyperjump/json-pointer"; -import * as Pact from "@hyperjump/pact"; import { JsonValueSet } from "./JsonValueSet.ts"; import { SubschemaTracker } from "./SubschemaTracker.ts"; import { Annotation } from "./Annotation.ts"; @@ -52,10 +51,22 @@ export class LspEvaluationPlugin implements EvaluationPlugin { private annotations: RecordedAnnotations = Object.create(null); private completions: Completions = Object.create(null); private incompleteLocations: Set; + // Incomplete locations by the pointer of their parent so the keywords that + // apply to an object only look at its own incomplete properties + private incompleteChildren = new Map(); private subschemaTracker = new SubschemaTracker(); constructor(incompleteLocations: Set = new Set()) { this.incompleteLocations = incompleteLocations; + for (const pointer of incompleteLocations) { + const [parentPointer, propertyName] = splitPointer(pointer); + let children = this.incompleteChildren.get(parentPointer); + if (!children) { + children = []; + this.incompleteChildren.set(parentPointer, children); + } + children.push([pointer, propertyName]); + } } // The plugin is registered once, in build-server.ts @@ -107,9 +118,8 @@ export class LspEvaluationPlugin implements EvaluationPlugin { } } - for (const pointer of this.incompleteLocations) { - const [parentPointer, propertyName] = splitPointer(pointer); - if (parentPointer === instance.pointer && pattern.test(propertyName)) { + for (const [pointer, propertyName] of this.incompleteChildren.get(instance.pointer) ?? []) { + if (pattern.test(propertyName)) { addCompletions(schemaContext.completions!, pointer, completions); context.evaluatedProperties?.add(propertyName); } @@ -129,9 +139,8 @@ export class LspEvaluationPlugin implements EvaluationPlugin { } } - for (const pointer of this.incompleteLocations) { - const [parentPointer, propertyName] = splitPointer(pointer); - if (parentPointer === instance.pointer && !isDeclaredProperty.test(propertyName)) { + for (const [pointer, propertyName] of this.incompleteChildren.get(instance.pointer) ?? []) { + if (!isDeclaredProperty.test(propertyName)) { addCompletions(schemaContext.completions!, pointer, completions); context.evaluatedProperties?.add(propertyName); } @@ -150,9 +159,8 @@ export class LspEvaluationPlugin implements EvaluationPlugin { } } - for (const pointer of this.incompleteLocations) { - const [parentPointer, propertyName] = splitPointer(pointer); - if (parentPointer === instance.pointer && !context.schemaEvaluatedProperties!.has(propertyName)) { + for (const [pointer, propertyName] of this.incompleteChildren.get(instance.pointer) ?? []) { + if (!context.schemaEvaluatedProperties!.has(propertyName)) { addCompletions(schemaContext.completions!, pointer, completions); context.evaluatedProperties?.add(propertyName); } @@ -275,7 +283,7 @@ export class LspEvaluationPlugin implements EvaluationPlugin { const isConditional = conditionalKeywords.has(keywordId); for (const { pointer, annotations } of annotationValues) { - appendAnnotations(schemaContext.schemaAnnotations!, isConditional ? asValueAnnotations(annotations, pointer) : annotations); + appendAnnotations(schemaContext.schemaAnnotations!, annotations, isConditional ? pointer : undefined); } // The schema's completions are only shared once the schema is done, so @@ -363,12 +371,13 @@ export class LspEvaluationPlugin implements EvaluationPlugin { } private getAnnotationsOfKind(instanceLocation: string, kind: RecordedAnnotation["kind"]): AnnotationRecord[] { - return Pact.pipe( - this.annotations[instanceLocation], - Pact.filter((recorded) => recorded.kind === kind), - Pact.map(({ annotation }) => annotation), - Pact.collectArray - ); + const annotations: AnnotationRecord[] = []; + for (const recorded of this.annotations[instanceLocation]) { + if (recorded.kind === kind) { + annotations.push(recorded.annotation); + } + } + return annotations; } * getCompletions(pointer: string): Generator { @@ -564,38 +573,23 @@ function negateInto(target: Completions, source: Completions) { } function exclusiveUnion(records: Completions[]): Completions { - const pointers = new Set(); + const setsByPointer: Record = Object.create(null); for (const record of records) { for (const pointer in record) { - pointers.add(pointer); + setsByPointer[pointer] ??= []; + setsByPointer[pointer].push(record[pointer]); } } const result: Completions = Object.create(null); - for (const pointer of pointers) { - const sets = Pact.pipe( - records, - Pact.map((record) => record[pointer]), - Pact.filter((set) => set !== undefined), - Pact.collectArray - ); + for (const pointer in setsByPointer) { + const sets = setsByPointer[pointer]; result[pointer] = lazy(() => JsonValueSet.exclusiveUnion(sets.map((set) => set()))); } return result; } -function asValueAnnotations(annotations: RecordedAnnotations, pointer: string): RecordedAnnotations { - if (!(pointer in annotations)) { - return annotations; - } - - return { - ...annotations, - [pointer]: annotations[pointer].map(({ annotation }) => ({ kind: "value", annotation })) - }; -} - // Adds a keyword's annotation without an instance, for locations that // evaluation doesn't reach. // Returns whether the keyword contributed an annotation @@ -616,10 +610,17 @@ function toAnnotations(records: AnnotationRecord[]): Annotation[] { return records.map((record) => new Annotation(record)); } -function appendAnnotations(target: RecordedAnnotations, source: RecordedAnnotations) { +// Annotations at `valuePointer` are appended as value annotations +function appendAnnotations(target: RecordedAnnotations, source: RecordedAnnotations, valuePointer?: string) { for (const pointer in source) { target[pointer] ??= []; - target[pointer].push(...source[pointer]); + if (pointer === valuePointer) { + for (const { annotation } of source[pointer]) { + target[pointer].push({ kind: "value", annotation }); + } + } else { + target[pointer].push(...source[pointer]); + } } } diff --git a/language-server/src/evaluation/SubschemaTracker.ts b/language-server/src/evaluation/SubschemaTracker.ts index c06b45c..70bfb3c 100644 --- a/language-server/src/evaluation/SubschemaTracker.ts +++ b/language-server/src/evaluation/SubschemaTracker.ts @@ -14,6 +14,12 @@ type SchemaFailures = { failedType: boolean; }; +type TrackerState = { + failedLocations?: Set; + schemaFailures?: SchemaFailures; + subschemaResults?: SubschemaResult[]; +}; + /** * Tracks the result of each subschema evaluated by a keyword and the instance * locations where it failed. This allows anyOf/oneOf alternatives to be @@ -23,30 +29,32 @@ type SchemaFailures = { * separately because an object or array is discriminated by its type, not by * any other failure. * - * State is keyed by context object rather than stored on the context so that - * plugins using separate trackers don't share state. + * State is stored on the context under a key that's unique to the tracker so + * that plugins using separate trackers don't share state. A property is much + * faster than a WeakMap for something that happens for every keyword. */ export class SubschemaTracker { - private failedLocations = new WeakMap>(); - private schemaFailures = new WeakMap(); - private subschemaResults = new WeakMap[]>(); + private key = Symbol("SubschemaTracker"); + + private state(context: object): TrackerState { + return (context as Record>)[this.key] ??= {}; + } beforeSchema(context: object): void { - if (!this.failedLocations.has(context)) { - this.failedLocations.set(context, new Set()); - } - this.schemaFailures.set(context, { failedLocations: new Set(), failedType: false }); + const state = this.state(context); + state.failedLocations ??= new Set(); + state.schemaFailures = { failedLocations: new Set(), failedType: false }; } beforeKeyword(keywordContext: object): void { - this.subschemaResults.set(keywordContext, []); + this.state(keywordContext).subschemaResults = []; } afterKeyword(keywordId: string, keywordValue: unknown, instance: JsonNode, keywordContext: object, valid: boolean, schemaContext: object): void { if (!valid) { - const schemaFailures = this.schemaFailures.get(schemaContext)!; + const schemaFailures = this.state(schemaContext).schemaFailures!; schemaFailures.failedLocations.add(instance.pointer); - for (const location of this.failedLocations.get(keywordContext) ?? []) { + for (const location of this.state(keywordContext).failedLocations ?? []) { schemaFailures.failedLocations.add(location); } @@ -57,7 +65,8 @@ export class SubschemaTracker { } afterSchema(instance: JsonNode, context: object, valid: boolean, value: T): void { - const schemaFailures = this.schemaFailures.get(context)!; + const state = this.state(context); + const schemaFailures = state.schemaFailures!; if (!valid) { // A schema that fails without any of its keywords failing is a `false` // schema, which rules out every type @@ -67,12 +76,12 @@ export class SubschemaTracker { schemaFailures.failedLocations.add(instance.pointer); } - const failedLocations = this.failedLocations.get(context)!; + const failedLocations = state.failedLocations!; for (const location of schemaFailures.failedLocations) { failedLocations.add(location); } - this.subschemaResults.get(context)?.push({ + state.subschemaResults?.push({ value, pointer: instance.pointer, failedLocations: schemaFailures.failedLocations, @@ -81,7 +90,7 @@ export class SubschemaTracker { } getSubschemaResults(keywordContext: object): SubschemaResult[] { - return this.subschemaResults.get(keywordContext) ?? []; + return this.state(keywordContext).subschemaResults ?? []; } /** diff --git a/language-server/src/features/DocumentColors.ts b/language-server/src/features/DocumentColors.ts index a8dfad1..36bb662 100644 --- a/language-server/src/features/DocumentColors.ts +++ b/language-server/src/features/DocumentColors.ts @@ -38,21 +38,21 @@ export class DocumentColors { const plugin = LspEvaluationPlugin.from(result); - const stringNodes: Node[] = []; - jsonDocument.walkNodes(ast, (node) => { + const stringNodes: [Node, string][] = []; + jsonDocument.walkNodes(ast, (node, pointer) => { if (node.type === "string") { - stringNodes.push(node); + stringNodes.push([node, pointer]); } }); const colors: ColorInformation[] = []; - for (const node of stringNodes) { + for (const [node, pointer] of stringNodes) { const color = colorFromHex(node.value as string); if (!color) { continue; } - const annotations = plugin.getAnnotations(jsonDocument.getPointerForNode(node)); + const annotations = plugin.getAnnotations(pointer); if (annotations.some(isColorHex)) { colors.push({ color, range: jsonDocument.rangeAt(node.offset, node.offset + node.length) }); } diff --git a/language-server/src/features/diagnostics/Diagnostics.test.ts b/language-server/src/features/diagnostics/Diagnostics.test.ts index 857ef0b..d672b06 100644 --- a/language-server/src/features/diagnostics/Diagnostics.test.ts +++ b/language-server/src/features/diagnostics/Diagnostics.test.ts @@ -3,6 +3,7 @@ import { readFile } from "node:fs/promises"; import { fileURLToPath } from "node:url"; import { TestClient } from "../../test/TestClient.ts"; import { ReadFileRequest } from "../../protocol/hyperjump-readFile.ts"; +import { wait } from "../../test/test-utils.ts"; import type { Diagnostic } from "vscode-languageserver"; @@ -102,3 +103,70 @@ describe("Diagnostics", () => { await expect(reopenDiagnostics).resolves.toHaveLength(1); }); }); + +describe("Diagnostics delay", () => { + const diagnosticsDelay = 200; + // Long enough to validate, but well within the delay + const pause = async () => wait(diagnosticsDelay / 5); + let client: TestClient; + + beforeEach(async () => { + client = new TestClient(undefined, { diagnosticsDelay }); + await client.start(); + }); + + afterEach(async () => { + await client.stop(); + }); + + test("opening a document publishes diagnostics without waiting for the delay", async () => { + await client.writeDocument("test.json", `{ "name": }`); + const initialDiagnostics = client.getDiagnostics("test.json"); + + const start = performance.now(); + await client.openDocument("test.json"); + await expect(initialDiagnostics).resolves.toHaveLength(1); + + expect(performance.now() - start).toBeLessThan(diagnosticsDelay); + }); + + test("changes in quick succession publish diagnostics once, for the last change", async () => { + await client.writeDocument("test.json", `{ "name": "foo" }`); + const initialDiagnostics = client.getDiagnostics("test.json"); + await client.openDocument("test.json"); + await expect(initialDiagnostics).resolves.toEqual([]); + + const receivedDiagnostics: Diagnostic[][] = []; + await client.onDiagnostics("test.json", (diagnostics) => { + receivedDiagnostics.push(diagnostics); + }); + + await client.changeDocument("test.json", `{ "name": }`); + await pause(); + await client.changeDocument("test.json", `{ "name": "f }`); + await pause(); + await client.changeDocument("test.json", `{ "name": "foo" }`); + await wait(diagnosticsDelay * 4); + + expect(receivedDiagnostics).toEqual([[]]); + }); + + test("closing a document before the delay is over publishes only the empty list", async () => { + await client.writeDocument("test.json", `{ "name": "foo" }`); + const initialDiagnostics = client.getDiagnostics("test.json"); + await client.openDocument("test.json"); + await expect(initialDiagnostics).resolves.toEqual([]); + + const receivedDiagnostics: Diagnostic[][] = []; + await client.onDiagnostics("test.json", (diagnostics) => { + receivedDiagnostics.push(diagnostics); + }); + + await client.changeDocument("test.json", `{ "name": }`); + await pause(); + await client.closeDocument("test.json"); + await wait(diagnosticsDelay * 4); + + expect(receivedDiagnostics).toEqual([[]]); + }); +}); diff --git a/language-server/src/features/diagnostics/Diagnostics.ts b/language-server/src/features/diagnostics/Diagnostics.ts index d82d87f..8253f0d 100644 --- a/language-server/src/features/diagnostics/Diagnostics.ts +++ b/language-server/src/features/diagnostics/Diagnostics.ts @@ -10,19 +10,30 @@ export type DiagnosticsProvider = { getDiagnostics(jsonDocument: JsonDocument): Promise; }; +// Validating on every keystroke would evaluate the document against its schema +// more often than anyone can read the results, so diagnostics for an edit are +// sent once the document hasn't changed for this long. This is what VS Code's +// JSON language server does. +const DIAGNOSTICS_DELAY = 500; + export class Diagnostics { private server: Server; private jsonDocuments: JsonDocuments; private providers: DiagnosticsProvider[]; + private delay: number; private pendingSends: Map = new Map(); + // Opening a document creates a new instance, so a document that hasn't been + // seen yet was just opened and is validated without waiting + private seenDocuments: WeakSet = new WeakSet(); - constructor(server: Server, jsonDocuments: JsonDocuments, jsonSchema: JsonSchema, providers: DiagnosticsProvider[]) { + constructor(server: Server, jsonDocuments: JsonDocuments, jsonSchema: JsonSchema, providers: DiagnosticsProvider[], delay = DIAGNOSTICS_DELAY) { this.server = server; this.jsonDocuments = jsonDocuments; this.providers = providers; + this.delay = delay; - jsonDocuments.onDidChangeContent(async (change) => { - await this.sendDiagnostics(change.document); + jsonDocuments.onDidChangeContent((change) => { + this.scheduleDiagnostics(change.document); }); jsonDocuments.onDidClose(async ({ document }) => { @@ -35,22 +46,32 @@ export class Diagnostics { this.server.console.log(`clear diagnostics for ${abbreviateUri(document.uri)}`); }); - jsonSchema.onDidChangeDocumentSchema(async (change) => { - await this.sendDiagnostics(change.document); + jsonSchema.onDidChangeDocumentSchema((change) => { + this.scheduleDiagnostics(change.document); }); } - private async sendDiagnostics(document: JsonDocument) { - // The document might have been closed (and possibly reopened as a new instance) since this was triggered - if (this.jsonDocuments.get(document.uri) !== document) { - return; - } - + // Replaces any send that's waiting or in progress. Aborting the send cancels + // it whether it's still waiting or already validating. + private scheduleDiagnostics(document: JsonDocument) { this.cancelPendingSend(document.uri); const controller = new AbortController(); this.pendingSends.set(document.uri, controller); + const delay = this.seenDocuments.has(document) ? this.delay : 0; + this.seenDocuments.add(document); + + const timeout = setTimeout(() => this.sendDiagnostics(document, controller), delay); + controller.signal.addEventListener("abort", () => clearTimeout(timeout)); + } + + private async sendDiagnostics(document: JsonDocument, controller: AbortController) { + // The document might have been closed (and possibly reopened as a new instance) since this was triggered + if (this.jsonDocuments.get(document.uri) !== document) { + return; + } + try { const diagnostics = []; for (const provider of this.providers) { diff --git a/language-server/src/test/TestClient.ts b/language-server/src/test/TestClient.ts index 43b2140..569a47b 100644 --- a/language-server/src/test/TestClient.ts +++ b/language-server/src/test/TestClient.ts @@ -39,7 +39,7 @@ import type { PublishDiagnosticsParams, ServerCapabilities } from "vscode-languageserver"; -import type { LanguageServerSettings } from "../build-server.js"; +import type { BuildServerOptions, LanguageServerSettings } from "../build-server.js"; // Encodes a URI the way VS Code does, e.g. "file:///c:/a@b" becomes "file:///c%3A/a%40b". Every URI sent to // the server goes through this so tests cover URIs that aren't normalized. @@ -66,7 +66,8 @@ export class TestClient { sendProgress: Connection["sendProgress"]; mockAgent: MockAgent; - constructor(serverName = "hyperjumpJsonLanguageServer") { + // Diagnostics are sent without a delay so tests don't wait for it + constructor(serverName = "hyperjumpJsonLanguageServer", options: BuildServerOptions = { diagnosticsDelay: 0 }) { this.serverName = serverName; this.watchEnabled = false; this.openDocuments = new Set(); @@ -93,7 +94,7 @@ export class TestClient { const connection = createConnection(up, down); - const server = buildServer(connection); + const server = buildServer(connection, options); server.listen(); this.ready = server.isInitialized;