#76103 Make ids unique in markers model

src/vs/workbench/contrib/markers/browser/markersModel.ts

@@ -50,14 +50,7 @@ export class ResourceMarkers {
 	@memoize
 	get name(): string { return basename(this.resource); }
 
-	@memoize
-	get hash(): string {
-		const hasher = new Hasher();
-		hasher.hash(this.resource.toString());
-		return `${hasher.value}`;
-	}
-
-	constructor(readonly resource: URI, readonly markers: Marker[]) { }
+	constructor(readonly id: string, readonly resource: URI, readonly markers: Marker[]) { }
 }
 
 export class Marker {
@@ -73,12 +66,8 @@ export class Marker {
 		return this._lines;
 	}
 
-	@memoize
-	get hash(): string {
-		return IMarkerData.makeKey(this.marker);
-	}
-
 	constructor(
+		readonly id: string,
 		readonly marker: IMarker,
 		readonly relatedInformation: RelatedInformation[] = []
 	) { }
@@ -94,24 +83,8 @@ export class Marker {
 
 export class RelatedInformation {
 
-	@memoize
-	get hash(): string {
-		const hasher = new Hasher();
-		hasher.hash(this.resource.toString());
-		hasher.hash(this.marker.startLineNumber);
-		hasher.hash(this.marker.startColumn);
-		hasher.hash(this.marker.endLineNumber);
-		hasher.hash(this.marker.endColumn);
-		hasher.hash(this.raw.resource.toString());
-		hasher.hash(this.raw.startLineNumber);
-		hasher.hash(this.raw.startColumn);
-		hasher.hash(this.raw.endLineNumber);
-		hasher.hash(this.raw.endColumn);
-		return `${hasher.value}`;
-	}
-
 	constructor(
-		private resource: URI,
+		readonly id: string,
 		readonly marker: IMarker,
 		readonly raw: IRelatedInformation
 	) { }
@@ -146,23 +119,39 @@ export class MarkersModel {
 		if (isFalsyOrEmpty(rawMarkers)) {
 			this.resourcesByUri.delete(resource.toString());
 		} else {
-			const markers = mergeSort(rawMarkers.map(rawMarker => {
-				let relatedInformation: RelatedInformation[] | undefined = undefined;
 
+			const resourceMarkersId = this.id(resource.toString());
+			const markersCountByKey = new Map<string, number>();
+			const markers = mergeSort(rawMarkers.map((rawMarker) => {
+				const key = IMarkerData.makeKey(rawMarker);
+				const index = markersCountByKey.get(key) || 0;
+				markersCountByKey.set(key, index + 1);
+
+				const markerId = this.id(resourceMarkersId, key, index);
+
+				let relatedInformation: RelatedInformation[] | undefined = undefined;
 				if (rawMarker.relatedInformation) {
-					relatedInformation = rawMarker.relatedInformation.map(r => new RelatedInformation(resource, rawMarker, r));
+					relatedInformation = rawMarker.relatedInformation.map((r, index) => new RelatedInformation(this.id(markerId, r.resource.toString(), r.startLineNumber, r.startColumn, r.endLineNumber, r.endColumn, index), rawMarker, r));
 				}
 
-				return new Marker(rawMarker, relatedInformation);
+				return new Marker(markerId, rawMarker, relatedInformation);
 			}), compareMarkers);
 
-			this.resourcesByUri.set(resource.toString(), new ResourceMarkers(resource, markers));
+			this.resourcesByUri.set(resource.toString(), new ResourceMarkers(resourceMarkersId, resource, markers));
 		}
 
 		this.cachedSortedResources = undefined;
 		this._onDidChange.fire(resource);
 	}
 
+	private id(...values: (string | number)[]): string {
+		const hasher = new Hasher();
+		for (const value of values) {
+			hasher.hash(value);
+		}
+		return `${hasher.value}`;
+	}
+
 	dispose(): void {
 		this._onDidChange.dispose();
 		this.resourcesByUri.clear();

src/vs/workbench/contrib/markers/browser/markersPanel.ts

@@ -305,7 +305,7 @@ export class MarkersPanel extends Panel implements IMarkerFilterController {
 
 		const identityProvider = {
 			getId(element: TreeElement) {
-				return element.hash;
+				return element.id;
 			}
 		};
 

src/vs/workbench/contrib/markers/browser/markersTreeViewer.ts

@@ -630,7 +630,7 @@ export class MarkersViewModel extends Disposable {
 	}
 
 	add(marker: Marker): void {
-		if (!this.markersViewStates.has(marker.hash)) {
+		if (!this.markersViewStates.has(marker.id)) {
 			const viewModel = this.instantiationService.createInstance(MarkerViewModel, marker);
 			const disposables: IDisposable[] = [viewModel];
 			viewModel.multiline = this.multiline;
@@ -639,7 +639,7 @@ export class MarkersViewModel extends Disposable {
 					this._onDidChange.fire(marker);
 				}
 			}, this, disposables);
-			this.markersViewStates.set(marker.hash, { viewModel, disposables });
+			this.markersViewStates.set(marker.id, { viewModel, disposables });
 
 			const markers = this.markersPerResource.get(marker.resource.toString()) || [];
 			markers.push(marker);
@@ -650,11 +650,11 @@ export class MarkersViewModel extends Disposable {
 	remove(resource: URI): void {
 		const markers = this.markersPerResource.get(resource.toString()) || [];
 		for (const marker of markers) {
-			const value = this.markersViewStates.get(marker.hash);
+			const value = this.markersViewStates.get(marker.id);
 			if (value) {
 				dispose(value.disposables);
 			}
-			this.markersViewStates.delete(marker.hash);
+			this.markersViewStates.delete(marker.id);
 			if (this.hoveredMarker === marker) {
 				this.hoveredMarker = null;
 			}
@@ -663,7 +663,7 @@ export class MarkersViewModel extends Disposable {
 	}
 
 	getViewModel(marker: Marker): MarkerViewModel | null {
-		const value = this.markersViewStates.get(marker.hash);
+		const value = this.markersViewStates.get(marker.id);
 		return value ? value.viewModel : null;
 	}
 

src/vs/workbench/contrib/markers/test/electron-browser/markersModel.test.ts

@@ -27,6 +27,23 @@ class TestMarkersModel extends MarkersModel {
 
 suite('MarkersModel Test', () => {
 
+	test('marker ids are unique', function () {
+		const marker1 = anErrorWithRange(3);
+		const marker2 = anErrorWithRange(3);
+		const marker3 = aWarningWithRange(3);
+		const marker4 = aWarningWithRange(3);
+
+		const testObject = new TestMarkersModel([marker1, marker2, marker3, marker4]);
+		const actuals = testObject.resourceMarkers[0].markers;
+
+		assert.notEqual(actuals[0].id, actuals[1].id);
+		assert.notEqual(actuals[0].id, actuals[2].id);
+		assert.notEqual(actuals[0].id, actuals[3].id);
+		assert.notEqual(actuals[1].id, actuals[2].id);
+		assert.notEqual(actuals[1].id, actuals[3].id);
+		assert.notEqual(actuals[2].id, actuals[3].id);
+	});
+
 	test('sort palces resources with no errors at the end', function () {
 		const marker1 = aMarker('a/res1', MarkerSeverity.Warning);
 		const marker2 = aMarker('a/res2');
@@ -105,22 +122,22 @@ suite('MarkersModel Test', () => {
 	test('toString()', () => {
 		let marker = aMarker('a/res1');
 		marker.code = '1234';
-		assert.equal(JSON.stringify({ ...marker, resource: marker.resource.path }, null, '\t'), new Marker(marker).toString());
+		assert.equal(JSON.stringify({ ...marker, resource: marker.resource.path }, null, '\t'), new Marker('1', marker).toString());
 
 		marker = aMarker('a/res2', MarkerSeverity.Warning);
-		assert.equal(JSON.stringify({ ...marker, resource: marker.resource.path }, null, '\t'), new Marker(marker).toString());
+		assert.equal(JSON.stringify({ ...marker, resource: marker.resource.path }, null, '\t'), new Marker('2', marker).toString());
 
 		marker = aMarker('a/res2', MarkerSeverity.Info, 1, 2, 1, 8, 'Info', '');
-		assert.equal(JSON.stringify({ ...marker, resource: marker.resource.path }, null, '\t'), new Marker(marker).toString());
+		assert.equal(JSON.stringify({ ...marker, resource: marker.resource.path }, null, '\t'), new Marker('3', marker).toString());
 
 		marker = aMarker('a/res2', MarkerSeverity.Hint, 1, 2, 1, 8, 'Ignore message', 'Ignore');
-		assert.equal(JSON.stringify({ ...marker, resource: marker.resource.path }, null, '\t'), new Marker(marker).toString());
+		assert.equal(JSON.stringify({ ...marker, resource: marker.resource.path }, null, '\t'), new Marker('4', marker).toString());
 
 		marker = aMarker('a/res2', MarkerSeverity.Warning, 1, 2, 1, 8, 'Warning message', '', [{ startLineNumber: 2, startColumn: 5, endLineNumber: 2, endColumn: 10, message: 'some info', resource: URI.file('a/res3') }]);
-		const testObject = new Marker(marker, null!);
+		const testObject = new Marker('5', marker, null!);
 
 		// hack
-		(testObject as any).relatedInformation = marker.relatedInformation!.map(r => new RelatedInformation(marker.resource, marker, r));
+		(testObject as any).relatedInformation = marker.relatedInformation!.map(r => new RelatedInformation('6', marker, r));
 		assert.equal(JSON.stringify({ ...marker, resource: marker.resource.path, relatedInformation: marker.relatedInformation!.map(r => ({ ...r, resource: r.resource.path })) }, null, '\t'), testObject.toString());
 	});