From d12ba087bbec0ea36e7e3975c86ed1366cedacaf Mon Sep 17 00:00:00 2001 From: Danny Tuppeny Date: Wed, 22 Apr 2026 19:17:21 +0100 Subject: [PATCH] Snapshot TextDocuments when delayOpenNotifications=true to avoid serving updated info to middleware/server When delayOpenNotifications is enabled and an open notification is delayed, we cannot use the live VS Code TextDocument to pass to middleware or to the server because it may have updated version/content info. Instead, capture a snapshot of the document at the time the original notification would've been sent, and then when the notification will be sent, provide that snapshot to the middleware and build the server parameters from it. Fixes https://github.com/microsoft/vscode-languageserver-node/issues/1695 --- build/bin/symlink.js | 1 + client-node-tests/src/integration.test.ts | 57 +++++-- client/package-lock.json | 8 +- client/package.json | 1 + client/src/common/textSynchronization.ts | 186 +++++++++++++++++++++- 5 files changed, 237 insertions(+), 16 deletions(-) diff --git a/build/bin/symlink.js b/build/bin/symlink.js index 216aa53b8..55619fed7 100644 --- a/build/bin/symlink.js +++ b/build/bin/symlink.js @@ -33,6 +33,7 @@ const root = path.dirname(path.dirname(__dirname)); await ln.tryLinkJsonRpc(clientFolder); await ln.tryLinkTypes(clientFolder); await ln.tryLinkProtocol(clientFolder); + await ln.tryLinkTextDocuments(clientFolder); // test-extension let extensionFolder = path.join(root, 'client-node-tests'); diff --git a/client-node-tests/src/integration.test.ts b/client-node-tests/src/integration.test.ts index 29c5c92d2..7f5880614 100644 --- a/client-node-tests/src/integration.test.ts +++ b/client-node-tests/src/integration.test.ts @@ -11,6 +11,7 @@ import * as lsclient from 'vscode-languageclient/node'; import * as proto from 'vscode-languageserver-protocol'; import { MemoryFileSystemProvider } from './memoryFileSystemProvider'; import { vsdiag, DiagnosticProviderMiddleware } from 'vscode-languageclient'; +import { TextDocument } from 'vscode'; namespace GotNotifiedRequest { export const method: 'testing/gotNotified' = 'testing/gotNotified'; @@ -2242,6 +2243,11 @@ suite('Server activation', () => { suite('delayOpenNotifications', () => { let client: lsclient.LanguageClient; + let middleware: lsclient.Middleware = {}; + + suiteSetup(() => { + middleware = {}; + }); async function startClient(delayOpen: boolean): Promise { const serverModule = path.join(__dirname, './servers/textSyncServer.js'); @@ -2254,7 +2260,7 @@ suite('delayOpenNotifications', () => { documentSelector: [{ language: 'plaintext' }], synchronize: {}, initializationOptions: {}, - middleware: {}, + middleware, textSynchronization: { delayOpenNotifications: delayOpen } @@ -2269,7 +2275,7 @@ suite('delayOpenNotifications', () => { uri: 'untitled:test.txt', languageId: 'plaintext', version: 1, - getText: () => '', + getText: () => 'original line1\noriginal line2\noriginal line3', } as any as vscode.TextDocument; function sendDidOpen(document: vscode.TextDocument) { @@ -2314,25 +2320,33 @@ suite('delayOpenNotifications', () => { ); }); - test.skip('didOpen contains correct version/content for create+edit operation', async () => { - // Fails due to - // https://github.com/microsoft/vscode-languageserver-node/issues/1695 + test('didOpen contains correct version/content for create+edit operation', async () => { await startClient(true); + // Set up middleware to capture the (delayed) text document passed for + // didOpen. + let middlewareDidOpenTextDocument: TextDocument | undefined; + middleware.didOpen = (document, next) => { + middlewareDidOpenTextDocument = document; + return next(document); + }; + // Simulate did open await sendDidOpen(fakeDocument); // Modify the document and trigger change. + const originalText = fakeDocument.getText(); + const updatedText = 'NEW CONTENT'; (fakeDocument as any).version = 2; - fakeDocument.getText = () => 'NEW CONTENT'; + fakeDocument.getText = () => updatedText; await sendDidChange({ document: fakeDocument, reason: undefined, contentChanges: [{ range: new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0)), rangeOffset: 0, - rangeLength: 0, - text: 'NEW CONTENT', + rangeLength: originalText.length, + text: updatedText, }] }); @@ -2340,11 +2354,30 @@ suite('delayOpenNotifications', () => { const notifications = await client.sendRequest(GetNotificationsRequest.type); assert.equal(notifications.length, 2); const [openNotification, changeNotification] = notifications; + assert.equal(openNotification.method, 'textDocument/didOpen'); - assert.equal(openNotification.params.textDocument.version, 1); - assert.equal(openNotification.params.textDocument.text, ''); + const openTextDoc = openNotification.params.textDocument as proto.TextDocumentItem; + assert.equal(openTextDoc.version, 1); + assert.equal(openTextDoc.text, originalText); + assert.equal(changeNotification.method, 'textDocument/didChange'); - assert.equal(changeNotification.params.textDocument.version, 2); - assert.equal(changeNotification.params.textDocument.text, 'NEW CONTENT'); + const changeTextDoc = changeNotification.params.textDocument as proto.VersionedTextDocumentIdentifier; + assert.equal(changeTextDoc.version, 2); + + // Also verify the "VS Code" version of the TextDocument passed to the + // middleware behaves as the original document would. + const line3index = 2; // lines are 0-based + const offsetOfLine3word = originalText.indexOf('line3'); + const lineOffsetOfLine3word = originalText.split('\n')[line3index].indexOf('line3'); + const textDoc = middlewareDidOpenTextDocument!; + const positionOfLine3word = textDoc.positionAt(offsetOfLine3word); + const rangeOfLine3word = textDoc.getWordRangeAtPosition(positionOfLine3word); + assert.equal(positionOfLine3word.line, line3index); + assert.equal(positionOfLine3word.character, lineOffsetOfLine3word); + assert.equal(textDoc.lineAt(line3index).text, 'original line3'); + assert.equal(textDoc.getText(rangeOfLine3word), 'line3'); + assert.equal(textDoc.offsetAt(positionOfLine3word), offsetOfLine3word); + assert.ok(textDoc.validatePosition(positionOfLine3word).isEqual(positionOfLine3word)); + assert.ok(textDoc.validateRange(rangeOfLine3word!).isEqual(rangeOfLine3word!)); }); }); diff --git a/client/package-lock.json b/client/package-lock.json index f287534e6..6a37f49d7 100644 --- a/client/package-lock.json +++ b/client/package-lock.json @@ -11,7 +11,8 @@ "dependencies": { "minimatch": "^10.1.2", "semver": "^7.7.1", - "vscode-languageserver-protocol": "3.17.6-next.17" + "vscode-languageserver-protocol": "3.17.6-next.17", + "vscode-languageserver-textdocument": "1.0.12" }, "devDependencies": { "@types/minimatch": "^5.1.2", @@ -688,6 +689,11 @@ "vscode-languageserver-types": "3.17.6-next.6" } }, + "node_modules/vscode-languageserver-textdocument": { + "version": "1.0.12", + "resolved": "https://registry.npmjs.org/vscode-languageserver-textdocument/-/vscode-languageserver-textdocument-1.0.12.tgz", + "integrity": "sha512-cxWNPesCnQCcMPeenjKKsOCKQZ/L6Tv19DTRIGuLWe32lyzWhihGVJ/rcckZXJxfdKCFvRLS3fpBIsV/ZGX4zA==" + }, "node_modules/vscode-languageserver-types": { "version": "3.17.6-next.6", "resolved": "https://registry.npmjs.org/vscode-languageserver-types/-/vscode-languageserver-types-3.17.6-next.6.tgz", diff --git a/client/package.json b/client/package.json index d7ef52f41..10e8528f1 100644 --- a/client/package.json +++ b/client/package.json @@ -43,6 +43,7 @@ "dependencies": { "minimatch": "^10.1.2", "semver": "^7.7.1", + "vscode-languageserver-textdocument": "1.0.12", "vscode-languageserver-protocol": "3.17.6-next.17" }, "scripts": { diff --git a/client/src/common/textSynchronization.ts b/client/src/common/textSynchronization.ts index 7bef7c4a4..da28058a6 100644 --- a/client/src/common/textSynchronization.ts +++ b/client/src/common/textSynchronization.ts @@ -5,7 +5,7 @@ import { workspace as Workspace, languages as Languages, TextDocument, TextDocumentChangeEvent, TextDocumentWillSaveEvent, TextEdit as VTextEdit, - DocumentSelector as VDocumentSelector, Event, EventEmitter, Disposable, + DocumentSelector as VDocumentSelector, EndOfLine, Event, EventEmitter, Disposable, Position, Range, TextLine, workspace } from 'vscode'; @@ -22,6 +22,7 @@ import { } from './features'; import * as UUID from './utils/uuid'; +import { TextDocument as nodeTextDocument } from 'vscode-languageserver-textdocument'; export interface TextDocumentSynchronizationMiddleware { didOpen?: NextSignature>; @@ -77,6 +78,14 @@ export class DidOpenTextDocumentFeature extends TextDocumentEventFeature/?-]+)/g; + + private readonly liveVsCodeDocument: TextDocument; + private readonly snapshotDocument: nodeTextDocument; + private readonly content: string; + public readonly fileName: string; + public readonly isUntitled: boolean; + public readonly encoding: string; + public readonly isDirty: boolean; + public readonly isClosed: boolean; + public readonly eol: EndOfLine; + private _lineOffsets: number[] | undefined; + + constructor(document: TextDocument) { + // Keep the document to handle operations like save(). + this.liveVsCodeDocument = document; + + // Snapshot all the data. + this.content = document.getText(); + this.snapshotDocument = nodeTextDocument.create( + document.uri.toString(), + document.languageId, + document.version, + this.content, + ); + this.fileName = document.fileName; + this.isUntitled = document.isUntitled; + this.encoding = document.encoding; + this.isDirty = document.isDirty; + this.isClosed = document.isClosed; + this.eol = document.eol; + } + + public get uri(): TextDocument['uri'] { + return this.liveVsCodeDocument.uri; + } + + public get languageId(): string { + return this.snapshotDocument.languageId; + } + + public get version(): number { + return this.snapshotDocument.version; + } + + public save(): Thenable { + return this.liveVsCodeDocument.save(); + } + + public get lineCount(): number { + return this.snapshotDocument.lineCount; + } + + public lineAt(line: number): TextLine; + public lineAt(position: Position): TextLine; + public lineAt(lineOrPosition: number | Position): TextLine { + const line = typeof lineOrPosition === 'number' ? lineOrPosition : this.validatePosition(lineOrPosition).line; + if (line < 0 || line >= this.lineCount) { + throw new RangeError(`Illegal value for line: ${line}`); + } + const startOffset = this.getLineOffsets()[line]; + const endOffset = this.getLineEndOffset(line); + const text = this.content.substring(startOffset, endOffset); + const firstNonWhitespaceCharacterIndex = text.search(/\S/); + const range = new Range(new Position(line, 0), new Position(line, text.length)); + const rangeIncludingLineBreak = line + 1 < this.lineCount + ? new Range(range.start, new Position(line + 1, 0)) + : range; + return { + lineNumber: line, + text, + range, + rangeIncludingLineBreak, + firstNonWhitespaceCharacterIndex: firstNonWhitespaceCharacterIndex === -1 ? text.length : firstNonWhitespaceCharacterIndex, + isEmptyOrWhitespace: firstNonWhitespaceCharacterIndex === -1, + }; + } + + public offsetAt(position: Position): number { + return this.snapshotDocument.offsetAt(position); + } + + public positionAt(offset: number): Position { + const position = this.snapshotDocument.positionAt(offset); + return new Position(position.line, position.character); + } + + public getText(range?: Range): string { + return this.snapshotDocument.getText(range); + } + + public getWordRangeAtPosition(position: Position, regex?: RegExp): Range | undefined { + const validatedPosition = this.validatePosition(position); + const line = this.lineAt(validatedPosition); + if (line.text.length === 0) { + return undefined; + } + const wordDefinition = TextDocumentSnapshot.createWordRegExp(regex); + if (''.match(wordDefinition)?.[0]?.length === 0) { + return undefined; + } + let match: RegExpExecArray | null; + while ((match = wordDefinition.exec(line.text)) !== null) { + if (match[0].length === 0) { + break; + } + const start = match.index; + const end = start + match[0].length; + if (start <= validatedPosition.character && validatedPosition.character <= end) { + return new Range(new Position(line.lineNumber, start), new Position(line.lineNumber, end)); + } + } + return undefined; + } + + public validateRange(range: Range): Range { + return new Range(this.validatePosition(range.start), this.validatePosition(range.end)); + } + + public validatePosition(position: Position): Position { + const line = Math.min(Math.max(position.line, 0), this.lineCount - 1); + const character = Math.min(Math.max(position.character, 0), this.getLineLength(line)); + if (line === position.line && character === position.character) { + return position; + } + return new Position(line, character); + } + + private getLineOffsets(): number[] { + if (this._lineOffsets === undefined) { + this._lineOffsets = [0]; + for (let index = 0; index < this.content.length; index++) { + const charCode = this.content.charCodeAt(index); + if (TextDocumentSnapshot.isEol(charCode)) { + if (charCode === 13 && index + 1 < this.content.length && this.content.charCodeAt(index + 1) === 10) { + index++; + } + this._lineOffsets.push(index + 1); + } + } + } + return this._lineOffsets; + } + + private getLineEndOffset(line: number): number { + const lineOffsets = this.getLineOffsets(); + const startOffset = lineOffsets[line]; + let endOffset = line + 1 < lineOffsets.length ? lineOffsets[line + 1] : this.content.length; + while (endOffset > startOffset && TextDocumentSnapshot.isEol(this.content.charCodeAt(endOffset - 1))) { + endOffset--; + } + return endOffset; + } + + private getLineLength(line: number): number { + return this.getLineEndOffset(line) - this.getLineOffsets()[line]; + } + + private static createWordRegExp(regex?: RegExp): RegExp { + const wordDefinition = regex ?? TextDocumentSnapshot.DefaultWordRegExp; + const flags = wordDefinition.flags.includes('g') ? wordDefinition.flags : `${wordDefinition.flags}g`; + return new RegExp(wordDefinition.source, flags); + } + + private static isEol(charCode: number): boolean { + return charCode === 10 || charCode === 13; + } +}