Plugins as factory

docs/plugins.md

@@ -111,6 +111,8 @@ The top level object is the `ProgramBuilder` which runs the overall process: pre
 ### API definition
 
 ```typescript
+export type CompilerPluginFactory = () => CompilierPlugin;
+
 export interface CompilerPlugin {
     name: string;
     beforeProgramCreate?: (builder: ProgramBuilder) => void;
@@ -165,7 +167,7 @@ interface CallableContainer {
 
 ## Plugin API
 
-Plugins are JavaScript modules dynamically loaded by the compiler. Their entry point is a required default object following the `CompilerPlugin` interface and exposing event handlers.
+Plugins are JavaScript modules dynamically loaded by the compiler. Their entry point is a default function that returns an object that follows the `CompilerPlugin` interface.
 
 To walk/modify the AST, a number of helpers are provided in `brighterscript/dist/parser/ASTUtils`.
 
@@ -198,30 +200,29 @@ Note: in a language-server context, Scope validation happens every time a file c
 import { CompilerPlugin, BrsFile, XmlFile } from 'brighterscript';
 import { isBrsFile } from 'brighterscript/dist/parser/ASTUtils';
 
-// entry point
-const pluginInterface: CompilerPlugin = {
-    name: 'myDiagnosticPlugin',
-    afterFileValidate
-};
-export default pluginInterface;
-
-// post-parsing validation
-function afterFileValidate(file: BscFile) {
-    if (!isBrsFile(file)) {
-        return;
-    }
-    // visit function statements and validate their name
-    file.parser.functionStatements.forEach((fun) => {
-        if (fun.name.text.toLowerCase() === 'main') {
-            file.addDiagnostics([{
-                code: 9000,
-                message: 'Use RunUserInterface as entry point',
-                range: fun.name.range,
-                file
-            }]);
+// plugin factory
+export default function () {
+    return {
+        name: 'myDiagnosticPlugin',
+        // post-parsing validation
+        afterFileValidate: (file: BscFile) => {
+            if (!isBrsFile(file)) {
+                return;
+            }
+            // visit function statements and validate their name
+            file.parser.functionStatements.forEach((fun) => {
+                if (fun.name.text.toLowerCase() === 'main') {
+                    file.addDiagnostics([{
+                        code: 9000,
+                        message: 'Use RunUserInterface as entry point',
+                        range: fun.name.range,
+                        file
+                    }]);
+                }
+            });
         }
-    });
-}
+    } as CompilerPlugin;
+};
 ```
 
 ### Example AST modifier plugin
@@ -234,23 +235,22 @@ import { CompilerPlugin, Program, TranspileObj } from 'brighterscript';
 import { EmptyStatement } from 'brighterscript/dist/parser';
 import { isBrsFile, createStatementEditor, editStatements } from 'brighterscript/dist/parser/ASTUtils';
 
