Skip to content

Commit 4e1e02c

Browse files
SSlinkySSlinky
authored
Merge pull request #72 from SSlinky/dev
Better Syntax Errors
2 parents f6fa0be + 8aecc73 commit 4e1e02c

File tree

11 files changed

+199
-24
lines changed

11 files changed

+199
-24
lines changed

‎client/src/test/codeActions.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/* --------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
* ------------------------------------------------------------------------------------------ */
5+
6+
import * as vscode from 'vscode';
7+
import * as assert from 'assert';
8+
import { getDocUri, activate, runOnActivate } from './helper';
9+
import { toRange } from './util';
10+
11+
suite('Should get code actions', () => {
12+
test('actions.module.missingOptionExplicitCodeAction', async () => {
13+
const docUri = getDocUri('EmptyModule.bas');
14+
const edits = new vscode.WorkspaceEdit();
15+
edits.set(docUri, [
16+
vscode.TextEdit.insert(new vscode.Position(1, 1), "\nOption Explicit")
17+
]);
18+
await testCodeActions(docUri, toRange(2, 1, 2, 1), [
19+
{
20+
title: "Insert Option Explicit",
21+
kind: vscode.CodeActionKind.QuickFix,
22+
edit: edits
23+
}
24+
]);
25+
});
26+
});
27+
28+
async function testCodeActions(docUri: vscode.Uri, activationRange: vscode.Range, expectedResults: vscode.CodeAction[]) {
29+
await activate(docUri);
30+
31+
// Use this method first to ensure the extension is activated.
32+
const actualResults = await runOnActivate(
33+
// Action to run.
34+
() => vscode.commands.executeCommand<vscode.CodeAction[]>(
35+
'vscode.executeCodeActionProvider',
36+
docUri,
37+
activationRange
38+
),
39+
// Test that shows it ran.
40+
(result) => result.length > 0
41+
);
42+
43+
assert.equal(actualResults.length, expectedResults.length, `Expected ${expectedResults.length}, got ${actualResults.length}`);
44+
45+
expectedResults.forEach((expected, i) => {
46+
const actual = actualResults[i];
47+
assert.equal(actual.title, expected.title, `Title: expected ${expected.title}, got ${actual.title}`);
48+
assert.equal(actual.edit.has(docUri), true, "Missing actual edits");
49+
assert.equal(expected.edit.has(docUri), true, "Missing expected edits");
50+
51+
const actEdits = actual.edit.get(docUri);
52+
const expEdits = expected.edit.get(docUri);
53+
54+
assert.equal(actEdits.length, expEdits.length, `Count edits for ${actual.title}`);
55+
expEdits.forEach((expEdit, i) => {
56+
const actEdit = actEdits[i];
57+
assert.equal(actEdit.newText, expEdit.newText, `Edit text: expected ${expEdit.newText}, got ${actEdit.newText}`);
58+
assert.deepEqual(actEdit.range, expEdit.range, `Edit range: expected ${JSON.stringify(expEdit.range)}, got ${JSON.stringify(actEdit.range)}`);
59+
});
60+
});
61+
}

‎client/src/test/foldingRanges.test.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,18 @@ async function testFoldingRanges(docUri: vscode.Uri, expectedFoldingRanges: vsco
2727
const action = () => vscode.commands.executeCommand<vscode.FoldingRange[]>(
2828
'vscode.executeFoldingRangeProvider',
2929
docUri
30-
)
30+
);
3131

3232
// Use this method first to ensure the extension is activated.
3333
const actualFoldingRanges = await runOnActivate(
3434
action,
35-
(result) => Array.isArray(result) && result.length > 0
35+
// Test returns 7 folding ranges when run in normal mode
36+
// but six in debug. Appears to be an issue with timing,
37+
// probably due to the editor guessing before LSP kicks in.
38+
(result) => Array.isArray(result) && result.length === expectedFoldingRanges.length
3639
);
3740

