Merge pull request microsoft#165147 from microsoft/hediet/integrated-mule

Implements diffing fixture tests. Fixes microsoft#165041

Author: hediet

.vscode/settings.json

@@ -12,7 +12,8 @@
 		}
 	},
 	"files.associations": {
-		"cglicenses.json": "jsonc"
+		"cglicenses.json": "jsonc",
+		"*.tst": "typescript"
 	},
 	"search.exclude": {
 		"**/node_modules": true,
@@ -27,7 +28,8 @@
 		"test/automation/out/**": true,
 		"test/integration/browser/out/**": true,
 		"src/vs/base/test/node/uri.test.data.txt": true,
-		"src/vs/workbench/api/test/browser/extHostDocumentData.test.perf-data.ts": true
+		"src/vs/workbench/api/test/browser/extHostDocumentData.test.perf-data.ts": true,
+		"src/vs/editor/test/node/diffing/fixtures/**": true,
 	},
 	"lcov.path": [
 		"./.build/coverage/lcov.info",

src/vs/editor/common/diff/algorithms/joinSequenceDiffs.ts

@@ -85,11 +85,11 @@ export function shiftSequenceDiffs(sequence1: ISequence, sequence2: ISequence, s
 		const diff = sequenceDiffs[i];
 		if (diff.seq1Range.isEmpty) {
 			const seq2PrevEndExclusive = (i > 0 ? sequenceDiffs[i - 1].seq2Range.endExclusive : -1);
-			const seq2NextStart = (i + 1 < sequenceDiffs.length ? sequenceDiffs[i + 1].seq2Range.start : sequence2.length + 1);
+			const seq2NextStart = (i + 1 < sequenceDiffs.length ? sequenceDiffs[i + 1].seq2Range.start : sequence2.length);
 			sequenceDiffs[i] = shiftDiffToBetterPosition(diff, sequence1, sequence2, seq2NextStart, seq2PrevEndExclusive);
 		} else if (diff.seq2Range.isEmpty) {
 			const seq1PrevEndExclusive = (i > 0 ? sequenceDiffs[i - 1].seq1Range.endExclusive : -1);
-			const seq1NextStart = (i + 1 < sequenceDiffs.length ? sequenceDiffs[i + 1].seq1Range.start : sequence1.length + 1);
+			const seq1NextStart = (i + 1 < sequenceDiffs.length ? sequenceDiffs[i + 1].seq1Range.start : sequence1.length);
 			sequenceDiffs[i] = shiftDiffToBetterPosition(diff.reverse(), sequence2, sequence1, seq1NextStart, seq1PrevEndExclusive).reverse();
 		}
 	}
@@ -102,15 +102,15 @@ function shiftDiffToBetterPosition(diff: SequenceDiff, sequence1: ISequence, seq
 
 	// don't touch previous or next!
 	let deltaBefore = 1;
-	while (diff.seq1Range.start - deltaBefore > seq2PrevEndExclusive &&
+	while (diff.seq2Range.start - deltaBefore > seq2PrevEndExclusive &&
 		sequence2.getElement(diff.seq2Range.start - deltaBefore) ===
 		sequence2.getElement(diff.seq2Range.endExclusive - deltaBefore) && deltaBefore < maxShiftLimit) {
 		deltaBefore++;
 	}
 	deltaBefore--;
 
 	let deltaAfter = 1;
-	while (diff.seq1Range.start + deltaAfter < seq2NextStart &&
+	while (diff.seq2Range.start + deltaAfter < seq2NextStart &&
 		sequence2.getElement(diff.seq2Range.start + deltaAfter) ===
 		sequence2.getElement(diff.seq2Range.endExclusive + deltaAfter) && deltaAfter < maxShiftLimit) {
 		deltaAfter++;

src/vs/editor/test/node/diffing/README.md

@@ -0,0 +1,10 @@
+# Diffing Fixture Tests
+
+Every folder in `fixtures` represents a test.
+The file that starts with `1.` is diffed against the file that starts with `2.`. Use `tst` instead of `ts` to avoid compiler/linter errors for typescript diff files.
+
+* Missing `*.expected.diff.json` are created automatically (as well as an `*.invalid.diff.json` file).
+* If the actual diff does not equal the expected diff, the expected file is updated automatically. The previous value of the expected file is written to `*.invalid.diff.json`.
+* The test will fail if there are any `*.invalid.diff.json` files. This makes sure that the test keeps failing even if it is run a second time.
+
+When changing the diffing algorithm, run the fixture tests, review the diff of the `*.expected.diff.json` files and delete all `*.invalid.diff.json` files.

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

@@ -0,0 +1,99 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import assert = require('assert');
+import { readdirSync, readFileSync, existsSync, writeFileSync } from 'fs';
+import { join, resolve } from 'path';
+import { FileAccess } from 'vs/base/common/network';
+import { SmartLinesDiffComputer } from 'vs/editor/common/diff/smartLinesDiffComputer';
+import { StandardLinesDiffComputer } from 'vs/editor/common/diff/standardLinesDiffComputer';
+
+suite('diff fixtures', () => {
+	const fixturesOutDir = FileAccess.asFileUri('vs/editor/test/node/diffing/fixtures').fsPath;
+	// We want the dir in src, so we can directly update the source files if they disagree and create invalid files to capture the previous state.
+	// This makes it very easy to update the fixtures.
+	const fixturesSrcDir = resolve(fixturesOutDir).replaceAll('\\', '/').replace('/out/vs/editor/', '/src/vs/editor/');
+	const folders = readdirSync(fixturesSrcDir);
+
+	for (const folder of folders) {
+		for (const diffingAlgoName of ['smart', 'experimental']) {
+			test(`${folder}-${diffingAlgoName}`, () => {
+				const folderPath = join(fixturesSrcDir, folder);
+				const files = readdirSync(folderPath);
+
+				const firstFileName = files.find(f => f.startsWith('1.'))!;
+				const secondFileName = files.find(f => f.startsWith('2.'))!;
+
+				const firstContentLines = readFileSync(join(folderPath, firstFileName), 'utf8').split(/\r\n|\r|\n/);
+				const secondContentLines = readFileSync(join(folderPath, secondFileName), 'utf8').split(/\r\n|\r|\n/);
+
+				const diffingAlgo = diffingAlgoName === 'smart' ? new SmartLinesDiffComputer() : new StandardLinesDiffComputer();
+
+				const diff = diffingAlgo.computeDiff(firstContentLines, secondContentLines, { ignoreTrimWhitespace: false, maxComputationTime: Number.MAX_SAFE_INTEGER });
+
+				const actualDiffingResult: DiffingResult = {
+					originalFileName: `./${firstFileName}`,
+					modifiedFileName: `./${secondFileName}`,
+					diffs: diff.changes.map<IDetailedDiff>(c => ({
+						originalRange: c.originalRange.toString(),
+						modifiedRange: c.modifiedRange.toString(),
+						innerChanges: c.innerChanges?.map<IDiff>(c => ({
+							originalRange: c.originalRange.toString(),
+							modifiedRange: c.modifiedRange.toString(),
+						})) || null
+					}))
+				};
+
+				const expectedFilePath = join(folderPath, `${diffingAlgoName}.expected.diff.json`);
+				const invalidFilePath = join(folderPath, `${diffingAlgoName}.invalid.diff.json`);
+
+				const expectedFileContentFromActual = JSON.stringify(actualDiffingResult, null, '\t');
+
+				const invalidExists = existsSync(invalidFilePath);
+
+				if (!existsSync(expectedFilePath)) {
+					writeFileSync(expectedFilePath, expectedFileContentFromActual);
+					writeFileSync(invalidFilePath, '');
+					throw new Error('No expected file! Expected and invalid files were written. Delete the invalid file to make the test pass.');
+				} else {
+					const expectedFileContent = readFileSync(invalidExists ? invalidFilePath : expectedFilePath, 'utf8');
+					const expectedFileDiffResult: DiffingResult = JSON.parse(expectedFileContent);
+
+					try {
+						assert.deepStrictEqual(actualDiffingResult, expectedFileDiffResult);
+					} catch (e) {
+						if (!invalidExists) {
+							writeFileSync(invalidFilePath, expectedFileContent);
+						}
+						writeFileSync(expectedFilePath, expectedFileContentFromActual);
+						throw e;
+					}
+				}
+
+				if (invalidExists) {
+					throw new Error('Invalid file exists and agrees with expected file! Delete the invalid file to make the test pass.');
+				}
+			});
+		}
+	}
+});
+
+interface DiffingResult {
+	originalFileName: string;
+	modifiedFileName: string;
+
+	diffs: IDetailedDiff[];
+}
+
+interface IDetailedDiff {
+	originalRange: string; // [startLineNumber, endLineNumberExclusive)
+	modifiedRange: string; // [startLineNumber, endLineNumberExclusive)
+	innerChanges: IDiff[] | null;
+}
+
+interface IDiff {
+	originalRange: string; // [1,18 -> 1,19]
+	modifiedRange: string; // [1,18 -> 1,19]
+}

src/vs/editor/test/node/diffing/fixtures/bracket-aligning/1.tst

@@ -0,0 +1,131 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { CompareResult } from 'vs/base/common/arrays';
+import { autorun, derived } from 'vs/base/common/observable';
+import { IModelDeltaDecoration, MinimapPosition, OverviewRulerLane } from 'vs/editor/common/model';
+import { localize } from 'vs/nls';
+import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
+import { LineRange } from 'vs/workbench/contrib/mergeEditor/browser/model/lineRange';
+import { applyObservableDecorations, join } from 'vs/workbench/contrib/mergeEditor/browser/utils';
+import { handledConflictMinimapOverViewRulerColor, unhandledConflictMinimapOverViewRulerColor } from 'vs/workbench/contrib/mergeEditor/browser/view/colors';
+import { CodeEditorView } from './codeEditorView';
+
+export class ResultCodeEditorView extends CodeEditorView {
+	private readonly decorations = derived('result.decorations', reader => {
+		const viewModel = this.viewModel.read(reader);
+		if (!viewModel) {
+			return [];
+		}
+		const model = viewModel.model;
+		const result = new Array<IModelDeltaDecoration>();
+
+		const baseRangeWithStoreAndTouchingDiffs = join(
+			model.modifiedBaseRanges.read(reader),
+			model.resultDiffs.read(reader),
+			(baseRange, diff) => baseRange.baseRange.touches(diff.inputRange)
+				? CompareResult.neitherLessOrGreaterThan
+				: LineRange.compareByStart(
+					baseRange.baseRange,
+					diff.inputRange
+				)
+		);
+
+		const activeModifiedBaseRange = viewModel.activeModifiedBaseRange.read(reader);
+
+		for (const m of baseRangeWithStoreAndTouchingDiffs) {
+			const modifiedBaseRange = m.left;
+
+			if (modifiedBaseRange) {
+				const range = model.getRangeInResult(modifiedBaseRange.baseRange, reader).toInclusiveRange();
+				if (range) {
+					const blockClassNames = ['merge-editor-block'];
+					const isHandled = model.isHandled(modifiedBaseRange).read(reader);
+					if (isHandled) {
+						blockClassNames.push('handled');
+					}
+					if (modifiedBaseRange === activeModifiedBaseRange) {
+						blockClassNames.push('focused');
+					}
+					blockClassNames.push('result');
+
+					result.push({
+						range,
+						options: {
+							isWholeLine: true,
+							blockClassName: blockClassNames.join(' '),
+							description: 'Result Diff',
+							minimap: {
+								position: MinimapPosition.Gutter,
+								color: { id: isHandled ? handledConflictMinimapOverViewRulerColor : unhandledConflictMinimapOverViewRulerColor },
+							},
+							overviewRuler: {
+								position: OverviewRulerLane.Center,
+								color: { id: isHandled ? handledConflictMinimapOverViewRulerColor : unhandledConflictMinimapOverViewRulerColor },
+							}
+						}
+					});
+				}
+			}
+
+			for (const diff of m.rights) {
+				const range = diff.outputRange.toInclusiveRange();
+				if (range) {
+					result.push({
+						range,
+						options: {
+							className: `merge-editor-diff result`,
+							description: 'Merge Editor',
+							isWholeLine: true,
+						}
+					});
+				}
+
+				if (diff.rangeMappings) {
+					for (const d of diff.rangeMappings) {
+						result.push({
+							range: d.outputRange,
+							options: {
+								className: `merge-editor-diff-word result`,
+								description: 'Merge Editor'
+							}
+						});
+					}
+				}
+			}
+		}
+		return result;
+	});
+
+	constructor(
+		@IInstantiationService instantiationService: IInstantiationService
+	) {
+		super(instantiationService);
+
+		this._register(applyObservableDecorations(this.editor, this.decorations));
+
+
+		this._register(autorun('update remainingConflicts label', reader => {
+			const model = this.model.read(reader);
+			if (!model) {
+				return;
+			}
+			const count = model.unhandledConflictsCount.read(reader);
+
+			this.htmlElements.detail.innerText = count === 1
+				? localize(
+					'mergeEditor.remainingConflicts',
+					'{0} Conflict Remaining',
+					count
+				)
+				: localize(
+					'mergeEditor.remainingConflict',
+					'{0} Conflicts Remaining ',
+					count
+				);
+
+		}));
+	}
+}