Add option to configure when diagnostics are updated (#34)

This new configuration option allows users to decide when the diagnostic updates should occur. When set to "Save", it will only update diagnostics when the document is saved.

CHANGELOG.md

@@ -6,6 +6,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 ### Added
+- Linter's execution action can be set by the `phpCodeSniffer.lintAction` option.
 - Command `phpCodeSniffer.cancelProcessing` to cancel all active processing.
 
 ## [1.3.0] - 2021-05-24
@@ -24,7 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [1.1.0] - 2021-03-04
 ### Added
-- Ignore diagnostics for files using `phpCodeSniffer.ignorePatterns` option.
+- Ignore diagnostics for files using the `phpCodeSniffer.ignorePatterns` option.
 
 ### Fixed
 - "Ignore * for this line" action should be present for all diagnostics.

package.json

@@ -56,6 +56,20 @@
           },
           "scope": "window"
         },
+        "phpCodeSniffer.lintAction": {
+          "type": "string",
+          "description": "The editor action that will cause the linter to run.",
+          "default": "Change",
+          "enum": [
+            "Change",
+            "Save"
+          ],
+          "enumDescriptions": [
+            "Lints whenever a document is changed.",
+            "Lints when a document is saved."
+          ],
+          "scope": "resource"
+        },
         "phpCodeSniffer.standard": {
           "type": "string",
           "description": "The coding standard that should be used by PHPCS.",

src/listeners/workspace-listener.ts

@@ -5,7 +5,7 @@ import {
 	window as vsCodeWindow,
 	workspace as vsCodeWorkspace,
 } from 'vscode';
-import { Configuration } from '../services/configuration';
+import { Configuration, LintAction } from '../services/configuration';
 import { CodeActionEditResolver } from '../services/code-action-edit-resolver';
 import { DiagnosticUpdater } from '../services/diagnostic-updater';
 import { DocumentFormatter } from '../services/document-formatter';
@@ -114,7 +114,14 @@ export class WorkspaceListener implements Disposable {
 					return;
 				}
 
-				this.onUpdate(e.document);
+				this.onUpdate(e.document, LintAction.Change);
+			})
+		);
+
+		// When the text document is saved we should update the diagnostics.
+		this.subscriptions.push(
+			workspace.onDidSaveTextDocument((e) => {
+				this.onUpdate(e, LintAction.Save);
 			})
 		);
 
@@ -174,7 +181,7 @@ export class WorkspaceListener implements Disposable {
 		this.trackedDocuments.add(document.uri);
 
 		// Trigger an update so that the document will gather diagnostics.
-		this.onUpdate(document);
+		this.onUpdate(document, LintAction.Force);
 	}
 
 	/**
@@ -201,8 +208,9 @@ export class WorkspaceListener implements Disposable {
 	 * A callback for documents being changed.
 	 *
 	 * @param {TextDocument} document The affected document.
+	 * @param {LintAction} lintAction The editor action triggering the linting.
 	 */
-	private onUpdate(document: TextDocument): void {
+	private onUpdate(document: TextDocument, lintAction: LintAction): void {
 		// Don't update documents that we aren't tracking.
 		if (!this.trackedDocuments.has(document.uri)) {
 			return;
@@ -220,8 +228,8 @@ export class WorkspaceListener implements Disposable {
 			this.diagnosticUpdater.cancel(document);
 
 			// Update the diagnostics for the document.
-			this.diagnosticUpdater.update(document);
-		}, 100);
+			this.diagnosticUpdater.update(document, lintAction);
+		}, 200);
 		this.updateDebounceMap.set(document.uri, debounce);
 	}
 
@@ -263,7 +271,7 @@ export class WorkspaceListener implements Disposable {
 			this.diagnosticUpdater.cancel(document);
 
 			// Update the diagnostics for the document.
-			this.diagnosticUpdater.update(document);
+			this.diagnosticUpdater.update(document, LintAction.Force);
 		}
 	}
 
@@ -286,7 +294,7 @@ export class WorkspaceListener implements Disposable {
 			this.diagnosticUpdater.cancel(document);
 
 			// Update the diagnostics for the document.
-			this.diagnosticUpdater.update(document);
+			this.diagnosticUpdater.update(document, LintAction.Force);
 		}
 	}
 }

src/services/__tests__/configuration.spec.ts

