From 74d1e31c6cfe2bbac22feeeda1654100c4c10462 Mon Sep 17 00:00:00 2001 From: Dylon Edwards Date: Tue, 24 Dec 2024 02:47:28 -0500 Subject: [PATCH] Fixes infinite loop while extracting definitions from invalid character ranges; adds support for external document symbols; simplifies the logging mechanism by capturing the log context within a closure; adds bug reporting mechanism to capture when the character ranges are out-of-bounds while extracting definitions --- server/src/bug-report-provider.ts | 179 ++++++++++++++- server/src/lfortran-accessors.ts | 154 ++++++------- server/src/lfortran-language-server.ts | 213 +++++++++++------- server/src/logger.ts | 17 ++ server/test/spec/lfortran-accessors.spec.ts | 50 +++- .../spec/lfortran-language-server.spec.ts | 82 +++++-- 6 files changed, 496 insertions(+), 199 deletions(-) diff --git a/server/src/bug-report-provider.ts b/server/src/bug-report-provider.ts index 5938442..2227fa6 100644 --- a/server/src/bug-report-provider.ts +++ b/server/src/bug-report-provider.ts @@ -1,16 +1,185 @@ import { Diagnostic, + Location, + Position, + Range, RenameParams, WorkspaceEdit, } from 'vscode-languageserver/node'; -import { LFortranSettings } from './lfortran-types'; - export interface BugReportProvider { getTitle(): string; getBody(params: object): string; } +interface LineColStats { + numLines: number; + minNumCols: number; + meanNumCols: number; + stdDevNumCols: number; + medianNumCols: number; + maxNumCols: number; + lineCols: number[]; +} + +function getLineColStats(text: string): LineColStats { + const lineCols: number[] = []; + let minNumCols: number = Number.MAX_SAFE_INTEGER; + let maxNumCols: number = 0; + let meanNumCols: number = 0.0; + let stdDevNumCols: number = 0.0; + let medianNumCols: number = 0.0; + + let line = 0; + let col = 0; + for (let i = 0, k = text.length; i < k; i++) { + const c: string = text[i]; + switch (c) { + case '\r': { + if (((i + 1) < k) && (text[i + 1] === '\n')) { + i++; + } + // fallthrough + } + case '\n': { + lineCols.push(col + 1); + + line++; + col = 0; + break; + } + default: { + col++; + } + } + } + + lineCols.push(col + 1); + + const numLines: number = line + 1; + const sortedLineCols = [...lineCols]; + sortedLineCols.sort((a, b) => a - b); + + if ((sortedLineCols.length % 2) == 0) { + const i: number = sortedLineCols.length / 2; + const leftNumCols: number = sortedLineCols[i]; + const rightNumCols: number = sortedLineCols[i - 1]; + medianNumCols = (leftNumCols + rightNumCols) / 2.0; + } else { + const i: number = (sortedLineCols.length - 1) / 2; + medianNumCols = sortedLineCols[i]; + } + + let sumNumCols: number = 0; + for (let i = 0, k = sortedLineCols.length; i < k; i++) { + const numCols: number = sortedLineCols[i]; + sumNumCols += numCols; + if (numCols < minNumCols) { + minNumCols = numCols; + } + if (numCols > maxNumCols) { + maxNumCols = numCols; + } + } + + meanNumCols = sumNumCols / numLines; + + let varNumCols = 0.0; + for (let i = 0, k = sortedLineCols.length; i < k; i++) { + const numCols: number = sortedLineCols[i]; + const errNumCols: number = numCols - meanNumCols; + varNumCols += (errNumCols * errNumCols); + } + varNumCols /= numLines; + stdDevNumCols = Math.sqrt(varNumCols); + + return { + numLines: numLines, + minNumCols: minNumCols, + meanNumCols: meanNumCols, + stdDevNumCols: stdDevNumCols, + medianNumCols: medianNumCols, + maxNumCols: maxNumCols, + lineCols: lineCols, + }; +} + +export class ExtractDefinitionBugReportProvider implements BugReportProvider { + public location: Location; + public text: string; + + constructor(location: Location, text: string) { + this.location = location; + this.text = text; + } + + getTitle(): string { + return "[Generated] LFortranLanguageServer.extractDefinition received invalid location data." + } + + getBody({ version }: { + version: string, + }): string { + const range: Range = this.location.range; + + const start: Position = range.start; + const startLine: number = start.line; + const startCol: number = start.character; + + const end: Position = range.end; + const endLine: number = end.line; + const endCol: number = end.character; + + const stats: LineColStats = getLineColStats(this.text); + + let message: string; + + if (startLine >= stats.numLines) { + message = `The starting line [${startLine}] in the location data was outside the bounds of the number of available lines [${stats.numLines}].`; + } else if (endLine >= stats.numLines) { + message = `The ending line [${endLine}] in the location data was outside the bounds of the number of available lines [${stats.numLines}].`; + } else if (startCol >= stats.lineCols[startLine]) { + message = `The starting column [${startCol}] was greater than the maximum column [${stats.lineCols[startLine]}] on line [${startLine}].`; + } else if (endCol >= stats.lineCols[endLine]) { + message = `The ending column [${endCol}] was greater than the maximum column [${stats.lineCols[endLine]}] on line [${endLine}].`; + } else { + message = "An unexpected error occurred, please investigate." + } + + return ` +${message} + +### Text + +\`\`\`fortran +${this.text} +\`\`\` + +### Location Data + +\`\`\`json +${JSON.stringify(this.location, undefined, 2)} +\`\`\` + +### Line and Column Statistics + +\`\`\`json +${JSON.stringify(stats, undefined, 2)} +\`\`\` + +### LFortran Version + +\`\`\`shell +${version} +\`\`\` + +### Additional Information + +If you can provide additional information about what you were doing or how to reproduce this error, please include it here. +`.trim(); + } +} + export class RenameSymbolBugReportProvider implements BugReportProvider { public params: RenameParams; public inputText: string; @@ -27,7 +196,7 @@ export class RenameSymbolBugReportProvider implements BugReportProvider { } getBody({ version, outputText, diagnostics }: { - version: LFortranSettings, + version: string, outputText: string, diagnostics: Diagnostic[] }): string { @@ -69,6 +238,10 @@ ${JSON.stringify(diagnostics, undefined, 2)} \`\`\`shell ${version} \`\`\` + +### Additional Information + +If you can provide additional information about what you were doing or how to reproduce this error, please include it here. `.trim(); } } diff --git a/server/src/lfortran-accessors.ts b/server/src/lfortran-accessors.ts index b90dcec..13827c5 100644 --- a/server/src/lfortran-accessors.ts +++ b/server/src/lfortran-accessors.ts @@ -20,11 +20,16 @@ import which from 'which'; import fs from 'fs'; +import path from 'path'; + import tmp from 'tmp'; import { spawnSync } from 'node:child_process'; -import { Logger } from './logger'; +import { + Logger, + makeLoggable, +} from './logger'; import shellescape from 'shell-escape'; @@ -73,7 +78,6 @@ export interface LFortranAccessor { * Interacts with LFortran through its escapedCommand-line interface. */ export class LFortranCLIAccessor implements LFortranAccessor { - static LOG_CONTEXT: string = "LFortranCLIAccessor"; // File handle representing the temporary file used to pass document text to // LFortran. @@ -98,8 +102,7 @@ export class LFortranCLIAccessor implements LFortranAccessor { process.on("uncaughtException", this.cleanUpHandler); if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranCLIAccessor.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "logger", logger, @@ -115,17 +118,15 @@ export class LFortranCLIAccessor implements LFortranAccessor { try { if (fs.existsSync(this.tmpFile.name)) { try { - this.logger.debug( - LFortranCLIAccessor.LOG_CONTEXT, + this.logDebug( "Deleting temporary file: %s", this.tmpFile.name); this.tmpFile.removeCallback(); } catch (error: any) { - this.logger.error( - LFortranCLIAccessor.LOG_CONTEXT, + this.logError( "Failed to delete temporary file: %s", this.tmpFile.name); - this.logger.error(LFortranCLIAccessor.LOG_CONTEXT, error); + this.logError(error); } } } finally { @@ -135,8 +136,7 @@ export class LFortranCLIAccessor implements LFortranAccessor { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranCLIAccessor.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "args", args, @@ -162,8 +162,7 @@ export class LFortranCLIAccessor implements LFortranAccessor { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranCLIAccessor.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "path", path, @@ -194,15 +193,11 @@ export class LFortranCLIAccessor implements LFortranAccessor { let lfortranPath: string | null = settings.compiler.lfortranPath; if (lfortranPath === "lfortran" || !(await this.checkPathExistsAndIsExecutable(lfortranPath))) { lfortranPath = await which("lfortran", { nothrow: true }); - this.logger.debug( - LFortranCLIAccessor.LOG_CONTEXT, - "lfortranPath = %s", - lfortranPath); + this.logDebug("lfortranPath = %s", lfortranPath); } if (lfortranPath === null) { - this.logger.error( - LFortranCLIAccessor.LOG_CONTEXT, + this.logError( "Failed to locate lfortran, please specify its path in the configuration."); return output; } @@ -210,16 +205,12 @@ export class LFortranCLIAccessor implements LFortranAccessor { try { try { fs.accessSync(lfortranPath, fs.constants.X_OK); - this.logger.debug( - LFortranCLIAccessor.LOG_CONTEXT, + this.logDebug( "[%s] is executable", lfortranPath); } catch (err) { - this.logger.error( - LFortranCLIAccessor.LOG_CONTEXT, - "[%s] is NOT executable", - lfortranPath); - this.logger.error(LFortranCLIAccessor.LOG_CONTEXT, err); + this.logError("[%s] is NOT executable", lfortranPath); + this.logError(err); } params = params.concat(settings.compiler.flags).concat([this.tmpFile.name]); @@ -236,14 +227,12 @@ export class LFortranCLIAccessor implements LFortranAccessor { stdio: "pipe" }); - this.logger.benchmark( - LFortranCLIAccessor.LOG_CONTEXT, + this.logBenchmark( escapedCommand as string, commandStart as number); if (this.logger.isDebugEnabled()) { - this.logger.debug( - LFortranCLIAccessor.LOG_CONTEXT, + this.logDebug( "`%s` yielded status=%s, signal=%s, response=%s", escapedCommand, response.status, response.signal, JSON.stringify( @@ -258,9 +247,7 @@ export class LFortranCLIAccessor implements LFortranAccessor { if (response.stderr) { output = response.stderr.toString(); } else { - this.logger.error( - LFortranCLIAccessor.LOG_CONTEXT, - "Failed to get stderr from lfortran"); + this.logError("Failed to get stderr from lfortran"); } } else if (response.stderr) { output = response.stderr.toString(); @@ -268,12 +255,9 @@ export class LFortranCLIAccessor implements LFortranAccessor { if (response.stdout) { output = response.stdout.toString(); } else if (!noResponseIsSuccess) { - this.logger.error( - LFortranCLIAccessor.LOG_CONTEXT, - "Failed to get stdout from lfortran"); + this.logError("Failed to get stdout from lfortran"); } else { - this.logger.debug( - LFortranCLIAccessor.LOG_CONTEXT, + this.logDebug( "lfortran responded successfully with an empty string."); } } @@ -284,19 +268,16 @@ export class LFortranCLIAccessor implements LFortranAccessor { output = compileError.stdout; } if (compileError.signal !== null) { - this.logger.error( - LFortranCLIAccessor.LOG_CONTEXT, - "Compilation failed."); + this.logError("Compilation failed."); } throw compileError; } } catch (error: any) { - this.logger.error(LFortranCLIAccessor.LOG_CONTEXT, error); + this.logError(error); } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranCLIAccessor.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "settings", settings, @@ -327,25 +308,50 @@ export class LFortranCLIAccessor implements LFortranAccessor { const flags = ["--show-document-symbols", "--continue-compilation"]; const stdout = await this.runCompiler(settings, flags, text, "[]"); - let symbols: SymbolInformation[]; + let symbols: Record[]; try { symbols = JSON.parse(stdout); } catch (error) { - this.logger.warn( - LFortranCLIAccessor.LOG_CONTEXT, + this.logWarn( "Failed to parse response: %s", stdout); - this.logger.warn(LFortranCLIAccessor.LOG_CONTEXT, error); + this.logWarn(error); symbols = []; } if (Array.isArray(symbols)) { + const resolved: Map = new Map(); for (let i = 0, k = symbols.length; i < k; i++) { - const symbol: SymbolInformation = symbols[i]; + const symbol: Record = symbols[i]; + let symbolPath: string = symbol.filename; + + if (!fs.existsSync(symbolPath)) { + let resolution = resolved.get(symbolPath); + if (resolution === undefined) { + for (const flag of settings.compiler.flags) { + if (flag.startsWith("-I")) { + const includeDir = flag.substring(2); + resolution = path.join(includeDir, symbolPath); + if (fs.existsSync(resolution)) { + resolved.set(symbolPath, resolution); + break; + } + } + } + } + if (fs.existsSync(resolution as string)) { + symbolPath = resolution as string; + } + } + + if (!fs.existsSync(symbolPath)) { + this.logWarn("Failed to find file by URI: %s", symbolPath); + } const location: Location = symbol.location; - location.uri = uri; + // location.uri = uri; + location.uri = symbolPath; const range: Range = location.range; @@ -358,8 +364,7 @@ export class LFortranCLIAccessor implements LFortranAccessor { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranCLIAccessor.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "uri", uri, @@ -369,7 +374,8 @@ export class LFortranCLIAccessor implements LFortranAccessor { symbols ); } - return symbols; + + return symbols as SymbolInformation[]; } async lookupName(uri: string, @@ -412,16 +418,14 @@ export class LFortranCLIAccessor implements LFortranAccessor { } } } catch (error: any) { - this.logger.error( - LFortranCLIAccessor.LOG_CONTEXT, + this.logError( "Failed to lookup name at line=%d, column=%d", line, column); - this.logger.error(LFortranCLIAccessor.LOG_CONTEXT, error); + this.logError(error); } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranCLIAccessor.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "uri", uri, @@ -457,19 +461,15 @@ export class LFortranCLIAccessor implements LFortranAccessor { // FIXME: Remove this repair logic once the respective bug has been // fixed (lfortran/lfortran issue #5525) // ---------------------------------------------------------------- - this.logger.warn( - LFortranCLIAccessor.LOG_CONTEXT, + this.logWarn( "Failed to parse response, attempting to repair and re-parse it."); const repaired: string = stdout.substring(0, 28) + "{" + stdout.substring(28); try { results = JSON.parse(repaired); - this.logger.log( - LFortranCLIAccessor.LOG_CONTEXT, + this.logLog( "Repair succeeded, see: https://github.com/lfortran/lfortran/issues/5525"); } catch { - this.logger.error( - LFortranCLIAccessor.LOG_CONTEXT, - "Failed to repair response"); + this.logError("Failed to repair response"); throw error; } } @@ -484,21 +484,15 @@ export class LFortranCLIAccessor implements LFortranAccessor { } } } catch (error: any) { - this.logger.error( - LFortranCLIAccessor.LOG_CONTEXT, - "Failed to show errors"); + this.logError("Failed to show errors"); if (stdout !== null) { - this.logger.error( - LFortranCLIAccessor.LOG_CONTEXT, - "Failed to parse response: %s", - stdout); + this.logError("Failed to parse response: %s", stdout); } - this.logger.error(LFortranCLIAccessor.LOG_CONTEXT, error); + this.logError(error); } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranCLIAccessor.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "uri", uri, @@ -552,16 +546,14 @@ export class LFortranCLIAccessor implements LFortranAccessor { } } } catch (error: any) { - this.logger.error( - LFortranCLIAccessor.LOG_CONTEXT, + this.logError( "Failed to rename symbol at line=%d, column=%d", line, column); - this.logger.error(LFortranCLIAccessor.LOG_CONTEXT, error); + this.logError(error); } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranCLIAccessor.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "uri", uri, @@ -578,3 +570,5 @@ export class LFortranCLIAccessor implements LFortranAccessor { return edits; } } + +makeLoggable(LFortranCLIAccessor); diff --git a/server/src/lfortran-language-server.ts b/server/src/lfortran-language-server.ts index 546a12d..53d6cba 100644 --- a/server/src/lfortran-language-server.ts +++ b/server/src/lfortran-language-server.ts @@ -3,6 +3,8 @@ * Licensed under the MIT License. See License.txt in the project root for license information. * ------------------------------------------------------------------------------------------ */ +import fs from 'fs'; + import { CompletionItem, CompletionItemKind, @@ -42,10 +44,14 @@ import { LFortranAccessor } from './lfortran-accessors'; import { PrefixTrie } from './prefix-trie'; -import { Logger } from './logger'; +import { + Logger, + makeLoggable, +} from './logger'; import { BugReportProvider, + ExtractDefinitionBugReportProvider, RenameSymbolBugReportProvider, } from './bug-report-provider'; @@ -65,11 +71,16 @@ const defaultSettings: LFortranSettings = { }, }; -const RE_IDENTIFIABLE: RegExp = /^[a-zA-Z0-9_]$/; +const RE_IDENTIFIABLE: RegExp = /^[a-z0-9_]$/i; +const RE_ALPHABETIC: RegExp = /^[a-z]$/i; -export class LFortranLanguageServer { - static LOG_CONTEXT: string = "LFortranLanguageServer"; +interface FileCacheEntry { + mtime: number; + text: string; + uri: string; +} +export class LFortranLanguageServer { public lfortran: LFortranAccessor; public connection: _Connection; public documents: TextDocuments; @@ -89,6 +100,8 @@ export class LFortranLanguageServer { public hasError: boolean = false; public bugReportProvider: BugReportProvider | null = null; + public fileCache: Map = new Map(); + constructor(lfortran: LFortranAccessor, connection: _Connection, documents: TextDocuments, @@ -105,8 +118,7 @@ export class LFortranLanguageServer { logger.configure(settings); if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "lfortran", lfortran, @@ -120,6 +132,7 @@ export class LFortranLanguageServer { } reportBug(title: string, body: string): void { + this.logDebug("Reporting a bug; title = \"%s\"\n%s", title, body); this.connection.sendRequest("LFortranLanguageServer.action.openIssueReporter", { issueType: 0, // IssueType.Bug issueSource: "extension", // IssueSource.Extension @@ -172,8 +185,7 @@ export class LFortranLanguageServer { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "params", params, @@ -195,8 +207,7 @@ export class LFortranLanguageServer { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "params", params, @@ -205,12 +216,39 @@ export class LFortranLanguageServer { } } - extractDefinition(location: Location): string | null { + async extractDefinition(location: Location): Promise { const fnid: string = "extractDefinition"; const start: number = performance.now(); + let definition: string | null = null; - const document = this.documents.get(location.uri); - if (document !== undefined) { + + let uri: string = location.uri; + const document = this.documents.get(uri); + let text = document?.getText(); + if ((text === undefined) && uri.startsWith("file://")) { + uri = uri.substring(7); + } + if (text === undefined) { + if (fs.existsSync(uri)) { + let entry = this.fileCache.get(uri); + const stats = fs.statSync(uri); + if ((entry === undefined) || (entry.mtime !== stats.mtime)) { + text = fs.readFileSync(uri, 'utf8'); + entry = { + mtime: stats.mtime, + text: text, + uri: uri, + }; + this.fileCache.set(uri, entry); + } else { + text = entry.text; + } + } else { + this.logWarn("Failed to find file by URI: %s", uri); + } + } + + if (text !== undefined) { const range: Range = location.range; const start: Position = range.start; @@ -221,8 +259,6 @@ export class LFortranLanguageServer { const endLine: number = end.line; const endCol: number = end.character; - const text = document.getText(); - let currLine = 0; let currCol = 0; @@ -230,46 +266,62 @@ export class LFortranLanguageServer { let c: string; if ((currLine === startLine) && (currCol === startCol)) { let j = i; - for (; (currLine < endLine) || (currLine === endLine) && (currCol <= endCol); j++) { + for (; ((currLine < endLine) || ((currLine === endLine) && (currCol <= endCol))) && (j < k); j++) { c = text[j]; - if (c === '\n') { - currLine++; - currCol = 0; - } else if (c === '\r') { - const l = j + 1; - if ((l < k) && (text[l] === '\n')) { - j = l; + switch (c) { + case '\r': { + const l = j + 1; + if ((l < k) && (text[l] === '\n')) { + j = l; + } + // fallthrough + } + case '\n': { + currLine++; + currCol = 0; + break; + } + default: { + currCol++; } - currLine++; - currCol = 0; - } else { - currCol++; } } - definition = text.substring(i, j); + if ((currLine < endLine) || ((currLine === endLine) && (currCol < endCol))) { + const provider: BugReportProvider = + new ExtractDefinitionBugReportProvider(location, text); + const version: string = await this.lfortran.version(this.settings); + const title: string = provider.getTitle(); + const body: string = provider.getBody({ version }); + this.reportBug(title, body); + } else { + definition = text.substring(i, j); + } break; } else { c = text[i]; - if (c === '\n') { - currLine++; - currCol = 0; - } else if (c === '\r') { - const j = i + 1; - if ((j < k) && (text[j] === '\n')) { - i = j; + switch (c) { + case '\r': { + const j = i + 1; + if ((j < k) && (text[j] === '\n')) { + i = j; + } + // fallthrough + } + case '\n': { + currLine++; + currCol = 0; + break; + } + default: { + currCol++; } - currLine++; - currCol = 0; - } else { - currCol++; } } } } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "location", location, @@ -280,7 +332,7 @@ export class LFortranLanguageServer { return definition; } - index(uri: string, symbols: SymbolInformation[]): void { + async index(uri: string, symbols: SymbolInformation[]): Promise { const fnid: string = "index"; const start: number = performance.now(); @@ -291,7 +343,7 @@ export class LFortranLanguageServer { for (let i = 0, k = symbols.length; i < k; i++) { const symbol: SymbolInformation = symbols[i]; const definition: string | null = - this.extractDefinition(symbol.location); + await this.extractDefinition(symbol.location); terms.push(symbol.name); values.push({ label: symbol.name, @@ -308,8 +360,7 @@ export class LFortranLanguageServer { this.dictionaries.set(uri, dictionary); if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "uri", uri, @@ -335,13 +386,12 @@ export class LFortranLanguageServer { if (symbols.length > 0) { // (symbols.length == 0) => error with document, but we still need to // support auto-completion. - this.index(uri, symbols); + await this.index(uri, symbols); } } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "request", request, @@ -362,8 +412,8 @@ export class LFortranLanguageServer { this.settings = await this.getDocumentSettings(uri); this.logger.configure(this.settings); const document = this.documents.get(uri); - const text = document?.getText(); - if (typeof text === "string") { + if (document !== undefined) { + const text = document.getText(); const line = request.position.line; const column = request.position.character; definitions = @@ -371,8 +421,7 @@ export class LFortranLanguageServer { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "request", request, @@ -402,8 +451,7 @@ export class LFortranLanguageServer { this.documents.all().forEach(this.validateTextDocument.bind(this)); if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "change", change, @@ -418,8 +466,7 @@ export class LFortranLanguageServer { if (!this.hasConfigurationCapability) { if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "uri", uri, @@ -442,8 +489,7 @@ export class LFortranLanguageServer { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "uri", uri, @@ -462,8 +508,7 @@ export class LFortranLanguageServer { this.documentSettings.delete(event.document.uri); if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "event", event, @@ -479,8 +524,7 @@ export class LFortranLanguageServer { this.validateTextDocument(event.document); if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "event", event, @@ -514,14 +558,11 @@ export class LFortranLanguageServer { // Send the computed diagnostics to VSCode. this.connection.sendDiagnostics({ uri: uri, diagnostics }); } else { - this.logger.error( - LFortranLanguageServer.LOG_CONTEXT, - 'Cannot validate a document with no diagnostic capability'); + this.logError('Cannot validate a document with no diagnostic capability'); } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "textDocument", textDocument, @@ -543,12 +584,19 @@ export class LFortranLanguageServer { if ((line === currLine) && (column === currCol)) { const re_identifiable: RegExp = RE_IDENTIFIABLE; + const re_alphabetic: RegExp = RE_ALPHABETIC; if (re_identifiable.test(c)) { let l = i; let u = i + 1; while ((l > 0) && re_identifiable.test(text[l - 1])) { l--; } + while (!re_alphabetic.test(text[l]) && (l <= i)) { + l++; + } + if (l > i) { + return null; + } while ((u < k) && re_identifiable.test(text[u])) { u++; } @@ -573,8 +621,7 @@ export class LFortranLanguageServer { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "text", text, @@ -607,8 +654,7 @@ export class LFortranLanguageServer { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "documentPosition", documentPosition, @@ -623,8 +669,7 @@ export class LFortranLanguageServer { const fnid: string = "onCompletionResolve"; const start: number = performance.now(); if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "item", item, @@ -665,8 +710,7 @@ export class LFortranLanguageServer { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "params", params, @@ -749,8 +793,7 @@ export class LFortranLanguageServer { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "text", text, @@ -773,8 +816,7 @@ export class LFortranLanguageServer { })); if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "text", text, @@ -815,8 +857,7 @@ export class LFortranLanguageServer { } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "params", params, @@ -847,17 +888,13 @@ export class LFortranLanguageServer { range: range, })); } else { - this.logger.warn( - LFortranLanguageServer.LOG_CONTEXT, - 'Cannot find symbol to highlight: "%s"', - query); + this.logWarn('Cannot find symbol to highlight: "%s"', query); } } } if (this.logger.isBenchmarkOrTraceEnabled()) { - this.logger.benchmarkAndTrace( - LFortranLanguageServer.LOG_CONTEXT, + this.logBenchmarkAndTrace( fnid, start, [ "params", params, @@ -869,3 +906,5 @@ export class LFortranLanguageServer { } } +makeLoggable(LFortranLanguageServer); + diff --git a/server/src/logger.ts b/server/src/logger.ts index a4c12e8..91e470d 100644 --- a/server/src/logger.ts +++ b/server/src/logger.ts @@ -24,6 +24,23 @@ export function logLevelFromName(lowerName: string): LogLevel { return level; } +interface Type { + new(...args: any[]): any; +} + +export function makeLoggable(kind: Type): Type { + const context: string = kind.name; + kind.prototype.logFatal = function (...args: any[]) { this.logger.fatal(context, ...args); } + kind.prototype.logError = function (...args: any[]) { this.logger.error(context, ...args); } + kind.prototype.logWarn = function (...args: any[]) { this.logger.warn(context, ...args); } + kind.prototype.logInfo = function (...args: any[]) { this.logger.info(context, ...args); } + kind.prototype.logDebug = function (...args: any[]) { this.logger.debug(context, ...args); } + kind.prototype.logTrace = function (...args: any[]) { this.logger.trace(context, ...args); } + kind.prototype.logBenchmark = function (...args: any[]) { this.logger.benchmark(context, ...args); } + kind.prototype.logBenchmarkAndTrace = function (...args: any[]) { this.logger.benchmarkAndTrace(context, ...args); } + return kind; +} + export class Logger { /** Current `LogLevel` for filtering and formatting messages. */ diff --git a/server/test/spec/lfortran-accessors.spec.ts b/server/test/spec/lfortran-accessors.spec.ts index 9933305..137b091 100644 --- a/server/test/spec/lfortran-accessors.spec.ts +++ b/server/test/spec/lfortran-accessors.spec.ts @@ -3,7 +3,6 @@ import { Diagnostic, DiagnosticSeverity, Range, - SymbolInformation, SymbolKind, TextEdit, } from "vscode-languageserver/node"; @@ -61,13 +60,13 @@ describe("LFortranCLIAccessor", () => { }); it("returns the expected symbol information", async () => { - const response: SymbolInformation[] = [ + const response: Record[] = [ { name: "foo", - // NOTE: Right now, the kind is hard-coded to Function ... kind: SymbolKind.Function, + filename: uri, location: { - uri: uri, + uri: "uri", range: { start: { line: 0, @@ -82,10 +81,10 @@ describe("LFortranCLIAccessor", () => { }, { name: "bar", - // NOTE: Right now, the kind is hard-coded to Function ... kind: SymbolKind.Function, + filename: uri, location: { - uri: uri, + uri: "uri", range: { start: { line: 3, @@ -101,7 +100,44 @@ describe("LFortranCLIAccessor", () => { ]; const stdout = JSON.stringify(response); sinon.stub(lfortran, "runCompiler").resolves(stdout); - const expected = response; + const expected: Record[] = [ + { + name: "foo", + kind: SymbolKind.Function, + filename: uri, + location: { + uri: uri, + range: { + start: { + line: 0, + character: 1 + }, + end: { + line: 0, + character: 5 + } + } + } + }, + { + name: "bar", + kind: SymbolKind.Function, + filename: uri, + location: { + uri: uri, + range: { + start: { + line: 3, + character: 15 + }, + end: { + line: 3, + character: 25 + } + } + } + }, + ]; for (const symbol of expected) { const range = symbol.location.range; range.start.character--; diff --git a/server/test/spec/lfortran-language-server.spec.ts b/server/test/spec/lfortran-language-server.spec.ts index 3428cb6..d417c70 100644 --- a/server/test/spec/lfortran-language-server.spec.ts +++ b/server/test/spec/lfortran-language-server.spec.ts @@ -118,13 +118,13 @@ describe("LFortranLanguageServer", () => { }; it("Returns all the symbols", async () => { - const response: SymbolInformation[] = [ + const response: Record[] = [ { name: "baz", - // NOTE: Right now, the kind is hard-coded to Function ... kind: SymbolKind.Function, + filename: uri, location: { - uri: uri, + uri: "uri", range: { start: { line: 0, @@ -139,10 +139,10 @@ describe("LFortranLanguageServer", () => { }, { name: "qux", - // NOTE: Right now, the kind is hard-coded to Function ... kind: SymbolKind.Function, + filename: uri, location: { - uri: uri, + uri: "uri", range: { start: { line: 3, @@ -161,7 +161,45 @@ describe("LFortranLanguageServer", () => { sinon.stub(lfortran, "runCompiler").resolves(stdout); document.getText.returns(""); - const expected = response; + const expected: Record[] = [ + { + name: "baz", + kind: SymbolKind.Function, + filename: uri, + location: { + uri: uri, + range: { + start: { + line: 0, + character: 1 + }, + end: { + line: 0, + character: 5 + } + } + } + }, + { + name: "qux", + kind: SymbolKind.Function, + filename: uri, + location: { + uri: uri, + range: { + start: { + line: 3, + character: 15 + }, + end: { + line: 3, + character: 25 + } + } + } + }, + ]; + for (const symbol of expected) { const range = symbol.location.range; range.start.character--; @@ -238,7 +276,7 @@ describe("LFortranLanguageServer", () => { it("returns nothing when the document has not been defined", async () => { const actual = await server.onDefinition(request); - assert.isNull(actual); + assert.isEmpty(actual); }); }); @@ -373,7 +411,7 @@ describe("LFortranLanguageServer", () => { }); describe("extractDefinition", () => { - it("extracts definitions from the respective range", () => { + it("extracts definitions from the respective range", async () => { const text: string = [ "Lorem ipsum dolor sit amet, consectetur adipiscing elit,", "sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.", @@ -400,14 +438,14 @@ describe("LFortranLanguageServer", () => { const range: Range = { start, end }; const location: Location = { uri, range }; - let definition: string = server.extractDefinition(location); + let definition: string = await server.extractDefinition(location); assert.equal(definition, "Lorem ipsum"); start.line = 2; start.character = 8; end.line = 5; end.character = 31; - definition = server.extractDefinition(location); + definition = await server.extractDefinition(location); assert.equal(definition, [ "ad minim veniam, quis nostrud exercitation ullamco laboris", "nisi ut aliquip ex ea commodo consequat.", @@ -418,9 +456,9 @@ describe("LFortranLanguageServer", () => { }); describe("index", () => { - it("indexes empty lists of symbols", () => { + it("indexes empty lists of symbols", async () => { const symbols: SymbolInformation[] = []; - server.index(uri, symbols); + await server.index(uri, symbols); const dictionary = server.dictionaries.get(uri); assert.isDefined(dictionary); const indexed: CompletionItem[] = @@ -428,7 +466,7 @@ describe("LFortranLanguageServer", () => { assert.isEmpty(indexed); }); - it("indexes singleton lists of symbols", () => { + it("indexes singleton lists of symbols", async () => { const text: string = "def foo; def bar; def baz; def qux; def quo;"; document.getText.returns(text); @@ -452,7 +490,7 @@ describe("LFortranLanguageServer", () => { }, ]; - server.index(uri, symbols); + await server.index(uri, symbols); const dictionary = server.dictionaries.get(uri); assert.isDefined(dictionary); const indexed: CompletionItem[] = @@ -466,7 +504,7 @@ describe("LFortranLanguageServer", () => { ]); }); - it("indexes lists of symbols", () => { + it("indexes lists of symbols", async () => { const text: string = "def foo; def bar; def baz; def qux; def quo;"; document.getText.returns(text); @@ -524,7 +562,7 @@ describe("LFortranLanguageServer", () => { }, ]; - server.index(uri, symbols); + await server.index(uri, symbols); const dictionary = server.dictionaries.get(uri); assert.isDefined(dictionary); const indexed: CompletionItem[] = @@ -565,7 +603,7 @@ describe("LFortranLanguageServer", () => { }); describe("onCompletion", () => { - it("completes queries based on indexed terms", () => { + it("completes queries based on indexed terms", async () => { const text: string = "def foo; def bar; def baz; def qux; def quo; ba"; document.getText.returns(text); @@ -623,7 +661,7 @@ describe("LFortranLanguageServer", () => { }, ]; - server.index(uri, symbols); + await server.index(uri, symbols); const documentPosition: TextDocumentPositionParams = { textDocument: { @@ -718,7 +756,7 @@ describe("LFortranLanguageServer", () => { }, ]; - server.index(uri, symbols); + await server.index(uri, symbols); const hoverParams: HoverParams = { textDocument: { @@ -1029,7 +1067,7 @@ describe("LFortranLanguageServer", () => { }, ]; - server.index(uri, symbols); + await server.index(uri, symbols); const expected: WorkspaceEdit = { changes: { @@ -1090,7 +1128,7 @@ describe("LFortranLanguageServer", () => { }, ]; - server.index(uri, symbols); + await server.index(uri, symbols); const params: DocumentHighlightParams = { textDocument: document, @@ -1128,7 +1166,7 @@ describe("LFortranLanguageServer", () => { }, ]; - server.index(uri, symbols); + await server.index(uri, symbols); const expected: DocumentHighlight[] = [ {