fix(language-server): correctly calculate coalesced document change (#…

…240)
volarjs · Oct 7, 2024 · 4733999 · 4733999
1 parent eb9864d
commit 4733999
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 33 deletions.
diff --git a/packages/language-server/lib/utils/snapshotDocument.ts b/packages/language-server/lib/utils/snapshotDocument.ts
@@ -57,16 +57,21 @@ export class SnapshotDocument implements TextDocument {
 	 */
 	update(contentChanges: vscode.TextDocumentContentChangeEvent[], version: number) {
 		if (contentChanges.every(change => 'range' in change)) {
-			const { minStart, oldLength, lengthDiff } = this.calculateChangeRange(contentChanges);
-			TextDocument.update(this.document, contentChanges, version);
+			let changeRanges: ts.TextChangeRange[] = [];
+			for (const contentChange of contentChanges) {
+				if (!('range' in contentChange)) {
+					continue;
+				}
+				const start = this.offsetAt(contentChange.range.start);
+				const length = contentChange.rangeLength ?? this.offsetAt(contentChange.range.end) - start;
+				changeRanges.push({
+					span: {start, length},
+					newLength: contentChange.text.length
+				});
+				TextDocument.update(this.document, [contentChange], version);
+			}
 			this.snapshots.push({
-				changeRange: {
-					span: {
-						start: minStart,
-						length: oldLength,
-					},
-					newLength: oldLength + lengthDiff,
-				},
+				changeRange: combineChangeRanges(...changeRanges),
 				version,
 				ref: undefined,
 			});
@@ -129,30 +134,6 @@ export class SnapshotDocument implements TextDocument {
 		];
 	}
 
-	/**
-	 * Calculate the change range from the given content changes.
-	 */
-	private calculateChangeRange(contentChanges: vscode.TextDocumentContentChangeEvent[]) {
-		let lengthDiff = 0;
-		const starts: number[] = [];
-		const ends: number[] = [];
-		for (const contentChange of contentChanges) {
-			if (!('range' in contentChange)) {
-				continue;
-			}
-			const start = this.offsetAt(contentChange.range.start);
-			const length = contentChange.rangeLength ?? this.offsetAt(contentChange.range.end) - start;
-			const end = start + length;
-			starts.push(start);
-			ends.push(end);
-			lengthDiff += contentChange.text.length - length;
-		}
-		const minStart = Math.min(...starts);
-		const maxEnd = Math.max(...ends);
-		const oldLength = maxEnd - minStart;
-		return { minStart, oldLength, lengthDiff };
-	}
-
 	private clearUnreferencedVersions() {
 		let firstReferencedIndex = 0;
 		while (firstReferencedIndex < this.snapshots.length - 1 && !this.snapshots[firstReferencedIndex].ref?.deref()) {

diff --git a/packages/language-server/tests/snapshotDocument.spec.ts b/packages/language-server/tests/snapshotDocument.spec.ts
@@ -47,6 +47,58 @@ describe('SnapshotDocument', () => {
 		expect(changeRange).toEqual({ span: { start: 5, length: 0 }, newLength: 5 });
 	});
 
+	it('returns correct change range with multiple edits', () => {
+		snapshotDocument.update([{
+			range: { start: snapshotDocument.positionAt(0), end: snapshotDocument.positionAt(0) },
+			text: 'HelloXXWorld',
+		}], 1);
+		const snapshot1 = snapshotDocument.getSnapshot();
+
+		snapshotDocument.update([
+			// -> HelloXWorld
+			{
+				range: { start: snapshotDocument.positionAt(5), end: snapshotDocument.positionAt(6) },
+				text: '',
+			},
+			// -> HelloWorld
+			{
+				range: { start: snapshotDocument.positionAt(5), end: snapshotDocument.positionAt(6) },
+				text: '',
+			},
+		], 2);
+		const snapshot2 = snapshotDocument.getSnapshot();
+
+		expect(snapshot2.getText(0, snapshot2.getLength())).toEqual('HelloWorld');
+		const changeRange = snapshot2.getChangeRange(snapshot1);
+		expect(changeRange).toEqual({ span: { start: 5, length: 2 }, newLength: 0 });
+	});
+
+	it('returns correct change range with multiple overlapping edits', () => {
+		snapshotDocument.update([{
+			range: { start: snapshotDocument.positionAt(0), end: snapshotDocument.positionAt(0) },
+			text: 'HelloXYYXWorld',
+		}], 1);
+		const snapshot1 = snapshotDocument.getSnapshot();
+
+		snapshotDocument.update([
+			// -> HelloXXWorld
+			{
+				range: { start: snapshotDocument.positionAt(6), end: snapshotDocument.positionAt(8) },
+				text: '',
+			},
+			// -> HelloWorld
+			{
+				range: { start: snapshotDocument.positionAt(5), end: snapshotDocument.positionAt(7) },
+				text: '',
+			},
+		], 2);
+		const snapshot2 = snapshotDocument.getSnapshot();
+
+		expect(snapshot2.getText(0, snapshot2.getLength())).toEqual('HelloWorld');
+		const changeRange = snapshot2.getChangeRange(snapshot1);
+		expect(changeRange).toEqual({ span: { start: 5, length: 4 }, newLength: 0 });
+	});
+
 	it('allows GC of unreferenced snapshots', () => {
 		const _WeakRef = globalThis.WeakRef;