38-
assert.equal(actualFoldingRanges.length ?? 0, expectedFoldingRanges.length, "Count");
39-
41+
// No need to assert length as this test will throw if it's not the same.
4042
expectedFoldingRanges.forEach((expectedFoldingRange, i) => {
4143
const actualFoldingRange = actualFoldingRanges[i];
4244
assert.deepEqual(actualFoldingRange, expectedFoldingRange, `FoldingRange ${i}`);

‎package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"icon": "images/vba-lsp-icon.png",
77
"author": "SSlinky",
88
"license": "MIT",
9-
"version": "1.5.9",
9+
"version": "1.5.10",
1010
"repository": {
1111
"type": "git",
1212
"url": "https://github.com/SSlinky/VBA-LanguageServer"

‎server/src/capabilities/codeActions.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import { CodeAction, Command, Diagnostic } from "vscode-languageserver";
2+
import { BaseDiagnostic } from "./diagnostics";
3+
import { Services } from "../injection/services";
4+
5+
/**
6+
* This class is not designed to be referenced or instantiated directly.
7+
* Please use Services to get its singleton instance.
8+
*/
9+
export class CodeActionsRegistry {
10+
actionFactory: Map<string | number, (diagnostic: Diagnostic, uri: string) => Command | CodeAction | undefined> = new Map();
11+
private logger = Services.logger;
12+
13+
getDiagnosticAction(diagnostic: Diagnostic, uri: string): Command | CodeAction | undefined {
14+
if (diagnostic.code && this.actionFactory.has(diagnostic.code)) {
15+
this.logger.debug(`Getting code action for ${diagnostic.code}`, 1);
16+
return this.actionFactory.get(diagnostic.code)!(diagnostic, uri);
17+
}
18+
}
19+
20+
/**
21+
* Allows a diagnostic to lazily register its action factory when it is
22+
* instantiated. This means code actions for diagnostics are managed on
23+
* the diagnostic itself without any additional wiring up.
24+
*/
25+
registerDiagnosticAction(diagnostic: BaseDiagnostic): void {
26+
// A diagnostic must have a code and a factory and must not already be registered.
27+
if (diagnostic.code && diagnostic.actionFactory && !this.actionFactory.has(diagnostic.code)) {
28+
this.logger.debug(`Registering code action for ${diagnostic.code}`, 1);
29+
this.actionFactory.set(diagnostic.code, diagnostic.actionFactory);
30+
}
31+
}
32+
}
33+
34+
// Example params that are related to a diagnostic.
35+
const onCodeActionParams = {
36+
"textDocument":{
37+
"uri":"file:///c%3A/Repos/vba-LanguageServer/sample/b.bas"
38+
},
39+
"range":{"start":{"line":4,"character":1},"end":{"line":4,"character":1}},
40+
"context":{
41+
"diagnostics":[{
42+
"range":{"start":{"line":4,"character":1},"end":{"line":4,"character":1}},
43+
"message":"Option Explicit is missing from module header.",
44+
"data":{"uri":"file:///c%3A/Repos/vba-LanguageServer/sample/b.bas"},
45+
"code":"W001",
46+
"severity":2
47+
}],
48+
"only":["quickfix"],
49+
"triggerKind":1
50+
}
51+
};

‎server/src/capabilities/diagnostics.ts

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// Core
2-
import { CodeDescription, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Position, Range } from 'vscode-languageserver';
2+
import { CodeAction, CodeActionKind, CodeDescription, Command, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Position, Range } from 'vscode-languageserver';
3+
import { Services } from '../injection/services';
34

45

56
export type DiagnosticConstructor<T extends BaseDiagnostic> =
@@ -9,12 +10,13 @@ export type DiagnosticConstructor<T extends BaseDiagnostic> =
910
export abstract class BaseDiagnostic implements Diagnostic {
1011
range: Range;
1112
message: string;
12-
severity?: DiagnosticSeverity | undefined;
13-
code?: string | number | undefined;
14-
codeDescription?: CodeDescription | undefined;
15-
source?: string | undefined;
16-
tags?: DiagnosticTag[] | undefined;
17-
relatedInformation?: DiagnosticRelatedInformation[] | undefined;
13+
severity?: DiagnosticSeverity;
14+
code?: string | number;
15+
actionFactory?: (diagnostic: Diagnostic, uri: string) => CodeAction | Command;
16+
codeDescription?: CodeDescription;
17+
source?: string;
18+
tags?: DiagnosticTag[];
19+
relatedInformation?: DiagnosticRelatedInformation[];
1820
data?: unknown;
1921

2022
constructor(range: Range)
@@ -151,6 +153,21 @@ export class MissingOptionExplicitDiagnostic extends BaseDiagnostic {
151153
severity = DiagnosticSeverity.Warning;
152154
constructor(range: Range) {
153155
super(range);
156+
157+
// Set up the properties that will enable the action.
158+
this.code = 'W001';
159+
this.actionFactory = (diagnostic: Diagnostic, uri: string) =>
160+
CodeAction.create(
161+
"Insert Option Explicit",
162+
{ changes: { [uri]: [{
163+
range: diagnostic.range,
164+
newText: "\nOption Explicit"
165+
}]}},
166+
CodeActionKind.QuickFix
167+
);
168+
169+
// Register the action factory to enable onActionRequest.
170+
Services.codeActionsRegistry.registerDiagnosticAction(this);
154171
}
155172
}
156173

‎server/src/injection/services.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@ import { Logger, IWorkspace, ILanguageServer } from './interface';
33
import { LspLogger } from '../utils/logger';
44
import { _Connection, createConnection, ProposedFeatures } from 'vscode-languageserver/node';
55
import { ScopeItemCapability } from '../capabilities/capabilities';
6+
import { CodeActionsRegistry } from '../capabilities/codeActions';
67

78

89
export class Services {
910
static registerServices(): void {
1011
container.registerSingleton("ILogger", LspLogger);
1112
container.registerInstance("_Connection", createConnection(ProposedFeatures.all));
13+
container.registerSingleton("CodeActions", CodeActionsRegistry);
1214
}
1315

1416
static registerProjectScope(scope: ScopeItemCapability): void {
@@ -35,6 +37,10 @@ export class Services {
3537
return container.resolve("ILanguageServer");
3638
}
3739

40+
static get codeActionsRegistry(): CodeActionsRegistry {
41+
return container.resolve("CodeActions");
42+
}
43+
3844
static get projectScope(): ScopeItemCapability {
3945
return container.resolve("ProjectScope");
4046
}

‎server/src/project/document.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ export abstract class BaseProjectDocument {
219219
* Auto registers the element based on capabilities.
220220
* @returns This for chaining.
221221
*/
222-
registerElement<T extends ParserRuleContext>(element: BaseSyntaxElement<T>) {
222+
registerElement(element: BaseSyntaxElement) {
223223
if (element.diagnosticCapability) this.registerDiagnosticElement(element as HasDiagnosticCapability);
224224
if (element.foldingRangeCapability) this.registerFoldableElement(element as HasFoldingRangeCapability);
225225
if (element.semanticTokenCapability) this.registerSemanticToken(element as HasSemanticTokenCapability);
@@ -235,12 +235,12 @@ export abstract class BaseProjectDocument {
235235
return this;
236236
}
237237

238-
registerDiagnosticElement(element: HasDiagnosticCapability) {
238+
private registerDiagnosticElement(element: HasDiagnosticCapability) {
239239
this.hasDiagnosticElements.push(element);
240240
return this;
241241
}
242242

243-
registerFoldableElement = (element: HasFoldingRangeCapability) => {
243+
private registerFoldableElement = (element: HasFoldingRangeCapability) => {
244244
this.foldableElements.push(element.foldingRangeCapability.foldingRange);
245245
return this;
246246
};
@@ -250,7 +250,7 @@ export abstract class BaseProjectDocument {
250250
* @param element element The element that has a semantic token.
251251
* @returns this for chaining.
252252
*/
253-
registerSemanticToken = (element: HasSemanticTokenCapability) => {
253+
private registerSemanticToken = (element: HasSemanticTokenCapability) => {
254254
this.semanticTokens.add(element);
255255
return this;
256256
};
@@ -265,12 +265,12 @@ export abstract class BaseProjectDocument {
265265
* @param element The element that has symbol information.
266266
* @returns this for chaining.
267267
*/
268-
registerSymbolInformation = (element: HasSymbolInformationCapability) => {
268+
private registerSymbolInformation = (element: HasSymbolInformationCapability) => {
269269
this.symbolInformations.push(element.symbolInformationCapability.SymbolInformation);
270270
return this;
271271
};
272272

273-
registerScopeItem = (element: HasScopeItemCapability) =>
273+
private registerScopeItem = (element: HasScopeItemCapability) =>
274274
this.currentScope = this.currentScope.registerScopeItem(element.scopeItemCapability);
275275

276276
exitContext = (ctx: ParserRuleContext) => {

‎server/src/project/elements/module.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717

1818
// Project
1919
import { BaseRuleSyntaxElement, BaseIdentifyableSyntaxElement, HasDiagnosticCapability } from './base';
20-
import { DiagnosticCapability, FoldingRangeCapability, IdentifierCapability, ItemType, ScopeItemCapability, SymbolInformationCapability } from '../../capabilities/capabilities';
20+
import { DiagnosticCapability, IdentifierCapability, ItemType, ScopeItemCapability, SymbolInformationCapability } from '../../capabilities/capabilities';
2121
import { DuplicateAttributeDiagnostic, IgnoredAttributeDiagnostic, MissingAttributeDiagnostic, MissingOptionExplicitDiagnostic } from '../../capabilities/diagnostics';
2222

2323

@@ -32,13 +32,11 @@ abstract class BaseModuleElement<T extends ParserRuleContext> extends BaseIdenti
3232
abstract hasOptionExplicit: boolean;
3333

3434
settings: DocumentSettings;
35-
// foldingRangeCapability: FoldingRangeCapability;
3635
symbolInformationCapability: SymbolInformationCapability;
3736

3837
constructor(ctx: T, doc: TextDocument, documentSettings: DocumentSettings, symbolKind: SymbolKind) {
3938
super(ctx, doc);
4039
this.settings = documentSettings;
41-
// this.foldingRangeCapability = new FoldingRangeCapability(this);
4240
this.symbolInformationCapability = new SymbolInformationCapability(this, symbolKind);
4341
this.scopeItemCapability = new ScopeItemCapability(this, ItemType.MODULE);
4442
}

‎server/src/project/workspace.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@ import { TextDocument } from 'vscode-languageserver-textdocument';
33
import {
44
CancellationToken,
55
CancellationTokenSource,
6+
CodeAction,
7+
CodeActionParams,
8+
Command,
69
CompletionItem,
710
CompletionParams,
811
DidChangeConfigurationNotification,
@@ -277,6 +280,7 @@ class WorkspaceEvents {
277280
connection.onHover(params => this.onHover(params));
278281
connection.onDocumentFormatting(async (params, token) => await this.onDocumentFormatting(params, token));
279282
connection.onDidCloseTextDocument(params => { Services.logger.debug('[event] onDidCloseTextDocument'); Services.logger.debug(JSON.stringify(params), 1); });
283+
connection.onCodeAction((params, token) => this.onCodeActionRequest(params, token));
280284

281285
if (hasWorkspaceConfigurationCapability(Services.server)) {
282286
connection.onFoldingRanges(async (params, token) => await cancellableOnFoldingRanges(params, token));
@@ -393,6 +397,42 @@ class WorkspaceEvents {
393397

394398
}
395399

400+
private async onCodeActionRequest(params: CodeActionParams, token: CancellationToken): Promise<(Command | CodeAction)[] | null | undefined> {
401+
const logger = Services.logger;
402+
logger.debug(`[event] onCodeAction: ${JSON.stringify(params)}`);
403+
404+
// For now, if we have no diagnostics then don't return any actions.
405+
if (params.context.diagnostics.length === 0) {
406+
return [];
407+
}
408+
409+
if (token.isCancellationRequested) {
410+
logger.debug(`[cbs] onCodeAction`);
411+
return;
412+
}
413+
414+
try {
415+
// We don't actually need the document but await to ensure it's parsed.
416+
await this.getParsedProjectDocument(params.textDocument.uri, 0, token);
417+
const uri = params.textDocument.uri;
418+
const result: (Command | CodeAction)[] = [];
419+
const codeActionRegistry = Services.codeActionsRegistry;
420+
params.context.diagnostics.forEach(d => {
421+
const action = codeActionRegistry.getDiagnosticAction(d, uri);
422+
if (action) {
423+
result.push(action);
424+
}
425+
});
426+
return result;
427+
} catch (e) {
428+
// If cancelled or something went wrong, just return.
429+
if (e instanceof Error) {
430+
logger.stack(e);
431+
}
432+
return;
433+
}
434+
}
435+
396436
/** Documents event handlers */
397437

398438
/**

‎server/src/server.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export class LanguageServerConfiguration {
8282
// },
8383

8484
// Implement soon.
85-
codeActionProvider: false,
85+
codeActionProvider: true,
8686
completionProvider: undefined,
8787
hoverProvider: false,
8888

0 commit comments

Comments
 (0)