Merge pull request #72 from SSlinky/dev

Better Syntax Errors

Author: SSlinky

client/src/test/codeActions.test.ts

@@ -0,0 +1,61 @@
+/* --------------------------------------------------------------------------------------------
+ * Copyright (c) Microsoft Corporation. All rights reserved.
+ * Licensed under the MIT License. See License.txt in the project root for license information.
+ * ------------------------------------------------------------------------------------------ */
+
+import * as vscode from 'vscode';
+import * as assert from 'assert';
+import { getDocUri, activate, runOnActivate } from './helper';
+import { toRange } from './util';
+
+suite('Should get code actions', () => {
+	test('actions.module.missingOptionExplicitCodeAction', async () => {
+		const docUri = getDocUri('EmptyModule.bas');
+		const edits = new vscode.WorkspaceEdit();
+		edits.set(docUri, [
+			vscode.TextEdit.insert(new vscode.Position(1, 1), "\nOption Explicit")
+		]);
+		await testCodeActions(docUri, toRange(2, 1, 2, 1), [
+			{
+				title: "Insert Option Explicit",
+				kind: vscode.CodeActionKind.QuickFix,
+				edit: edits
+			}
+		]);
+	});
+});
+
+async function testCodeActions(docUri: vscode.Uri, activationRange: vscode.Range, expectedResults: vscode.CodeAction[]) {
+	await activate(docUri);
+
+	// Use this method first to ensure the extension is activated.
+	const actualResults = await runOnActivate(
+		// Action to run.
+		() => vscode.commands.executeCommand<vscode.CodeAction[]>(
+			'vscode.executeCodeActionProvider',
+			docUri,
+			activationRange
+		),
+		// Test that shows it ran.
+		(result) => result.length > 0
+	);
+
+	assert.equal(actualResults.length, expectedResults.length, `Expected ${expectedResults.length}, got ${actualResults.length}`);
+
+	expectedResults.forEach((expected, i) => {
+		const actual = actualResults[i];
+		assert.equal(actual.title, expected.title, `Title: expected ${expected.title}, got ${actual.title}`);
+		assert.equal(actual.edit.has(docUri), true, "Missing actual edits");
+		assert.equal(expected.edit.has(docUri), true, "Missing expected edits");
+
+		const actEdits = actual.edit.get(docUri);
+		const expEdits = expected.edit.get(docUri);
+
+		assert.equal(actEdits.length, expEdits.length, `Count edits for ${actual.title}`);
+		expEdits.forEach((expEdit, i) => {
+			const actEdit = actEdits[i];
+			assert.equal(actEdit.newText, expEdit.newText, `Edit text: expected ${expEdit.newText}, got ${actEdit.newText}`);
+			assert.deepEqual(actEdit.range, expEdit.range, `Edit range: expected ${JSON.stringify(expEdit.range)}, got ${JSON.stringify(actEdit.range)}`);
+		});
+	});
+}

client/src/test/foldingRanges.test.ts

@@ -27,16 +27,18 @@ async function testFoldingRanges(docUri: vscode.Uri, expectedFoldingRanges: vsco
 	const action = () => vscode.commands.executeCommand<vscode.FoldingRange[]>(
 		'vscode.executeFoldingRangeProvider',
 		docUri
-	)
+	);
 
 	// Use this method first to ensure the extension is activated.
 	const actualFoldingRanges = await runOnActivate(
 		action,
-		(result) => Array.isArray(result) && result.length > 0
+		// Test returns 7 folding ranges when run in normal mode
+		// but six in debug. Appears to be an issue with timing,
+		// probably due to the editor guessing before LSP kicks in.
+		(result) => Array.isArray(result) && result.length === expectedFoldingRanges.length
 	);
 