-// entry point
-const pluginInterface: CompilerPlugin = {
-    name: 'removePrint',
-    beforeFileTranspile
+// plugin factory
+export default function () {
+    return {
+        name: 'removePrint',
+        // transform AST before transpilation
+        beforeFileTranspile: (entry: TranspileObj) => {
+            if (isBrsFile(entry.file)) {
+                // visit functions bodies and replace `PrintStatement` nodes with `EmptyStatement`
+                entry.file.parser.functionExpressions.forEach((fun) => {
+                    const visitor = createStatementEditor({
+                        PrintStatement: (statement) => new EmptyStatement()
+                    });
+                    editStatements(fun.body, visitor);
+                });
+            }
+        }
+    } as CompilerPlugin;
 };
-export default pluginInterface;
-
-// transform AST before transpilation
-function beforeFileTranspile(entry: TranspileObj) {
-    if (isBrsFile(entry.file)) {
-        // visit functions bodies and replace `PrintStatement` nodes with `EmptyStatement`
-        entry.file.parser.functionExpressions.forEach((fun) => {
-            const visitor = createStatementEditor({
-                PrintStatement: (statement) => new EmptyStatement()
-            });
-            editStatements(fun.body, visitor);
-        });
-    }
-}
 ```

src/ProgramBuilder.ts

@@ -5,7 +5,7 @@ import type { BsConfig } from './BsConfig';
 import type { BsDiagnostic, File, FileObj } from './interfaces';
 import type { FileResolver } from './Program';
 import { Program } from './Program';
-import { standardizePath as s, util, loadPlugins } from './util';
+import { standardizePath as s, util } from './util';
 import { Watcher } from './Watcher';
 import { DiagnosticSeverity } from 'vscode-languageserver';
 import { Logger, LogLevel } from './Logger';
@@ -114,7 +114,7 @@ export class ProgramBuilder {
     }
 
     protected loadPlugins() {
-        const plugins = loadPlugins(
+        const plugins = util.loadPlugins(
             this.options.cwd ?? process.cwd(),
             this.options.plugins,
             (pathOrModule, err) => this.logger.error(`Error when loading plugin '${pathOrModule}':`, err)

src/examples/plugins/removePrint.ts

@@ -3,25 +3,24 @@ import { createVisitor, WalkMode } from '../../astUtils/visitors';
 import type { BscFile, CompilerPlugin } from '../../interfaces';
 import { EmptyStatement } from '../../parser/Statement';
 
-// entry point
-const pluginInterface: CompilerPlugin = {
-    name: 'removePrint',
-    afterFileParse: afterFileParse
-};
-export default pluginInterface;
-
-// note: it is normally not recommended to modify the AST too much at this stage,
-// because if the plugin runs in a language-server context it could break intellisense
-function afterFileParse(file: BscFile) {
-    if (!isBrsFile(file)) {
-        return;
-    }
-    // visit functions bodies and replace `PrintStatement` nodes with `EmptyStatement`
-    for (const func of file.parser.references.functionExpressions) {
-        func.body.walk(createVisitor({
-            PrintStatement: (statement) => new EmptyStatement()
-        }), {
-            walkMode: WalkMode.visitStatements
-        });
-    }
+export default function plugin() {
+    return {
+        name: 'removePrint',
+        // note: it is normally not recommended to modify the AST too much at this stage,
+        // because if the plugin runs in a language-server context it could break intellisense
+        afterFileParse: (file: BscFile) => {
+            if (!isBrsFile(file)) {
+                return;
+            }
+            // visit functions bodies and replace `PrintStatement` nodes with `EmptyStatement`
+            for (const func of file.parser.references.functionExpressions) {
+                func.body.walk(createVisitor({
+                    PrintStatement: (statement) => new EmptyStatement()
+                }), {
+                    walkMode: WalkMode.visitStatements
+                });
+            }
+        }
+    } as CompilerPlugin;
 }
+

src/files/BrsFile.spec.ts

@@ -14,7 +14,7 @@ import { SourceMapConsumer } from 'source-map';
 import { TokenKind, Lexer, Keywords } from '../lexer';
 import { DiagnosticMessages } from '../DiagnosticMessages';
 import type { StandardizedFileEntry } from 'roku-deploy';
-import util, { loadPlugins, standardizePath as s } from '../util';
+import util, { standardizePath as s } from '../util';
 import PluginInterface from '../PluginInterface';
 import { trim, trimMap } from '../testHelpers.spec';
 import { ParseMode } from '../parser/Parser';
@@ -2554,7 +2554,7 @@ describe('BrsFile', () => {
 
         it('can use a plugin object which transforms the AST', async () => {
             program.plugins = new PluginInterface(
-                loadPlugins('', [
+                util.loadPlugins('', [
                     require.resolve('../examples/plugins/removePrint')
                 ]),
                 undefined
@@ -2564,7 +2564,7 @@ describe('BrsFile', () => {
 
         it('can load an absolute plugin which transforms the AST', async () => {
             program.plugins = new PluginInterface(
-                loadPlugins('', [
+                util.loadPlugins('', [
                     path.resolve(process.cwd(), './dist/examples/plugins/removePrint.js')
                 ]),
                 undefined
@@ -2574,7 +2574,7 @@ describe('BrsFile', () => {
 
         it('can load a relative plugin which transforms the AST', async () => {
             program.plugins = new PluginInterface(
-                loadPlugins(process.cwd(), [
+                util.loadPlugins(process.cwd(), [
                     './dist/examples/plugins/removePrint.js'
                 ]),
                 undefined

src/interfaces.ts

@@ -161,6 +161,8 @@ export interface CommentFlag {
 
 type ValidateHandler = (scope: Scope, files: BscFile[], callables: CallableContainerMap) => void;
 
+export type CompilerPluginFactory = () => CompilerPlugin;
+
 export interface CompilerPlugin {
     name: string;
     beforeProgramCreate?: (builder: ProgramBuilder) => void;

src/util.spec.ts

@@ -5,18 +5,21 @@ import { Range } from 'vscode-languageserver';
 import { Lexer } from './lexer';
 import type { BsConfig } from './BsConfig';
 import * as fsExtra from 'fs-extra';
-
+import { createSandbox } from 'sinon';
+const sinon = createSandbox();
 let tempDir = s`${process.cwd()}/.tmp`;
 let rootDir = s`${tempDir}/rootDir`;
 let cwd = process.cwd();
 
 describe('util', () => {
     beforeEach(() => {
+        sinon.restore();
         fsExtra.ensureDirSync(tempDir);
         fsExtra.emptyDirSync(tempDir);
     });
 
     afterEach(() => {
+        sinon.restore();
         fsExtra.ensureDirSync(tempDir);
         fsExtra.emptyDirSync(tempDir);
     });
@@ -532,6 +535,71 @@ describe('util', () => {
             expect(util.getExtension('main.component.xml')).to.eql('.xml');
         });
     });
+
+    describe('loadPlugins', () => {
+
+        let pluginPath: string;
+        let id = 1;
+
+        beforeEach(() => {
+            // `require` caches plugins, so  generate a unique plugin name for every test
+            pluginPath = `${tempDir}/plugin${id++}.js`;
+        });
+
+        it('shows warning when loading plugin with old "object" format', () => {
+            fsExtra.writeFileSync(pluginPath, `
+                module.exports = {
+                    name: 'AwesomePlugin'
+                };
+            `);
+            const stub = sinon.stub(console, 'warn').callThrough();
+            const plugins = util.loadPlugins(cwd, [pluginPath]);
+            expect(plugins[0].name).to.eql('AwesomePlugin');
+            expect(stub.callCount).to.equal(1);
+        });
+
+        it('shows warning when loading plugin with old "object" format and exports.default', () => {
+            fsExtra.writeFileSync(pluginPath, `
+                module.exports.default = {
+                    name: 'AwesomePlugin'
+                };
+            `);
+            const stub = sinon.stub(console, 'warn').callThrough();
+            const plugins = util.loadPlugins(cwd, [pluginPath]);
+            expect(plugins[0].name).to.eql('AwesomePlugin');
+            expect(stub.callCount).to.equal(1);
+        });
+
+        it('loads plugin with factory pattern', () => {
+            fsExtra.writeFileSync(pluginPath, `
+                module.exports = function() {
+                    return {
+                        name: 'AwesomePlugin'
+                    };
+                };
+            `);
+            const stub = sinon.stub(console, 'warn').callThrough();
+            const plugins = util.loadPlugins(cwd, [pluginPath]);
+            expect(plugins[0].name).to.eql('AwesomePlugin');
+            //does not warn about factory pattern
+            expect(stub.callCount).to.equal(0);
+        });
+
+        it('loads plugin with factory pattern and `default`', () => {
+            fsExtra.writeFileSync(pluginPath, `
+                module.exports.default = function() {
+                    return {
+                        name: 'AwesomePlugin'
+                    };
+                };
+            `);
+            const stub = sinon.stub(console, 'warn').callThrough();
+            const plugins = util.loadPlugins(cwd, [pluginPath]);
+            expect(plugins[0].name).to.eql('AwesomePlugin');
+            //does not warn about factory pattern
+            expect(stub.callCount).to.equal(0);
+        });
+    });
 });
 
 async function expectThrowAsync(callback) {