From b5fe16df9bab55625566b9dbcd12e6cbe6edb2c4 Mon Sep 17 00:00:00 2001 From: sslinky <39886505+SSlinky@users.noreply.github.com> Date: Wed, 16 Apr 2025 17:13:55 +0800 Subject: [PATCH 1/5] Removed folding from module --- server/src/project/elements/module.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/server/src/project/elements/module.ts b/server/src/project/elements/module.ts index 01b0c7d..c8bdc2f 100644 --- a/server/src/project/elements/module.ts +++ b/server/src/project/elements/module.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 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); } From 4dc2d1773b3cca173f7207c6c885387f3dc16cef Mon Sep 17 00:00:00 2001 From: sslinky <39886505+SSlinky@users.noreply.github.com> Date: Wed, 16 Apr 2025 23:41:01 +0800 Subject: [PATCH 2/5] Fixed test failing when run normally (not debug). --- client/src/test/foldingRanges.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/client/src/test/foldingRanges.test.ts b/client/src/test/foldingRanges.test.ts index 114977f..6b80841 100644 --- a/client/src/test/foldingRanges.test.ts +++ b/client/src/test/foldingRanges.test.ts @@ -27,16 +27,18 @@ async function testFoldingRanges(docUri: vscode.Uri, expectedFoldingRanges: vsco const action = () => vscode.commands.executeCommand( '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}`); From 26fa882afd719d4421df27a720c558a37ef440f0 Mon Sep 17 00:00:00 2001 From: sslinky <39886505+SSlinky@users.noreply.github.com> Date: Wed, 16 Apr 2025 23:43:44 +0800 Subject: [PATCH 3/5] New functionality to register an action to a diagnostic --- client/src/test/codeActions.test.ts | 61 ++++++++++++++++++++++++++ server/src/capabilities/codeActions.ts | 51 +++++++++++++++++++++ server/src/capabilities/diagnostics.ts | 31 ++++++++++--- server/src/injection/services.ts | 6 +++ server/src/project/workspace.ts | 40 +++++++++++++++++ server/src/server.ts | 2 +- 6 files changed, 183 insertions(+), 8 deletions(-) create mode 100644 client/src/test/codeActions.test.ts create mode 100644 server/src/capabilities/codeActions.ts diff --git a/client/src/test/codeActions.test.ts b/client/src/test/codeActions.test.ts new file mode 100644 index 0000000..07c78b5 --- /dev/null +++ b/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.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)}`); + }); + }); +} \ No newline at end of file diff --git a/server/src/capabilities/codeActions.ts b/server/src/capabilities/codeActions.ts new file mode 100644 index 0000000..3c3dde3 --- /dev/null +++ b/server/src/capabilities/codeActions.ts @@ -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 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 + } +}; diff --git a/server/src/capabilities/diagnostics.ts b/server/src/capabilities/diagnostics.ts index d63be3d..4dd28d8 100644 --- a/server/src/capabilities/diagnostics.ts +++ b/server/src/capabilities/diagnostics.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 = @@ -9,12 +10,13 @@ export type DiagnosticConstructor = 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); } } diff --git a/server/src/injection/services.ts b/server/src/injection/services.ts index 7397670..8cf22a0 100644 --- a/server/src/injection/services.ts +++ b/server/src/injection/services.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"); } diff --git a/server/src/project/workspace.ts b/server/src/project/workspace.ts index 3877655..14ab298 100644 --- a/server/src/project/workspace.ts +++ b/server/src/project/workspace.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 */ /** diff --git a/server/src/server.ts b/server/src/server.ts index d666330..f42fff0 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -82,7 +82,7 @@ export class LanguageServerConfiguration { // }, // Implement soon. - codeActionProvider: false, + codeActionProvider: true, completionProvider: undefined, hoverProvider: false, From a78145b39d46b9ea279c924e3d34cda0a1384753 Mon Sep 17 00:00:00 2001 From: sslinky <39886505+SSlinky@users.noreply.github.com> Date: Wed, 16 Apr 2025 23:44:19 +0800 Subject: [PATCH 4/5] Made individual registration methods private in favour of generic --- server/src/project/document.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/src/project/document.ts b/server/src/project/document.ts index 5955be4..31d3458 100644 --- a/server/src/project/document.ts +++ b/server/src/project/document.ts @@ -219,7 +219,7 @@ export abstract class BaseProjectDocument { * Auto registers the element based on capabilities. * @returns This for chaining. */ - registerElement(element: BaseSyntaxElement) { + 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) => { From 8aecc73306a9c9f917b6ec6aaf642d77c0d17bb5 Mon Sep 17 00:00:00 2001 From: sslinky <39886505+SSlinky@users.noreply.github.com> Date: Thu, 17 Apr 2025 02:21:16 +0800 Subject: [PATCH 5/5] 1.5.10 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4b4bcce..09bb129 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "vba-lsp", - "version": "1.5.9", + "version": "1.5.10", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "vba-lsp", - "version": "1.5.9", + "version": "1.5.10", "hasInstallScript": true, "license": "MIT", "dependencies": { diff --git a/package.json b/package.json index 0cca9b2..a3acd42 100644 --- a/package.json +++ b/package.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"