-	assert.equal(actualFoldingRanges.length ?? 0, expectedFoldingRanges.length, "Count");
-
+	// No need to assert length as this test will throw if it's not the same.
 	expectedFoldingRanges.forEach((expectedFoldingRange, i) => {
 		const actualFoldingRange = actualFoldingRanges[i];
 		assert.deepEqual(actualFoldingRange, expectedFoldingRange, `FoldingRange ${i}`);

package-lock.json

@@ -6,7 +6,7 @@
 	"icon": "images/vba-lsp-icon.png",
 	"author": "SSlinky",
 	"license": "MIT",
-	"version": "1.5.9",
+	"version": "1.5.10",
 	"repository": {
 		"type": "git",
 		"url": "https://github.com/SSlinky/VBA-LanguageServer"

package.json

@@ -0,0 +1,51 @@
+import { CodeAction, Command, Diagnostic } from "vscode-languageserver";
+import { BaseDiagnostic } from "./diagnostics";
+import { Services } from "../injection/services";
+
+/**
+ * This class is not designed to be referenced or instantiated directly.
+ * Please use Services to get its singleton instance.
+ */
+export class CodeActionsRegistry {
+    actionFactory: Map<string | number, (diagnostic: Diagnostic, uri: string) => Command | CodeAction | undefined> = new Map();
+    private logger = Services.logger;
+
+    getDiagnosticAction(diagnostic: Diagnostic, uri: string): Command | CodeAction | undefined {
+        if (diagnostic.code && this.actionFactory.has(diagnostic.code)) {
+            this.logger.debug(`Getting code action for ${diagnostic.code}`, 1);
+            return this.actionFactory.get(diagnostic.code)!(diagnostic, uri);
+        }
+    }
+
+    /**
+     * Allows a diagnostic to lazily register its action factory when it is
+     * instantiated. This means code actions for diagnostics are managed on
+     * the diagnostic itself without any additional wiring up.
+     */
+    registerDiagnosticAction(diagnostic: BaseDiagnostic): void {
+        // A diagnostic must have a code and a factory and must not already be registered.
+        if (diagnostic.code && diagnostic.actionFactory && !this.actionFactory.has(diagnostic.code)) {
+            this.logger.debug(`Registering code action for ${diagnostic.code}`, 1);
+            this.actionFactory.set(diagnostic.code, diagnostic.actionFactory);
+        }
+    }
+}
+
+// Example params that are related to a diagnostic.
+const onCodeActionParams = {
+    "textDocument":{
+        "uri":"file:///c%3A/Repos/vba-LanguageServer/sample/b.bas"
+    },
+    "range":{"start":{"line":4,"character":1},"end":{"line":4,"character":1}},
+    "context":{
+        "diagnostics":[{
+            "range":{"start":{"line":4,"character":1},"end":{"line":4,"character":1}},
+            "message":"Option Explicit is missing from module header.",
+            "data":{"uri":"file:///c%3A/Repos/vba-LanguageServer/sample/b.bas"},
+            "code":"W001",
+            "severity":2
+        }],
+        "only":["quickfix"],
+        "triggerKind":1
+    }
+};

server/src/capabilities/codeActions.ts

@@ -1,5 +1,6 @@
 // Core
-import { CodeDescription, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Position, Range } from 'vscode-languageserver';
+import { CodeAction, CodeActionKind, CodeDescription, Command, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Position, Range } from 'vscode-languageserver';
+import { Services } from '../injection/services';
 
 
 export type DiagnosticConstructor<T extends BaseDiagnostic> =
@@ -9,12 +10,13 @@ export type DiagnosticConstructor<T extends BaseDiagnostic> =
 export abstract class BaseDiagnostic implements Diagnostic {
 	range: Range;
 	message: string;
-	severity?: DiagnosticSeverity | undefined;
-	code?: string | number | undefined;
-	codeDescription?: CodeDescription | undefined;
-	source?: string | undefined;
-	tags?: DiagnosticTag[] | undefined;
-	relatedInformation?: DiagnosticRelatedInformation[] | undefined;
+	severity?: DiagnosticSeverity;
+	code?: string | number;
+	actionFactory?: (diagnostic: Diagnostic, uri: string) => CodeAction | Command;
+	codeDescription?: CodeDescription;
+	source?: string;
+	tags?: DiagnosticTag[];
+	relatedInformation?: DiagnosticRelatedInformation[];
 	data?: unknown;
 
 	constructor(range: Range)
@@ -151,6 +153,21 @@ export class MissingOptionExplicitDiagnostic extends BaseDiagnostic {
 	severity = DiagnosticSeverity.Warning;
 	constructor(range: Range) {
 		super(range);
+
+		// Set up the properties that will enable the action.
+		this.code = 'W001';
+		this.actionFactory = (diagnostic: Diagnostic, uri: string) =>
+			CodeAction.create(
+				"Insert Option Explicit",
+				{ changes: { [uri]: [{
+					range: diagnostic.range,
+					newText: "\nOption Explicit"
+				}]}},
+				CodeActionKind.QuickFix
+			);
+
+		// Register the action factory to enable onActionRequest.
+		Services.codeActionsRegistry.registerDiagnosticAction(this);
 	}
 }
 

server/src/capabilities/diagnostics.ts

@@ -3,12 +3,14 @@ import { Logger, IWorkspace, ILanguageServer } from './interface';
 import { LspLogger } from '../utils/logger';
 import { _Connection, createConnection, ProposedFeatures } from 'vscode-languageserver/node';
 import { ScopeItemCapability } from '../capabilities/capabilities';
+import { CodeActionsRegistry } from '../capabilities/codeActions';
 
 
 export class Services {
 	static registerServices(): void {
 		container.registerSingleton("ILogger", LspLogger);
 		container.registerInstance("_Connection", createConnection(ProposedFeatures.all));
+		container.registerSingleton("CodeActions", CodeActionsRegistry);
 	}
 
 	static registerProjectScope(scope: ScopeItemCapability): void {
@@ -35,6 +37,10 @@ export class Services {
 		return container.resolve("ILanguageServer");
 	}
 
+	static get codeActionsRegistry(): CodeActionsRegistry {
+		return container.resolve("CodeActions");
+	}
+
 	static get projectScope(): ScopeItemCapability {
 		return container.resolve("ProjectScope");
 	}

server/src/injection/services.ts

@@ -219,7 +219,7 @@ export abstract class BaseProjectDocument {
 	 * Auto registers the element based on capabilities.
 	 * @returns This for chaining.
 	 */
-	registerElement<T extends ParserRuleContext>(element: BaseSyntaxElement<T>) {
+	registerElement(element: BaseSyntaxElement) {
 		if (element.diagnosticCapability) this.registerDiagnosticElement(element as HasDiagnosticCapability);
 		if (element.foldingRangeCapability) this.registerFoldableElement(element as HasFoldingRangeCapability);
 		if (element.semanticTokenCapability) this.registerSemanticToken(element as HasSemanticTokenCapability);
@@ -235,12 +235,12 @@ export abstract class BaseProjectDocument {
 		return this;
 	}
 
-	registerDiagnosticElement(element: HasDiagnosticCapability) {
+	private registerDiagnosticElement(element: HasDiagnosticCapability) {
 		this.hasDiagnosticElements.push(element);
 		return this;
 	}
 
-	registerFoldableElement = (element: HasFoldingRangeCapability) => {
+	private registerFoldableElement = (element: HasFoldingRangeCapability) => {
 		this.foldableElements.push(element.foldingRangeCapability.foldingRange);
 		return this;
 	};
@@ -250,7 +250,7 @@ export abstract class BaseProjectDocument {
 	 * @param element element The element that has a semantic token.
 	 * @returns this for chaining.
 	 */
-	registerSemanticToken = (element: HasSemanticTokenCapability) => {
+	private registerSemanticToken = (element: HasSemanticTokenCapability) => {
 		this.semanticTokens.add(element);
 		return this;
 	};
@@ -265,12 +265,12 @@ export abstract class BaseProjectDocument {
 	 * @param element The element that has symbol information.
 	 * @returns this for chaining.
 	 */
-	registerSymbolInformation = (element: HasSymbolInformationCapability) => {
+	private registerSymbolInformation = (element: HasSymbolInformationCapability) => {
 		this.symbolInformations.push(element.symbolInformationCapability.SymbolInformation);
 		return this;
 	};
 
-	registerScopeItem = (element: HasScopeItemCapability) =>
+	private registerScopeItem = (element: HasScopeItemCapability) =>
 		this.currentScope = this.currentScope.registerScopeItem(element.scopeItemCapability);
 
 	exitContext = (ctx: ParserRuleContext) => {

server/src/project/document.ts

@@ -17,7 +17,7 @@ import {
 
 // Project
 import { BaseRuleSyntaxElement, BaseIdentifyableSyntaxElement, HasDiagnosticCapability } from './base';
-import { DiagnosticCapability, FoldingRangeCapability, IdentifierCapability, ItemType, ScopeItemCapability, SymbolInformationCapability } from '../../capabilities/capabilities';
+import { DiagnosticCapability, IdentifierCapability, ItemType, ScopeItemCapability, SymbolInformationCapability } from '../../capabilities/capabilities';
 import { DuplicateAttributeDiagnostic, IgnoredAttributeDiagnostic, MissingAttributeDiagnostic, MissingOptionExplicitDiagnostic } from '../../capabilities/diagnostics';
 
 
@@ -32,13 +32,11 @@ abstract class BaseModuleElement<T extends ParserRuleContext> extends BaseIdenti
 	abstract hasOptionExplicit: boolean;
 
 	settings: DocumentSettings;
-	// foldingRangeCapability: FoldingRangeCapability;
 	symbolInformationCapability: SymbolInformationCapability;
 
 	constructor(ctx: T, doc: TextDocument, documentSettings: DocumentSettings, symbolKind: SymbolKind) {
 		super(ctx, doc);
 		this.settings = documentSettings;
-		// this.foldingRangeCapability = new FoldingRangeCapability(this);
 		this.symbolInformationCapability = new SymbolInformationCapability(this, symbolKind);
 		this.scopeItemCapability = new ScopeItemCapability(this, ItemType.MODULE);
 	}

server/src/project/elements/module.ts

@@ -3,6 +3,9 @@ import { TextDocument } from 'vscode-languageserver-textdocument';
 import {
 	CancellationToken,
 	CancellationTokenSource,
+	CodeAction,
+	CodeActionParams,
+	Command,
 	CompletionItem,
 	CompletionParams,
 	DidChangeConfigurationNotification,
@@ -277,6 +280,7 @@ class WorkspaceEvents {
 		connection.onHover(params => this.onHover(params));
 		connection.onDocumentFormatting(async (params, token) => await this.onDocumentFormatting(params, token));
 		connection.onDidCloseTextDocument(params => { Services.logger.debug('[event] onDidCloseTextDocument'); Services.logger.debug(JSON.stringify(params), 1); });
+		connection.onCodeAction((params, token) => this.onCodeActionRequest(params, token));
 
 		if (hasWorkspaceConfigurationCapability(Services.server)) {
 			connection.onFoldingRanges(async (params, token) => await cancellableOnFoldingRanges(params, token));
@@ -393,6 +397,42 @@ class WorkspaceEvents {
 
 	}
 
+	private async onCodeActionRequest(params: CodeActionParams, token: CancellationToken): Promise<(Command | CodeAction)[] | null | undefined> {
+		const logger = Services.logger;
+		logger.debug(`[event] onCodeAction: ${JSON.stringify(params)}`);
+
+		// For now, if we have no diagnostics then don't return any actions.
+		if (params.context.diagnostics.length === 0) {
+			return [];
+		}
+
+		if (token.isCancellationRequested) {
+			logger.debug(`[cbs] onCodeAction`);
+			return;
+		}
+
+		try {
+			// We don't actually need the document but await to ensure it's parsed.
+			await this.getParsedProjectDocument(params.textDocument.uri, 0, token);
+			const uri = params.textDocument.uri;
+			const result: (Command | CodeAction)[] = [];
+			const codeActionRegistry = Services.codeActionsRegistry;
+			params.context.diagnostics.forEach(d => {
+				const action = codeActionRegistry.getDiagnosticAction(d, uri);
+				if (action) {
+					result.push(action);
+				}
+			});
+			return result;
+		} catch (e) {
+			// If cancelled or something went wrong, just return.
+			if (e instanceof Error) {
+				logger.stack(e);
+			}
+			return;
+		}
+	}
+
 	/** Documents event handlers */
 
 	/**