Fixes microsoft/vscode-copilot#6556 (#219996)

* Fixes microsoft/vscode-copilot#6556

src/vs/editor/common/core/textEdit.ts

@@ -225,6 +225,15 @@ export class LineBasedText extends AbstractText {
 	}
 }
 
+export class ArrayText extends LineBasedText {
+	constructor(lines: string[]) {
+		super(
+			lineNumber => lines[lineNumber - 1],
+			lines.length
+		);
+	}
+}
+
 export class StringText extends AbstractText {
 	private readonly _t = new PositionOffsetTransformer(this.value);
 

src/vs/editor/common/diff/defaultLinesDiffComputer/algorithms/diffAlgorithm.ts

@@ -52,6 +52,18 @@ export class SequenceDiff {
 		);
 	}
 
+	public static assertSorted(sequenceDiffs: SequenceDiff[]): void {
+		let last: SequenceDiff | undefined = undefined;
+		for (const cur of sequenceDiffs) {
+			if (last) {
+				if (!(last.seq1Range.endExclusive <= cur.seq1Range.start && last.seq2Range.endExclusive <= cur.seq2Range.start)) {
+					throw new BugIndicatingError('Sequence diffs must be sorted');
+				}
+			}
+			last = cur;
+		}
+	}
+
 	constructor(
 		public readonly seq1Range: OffsetRange,
 		public readonly seq2Range: OffsetRange,

src/vs/editor/common/diff/defaultLinesDiffComputer/computeMovedLines.ts

@@ -9,10 +9,10 @@ import { pushMany, compareBy, numberComparator, reverseOrder } from 'vs/base/com
 import { MonotonousArray, findLastMonotonous } from 'vs/base/common/arraysFind';
 import { SetMap } from 'vs/base/common/map';
 import { LineRange, LineRangeSet } from 'vs/editor/common/core/lineRange';
-import { OffsetRange } from 'vs/editor/common/core/offsetRange';
 import { LinesSliceCharSequence } from 'vs/editor/common/diff/defaultLinesDiffComputer/linesSliceCharSequence';
 import { LineRangeFragment, isSpace } from 'vs/editor/common/diff/defaultLinesDiffComputer/utils';
 import { MyersDiffAlgorithm } from 'vs/editor/common/diff/defaultLinesDiffComputer/algorithms/myersDiffAlgorithm';
+import { Range } from 'vs/editor/common/core/range';
 
 export function computeMovedLines(
 	changes: DetailedLineRangeMapping[],
@@ -260,8 +260,8 @@ function areLinesSimilar(line1: string, line2: string, timeout: ITimeout): boole
 
 	const myersDiffingAlgorithm = new MyersDiffAlgorithm();
 	const result = myersDiffingAlgorithm.compute(
-		new LinesSliceCharSequence([line1], new OffsetRange(0, 1), false),
-		new LinesSliceCharSequence([line2], new OffsetRange(0, 1), false),
+		new LinesSliceCharSequence([line1], new Range(1, 1, 1, line1.length), false),
+		new LinesSliceCharSequence([line2], new Range(1, 1, 1, line2.length), false),
 		timeout
 	);
 	let commonNonSpaceCharCount = 0;

src/vs/editor/common/diff/defaultLinesDiffComputer/defaultLinesDiffComputer.ts

@@ -17,7 +17,7 @@ import { extendDiffsToEntireWordIfAppropriate, optimizeSequenceDiffs, removeShor
 import { LineSequence } from 'vs/editor/common/diff/defaultLinesDiffComputer/lineSequence';
 import { LinesSliceCharSequence } from 'vs/editor/common/diff/defaultLinesDiffComputer/linesSliceCharSequence';
 import { ILinesDiffComputer, ILinesDiffComputerOptions, LinesDiff, MovedText } from 'vs/editor/common/diff/linesDiffComputer';
-import { DetailedLineRangeMapping, RangeMapping } from '../rangeMapping';
+import { DetailedLineRangeMapping, LineRangeMapping, RangeMapping } from '../rangeMapping';
 
 export class DefaultLinesDiffComputer implements ILinesDiffComputer {
 	private readonly dynamicProgrammingDiffing = new DynamicProgrammingDiffing();
@@ -167,7 +167,9 @@ export class DefaultLinesDiffComputer implements ILinesDiffComputer {
 				for (const ic of c.innerChanges) {
 					const valid = validatePosition(ic.modifiedRange.getStartPosition(), modifiedLines) && validatePosition(ic.modifiedRange.getEndPosition(), modifiedLines) &&
 						validatePosition(ic.originalRange.getStartPosition(), originalLines) && validatePosition(ic.originalRange.getEndPosition(), originalLines);
-					if (!valid) { return false; }
+					if (!valid) {
+						return false;
+					}
 				}
 				if (!validateRange(c.modified, modifiedLines) || !validateRange(c.original, originalLines)) {
 					return false;
@@ -208,18 +210,28 @@ export class DefaultLinesDiffComputer implements ILinesDiffComputer {
 	}
 
 	private refineDiff(originalLines: string[], modifiedLines: string[], diff: SequenceDiff, timeout: ITimeout, considerWhitespaceChanges: boolean): { mappings: RangeMapping[]; hitTimeout: boolean } {
-		const slice1 = new LinesSliceCharSequence(originalLines, diff.seq1Range, considerWhitespaceChanges);
-		const slice2 = new LinesSliceCharSequence(modifiedLines, diff.seq2Range, considerWhitespaceChanges);
+		const lineRangeMapping = toLineRangeMapping(diff);
+		const rangeMapping = lineRangeMapping.toRangeMapping2(originalLines, modifiedLines);
+
+		const slice1 = new LinesSliceCharSequence(originalLines, rangeMapping.originalRange, considerWhitespaceChanges);
+		const slice2 = new LinesSliceCharSequence(modifiedLines, rangeMapping.modifiedRange, considerWhitespaceChanges);
 
 		const diffResult = slice1.length + slice2.length < 500
 			? this.dynamicProgrammingDiffing.compute(slice1, slice2, timeout)
 			: this.myersDiffingAlgorithm.compute(slice1, slice2, timeout);
 
+		const check = false;
+
 		let diffs = diffResult.diffs;
+		if (check) { SequenceDiff.assertSorted(diffs); }
 		diffs = optimizeSequenceDiffs(slice1, slice2, diffs);
+		if (check) { SequenceDiff.assertSorted(diffs); }
 		diffs = extendDiffsToEntireWordIfAppropriate(slice1, slice2, diffs);
+		if (check) { SequenceDiff.assertSorted(diffs); }
 		diffs = removeShortMatches(slice1, slice2, diffs);
+		if (check) { SequenceDiff.assertSorted(diffs); }
 		diffs = removeVeryShortMatchingTextBetweenLongDiffs(slice1, slice2, diffs);
+		if (check) { SequenceDiff.assertSorted(diffs); }
 
 		const result = diffs.map(
 			(d) =>
@@ -229,6 +241,8 @@ export class DefaultLinesDiffComputer implements ILinesDiffComputer {
 				)
 		);
 
+		if (check) { RangeMapping.assertSorted(result); }
+
 		// Assert: result applied on original should be the same as diff applied to original
 
 		return {
@@ -312,3 +326,10 @@ export function getLineRangeMapping(rangeMapping: RangeMapping, originalLines: s
 
 	return new DetailedLineRangeMapping(originalLineRange, modifiedLineRange, [rangeMapping]);
 }
+
+function toLineRangeMapping(sequenceDiff: SequenceDiff) {
+	return new LineRangeMapping(
+		new LineRange(sequenceDiff.seq1Range.start + 1, sequenceDiff.seq1Range.endExclusive + 1),
+		new LineRange(sequenceDiff.seq2Range.start + 1, sequenceDiff.seq2Range.endExclusive + 1),
+	);
+}

src/vs/editor/common/diff/defaultLinesDiffComputer/linesSliceCharSequence.ts

@@ -13,52 +13,39 @@ import { isSpace } from 'vs/editor/common/diff/defaultLinesDiffComputer/utils';
 
 export class LinesSliceCharSequence implements ISequence {
 	private readonly elements: number[] = [];
-	private readonly firstCharOffsetByLine: number[] = [];
-	public readonly lineRange: OffsetRange;
-	// To account for trimming
-	private readonly additionalOffsetByLine: number[] = [];
-
-	constructor(public readonly lines: string[], lineRange: OffsetRange, public readonly considerWhitespaceChanges: boolean) {
-		// This slice has to have lineRange.length many \n! (otherwise diffing against an empty slice will be problematic)
-		// (Unless it covers the entire document, in that case the other slice also has to cover the entire document ands it's okay)
-
-		// If the slice covers the end, but does not start at the beginning, we include just the \n of the previous line.
-		let trimFirstLineFully = false;
-		if (lineRange.start > 0 && lineRange.endExclusive >= lines.length) {
-			lineRange = new OffsetRange(lineRange.start - 1, lineRange.endExclusive);
-			trimFirstLineFully = true;
-		}
+	private readonly firstElementOffsetByLineIdx: number[] = [];
+	private readonly lineStartOffsets: number[] = [];
+	private readonly trimmedWsLengthsByLineIdx: number[] = [];
+
+	constructor(public readonly lines: string[], private readonly range: Range, public readonly considerWhitespaceChanges: boolean) {
+		this.firstElementOffsetByLineIdx.push(0);
+		for (let lineNumber = this.range.startLineNumber; lineNumber <= this.range.endLineNumber; lineNumber++) {
+			let line = lines[lineNumber - 1];
+			let lineStartOffset = 0;
+			if (lineNumber === this.range.startLineNumber && this.range.startColumn > 1) {
+				lineStartOffset = this.range.startColumn - 1;
+				line = line.substring(lineStartOffset);
+			}
+			this.lineStartOffsets.push(lineStartOffset);
 
-		this.lineRange = lineRange;
-
-		this.firstCharOffsetByLine[0] = 0;
-		for (let i = this.lineRange.start; i < this.lineRange.endExclusive; i++) {
-			let line = lines[i];
-			let offset = 0;
-			if (trimFirstLineFully) {
-				offset = line.length;
-				line = '';
-				trimFirstLineFully = false;
-			} else if (!considerWhitespaceChanges) {
+			let trimmedWsLength = 0;
+			if (!considerWhitespaceChanges) {
 				const trimmedStartLine = line.trimStart();
-				offset = line.length - trimmedStartLine.length;
+				trimmedWsLength = line.length - trimmedStartLine.length;
 				line = trimmedStartLine.trimEnd();
 			}
+			this.trimmedWsLengthsByLineIdx.push(trimmedWsLength);
 
-			this.additionalOffsetByLine.push(offset);
-
-			for (let i = 0; i < line.length; i++) {
+			const lineLength = lineNumber === this.range.endLineNumber ? Math.min(this.range.endColumn - 1 - lineStartOffset - trimmedWsLength, line.length) : line.length;
+			for (let i = 0; i < lineLength; i++) {
 				this.elements.push(line.charCodeAt(i));
 			}
 
-			// Don't add an \n that does not exist in the document.
-			if (i < lines.length - 1) {
+			if (lineNumber < this.range.endLineNumber) {
 				this.elements.push('\n'.charCodeAt(0));
-				this.firstCharOffsetByLine[i - this.lineRange.start + 1] = this.elements.length;
+				this.firstElementOffsetByLineIdx.push(this.elements.length);
 			}
 		}
-		// To account for the last line
-		this.additionalOffsetByLine.push(0);
 	}
 
 	toString() {
@@ -111,18 +98,23 @@ export class LinesSliceCharSequence implements ISequence {
 		return score;
 	}
 
-	public translateOffset(offset: number): Position {
+	public translateOffset(offset: number, preference: 'left' | 'right' = 'right'): Position {
 		// find smallest i, so that lineBreakOffsets[i] <= offset using binary search
-		if (this.lineRange.isEmpty) {
-			return new Position(this.lineRange.start + 1, 1);
-		}
-
-		const i = findLastIdxMonotonous(this.firstCharOffsetByLine, (value) => value <= offset);
-		return new Position(this.lineRange.start + i + 1, offset - this.firstCharOffsetByLine[i] + this.additionalOffsetByLine[i] + 1);
+		const i = findLastIdxMonotonous(this.firstElementOffsetByLineIdx, (value) => value <= offset);
+		const lineOffset = offset - this.firstElementOffsetByLineIdx[i];
+		return new Position(
+			this.range.startLineNumber + i,
+			1 + this.lineStartOffsets[i] + lineOffset + ((lineOffset === 0 && preference === 'left') ? 0 : this.trimmedWsLengthsByLineIdx[i])
+		);
 	}
 
 	public translateRange(range: OffsetRange): Range {
-		return Range.fromPositions(this.translateOffset(range.start), this.translateOffset(range.endExclusive));
+		const pos1 = this.translateOffset(range.start, 'right');
+		const pos2 = this.translateOffset(range.endExclusive, 'left');
+		if (pos2.isBefore(pos1)) {
+			return Range.fromPositions(pos2, pos2);
+		}
+		return Range.fromPositions(pos1, pos2);
 	}
 
 	/**
@@ -161,8 +153,8 @@ export class LinesSliceCharSequence implements ISequence {
 	}
 
 	public extendToFullLines(range: OffsetRange): OffsetRange {
-		const start = findLastMonotonous(this.firstCharOffsetByLine, x => x <= range.start) ?? 0;
-		const end = findFirstMonotonous(this.firstCharOffsetByLine, x => range.endExclusive <= x) ?? this.elements.length;
+		const start = findLastMonotonous(this.firstElementOffsetByLineIdx, x => x <= range.start) ?? 0;
+		const end = findFirstMonotonous(this.firstElementOffsetByLineIdx, x => range.endExclusive <= x) ?? this.elements.length;
 		return new OffsetRange(start, end);
 	}
 }

src/vs/editor/common/diff/rangeMapping.ts

@@ -5,6 +5,7 @@
 
 import { BugIndicatingError } from 'vs/base/common/errors';
 import { LineRange } from 'vs/editor/common/core/lineRange';
+import { Position } from 'vs/editor/common/core/position';
 import { Range } from 'vs/editor/common/core/range';
 import { AbstractText, SingleTextEdit } from 'vs/editor/common/core/textEdit';
 
@@ -118,6 +119,70 @@ export class LineRangeMapping {
 			);
 		}
 	}
+
+	/**
+	 * This method assumes that the LineRangeMapping describes a valid diff!
+	 * I.e. if one range is empty, the other range cannot be the entire document.
+	 * It avoids various problems when the line range points to non-existing line-numbers.
+	*/
+	public toRangeMapping2(original: string[], modified: string[]): RangeMapping {
+		if (isValidLineNumber(this.original.endLineNumberExclusive, original)
+			&& isValidLineNumber(this.modified.endLineNumberExclusive, modified)) {
+			return new RangeMapping(
+				new Range(this.original.startLineNumber, 1, this.original.endLineNumberExclusive, 1),
+				new Range(this.modified.startLineNumber, 1, this.modified.endLineNumberExclusive, 1),
+			);
+		}
+
+		if (!this.original.isEmpty && !this.modified.isEmpty) {
+			return new RangeMapping(
+				Range.fromPositions(
+					new Position(this.original.startLineNumber, 1),
+					normalizePosition(new Position(this.original.endLineNumberExclusive - 1, Number.MAX_SAFE_INTEGER), original)
+				),
+				Range.fromPositions(
+					new Position(this.modified.startLineNumber, 1),
+					normalizePosition(new Position(this.modified.endLineNumberExclusive - 1, Number.MAX_SAFE_INTEGER), modified)
+				),
+			);
+		}
+
+		if (this.original.startLineNumber > 1 && this.modified.startLineNumber > 1) {
+			return new RangeMapping(
+				Range.fromPositions(
+					normalizePosition(new Position(this.original.startLineNumber - 1, Number.MAX_SAFE_INTEGER), original),
+					normalizePosition(new Position(this.original.endLineNumberExclusive - 1, Number.MAX_SAFE_INTEGER), original)
+				),
+				Range.fromPositions(
+					normalizePosition(new Position(this.modified.startLineNumber - 1, Number.MAX_SAFE_INTEGER), modified),
+					normalizePosition(new Position(this.modified.endLineNumberExclusive - 1, Number.MAX_SAFE_INTEGER), modified)
+				),
+			);
+		}
+
+		// Situation now: one range is empty and one range touches the last line and one range starts at line 1.
+		// I don't think this can happen.
+
+		throw new BugIndicatingError();
+	}
+}
+
+function normalizePosition(position: Position, content: string[]): Position {
+	if (position.lineNumber < 1) {
+		return new Position(1, 1);
+	}
+	if (position.lineNumber > content.length) {
+		return new Position(content.length, content[content.length - 1].length + 1);
+	}
+	const line = content[position.lineNumber - 1];
+	if (position.column > line.length + 1) {
+		return new Position(position.lineNumber, line.length + 1);
+	}
+	return position;
+}
+
+function isValidLineNumber(lineNumber: number, lines: string[]): boolean {
+	return lineNumber >= 1 && lineNumber <= lines.length;
 }
 
 /**
@@ -161,6 +226,19 @@ export class DetailedLineRangeMapping extends LineRangeMapping {
  * Maps a range in the original text model to a range in the modified text model.
  */
 export class RangeMapping {
+	public static assertSorted(rangeMappings: RangeMapping[]): void {
+		for (let i = 1; i < rangeMappings.length; i++) {
+			const previous = rangeMappings[i - 1];
+			const current = rangeMappings[i];
+			if (!(
+				previous.originalRange.getEndPosition().isBeforeOrEqual(current.originalRange.getStartPosition())
+				&& previous.modifiedRange.getEndPosition().isBeforeOrEqual(current.modifiedRange.getStartPosition())
+			)) {
+				throw new BugIndicatingError('Range mappings must be sorted');
+			}
+		}
+	}
+
 	/**
 	 * The original range.
 	 */

src/vs/editor/test/node/diffing/defaultLinesDiffComputer.test.ts

@@ -17,8 +17,8 @@ suite('myers', () => {
 	ensureNoDisposablesAreLeakedInTestSuite();
 
 	test('1', () => {
-		const s1 = new LinesSliceCharSequence(['hello world'], new OffsetRange(0, 1), true);
-		const s2 = new LinesSliceCharSequence(['hallo welt'], new OffsetRange(0, 1), true);
+		const s1 = new LinesSliceCharSequence(['hello world'], new Range(1, 1, 1, Number.MAX_SAFE_INTEGER), true);
+		const s2 = new LinesSliceCharSequence(['hallo welt'], new Range(1, 1, 1, Number.MAX_SAFE_INTEGER), true);
 
 		const a = true ? new MyersDiffAlgorithm() : new DynamicProgrammingDiffing();
 		a.compute(s1, s2);
@@ -83,7 +83,7 @@ suite('LinesSliceCharSequence', () => {
 			'line4: hello world',
 			'line5: bazz',
 		],
-		new OffsetRange(1, 4), true
+		new Range(2, 1, 5, 1), true
 	);
 
 	test('translateOffset', () => {