From d2adf057779f84a64653a1bd5d8ef19756e73814 Mon Sep 17 00:00:00 2001 From: Michael Martinez Date: Wed, 27 Dec 2023 18:00:25 +0000 Subject: [PATCH 1/8] Added file exclusion for empty files --- src/BsConfig.ts | 8 +++++++- src/Program.ts | 40 ++++++++++++++++++------------------ src/files/BrsFile.ts | 21 +++++++++++++++++++ src/files/XmlFile.ts | 48 +++++++++++++++++++++++++++++++++++++------- src/util.ts | 1 + 5 files changed, 91 insertions(+), 27 deletions(-) diff --git a/src/BsConfig.ts b/src/BsConfig.ts index dd9a1be4a..ba2f8befc 100644 --- a/src/BsConfig.ts +++ b/src/BsConfig.ts @@ -186,7 +186,13 @@ export interface BsConfig { * @default true */ sourceMap?: boolean; - + /** + * Enables the publishing of empty tranpiled files. Some Brighterscript files + * and the default behaviour is to write these to disk after transpilation. + * empty files being written and will remove associated script tags from XM + * @default true + */ + publishEmptyFiles?: boolean; /** * Allow brighterscript features (classes, interfaces, etc...) to be included in BrightScript (`.brs`) files, and force those files to be transpiled. * @default false diff --git a/src/Program.ts b/src/Program.ts index 27022543f..31bd071b0 100644 --- a/src/Program.ts +++ b/src/Program.ts @@ -1213,28 +1213,30 @@ export class Program { //mark this file as processed so we don't process it more than once processedFiles.add(outputPath?.toLowerCase()); - //skip transpiling typedef files - if (isBrsFile(file) && file.isTypedef) { - return; - } + if (this.options.publishEmptyFiles || file.isPublishable) { + //skip transpiling typedef files + if (isBrsFile(file) && file.isTypedef) { + return; + } - const fileTranspileResult = this._getTranspiledFileContents(file, outputPath); + const fileTranspileResult = this._getTranspiledFileContents(file, outputPath); - //make sure the full dir path exists - await fsExtra.ensureDir(path.dirname(outputPath)); + //make sure the full dir path exists + await fsExtra.ensureDir(path.dirname(outputPath)); - if (await fsExtra.pathExists(outputPath)) { - throw new Error(`Error while transpiling "${file.srcPath}". A file already exists at "${outputPath}" and will not be overwritten.`); - } - const writeMapPromise = fileTranspileResult.map ? fsExtra.writeFile(`${outputPath}.map`, fileTranspileResult.map.toString()) : null; - await Promise.all([ - fsExtra.writeFile(outputPath, fileTranspileResult.code), - writeMapPromise - ]); - - if (fileTranspileResult.typedef) { - const typedefPath = outputPath.replace(/\.brs$/i, '.d.bs'); - await fsExtra.writeFile(typedefPath, fileTranspileResult.typedef); + if (await fsExtra.pathExists(outputPath)) { + throw new Error(`Error while transpiling "${file.srcPath}". A file already exists at "${outputPath}" and will not be overwritten.`); + } + const writeMapPromise = fileTranspileResult.map ? fsExtra.writeFile(`${outputPath}.map`, fileTranspileResult.map.toString()) : null; + await Promise.all([ + fsExtra.writeFile(outputPath, fileTranspileResult.code), + writeMapPromise + ]); + + if (fileTranspileResult.typedef) { + const typedefPath = outputPath.replace(/\.brs$/i, '.d.bs'); + await fsExtra.writeFile(typedefPath, fileTranspileResult.typedef); + } } }; diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index 9edef7fd8..13209c75e 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -77,6 +77,27 @@ export class BrsFile { this.srcPath = value; } + /** + * Should this file be written to disk + */ + public get isPublishable() { + let isPublishable = false; + this.ast.walk(createVisitor({ + FunctionStatement: () => { + isPublishable = true; + }, + MethodStatement: () => { + isPublishable = true; + }, + ClassStatement: () => { + isPublishable = true; + } + }), { + walkMode: WalkMode.visitStatementsRecursive + }); + return isPublishable; + } + /** * The parseMode used for the parser for this file */ diff --git a/src/files/XmlFile.ts b/src/files/XmlFile.ts index 23ce5a203..d5d4e90c3 100644 --- a/src/files/XmlFile.ts +++ b/src/files/XmlFile.ts @@ -70,6 +70,11 @@ export class XmlFile { public commentFlags = [] as CommentFlag[]; + /** + * should this file be written to disk + */ + readonly isPublishable = true; + /** * The list of script imports delcared in the XML of this file. * This excludes parent imports and auto codebehind imports @@ -458,27 +463,56 @@ export class XmlFile { this.program.logger.debug('XmlFile', chalk.green(this.pkgPath), ...args); } + private determinePkgMapPath(uri) { + if (/^pkg:\/.+/.test(uri)) { + return uri.replace(/^pkg:\//, ''); + } + const fileDirectory = this.pkgPath.replace(/\/\w+\.xml$/, ''); + const pkgMapPath = path.normalize(`${fileDirectory}/${uri}`); + return pkgMapPath; + } + + private checkScriptsForPublishableImports(scripts: SGScript[]): [boolean, SGScript[]] { + if (this.program.options.publishEmptyFiles) { + return [false, scripts]; + } + const publishableScripts = scripts.filter(script => { + const uriAttributeValue = script.attributes.find((v) => v.key.text === 'uri')?.value.text; + const pkgMapPath = this.determinePkgMapPath(uriAttributeValue); + let file = this.program.getFile(pkgMapPath); + if (!file && pkgMapPath.endsWith(this.program.bslibPkgPath)) { + return true; + } + if (!file && pkgMapPath.endsWith('.brs')) { + file = this.program.getFile(pkgMapPath.replace(/\.brs$/, '.bs')); + } + return file?.isPublishable; + }); + return [publishableScripts.length !== scripts.length, publishableScripts]; + } + /** * Convert the brightscript/brighterscript source code into valid brightscript */ public transpile(): CodeWithSourceMap { const state = new TranspileState(this.srcPath, this.program.options); + const originalScripts = this.ast.component?.scripts ?? []; const extraImportScripts = this.getMissingImportsForTranspile().map(uri => { const script = new SGScript(); script.uri = util.getRokuPkgPath(uri.replace(/\.bs$/, '.brs')); return script; }); - let transpileResult: SourceNode | undefined; + const [scriptsHaveChanged, publishableScripts] = this.checkScriptsForPublishableImports([ + ...originalScripts, + ...extraImportScripts + ]); - if (this.needsTranspiled || extraImportScripts.length > 0) { + let transpileResult: SourceNode | undefined; + if (this.needsTranspiled || extraImportScripts.length > 0 || scriptsHaveChanged) { //temporarily add the missing imports as script tags - const originalScripts = this.ast.component?.scripts ?? []; - this.ast.component.scripts = [ - ...originalScripts, - ...extraImportScripts - ]; + this.ast.component.scripts = publishableScripts; transpileResult = new SourceNode(null, null, state.srcPath, this.parser.ast.transpile(state)); diff --git a/src/util.ts b/src/util.ts index 4253f72ad..fe56a7baf 100644 --- a/src/util.ts +++ b/src/util.ts @@ -341,6 +341,7 @@ export class Util { config.diagnosticSeverityOverrides = config.diagnosticSeverityOverrides ?? {}; config.diagnosticFilters = config.diagnosticFilters ?? []; config.plugins = config.plugins ?? []; + config.publishEmptyFiles = config.publishEmptyFiles === false ? false : true; config.autoImportComponentScript = config.autoImportComponentScript === true ? true : false; config.showDiagnosticsInConsole = config.showDiagnosticsInConsole === false ? false : true; config.sourceRoot = config.sourceRoot ? standardizePath(config.sourceRoot) : undefined; From 26abeb8e25dc3469dd6ad55bd9e26eec8a5d3c50 Mon Sep 17 00:00:00 2001 From: Michael Martinez Date: Thu, 28 Dec 2023 15:22:18 +0000 Subject: [PATCH 2/8] Added unit tests --- src/Program.spec.ts | 46 ++++++++++++++++++++++++++++++ src/files/BrsFile.spec.ts | 60 +++++++++++++++++++++++++++++++++++++++ src/files/XmlFile.spec.ts | 59 ++++++++++++++++++++++++++++++++++++++ src/util.spec.ts | 6 ++++ 4 files changed, 171 insertions(+) diff --git a/src/Program.spec.ts b/src/Program.spec.ts index 04b405980..d7474812d 100644 --- a/src/Program.spec.ts +++ b/src/Program.spec.ts @@ -2158,6 +2158,52 @@ describe('Program', () => { s`${sourceRoot}/source/main.bs` ); }); + + it('does not publish files that are empty', async () => { + let sourceRoot = s`${tempDir}/sourceRootFolder`; + program = new Program({ + rootDir: rootDir, + stagingDir: stagingDir, + sourceRoot: sourceRoot, + sourceMap: true, + publishEmptyFiles: false + }); + program.setFile('source/types.bs', ` + enum mainstyle + dark = "dark" + light = "light" + end enum + `); + program.setFile('source/main.bs', ` + import "pkg:/source/types.bs" + + sub main() + ? "The night is " + mainstyle.dark + " and full of terror" + end sub + `); + await program.transpile([ + { + src: s`${rootDir}/source/main.bs`, + dest: s`source/main.bs` + }, + { + src: s`${rootDir}/source/types.bs`, + dest: s`source/types.bs` + } + ], stagingDir); + + expect(trimMap( + fsExtra.readFileSync(s`${stagingDir}/source/main.brs`).toString() + )).to.eql(trim` + 'import "pkg:/source/types.bs" + + sub main() + ? "The night is " + "dark" + " and full of terror" + end sub + `); + expect(() => fsExtra.readFileSync(s`${stagingDir}/source/types.brs`)) + .to.throw(/ENOENT: no such file or directory, open .+\/source\/types.brs/); + }); }); describe('typedef', () => { diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 9a71d26ad..340330b55 100644 --- a/src/files/BrsFile.spec.ts +++ b/src/files/BrsFile.spec.ts @@ -182,6 +182,66 @@ describe('BrsFile', () => { }); }); + describe('isPublishable', () => { + it('returns true is target file has contains a function statement', () => { + program.setFile('source/main.brs', ` + sub main() + print \`pkg:\` + end sub + `); + const file = program.getFile('source/main.brs'); + expect(file.isPublishable).to.be.true; + }); + + it('returns true if target file contains a class statement', () => { + program.setFile('source/main.brs', ` + class Animal + public name as string + end class + `); + const file = program.getFile('source/main.brs'); + expect(file.isPublishable).to.be.true; + }); + + it('returns true if target file contains a class statement', () => { + program.setFile('source/main.brs', ` + namespace Vertibrates.Birds + function GetDucks() + end function + end namespace + `); + const file = program.getFile('source/main.brs'); + expect(file.isPublishable).to.be.true; + }); + + it('returns false if target file contains only enum', () => { + program.setFile('source/main.brs', ` + enum Direction + up + down + left + right + end enum + `); + const file = program.getFile('source/main.brs'); + expect(file.isPublishable).to.be.false; + }); + + it('returns false if target file is empty', () => { + program.setFile('source/main.brs', ''); + const file = program.getFile('source/main.brs'); + expect(file.isPublishable).to.be.false; + }); + + it('returns false if target file only has comments', () => { + program.setFile('source/main.brs', ` + ' this is an interesting comment + `); + const file = program.getFile('source/main.brs'); + expect(file.isPublishable).to.be.false; + }); + }); + describe('getScopesForFile', () => { it('finds the scope for the file', () => { let file = program.setFile('source/main.brs', ``); diff --git a/src/files/XmlFile.spec.ts b/src/files/XmlFile.spec.ts index 610a9e50a..68e1401cd 100644 --- a/src/files/XmlFile.spec.ts +++ b/src/files/XmlFile.spec.ts @@ -899,6 +899,65 @@ describe('XmlFile', () => { const code = file.transpile().code; expect(code.endsWith(``)).to.be.true; }); + + it('removes script imports if given file is not publishable', () => { + program.options.publishEmptyFiles = false; + program.setFile(`components/SimpleScene.bs`, ` + enum simplescenetypes + hero + intro + end enum + `); + + testTranspile(trim` + + +