From 5ef936f95e75c9c309dde193945ae779884cef32 Mon Sep 17 00:00:00 2001 From: Bronley Date: Mon, 18 Jan 2021 06:34:07 -0500 Subject: [PATCH 1/5] API change --- src/LanguageServer.ts | 91 +++++++++++++++++-------------------- src/Program.ts | 101 +++++++++--------------------------------- src/ProgramBuilder.ts | 78 +++++++++++++++++++++++++------- src/util.ts | 46 ++++++++++++------- 4 files changed, 154 insertions(+), 162 deletions(-) diff --git a/src/LanguageServer.ts b/src/LanguageServer.ts index d59c40ab1..e1558e32f 100644 --- a/src/LanguageServer.ts +++ b/src/LanguageServer.ts @@ -335,7 +335,7 @@ export class LanguageServer { //if there's a setting, we need to find the file or show error if it can't be found if (config?.configFile) { configFilePath = path.resolve(workspacePath, config.configFile); - if (await util.fileExists(configFilePath)) { + if (await util.pathExists(configFilePath)) { return configFilePath; } else { this.sendCriticalFailure(`Cannot find config file specified in user/workspace settings at '${configFilePath}'`); @@ -344,13 +344,13 @@ export class LanguageServer { //default to config file path found in the root of the workspace configFilePath = path.resolve(workspacePath, 'bsconfig.json'); - if (await util.fileExists(configFilePath)) { + if (await util.pathExists(configFilePath)) { return configFilePath; } //look for the deprecated `brsconfig.json` file configFilePath = path.resolve(workspacePath, 'brsconfig.json'); - if (await util.fileExists(configFilePath)) { + if (await util.pathExists(configFilePath)) { return configFilePath; } @@ -371,8 +371,9 @@ export class LanguageServer { builder.allowConsoleClearing = false; //look for files in our in-memory cache before going to the file system - builder.addFileResolver((pathAbsolute) => { - return this.documentFileResolver(pathAbsolute); + builder.addFileResolver({ + readFile: (pathAbsolute) => this.documentFileResolver(pathAbsolute), + readFileSync: (pathAbsolute) => this.documentFileResolver(pathAbsolute) }); let configFilePath = await this.getConfigFilePath(workspacePath); @@ -380,7 +381,7 @@ export class LanguageServer { let cwd = workspacePath; //if the config file exists, use it and its folder as cwd - if (configFilePath && await util.fileExists(configFilePath)) { + if (configFilePath && await util.pathExists(configFilePath)) { cwd = path.dirname(configFilePath); } else { //config file doesn't exist...let `brighterscript` resolve the default way @@ -442,8 +443,9 @@ export class LanguageServer { builder.allowConsoleClearing = false; //look for files in our in-memory cache before going to the file system - builder.addFileResolver((pathAbsolute) => { - return this.documentFileResolver(pathAbsolute); + builder.addFileResolver({ + readFile: (pathAbsolute) => this.documentFileResolver(pathAbsolute), + readFileSync: (pathAbsolute) => this.documentFileResolver(pathAbsolute) }); //get the path to the directory where this file resides @@ -521,23 +523,9 @@ export class LanguageServer { //wait until the file has settled await this.keyedThrottler.onIdleOnce(filePath, true); - let workspaceCompletionPromises = [] as Array>; - let workspaces = this.getWorkspaces(); - //get completions from every workspace - for (let workspace of workspaces) { - //if this workspace has the file in question, get its completions - if (workspace.builder.program.hasFile(filePath)) { - workspaceCompletionPromises.push( - workspace.builder.program.getCompletions(filePath, position) - ); - } - } - - let completions = ([] as CompletionItem[]) - //wait for all promises to resolve, and then flatten them into a single array - .concat(...await Promise.all(workspaceCompletionPromises)) - //throw out falsey values - .filter(x => !!x); + let completions = this + .getWorkspaces() + .flatMap(workspace => workspace.builder.program.getCompletions(filePath, position)); for (let completion of completions) { completion.commitCharacters = ['.']; @@ -804,10 +792,13 @@ export class LanguageServer { //if we got a dest path, then the program wants this file if (destPath) { - await program.addOrReplaceFile({ - src: change.pathAbsolute, - dest: rokuDeploy.getDestPath(change.pathAbsolute, options.files, rootDir) - }); + program.addOrReplaceFile( + { + src: change.pathAbsolute, + dest: rokuDeploy.getDestPath(change.pathAbsolute, options.files, rootDir) + }, + await workspace.builder.getFileContents(change.pathAbsolute) + ); } else { //no dest path means the program doesn't want this file } @@ -816,11 +807,14 @@ export class LanguageServer { } else if (program.hasFile(change.pathAbsolute)) { //sometimes "changed" events are emitted on files that were actually deleted, //so determine file existance and act accordingly - if (await util.fileExists(change.pathAbsolute)) { - await program.addOrReplaceFile({ - src: change.pathAbsolute, - dest: rokuDeploy.getDestPath(change.pathAbsolute, options.files, rootDir) - }); + if (await util.pathExists(change.pathAbsolute)) { + program.addOrReplaceFile( + { + src: change.pathAbsolute, + dest: rokuDeploy.getDestPath(change.pathAbsolute, options.files, rootDir) + }, + await workspace.builder.getFileContents(change.pathAbsolute) + ); } else { program.removeFile(change.pathAbsolute); } @@ -875,26 +869,23 @@ export class LanguageServer { try { //throttle file processing. first call is run immediately, and then the last call is processed. - await this.keyedThrottler.run(filePath, async () => { + await this.keyedThrottler.run(filePath, () => { this.connection.sendNotification('build-status', 'building'); let documentText = textDocument.getText(); - let workspaces = this.getWorkspaces(); - await Promise.all( - workspaces.map(async (x) => { - //only add or replace existing files. All of the files in the project should - //have already been loaded by other means - if (x.builder.program.hasFile(filePath)) { - let rootDir = x.builder.program.options.rootDir ?? x.builder.program.options.cwd; - let dest = rokuDeploy.getDestPath(filePath, x.builder.program.options.files, rootDir); - await x.builder.program.addOrReplaceFile({ - src: filePath, - dest: dest - }, documentText); - } - }) - ); + for (const workspace of this.getWorkspaces()) { + //only add or replace existing files. All of the files in the project should + //have already been loaded by other means + if (workspace.builder.program.hasFile(filePath)) { + let rootDir = workspace.builder.program.options.rootDir ?? workspace.builder.program.options.cwd; + let dest = rokuDeploy.getDestPath(filePath, workspace.builder.program.options.files, rootDir); + workspace.builder.program.addOrReplaceFile({ + src: filePath, + dest: dest + }, documentText); + } + } }); // validate all workspaces await this.validateAllThrottled(); diff --git a/src/Program.ts b/src/Program.ts index 3ffa3ad2a..94f005c0b 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -73,12 +73,6 @@ export class Program { this.options.rootDir = util.getRootDir(this.options); this.createGlobalScope(); - - //add the default file resolver (used by this program to load source file contents). - this.fileResolvers.push(async (pathAbsolute) => { - let contents = await this.util.getFileContents(pathAbsolute); - return contents; - }); } public logger: Logger; @@ -106,16 +100,6 @@ export class Program { private diagnosticFilterer = new DiagnosticFilterer(); - private util = util; - - /** - * A list of functions that will be used to load file contents. - * In most cases, there will only be the "read from filesystem" resolver. - * However, when running inside the LanguageServer, a second resolver will be added - * to resolve the opened file contents from memory instead of going to disk. - */ - public fileResolvers = [] as FileResolver[]; - /** * A scope that contains all built-in global functions. * All scopes should directly or indirectly inherit from this scope @@ -180,23 +164,6 @@ export class Program { delete this.components[(xmlFile.componentName?.text ?? xmlFile.pkgPath).toLowerCase()]; } - /** - * Get the contents of the specified file as a string. - * This walks backwards through the file resolvers until we get a value. - * This allow the language server to provide file contents directly from memory. - */ - public async getFileContents(pathAbsolute: string) { - pathAbsolute = s`${pathAbsolute}`; - let reversedResolvers = [...this.fileResolvers].reverse(); - for (let fileResolver of reversedResolvers) { - let result = await fileResolver(pathAbsolute); - if (typeof result === 'string') { - return result; - } - } - throw new Error(`Could not load file "${pathAbsolute}"`); - } - /** * Get a list of all files that are included in the project but are not referenced * by any scope in the program. @@ -257,22 +224,6 @@ export class Program { return this.files[filePath] !== undefined; } - /** - * Add and parse all of the provided files. - * Files that are already loaded will be replaced by the latest - * contents from the file system. - * @param filePaths - */ - public async addOrReplaceFiles(fileObjects: Array) { - let promises = []; - for (let fileObject of fileObjects) { - promises.push( - this.addOrReplaceFile(fileObject) - ); - } - return Promise.all(promises) as Promise; - } - public getPkgPath(...args: any[]): any { //eslint-disable-line throw new Error('Not implemented'); } @@ -310,17 +261,16 @@ export class Program { * Load a file into the program. If that file already exists, it is replaced. * If file contents are provided, those are used, Otherwise, the file is loaded from the file system * @param relativePath the file path relative to the root dir - * @param fileContents the file contents. If not provided, the file will be loaded from disk + * @param fileContents the file contents */ - public async addOrReplaceFile(relativePath: string, fileContents?: string): Promise; + public addOrReplaceFile(relativePath: string, fileContents: string): T; /** * Load a file into the program. If that file already exists, it is replaced. - * If file contents are provided, those are used, Otherwise, the file is loaded from the file system * @param fileEntry an object that specifies src and dest for the file. * @param fileContents the file contents. If not provided, the file will be loaded from disk */ - public async addOrReplaceFile(fileEntry: FileObj, fileContents?: string): Promise; - public async addOrReplaceFile(fileParam: FileObj | string, fileContents?: string): Promise { + public addOrReplaceFile(fileEntry: FileObj, fileContents: string): T; + public addOrReplaceFile(fileParam: FileObj | string, fileContents: string): T { assert.ok(fileParam, 'fileEntry is required'); let srcPath: string; let pkgPath: string; @@ -331,7 +281,7 @@ export class Program { srcPath = s`${fileParam.src}`; pkgPath = s`${fileParam.dest}`; } - let file = await this.logger.time(LogLevel.debug, ['Program.addOrReplaceFile()', chalk.green(srcPath)], async () => { + let file = this.logger.time(LogLevel.debug, ['Program.addOrReplaceFile()', chalk.green(srcPath)], () => { assert.ok(srcPath, 'fileEntry.src is required'); assert.ok(pkgPath, 'fileEntry.dest is required'); @@ -343,15 +293,6 @@ export class Program { let fileExtension = path.extname(srcPath).toLowerCase(); let file: BscFile | undefined; - //load the file contents by file path if not provided - let getFileContents = async () => { - if (fileContents === undefined) { - return this.getFileContents(srcPath); - } else { - return fileContents; - } - }; - if (fileExtension === '.brs' || fileExtension === '.bs') { let brsFile = new BrsFile(srcPath, pkgPath, this); @@ -365,14 +306,14 @@ export class Program { //add the file to the program this.files[srcPath] = brsFile; this.pkgMap[brsFile.pkgPath.toLowerCase()] = brsFile; - let fileContents: SourceObj = { + let sourceObj: SourceObj = { pathAbsolute: srcPath, - source: await getFileContents() + source: fileContents }; - this.plugins.emit('beforeFileParse', fileContents); + this.plugins.emit('beforeFileParse', sourceObj); this.logger.time(LogLevel.info, ['parse', chalk.green(srcPath)], () => { - brsFile.parse(fileContents.source); + brsFile.parse(sourceObj.source); }); file = brsFile; @@ -389,12 +330,12 @@ export class Program { //add the file to the program this.files[srcPath] = xmlFile; this.pkgMap[xmlFile.pkgPath.toLowerCase()] = xmlFile; - let fileContents: SourceObj = { + let sourceObj: SourceObj = { pathAbsolute: srcPath, - source: await getFileContents() + source: fileContents }; - this.plugins.emit('beforeFileParse', fileContents); - xmlFile.parse(fileContents.source); + this.plugins.emit('beforeFileParse', sourceObj); + xmlFile.parse(sourceObj.source); file = xmlFile; @@ -525,8 +466,8 @@ export class Program { * Traverse the entire project, and validate all scopes * @param force - if true, then all scopes are force to validate, even if they aren't marked as dirty */ - public async validate() { - await this.logger.time(LogLevel.debug, ['Program.validate()'], async () => { + public validate() { + this.logger.time(LogLevel.debug, ['Program.validate()'], () => { this.diagnostics = []; this.plugins.emit('beforeProgramValidate', this); @@ -553,7 +494,6 @@ export class Program { this.detectDuplicateComponentNames(); this.plugins.emit('afterProgramValidate', this); - await Promise.resolve(); }); } @@ -703,7 +643,7 @@ export class Program { * @param lineIndex * @param columnIndex */ - public async getCompletions(pathAbsolute: string, position: Position) { + public getCompletions(pathAbsolute: string, position: Position) { let file = this.getFile(pathAbsolute); if (!file) { return []; @@ -737,7 +677,6 @@ export class Program { c => c ); - //only keep completions common to every scope for this file let keyCounts = {} as Record; for (let completion of allCompletions) { @@ -747,7 +686,7 @@ export class Program { result.push(completion); } } - return Promise.resolve(result); + return result; } /** @@ -1192,4 +1131,8 @@ export class Program { } } -export type FileResolver = (pathAbsolute: string) => string | undefined | Thenable | void; +export interface FileResolver { + readFile(pathAbsolute: string): string | undefined | Thenable | void; + readFileSync(pathAbsolute: string): string | undefined; +} + diff --git a/src/ProgramBuilder.ts b/src/ProgramBuilder.ts index 9e87ed1b9..e4d1880bc 100644 --- a/src/ProgramBuilder.ts +++ b/src/ProgramBuilder.ts @@ -11,10 +11,16 @@ import { DiagnosticSeverity } from 'vscode-languageserver'; import { Logger, LogLevel } from './Logger'; import PluginInterface from './PluginInterface'; import * as diagnosticUtils from './diagnosticUtils'; + /** * A runner class that handles */ export class ProgramBuilder { + + public constructor() { + //add the default file resolver (used to load source file contents). + this.addFileResolver(util.fileResolver); + } /** * Determines whether the console should be cleared after a run (true for cli, false for languageserver) */ @@ -30,9 +36,40 @@ export class ProgramBuilder { public addFileResolver(fileResolver: FileResolver) { this.fileResolvers.push(fileResolver); - if (this.program) { - this.program.fileResolvers.push(fileResolver); + } + + /** + * Get the contents of the specified file as a string. + * This walks backwards through the file resolvers until we get a value. + * This allow the language server to provide file contents directly from memory. + */ + public async getFileContents(pathAbsolute: string) { + pathAbsolute = s`${pathAbsolute}`; + let reversedResolvers = [...this.fileResolvers].reverse(); + for (let fileResolver of reversedResolvers) { + let result = await fileResolver.readFile(pathAbsolute); + if (typeof result === 'string') { + return result; + } } + throw new Error(`Could not load file "${pathAbsolute}"`); + } + + /** + * Get the contents of the specified file as a string. + * This walks backwards through the file resolvers until we get a value. + * This allow the language server to provide file contents directly from memory. + */ + public getFileContentsSync(pathAbsolute: string) { + pathAbsolute = s`${pathAbsolute}`; + let reversedResolvers = [...this.fileResolvers].reverse(); + for (let fileResolver of reversedResolvers) { + let result = fileResolver.readFileSync(pathAbsolute); + if (typeof result === 'string') { + return result; + } + } + throw new Error(`Could not load file "${pathAbsolute}"`); } /** @@ -106,9 +143,6 @@ export class ProgramBuilder { protected createProgram() { const program = new Program(this.options, undefined, this.plugins); - //add the initial FileResolvers - program.fileResolvers.push(...this.fileResolvers); - this.plugins.emit('afterProgramCreate', program); return program; } @@ -163,10 +197,15 @@ export class ProgramBuilder { this.watcher.on('all', async (event: string, thePath: string) => { //eslint-disable-line @typescript-eslint/no-misused-promises thePath = s`${path.resolve(this.rootDir, thePath)}`; if (event === 'add' || event === 'change') { - await this.program.addOrReplaceFile({ + const fileObj = { src: thePath, dest: rokuDeploy.getDestPath(thePath, this.program.options.files, this.rootDir) - }); + }; + this.program.addOrReplaceFile( + fileObj, + //load the file synchronously because that's faster than async for a small number of filies + await this.getFileContents(fileObj.src) + ); } else if (event === 'unlink') { this.program.removeFile(thePath); } @@ -273,7 +312,7 @@ export class ProgramBuilder { } this.logger.log('Validating project'); //validate program - await this.validateProject(); + this.validateProject(); //maybe cancel? if (cancellationToken.isCanceled === true) { @@ -408,9 +447,12 @@ export class ProgramBuilder { //preload every type definition file first, which eliminates duplicate file loading await Promise.all( - typedefFiles.map(async (file) => { + typedefFiles.map(async (fileObj) => { try { - await this.program.addOrReplaceFile(file); + this.program.addOrReplaceFile( + fileObj, + await this.getFileContents(fileObj.src) + ); } catch (e) { //log the error, but don't fail this process because the file might be fixable later this.logger.log(e); @@ -418,15 +460,19 @@ export class ProgramBuilder { }) ); + const acceptableExtensions = ['.bs', '.brs', '.xml']; //parse every file other than the type definitions await Promise.all( - nonTypedefFiles.map(async (file) => { + nonTypedefFiles.map(async (fileObj) => { try { - let fileExtension = path.extname(file.src).toLowerCase(); + let fileExtension = path.extname(fileObj.src).toLowerCase(); //only process certain file types - if (['.bs', '.brs', '.xml'].includes(fileExtension)) { - await this.program.addOrReplaceFile(file); + if (acceptableExtensions.includes(fileExtension)) { + this.program.addOrReplaceFile( + fileObj, + await this.getFileContents(fileObj.src) + ); } } catch (e) { //log the error, but don't fail this process because the file might be fixable later @@ -454,8 +500,8 @@ export class ProgramBuilder { * Scan every file and resolve all variable references. * If no errors were encountered, return true. Otherwise return false. */ - private async validateProject() { - await this.program.validate(); + private validateProject() { + this.program.validate(); } public dispose() { diff --git a/src/util.ts b/src/util.ts index 518826159..1bdc60f9f 100644 --- a/src/util.ts +++ b/src/util.ts @@ -9,7 +9,7 @@ import { URI } from 'vscode-uri'; import * as xml2js from 'xml2js'; import type { BsConfig } from './BsConfig'; import { DiagnosticMessages } from './DiagnosticMessages'; -import type { CallableContainer, BsDiagnostic, FileReference, CallableContainerMap, CompilerPluginFactory } from './interfaces'; +import type { CallableContainer, BsDiagnostic, FileReference, CallableContainerMap, CompilerPluginFactory, CompilerPlugin } from './interfaces'; import { BooleanType } from './types/BooleanType'; import { DoubleType } from './types/DoubleType'; import { DynamicType } from './types/DynamicType'; @@ -26,12 +26,23 @@ import type { DottedGetExpression, VariableExpression } from './parser/Expressio import { Logger, LogLevel } from './Logger'; import type { Token } from './lexer'; import { TokenKind } from './lexer'; -import type { CompilerPlugin } from '.'; import { isBrsFile, isDottedGetExpression, isVariableExpression } from './astUtils'; import { CustomType } from './types/CustomType'; +import type { FileResolver } from './Program'; export class Util { + public fileResolver = { + readFile: (filePath) => { + return fsExtra.readFile(filePath).then((value) => { + return value.toString(); + }); + }, + readFileSync: (filePath) => { + return fsExtra.readFileSync(filePath).toString(); + } + } as FileResolver; + public clearConsole() { // process.stdout.write('\x1Bc'); } @@ -40,7 +51,7 @@ export class Util { * Determine if the file exists * @param filePath */ - public async fileExists(filePath: string | undefined) { + public async pathExists(filePath: string | undefined) { if (!filePath) { return false; } else { @@ -49,20 +60,22 @@ export class Util { } /** - * Determine if this path is a directory + * Determine if the file exists + * @param filePath */ - public isDirectorySync(dirPath: string | undefined) { - return fs.existsSync(dirPath) && fs.lstatSync(dirPath).isDirectory(); + public pathExistsSync(filePath: string | undefined) { + if (!filePath) { + return false; + } else { + return fsExtra.pathExistsSync(filePath); + } } /** - * Load a file from disc into a string - * @param filePath + * Determine if this path is a directory */ - public async getFileContents(filePath: string) { - let file = await fsExtra.readFile(filePath); - let fileContents = file.toString(); - return fileContents; + public isDirectorySync(dirPath: string | undefined) { + return fs.existsSync(dirPath) && fs.lstatSync(dirPath).isDirectory(); } /** @@ -95,7 +108,7 @@ export class Util { let configPath = path.join(cwd, 'bsconfig.json'); //find the nearest config file path for (let i = 0; i < 100; i++) { - if (await this.fileExists(configPath)) { + if (await this.pathExists(configPath)) { return configPath; } else { let parentDirPath = path.dirname(path.dirname(configPath)); @@ -149,7 +162,7 @@ export class Util { throw new Error('Circular dependency detected: "' + parentProjectPaths.join('" => ') + '"'); } //load the project file - let projectFileContents = await this.getFileContents(configFilePath); + let projectFileContents = (await fsExtra.readFile(configFilePath)).toString(); let parseErrors = [] as ParseError[]; let projectConfig = parseJsonc(projectFileContents, parseErrors) as BsConfig; if (parseErrors.length > 0) { @@ -752,9 +765,9 @@ export class Util { let bsPath = path.join(currentPath, 'bsconfig.json'); let brsPath = path.join(currentPath, 'brsconfig.json'); - if (await this.fileExists(bsPath)) { + if (await this.pathExists(bsPath)) { return bsPath; - } else if (await this.fileExists(brsPath)) { + } else if (await this.pathExists(brsPath)) { return brsPath; } else { //walk upwards one directory @@ -1130,6 +1143,5 @@ export function standardizePath(stringParts, ...expressions: any[]) { ); } - export let util = new Util(); export default util; From 92776895fe1e5cf4751816e6a690e1e3c4f080e0 Mon Sep 17 00:00:00 2001 From: Bronley Date: Mon, 18 Jan 2021 06:34:29 -0500 Subject: [PATCH 2/5] test changes to match new api --- src/FunctionScope.spec.ts | 4 +- src/LanguageServer.spec.ts | 53 +- src/Program.spec.ts | 728 ++++++++---------- src/ProgramBuilder.spec.ts | 36 +- src/Scope.spec.ts | 272 +++---- src/XmlScope.spec.ts | 38 +- src/astUtils/visitors.spec.ts | 112 +-- src/files/BrsFile.Class.spec.ts | 226 +++--- src/files/BrsFile.spec.ts | 508 ++++++------ src/files/XmlFile.spec.ts | 214 ++--- src/files/tests/imports.spec.ts | 74 +- src/globalCallables.spec.ts | 24 +- src/parser/Statement.spec.ts | 4 +- .../SourceLiteralExpression.spec.ts | 48 +- .../TemplateStringExpression.spec.ts | 52 +- src/preprocessor/Manifest.spec.ts | 2 +- src/util.spec.ts | 2 +- 17 files changed, 1174 insertions(+), 1223 deletions(-) diff --git a/src/FunctionScope.spec.ts b/src/FunctionScope.spec.ts index ef1cebf1c..99f632d63 100644 --- a/src/FunctionScope.spec.ts +++ b/src/FunctionScope.spec.ts @@ -22,8 +22,8 @@ describe('FunctionScope', () => { expect(variables).to.be.lengthOf(0); }); - it('returns variables defined above the specified line number', async () => { - let file = await program.addOrReplaceFile({ src: `${rootDir}/source/main.brs`, dest: 'source/main.brs' }, ` + it('returns variables defined above the specified line number', () => { + let file = program.addOrReplaceFile({ src: `${rootDir}/source/main.brs`, dest: 'source/main.brs' }, ` sub main() var1 = 1 var2 = 2 diff --git a/src/LanguageServer.spec.ts b/src/LanguageServer.spec.ts index 0910526db..79f84d584 100644 --- a/src/LanguageServer.spec.ts +++ b/src/LanguageServer.spec.ts @@ -103,7 +103,7 @@ describe('LanguageServer', () => { server.dispose(); }); - async function addXmlFile(name: string, additionalXmlContents = '') { + function addXmlFile(name: string, additionalXmlContents = '') { const filePath = `components/${name}.xml`; const contents = ` @@ -111,12 +111,12 @@ describe('LanguageServer', () => { ${additionalXmlContents}