Improving null safety: Add FinalizedBsConfig and tweak plugin events

src/BsConfig.ts

@@ -209,3 +209,22 @@ export interface BsConfig {
      */
     bslibDestinationDir?: string;
 }
+
+type OptionalBsConfigFields =
+    | '_ancestors'
+    | 'sourceRoot'
+    | 'project'
+    | 'manifest'
+    | 'noProject'
+    | 'extends'
+    | 'host'
+    | 'password'
+    | 'require'
+    | 'stagingFolderPath'
+    | 'diagnosticLevel'
+    | 'rootDir'
+    | 'stagingDir';
+
+export type FinalizedBsConfig =
+    Omit<Required<BsConfig>, OptionalBsConfigFields>
+    & Pick<BsConfig, OptionalBsConfigFields>;

src/PluginInterface.ts

@@ -2,10 +2,26 @@ import type { CompilerPlugin } from './interfaces';
 import type { Logger } from './Logger';
 import { LogLevel } from './Logger';
 
-// inspiration: https://github.com/andywer/typed-emitter/blob/master/index.d.ts
-export type Arguments<T> = [T] extends [(...args: infer U) => any]
-    ? U
-    : [T] extends [void] ? [] : [T];
+/*
+ * we use `Required` everywhere here because we expect that the methods on plugin objects will
+ * be optional, and we don't want to deal with `undefined`.
+ * `extends (...args: any[]) => any` determines whether the thing we're dealing with is a function.
+ * Returning `never` in the `as` clause of the `[key in object]` step deletes that key from the
+ * resultant object.
+ * on the right-hand side of the mapped type we are forced to use a conditional type a second time,
+ * in order to be able to use the `Parameters` utility type on `Required<T>[K]`. This will always
+ * be true because of the filtering done by the `[key in object]` clause, but TS requires the duplication.
+ *
+ * so put together: we iterate over all of the fields in T, deleting ones which are not (potentially
+ * optional) functions. For the ones that are, we replace them with their parameters.
+ *
+ * this returns the type of an object whose keys are the names of the methods of T and whose values
+ * are tuples containing the arguments that each method accepts.
+ */
+export type PluginEventArgs<T> = {
+    [K in keyof Required<T> as Required<T>[K] extends (...args: any[]) => any ? K : never]:
+    Required<T>[K] extends (...args: any[]) => any ? Parameters<Required<T>[K]> : never
+};
 
 export default class PluginInterface<T extends CompilerPlugin = CompilerPlugin> {
 
@@ -48,7 +64,7 @@ export default class PluginInterface<T extends CompilerPlugin = CompilerPlugin>
     /**
      * Call `event` on plugins
      */
-    public emit<K extends keyof T & string>(event: K, ...args: Arguments<T[K]>) {
+    public emit<K extends keyof PluginEventArgs<T> & string>(event: K, ...args: PluginEventArgs<T>[K]) {
         for (let plugin of this.plugins) {
             if ((plugin as any)[event]) {
                 try {

src/Program.spec.ts

@@ -3111,9 +3111,9 @@ describe('Program', () => {
                 supports_input_launch=1
                 bs_const=DEBUG=false
             `);
-            program.options = {
+            program.options = util.normalizeConfig({
                 rootDir: tempDir
-            };
+            });
         });
 
         afterEach(() => {

src/Program.ts

@@ -3,7 +3,7 @@ import * as fsExtra from 'fs-extra';
 import * as path from 'path';
 import type { CodeAction, CompletionItem, Position, Range, SignatureInformation, Location } from 'vscode-languageserver';
 import { CompletionItemKind } from 'vscode-languageserver';
-import type { BsConfig } from './BsConfig';
+import type { BsConfig, FinalizedBsConfig } from './BsConfig';
 import { Scope } from './Scope';
 import { DiagnosticMessages } from './DiagnosticMessages';
 import { BrsFile } from './files/BrsFile';
@@ -60,7 +60,7 @@ export class Program {
         /**
          * The root directory for this program
          */
-        public options: BsConfig,
+        options: BsConfig,
         logger?: Logger,
         plugins?: PluginInterface
     ) {
@@ -77,6 +77,7 @@ export class Program {
         this.createGlobalScope();
     }
 
+    public options: FinalizedBsConfig;
     public logger: Logger;
 
     private createGlobalScope() {
@@ -109,7 +110,7 @@ export class Program {
      * A scope that contains all built-in global functions.
      * All scopes should directly or indirectly inherit from this scope
      */
-    public globalScope: Scope;
+    public globalScope: Scope = undefined as any;
 
     /**
      * Plugins which can provide extra diagnostics or transform AST
@@ -320,15 +321,15 @@ export class Program {
     /**
      * roku filesystem is case INsensitive, so find the scope by key case insensitive
      */
-    public getScopeByName(scopeName: string) {
+    public getScopeByName(scopeName: string): Scope | undefined {
         if (!scopeName) {
             return undefined;
         }
         //most scopes are xml file pkg paths. however, the ones that are not are single names like "global" and "scope",
         //so it's safe to run the standardizePkgPath method
         scopeName = s`${scopeName}`;
         let key = Object.keys(this.scopes).find(x => x.toLowerCase() === scopeName.toLowerCase());
-        return this.scopes[key];
+        return this.scopes[key!];
     }
 
     /**
@@ -495,8 +496,8 @@ export class Program {
      * @param rootDir must be a pre-normalized path
      */
     private getPaths(fileParam: string | FileObj | { srcPath?: string; pkgPath?: string }, rootDir: string) {
-        let srcPath: string;
-        let pkgPath: string;
+        let srcPath: string | undefined;
+        let pkgPath: string | undefined;
 
         assert.ok(fileParam, 'fileParam is required');
 
@@ -631,7 +632,7 @@ export class Program {
                 this.plugins.emit('beforeScopeDispose', scope);
                 scope.dispose();
                 //notify dependencies of this scope that it has been removed
-                this.dependencyGraph.remove(scope.dependencyGraphKey);
+                this.dependencyGraph.remove(scope.dependencyGraphKey!);
                 delete this.scopes[file.pkgPath];
                 this.plugins.emit('afterScopeDispose', scope);
             }
@@ -777,15 +778,15 @@ export class Program {
      * @param file the file
      */
     public getScopesForFile(file: XmlFile | BrsFile | string) {
-        if (typeof file === 'string') {
-            file = this.getFile(file);
-        }
+
+        const resolvedFile = typeof file === 'string' ? this.getFile(file) : file;
+
         let result = [] as Scope[];
-        if (file) {
+        if (resolvedFile) {
             for (let key in this.scopes) {
                 let scope = this.scopes[key];
 
-                if (scope.hasFile(file)) {
+                if (scope.hasFile(resolvedFile)) {
                     result.push(scope);
                 }
             }

src/ProgramBuilder.ts

@@ -1,7 +1,7 @@
 import * as debounce from 'debounce-promise';
 import * as path from 'path';
 import { rokuDeploy } from 'roku-deploy';
-import type { BsConfig } from './BsConfig';
+import type { BsConfig, FinalizedBsConfig } from './BsConfig';
 import type { BscFile, BsDiagnostic, FileObj, FileResolver } from './interfaces';
 import { Program } from './Program';
 import { standardizePath as s, util } from './util';
@@ -33,10 +33,10 @@ export class ProgramBuilder {
      */
     public allowConsoleClearing = true;
 
-    public options: BsConfig;
+    public options: FinalizedBsConfig = util.normalizeConfig({});
     private isRunning = false;
-    private watcher: Watcher;
-    public program: Program;
+    private watcher: Watcher | undefined;
+    public program: Program | undefined;
     public logger = new Logger();
     public plugins: PluginInterface = new PluginInterface([], { logger: this.logger });
     private fileResolvers = [] as FileResolver[];
@@ -68,7 +68,10 @@ export class ProgramBuilder {
     private staticDiagnostics = [] as BsDiagnostic[];
 
     public addDiagnostic(srcPath: string, diagnostic: Partial<BsDiagnostic>) {
-        let file: BscFile = this.program.getFile(srcPath);
+        if (!this.program) {
+            throw new Error('Cannot call `ProgramBuilder.addDiagnostic` before `ProgramBuilder.run()`');
+        }
+        let file: BscFile | undefined = this.program.getFile(srcPath);
         if (!file) {
             file = {
                 pkgPath: this.program.getPkgPath(srcPath),
@@ -180,7 +183,7 @@ export class ProgramBuilder {
      * A handle for the watch mode interval that keeps the process alive.
      * We need this so we can clear it if the builder is disposed
      */
-    private watchInterval: NodeJS.Timer;
+    private watchInterval: NodeJS.Timer | undefined;
 
     public enableWatchMode() {
         this.watcher = new Watcher(this.options);
@@ -211,6 +214,9 @@ export class ProgramBuilder {
 
         //on any file watcher event
         this.watcher.on('all', async (event: string, thePath: string) => { //eslint-disable-line @typescript-eslint/no-misused-promises
+            if (!this.program) {
+                throw new Error('Internal invariant exception: somehow file watcher ran before `ProgramBuilder.run()`');
+            }
             thePath = s`${path.resolve(this.rootDir, thePath)}`;
             if (event === 'add' || event === 'change') {
                 const fileObj = {
@@ -238,6 +244,9 @@ export class ProgramBuilder {
      * The rootDir for this program.
      */
     public get rootDir() {
+        if (!this.program) {
+            throw new Error('Cannot access `ProgramBuilder.rootDir` until after `ProgramBuilder.run()`');
+        }
         return this.program.options.rootDir;
     }
 
@@ -412,16 +421,19 @@ export class ProgramBuilder {
                     logLevel: this.options.logLevel as LogLevel,
                     outDir: util.getOutDir(this.options),
                     outFile: path.basename(this.options.outFile)
-                });
+
+                    //rokuDeploy's return type says all its fields can be nullable, but it sets values for all of them.
+                }) as any as Required<ReturnType<typeof rokuDeploy.getOptions>>;
             });
 
             //get every file referenced by the files array
             let fileMap = await rokuDeploy.getFilePaths(options.files, options.rootDir);
 
             //remove files currently loaded in the program, we will transpile those instead (even if just for source maps)
             let filteredFileMap = [] as FileObj[];
+
             for (let fileEntry of fileMap) {
-                if (this.program.hasFile(fileEntry.src) === false) {
+                if (this.program!.hasFile(fileEntry.src) === false) {
                     filteredFileMap.push(fileEntry);
                 }
             }
@@ -441,7 +453,7 @@ export class ProgramBuilder {
 
             await this.logger.time(LogLevel.log, ['Transpiling'], async () => {
                 //transpile any brighterscript files
-                await this.program.transpile(fileMap, options.stagingDir);
+                await this.program!.transpile(fileMap, options.stagingDir);
             });
 
             this.plugins.emit('afterPublish', this, fileMap);
@@ -492,12 +504,12 @@ export class ProgramBuilder {
             }
 
             if (manifestFile) {
-                this.program.loadManifest(manifestFile);
+                this.program!.loadManifest(manifestFile);
             }
 
             const loadFile = async (fileObj) => {
                 try {
-                    this.program.setFile(fileObj, await this.getFileContents(fileObj.src));
+                    this.program!.setFile(fileObj, await this.getFileContents(fileObj.src));
                 } catch (e) {
                     this.logger.log(e); // log the error, but don't fail this process because the file might be fixable later
                 }

src/Scope.ts

@@ -369,8 +369,8 @@ export class Scope {
      * XmlScope overrides this to return the parent xml scope if available.
      * For globalScope this will return null.
      */
-    public getParentScope() {
-        let scope: Scope;
+    public getParentScope(): Scope | null {
+        let scope: Scope | undefined;
         //use the global scope if we didn't find a sope and this is not the global scope
         if (this.program.globalScope !== this) {
             scope = this.program.globalScope;

src/XmlScope.ts

@@ -27,15 +27,15 @@ export class XmlScope extends Scope {
      */
     public getParentScope() {
         return this.cache.getOrAdd('parentScope', () => {
-            let scope: Scope;
+            let scope: Scope | undefined;
             let parentComponentName = this.xmlFile.parentComponentName?.text;
             if (parentComponentName) {
                 scope = this.program.getComponentScope(parentComponentName);
             }
             if (scope) {
                 return scope;
             } else {
-                return this.program.globalScope;
+                return this.program.globalScope ?? null;
             }
         });
     }
@@ -64,7 +64,7 @@ export class XmlScope extends Scope {
             } else if (!callableContainerMap.has(name.toLowerCase())) {
                 this.diagnostics.push({
                     ...DiagnosticMessages.xmlFunctionNotFound(name),
-                    range: fun.getAttribute('name').value.range,
+                    range: fun.getAttribute('name')?.value.range,
                     file: this.xmlFile
                 });
             }
@@ -82,15 +82,15 @@ export class XmlScope extends Scope {
             } else if (!SGFieldTypes.includes(type.toLowerCase())) {
                 this.diagnostics.push({
                     ...DiagnosticMessages.xmlInvalidFieldType(type),
-                    range: field.getAttribute('type').value.range,
+                    range: field.getAttribute('type')?.value.range,
                     file: this.xmlFile
                 });
             }
             if (onChange) {
                 if (!callableContainerMap.has(onChange.toLowerCase())) {
                     this.diagnostics.push({
                         ...DiagnosticMessages.xmlFunctionNotFound(onChange),
-                        range: field.getAttribute('onchange').value.range,
+                        range: field.getAttribute('onchange')?.value.range,
                         file: this.xmlFile
                     });
                 }

src/files/XmlFile.ts

@@ -398,8 +398,8 @@ export class XmlFile {
     /**
      * Walk up the ancestor chain and aggregate all of the script tag imports
      */
-    public getAncestorScriptTagImports() {
-        let result = [];
+    public getAncestorScriptTagImports(): FileReference[] {
+        let result = [] as FileReference[];
         let parent = this.parentComponent;
         while (parent) {
             result.push(...parent.scriptTagImports);