@@ -3,11 +3,16 @@ import {
 	workspace,
 	Uri as vsCodeUri,
 	FileSystemError,
+	CancellationError,
 } from 'vscode';
-import { MockTextDocument, Uri } from '../../__mocks__/vscode';
+import {
+	MockCancellationToken,
+	MockTextDocument,
+	Uri,
+} from '../../__mocks__/vscode';
 import { TextEncoder } from 'util';
 import { mocked } from 'ts-jest/utils';
-import { Configuration, StandardType } from '../configuration';
+import { Configuration, LintAction, StandardType } from '../configuration';
 
 describe('Configuration', () => {
 	let mockDocument: TextDocument;
@@ -66,6 +71,8 @@ describe('Configuration', () => {
 					return 'test.exec';
 				case 'ignorePatterns':
 					return ['test'];
+				case 'lintAction':
+					return LintAction.Change;
 				case 'standard':
 					return StandardType.Disabled;
 			}
@@ -148,6 +155,8 @@ describe('Configuration', () => {
 					return 'test.exec';
 				case 'ignorePatterns':
 					return ['test'];
+				case 'lintAction':
+					return LintAction.Change;
 				case 'standard':
 					return StandardType.Disabled;
 			}
@@ -176,4 +185,78 @@ describe('Configuration', () => {
 		expect(workspace.getConfiguration).toHaveBeenCalledTimes(1);
 		expect(cached).toMatchObject(result);
 	});
+
+	it('should support cancellation', () => {
+		const cancellationToken = new MockCancellationToken();
+
+		const mockConfiguration = { get: jest.fn() };
+		mocked(workspace).getConfiguration.mockReturnValue(
+			mockConfiguration as never
+		);
+
+		const workspaceUri = new Uri();
+		workspaceUri.path = 'test';
+		workspaceUri.fsPath = 'test';
+		mocked(workspace).getWorkspaceFolder.mockReturnValue({
+			uri: workspaceUri,
+		} as never);
+
+		// We will traverse from the file directory up.
+		mocked(workspace.fs.readFile).mockImplementation((uri) => {
+			switch (uri.path) {
+				case 'test/file/composer.json':
+					return Promise.reject(new FileSystemError(uri));
+				case 'test/composer.json': {
+					const json = JSON.stringify({
+						config: {
+							'vendor-dir': 'newvendor',
+						},
+					});
+					const encoder = new TextEncoder();
+					return Promise.resolve(encoder.encode(json));
+				}
+			}
+
+			throw new Error('Invalid path: ' + uri.path);
+		});
+
+		mocked(workspace.fs.stat).mockImplementation((uri) => {
+			switch (uri.path) {
+				case 'test/newvendor/bin/phpcs': {
+					const ret = new Uri();
+					ret.path = 'test';
+					ret.fsPath = 'test';
+					return Promise.resolve(ret);
+				}
+			}
+
+			throw new Error('Invalid path: ' + uri.path);
+		});
+
+		mockConfiguration.get.mockImplementation((key) => {
+			switch (key) {
+				case 'autoExecutable':
+					return true;
+				case 'executable':
+					return 'test.exec';
+				case 'ignorePatterns':
+					return ['test'];
+				case 'lintAction':
+					return LintAction.Change;
+				case 'standard':
+					return StandardType.Disabled;
+			}
+
+			fail(
+				'An unexpected configuration key of ' + key + ' was received.'
+			);
+		});
+
+		const promise = configuration.get(mockDocument, cancellationToken);
+
+		// Mock a cancellation so that resolution will reject.
+		cancellationToken.mockCancel();
+
+		return expect(promise).rejects.toMatchObject(new CancellationError());
+	});
 });

src/services/__tests__/diagnostic-updater.spec.ts

@@ -8,15 +8,19 @@ import {
 	CodeActionKind,
 } from 'vscode';
 import { CodeAction, CodeActionCollection } from '../../types';
-import { Configuration, StandardType } from '../../services/configuration';
+import {
+	Configuration,
+	LintAction,
+	StandardType,
+} from '../../services/configuration';
 import { WorkerPool } from '../../phpcs-report/worker-pool';
 import { DiagnosticUpdater } from '../diagnostic-updater';
 import {
 	MockDiagnosticCollection,
 	MockTextDocument,
 } from '../../__mocks__/vscode';
 import { mocked } from 'ts-jest/utils';
-import { Worker } from '../../phpcs-report/worker';
+import { PHPCSError, Worker } from '../../phpcs-report/worker';
 import { ReportType, Response } from '../../phpcs-report/response';
 import { Logger } from '../../services/logger';
 import { LinterStatus } from '../linter-status';
@@ -96,6 +100,7 @@ describe('DiagnosticUpdater', () => {
 			workingDirectory: 'test-dir',
 			executable: 'phpcs-test',
 			ignorePatterns: [],
+			lintAction: LintAction.Change,
 			standard: StandardType.PSR12,
 		});
 		mocked(mockWorker).execute.mockImplementation((request) => {
@@ -124,49 +129,86 @@ describe('DiagnosticUpdater', () => {
 				},
 			};
 
-			// Wait for the updater to process the result before completing the test.
-			setTimeout(() => {
-				expect(mockDiagnosticCollection.set).toHaveBeenCalled();
-				expect(mockCodeActionCollection.set).toHaveBeenCalled();
-				done();
-			}, 5);
-
 			return Promise.resolve(response);
 		});
 
-		diagnosticUpdater.update(document);
+		// Wait for the updater to process the result before completing the test.
+		setTimeout(() => {
+			expect(mockDiagnosticCollection.set).toHaveBeenCalled();
+			expect(mockCodeActionCollection.set).toHaveBeenCalled();
+			done();
+		}, 5);
+
+		diagnosticUpdater.update(document, LintAction.Force);
 	});
 
