Clear Status When Document Closes (#30)

When a diagnostic is cancelled, we were leaving behind the status
entry to dangle. This adds a cancellation event to stop the status,
thus removing it from the status bar when cancelled. We were also
using the Uri as a key in many different places, but this led to bugs
when they were not identical objects (even though identical Uris).
For convenience, we've wrapped these places with a class that
gets the string from the Uri as the key.

CHANGELOG.md

@@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Fixed
+- Maps and Sets should not be keyed by the Uri instance directly.
+- Linter status should be cleared when a diagnostic is cancelled.
 
 ## [1.2.0] - 2021-05-21
 ### Added

src/__mocks__/vscode.ts

@@ -1,13 +1,23 @@
 const Uri: any = jest.fn().mockImplementation(() => {
-	return {
+	const created = {
 		scheme: 'file',
 		authority: '',
 		path: 'test/file/path.php',
 		query: '',
 		fragment: '',
 		fsPath: 'test/file/path.php',
-		toString: jest.fn(),
+		toString: (): string => {
+			let str = created.scheme + '://' + created.path;
+			if (created.query) {
+				str += '?' + created.query;
+			}
+			if (created.fragment) {
+				str += '#' + created.fragment;
+			}
+			return str;
+		},
 	};
+	return created;
 });
 Uri.joinPath = jest.fn();
 

src/common/uri-map.ts

@@ -0,0 +1,46 @@
+import { Uri } from 'vscode';
+
+/**
+ * A map for storing data keyed by a Uri.
+ */
+export class UriMap<V> extends Map<string, V> {
+	public has(key: Uri): boolean;
+	public has(key: string): boolean;
+	public has(key: Uri | string): boolean {
+		if (key instanceof Uri) {
+			key = key.toString();
+		}
+
+		return super.has(key);
+	}
+
+	public set(key: Uri, value: V): this;
+	public set(key: string, value: V): this;
+	public set(key: Uri | string, value: V): this {
+		if (key instanceof Uri) {
+			key = key.toString();
+		}
+
+		return super.set(key, value);
+	}
+
+	public get(key: Uri): V | undefined;
+	public get(key: string): V | undefined;
+	public get(key: Uri | string): V | undefined {
+		if (key instanceof Uri) {
+			key = key.toString();
+		}
+
+		return super.get(key);
+	}
+
+	public delete(key: Uri): boolean;
+	public delete(key: string): boolean;
+	public delete(key: Uri | string): boolean {
+		if (key instanceof Uri) {
+			key = key.toString();
+		}
+
+		return super.delete(key);
+	}
+}

src/common/uri-set.ts

@@ -0,0 +1,36 @@
+import { Uri } from 'vscode';
+
+/**
+ * A set for storing Uris.
+ */
+export class UriSet extends Set<string> {
+	public has(key: Uri): boolean;
+	public has(key: string): boolean;
+	public has(key: Uri | string): boolean {
+		if (key instanceof Uri) {
+			key = key.toString();
+		}
+
+		return super.has(key);
+	}
+
+	public add(key: Uri): this;
+	public add(key: string): this;
+	public add(key: Uri | string): this {
+		if (key instanceof Uri) {
+			key = key.toString();
+		}
+
+		return super.add(key);
+	}
+
+	public delete(key: Uri): boolean;
+	public delete(key: string): boolean;
+	public delete(key: Uri | string): boolean {
+		if (key instanceof Uri) {
+			key = key.toString();
+		}
+
+		return super.delete(key);
+	}
+}

src/listeners/workspace-listener.ts

@@ -9,6 +9,8 @@ import { Configuration } from '../services/configuration';
 import { CodeActionEditResolver } from '../services/code-action-edit-resolver';
 import { DiagnosticUpdater } from '../services/diagnostic-updater';
 import { DocumentFormatter } from '../services/document-formatter';
+import { UriMap } from '../common/uri-map';
+import { UriSet } from '../common/uri-set';
 
 /**
  * A class for listening to the workspace and responding to events that occur.
@@ -37,12 +39,12 @@ export class WorkspaceListener implements Disposable {
 	/**
 	 * A set containing the Uris of all the documents that we're tracking.
 	 */
-	private readonly trackedDocuments: Set<Uri>;
+	private readonly trackedDocuments: UriSet;
 
 	/**
 	 * A map for applying a debounce to document updates.
 	 */
-	private readonly updateDebounceMap: Map<Uri, NodeJS.Timeout>;
+	private readonly updateDebounceMap: UriMap<NodeJS.Timeout>;
 
 	/**
 	 * The subscriptions we have to VS Code events.
@@ -67,8 +69,8 @@ export class WorkspaceListener implements Disposable {
 		this.diagnosticUpdater = diagnosticUpdater;
 		this.codeActionEditResolver = codeActionEditResolver;
 		this.documentFormatter = documentFormatter;
-		this.trackedDocuments = new Set();
-		this.updateDebounceMap = new Map();
+		this.trackedDocuments = new UriSet();
+		this.updateDebounceMap = new UriMap();
 		this.subscriptions = [];
 	}
 

src/services/configuration.ts

@@ -5,6 +5,7 @@ import {
 	Uri,
 	workspace as vsCodeWorkspace,
 } from 'vscode';
+import { UriMap } from '../common/uri-map';
 
 /**
  * An enum describing the values in the `phpcsCodeSniffer.standard` configuration.
@@ -83,7 +84,7 @@ export class Configuration {
 	/**
 	 * A cache containing all of the configurations we've loaded.
 	 */
-	private readonly cache: Map<Uri, DocumentConfiguration>;
+	private readonly cache: UriMap<DocumentConfiguration>;
 
 	/**
 	 * The decoder for parsing file content into strings for consumption.
@@ -97,7 +98,7 @@ export class Configuration {
 	 */
 	public constructor(workspace: typeof vsCodeWorkspace) {
 		this.workspace = workspace;
-		this.cache = new Map();
+		this.cache = new UriMap();
 		this.textDecoder = new TextDecoder();
 	}
 

src/services/diagnostic-updater.ts

@@ -94,6 +94,11 @@ export class DiagnosticUpdater extends WorkerService {
 		// Record that we're going to start linting a document.
 		this.linterStatus.start(document.uri);
 
+		// Make sure we stop linting the document the update is cancelled.
+		cancellationToken.onCancellationRequested(() => {
+			this.linterStatus.stop(document.uri);
+		});
+
 		this.workerPool
 			.waitForAvailable(
 				'diagnostic:' + document.fileName,

src/services/linter-status.ts

@@ -5,6 +5,7 @@ import {
 	Uri,
 	window as vsCodeWindow,
 } from 'vscode';
+import { UriSet } from '../common/uri-set';
 
 /**
  * A class for managing the linter's status.
@@ -18,14 +19,14 @@ export class LinterStatus implements Disposable {
 	/**
 	 * The documents that are actively being linted.
 	 */
-	private readonly activeDocuments: Set<Uri>;
+	private readonly activeDocuments: UriSet;
 
 	/**
 	 * Constructor.
 	 */
 	public constructor(window: typeof vsCodeWindow) {
 		this.statusBar = window.createStatusBarItem(StatusBarAlignment.Left);
-		this.activeDocuments = new Set();
+		this.activeDocuments = new UriSet();
 	}
 
 	/**

src/services/worker-service.ts

@@ -3,11 +3,11 @@ import {
 	CancellationTokenSource,
 	Disposable,
 	TextDocument,
-	Uri,
 } from 'vscode';
 import { Logger } from './logger';
 import { Configuration } from './configuration';
 import { WorkerPool } from '../phpcs-report/worker-pool';
+import { UriMap } from '../common/uri-map';
 
 /**
  * A base class for all of the updates that interact with PHPCS.
@@ -31,10 +31,7 @@ export abstract class WorkerService implements Disposable {
 	/**
 	 * A map containing all of the cancellation token sources to prevent overlapping execution.
 	 */
-	private readonly cancellationTokenSourceMap: Map<
-		Uri,
-		CancellationTokenSource
-	>;
+	private readonly cancellationTokenSourceMap: UriMap<CancellationTokenSource>;
 
 	/**
 	 * Constructor.
@@ -51,7 +48,7 @@ export abstract class WorkerService implements Disposable {
 		this.logger = logger;
 		this.configuration = configuration;
 		this.workerPool = workerPool;
-		this.cancellationTokenSourceMap = new Map();
+		this.cancellationTokenSourceMap = new UriMap();
 	}
 
 	/**

src/types.ts

@@ -1,4 +1,5 @@
-import { CodeAction as BaseCodeAction, TextDocument, Uri } from 'vscode';
+import { CodeAction as BaseCodeAction, TextDocument } from 'vscode';
+import { UriMap } from './common/uri-map';
 
 /**
  * A custom code action class that adds a Uri for associating it with a document.
@@ -13,4 +14,4 @@ export class CodeAction extends BaseCodeAction {
 /**
  * A collection of CodeAction instances keyed by the Uri.
  */
-export class CodeActionCollection extends Map<Uri, CodeAction[]> {}
+export class CodeActionCollection extends UriMap<CodeAction[]> {}