diff --git a/server/src/project/document.ts b/server/src/project/document.ts index 5befb60..879d183 100644 --- a/server/src/project/document.ts +++ b/server/src/project/document.ts @@ -175,27 +175,29 @@ export abstract class BaseProjectDocument { return; } - // Parse the document. - await (new SyntaxParser(Services.logger)).parse(token, this); - const projectScope = this.currentScope.project; - const buildScope = projectScope?.isDirty ? projectScope : this.currentScope; - buildScope.build(); - buildScope.resolveUnused(); - - // Evaluate the diagnostics. - const diagnostics = this.hasDiagnosticElements - .map(e => e.diagnosticCapability.evaluate()) - .flat(); - - // Ensure diagnostics aren't reported twice. - // TODO: Redesign diagnostics so this isn't required. - diagnostics.forEach(diagnostic => { - if (!this.hasDiagnostic(diagnostic)) { - this.diagnostics.push(diagnostic); - } - }); - - this._isBusy = false; + try { + // Parse the document. + await (new SyntaxParser(Services.logger)).parse(token, this); + const projectScope = this.currentScope.project; + const buildScope = projectScope?.isDirty ? projectScope : this.currentScope; + buildScope.build(); + buildScope.resolveUnused(); + + // Evaluate the diagnostics. + const diagnostics = this.hasDiagnosticElements + .map(e => e.diagnosticCapability.evaluate()) + .flat(); + + // Ensure diagnostics aren't reported twice. + // TODO: Redesign diagnostics so this isn't required. + diagnostics.forEach(diagnostic => { + if (!this.hasDiagnostic(diagnostic)) { + this.diagnostics.push(diagnostic); + } + }); + } finally { + this._isBusy = false; + } }; async formatParse(token: CancellationToken): Promise { diff --git a/server/src/project/workspace.ts b/server/src/project/workspace.ts index 9638fc3..b73b736 100644 --- a/server/src/project/workspace.ts +++ b/server/src/project/workspace.ts @@ -34,15 +34,13 @@ import { import { ParseCancellationException } from 'antlr4ng'; // Project -import { sleep, walk } from '../utils/helpers'; +import { getMissingSymbolsLogSeverity, sleep, walk } from '../utils/helpers'; import { Services } from '../injection/services'; import { getFormattingEdits } from './formatter'; import { BaseProjectDocument } from './document'; -import { SyntaxParser } from './parser/vbaParser'; import { VbaFmtListener } from './parser/vbaListener'; import { hasWorkspaceConfigurationCapability } from '../capabilities/workspaceFolder'; import { Logger, ILanguageServer, IWorkspace } from '../injection/interface'; -import { returnDefaultOnCancelClientRequest } from '../utils/wrappers'; import { ScopeType, ScopeItemCapability } from '../capabilities/capabilities'; export interface ExtensionConfiguration { @@ -116,7 +114,6 @@ export class Workspace implements IWorkspace { } // Set up parser and dummy token because we won't cancel this. - const parser = new SyntaxParser(this.logger); const token = new CancellationTokenSource().token; // Handle each file in the workspace. @@ -134,7 +131,8 @@ export class Workspace implements IWorkspace { const textDocument = TextDocument.create(`${normalisedUri}`, 'vba', 1, file); const projectDocument = BaseProjectDocument.create(textDocument); this.projectDocuments.set(normalisedUri, projectDocument); - await parser.parse(token, projectDocument); + await projectDocument.parse(token); + this.connection.sendDiagnostics(projectDocument.languageServerDiagnostics()); this.logger.info(`Parsed ${projectDocument.name}`, 1); } catch (e) { // Log errors and anything else without failing. @@ -284,17 +282,19 @@ class WorkspaceEvents { // Handle token cancellation. if (token.isCancellationRequested) return undefined; + const normalisedUri = uri.toFilePath().toFileUri(); + let cancelled = false; token.onCancellationRequested(() => cancelled = true); let document: BaseProjectDocument | undefined; - document = this.projectDocuments.get(uri); + document = this.projectDocuments.get(normalisedUri); // Ensure we have the appropriately versioned document. while (!document || document.textDocument.version < version) { if (cancelled) return undefined; await sleep(5); - document = this.projectDocuments.get(uri); + document = this.projectDocuments.get(normalisedUri); } // Return nothing if the document version is newer than requested. @@ -308,7 +308,7 @@ class WorkspaceEvents { await sleep(5); // A didChange can replace the tracked document instance while an older // request is still waiting; re-read the latest instance to avoid stale waits - const latestDocument = this.projectDocuments.get(uri); + const latestDocument = this.projectDocuments.get(normalisedUri); if (latestDocument) { // For versioned requests, ensure we don't return a different version. if (version > 0 && latestDocument.textDocument.version !== version) { @@ -322,9 +322,6 @@ class WorkspaceEvents { } private initialiseConnectionEvents(connection: _Connection) { - const cancellableOnDocSymbol = returnDefaultOnCancelClientRequest( - (p: DocumentSymbolParams, t) => this.onDocumentSymbolAsync(p, t), [], 'Document Symbols'); - connection.onCodeAction(async (params, token) => this.onCodeActionRequest(params, token)); connection.onCompletion(params => this.onCompletion(params)); connection.onCompletionResolve(item => this.onCompletionResolve(item)); @@ -333,7 +330,7 @@ class WorkspaceEvents { connection.onDidChangeWatchedFiles(params => this.onDidChangeWatchedFiles(params)); connection.onDidCloseTextDocument(params => { Services.logger.debug('[event] onDidCloseTextDocument'); Services.logger.debug(JSON.stringify(params), 1); }); connection.onDocumentFormatting(async (params, token) => await this.onDocumentFormatting(params, token)); - connection.onDocumentSymbol(async (params, token) => await cancellableOnDocSymbol(params, token)); + connection.onDocumentSymbol(async (params, token) => await this.onDocumentSymbolAsync(params, token)); connection.onHover(params => this.onHover(params)); connection.onInitialized(() => this.onInitialized()); connection.onRenameRequest((params, token) => this.onRenameRequest(params, token)); @@ -411,8 +408,24 @@ class WorkspaceEvents { private async onDocumentSymbolAsync(params: DocumentSymbolParams, token: CancellationToken): Promise { Services.logger.debug('[event] onDocumentSymbol'); - const document = await this.getParsedProjectDocument(params.textDocument.uri, 0, token); - return document?.languageServerSymbolInformation() ?? []; + const normalisedUri = params.textDocument.uri.toFilePath().toFileUri(); + const document = await this.getParsedProjectDocument(normalisedUri, 0, token); + const symbols = document?.languageServerSymbolInformation() ?? []; + + if (document) { + switch (getMissingSymbolsLogSeverity(document.textDocument.getText(), symbols)) { + case 'error': + Services.logger.error(`No document symbols produced for ${document.name}`); + break; + case 'warn': + Services.logger.warn(`No member symbols produced for ${document.name}`); + break; + default: + break; + } + } + + return symbols; } private async onFoldingRangesAsync(params: FoldingRangeParams, token: CancellationToken): Promise { @@ -572,9 +585,14 @@ class WorkspaceEvents { Services.logger.debug('[event] onDidOpen'); this.printDocumentInformation(document); const normalisedUri = document.uri.toFilePath().toFileUri(); - if (this.projectDocuments.has(normalisedUri)) { - Services.workspace.openDocument(document); + + if (!this.projectDocuments.has(normalisedUri)) { + const projectDocument = BaseProjectDocument.create(document); + this.projectDocuments.set(normalisedUri, projectDocument); + Services.workspace.parseDocument(projectDocument); } + + Services.workspace.openDocument(document); } /** diff --git a/server/src/test/helpers.test.ts b/server/src/test/helpers.test.ts new file mode 100644 index 0000000..831a238 --- /dev/null +++ b/server/src/test/helpers.test.ts @@ -0,0 +1,131 @@ +import 'reflect-metadata'; + +import { describe, it } from 'mocha'; +import * as assert from 'assert'; +import dedent from 'dedent'; +import { SymbolKind } from 'vscode-languageserver'; + +import { getMissingSymbolsLogSeverity, shouldHaveSymbols } from '../utils/helpers'; + +describe('Helpers symbol classification', () => { + it('does not flag legitimate no-symbol module content', () => { + const moduleText = dedent` + Option Explicit + Attribute VB_Name = "Module1" + ' comment + Rem comment + #If VBA7 Then + #Else + #End If + `; + + const result = shouldHaveSymbols(moduleText); + + assert.strictEqual(result, false, 'Expected legitimate directive-only content to skip missing-symbol error logging'); + }); + + it('flags substantive code with empty symbol list', () => { + const moduleText = dedent` + Option Explicit + Public Sub Test() + End Sub + `; + + const result = shouldHaveSymbols(moduleText); + + assert.strictEqual(result, true, 'Expected substantive code to be flagged when symbols are missing'); + }); + + it('does not flag a procedure wrapped in conditional compilation', () => { + const moduleText = dedent` + Option Explicit + #If Win64 Then + Public Sub ConditionalProc() + End Sub + #End If + `; + + const result = shouldHaveSymbols(moduleText); + + assert.strictEqual(result, false, 'Expected conditional-compilation-only procedures to be treated as legitimate zero-symbol content'); + }); +}); + +describe('Helpers missing-symbol log severity', () => { + it('returns error when no symbols are produced at all', () => { + const moduleText = dedent` + Option Explicit + Public Sub Test() + End Sub + `; + + const severity = getMissingSymbolsLogSeverity( + moduleText, + [] + ); + + assert.strictEqual(severity, 'error'); + }); + + it('returns warn when only module symbol exists but member symbols are expected', () => { + const moduleText = dedent` + Option Explicit + Public Sub Test() + End Sub + `; + + const severity = getMissingSymbolsLogSeverity( + moduleText, + [{ kind: SymbolKind.File }] + ); + + assert.strictEqual(severity, 'warn'); + }); + + it('returns none when only module symbol exists and content is legitimately non-symbolic', () => { + const moduleText = dedent` + Attribute VB_Name = "Module1" + Option Explicit + `; + + const severity = getMissingSymbolsLogSeverity( + moduleText, + [{ kind: SymbolKind.File }] + ); + + assert.strictEqual(severity, 'none'); + }); + + it('returns none when only module symbol exists and procedures are in inactive compiler branch', () => { + const moduleText = dedent` + Option Explicit + #If Win64 Then + #Else + Public Sub ConditionalProc() + End Sub + #End If + `; + + const severity = getMissingSymbolsLogSeverity( + moduleText, + [{ kind: SymbolKind.File }] + ); + + assert.strictEqual(severity, 'none'); + }); + + it('returns none when member symbols are present', () => { + const moduleText = dedent` + Option Explicit + Public Sub Test() + End Sub + `; + + const severity = getMissingSymbolsLogSeverity( + moduleText, + [{ kind: SymbolKind.File }, { kind: SymbolKind.Method }] + ); + + assert.strictEqual(severity, 'none'); + }); +}); diff --git a/server/src/utils/helpers.ts b/server/src/utils/helpers.ts index 8276b6f..b9b4497 100644 --- a/server/src/utils/helpers.ts +++ b/server/src/utils/helpers.ts @@ -3,7 +3,7 @@ import * as fs from 'fs'; import * as path from 'path'; import { Services } from '../injection/services'; import { pathToFileURL } from 'url'; -import { Position, Range } from 'vscode-languageserver'; +import { Position, Range, SymbolInformation, SymbolKind } from 'vscode-languageserver'; export class Dictionary extends Map { private defaultFactory: (...args: any) => V; @@ -132,4 +132,65 @@ export function rangeEquals(r1?: Range, r2?: Range): boolean { && r1.start.character === r2.start.character && r1.end.line === r2.end.line && r1.end.character === r2.end.character; +} + +/** + * Returns true when member symbols would normally be expected from the provided VBA source. + */ +export function shouldHaveSymbols(text: string): boolean { + const lines = text.split(/\r?\n/); + let preprocessorDepth = 0; + + for (const rawLine of lines) { + const line = rawLine.trim(); + if (line.length === 0) continue; + if (/^'/.test(line)) continue; + if (/^rem(?:\s|$)/i.test(line)) continue; + if (/^option\b/i.test(line)) continue; + if (/^attribute\s+vb_/i.test(line)) continue; + + if (/^#if\b/i.test(line)) { + preprocessorDepth++; + continue; + } + + if (/^#elseif\b/i.test(line) || /^#else\b/i.test(line)) { + continue; + } + + if (/^#end\s*if\b/i.test(line)) { + preprocessorDepth = Math.max(0, preprocessorDepth - 1); + continue; + } + + if (/^#const\b/i.test(line)) continue; + + if (preprocessorDepth > 0) { + continue; + } + + return true; + } + + return false; +} + +/** + * Determines diagnostic log severity for missing outline symbols. + * + * - `error`: no symbols at all (module/class symbol missing) + * - `warn`: only module/class symbol exists but member symbols are expected + * - `none`: symbols are present as expected or document can legitimately have none + */ +export function getMissingSymbolsLogSeverity(text: string, symbols: Pick[]): 'none' | 'warn' | 'error' { + if (symbols.length === 0) { + return 'error'; + } + + const hasMemberSymbols = symbols.some(x => x.kind !== SymbolKind.File); + if (hasMemberSymbols) { + return 'none'; + } + + return shouldHaveSymbols(text) ? 'warn' : 'none'; } \ No newline at end of file