-	it('should respect ignore patterns', async (done) => {
+	it('should log PHPCS errors', (done) => {
 		const document = new MockTextDocument();
 		document.fileName = 'test-document';
 
 		const mockWorker = new Worker();
 		mocked(mockWorkerPool).waitForAvailable.mockImplementation(
 			(workerKey) => {
 				expect(workerKey).toBe('diagnostic:test-document');
-
-				// Wait for the updater to process the result before completing the test.
-				setTimeout(() => {
-					expect(
-						mockDiagnosticCollection.delete
-					).toHaveBeenCalledWith(document.uri);
-					expect(
-						mockCodeActionCollection.delete
-					).toHaveBeenCalledWith(document.uri);
-					done();
-				}, 5);
-
 				return Promise.resolve(mockWorker);
 			}
 		);
+		mocked(mockConfiguration).get.mockResolvedValue({
+			workingDirectory: 'test-dir',
+			executable: 'phpcs-test',
+			ignorePatterns: [],
+			lintAction: LintAction.Change,
+			standard: StandardType.PSR12,
+		});
+		mocked(mockWorker).execute.mockImplementation((request) => {
+			expect(request).toMatchObject({
+				type: ReportType.Diagnostic,
+				options: {
+					workingDirectory: 'test-dir',
+					executable: 'phpcs-test',
+					standard: StandardType.PSR12,
+				},
+			});
+
+			throw new PHPCSError('Test Failure', 'Test Failure');
+		});
+
+		// Wait for the updater to process the result before completing the test.
+		setTimeout(() => {
+			expect(mockLogger.error).toHaveBeenCalled();
+			done();
+		}, 5);
+
+		diagnosticUpdater.update(document, LintAction.Force);
+	});
+
+	it('should respect ignore patterns', () => {
+		const document = new MockTextDocument();
+		document.fileName = 'test-document';
+
 		mocked(mockConfiguration).get.mockResolvedValue({
 			workingDirectory: 'test-dir',
 			executable: 'phpcs-test',
 			ignorePatterns: [new RegExp('.*/file/.*')],
+			lintAction: LintAction.Change,
+			standard: StandardType.PSR12,
+		});
+
+		return diagnosticUpdater.update(document, LintAction.Force);
+	});
+
+	it('should not update on change when configured to save', () => {
+		const document = new MockTextDocument();
+		document.fileName = 'test-document';
+
+		mocked(mockConfiguration).get.mockResolvedValue({
+			workingDirectory: 'test-dir',
+			executable: 'phpcs-test',
+			ignorePatterns: [],
+			lintAction: LintAction.Save,
 			standard: StandardType.PSR12,
 		});
 
-		diagnosticUpdater.update(document);
+		return diagnosticUpdater.update(document, LintAction.Change);
 	});
 });