Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent publishing of empty files #997

8 changes: 7 additions & 1 deletion src/BsConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 46 additions & 0 deletions src/Program.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
40 changes: 21 additions & 19 deletions src/Program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
};

Expand Down
60 changes: 60 additions & 0 deletions src/files/BrsFile.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', ``);
Expand Down
21 changes: 21 additions & 0 deletions src/files/BrsFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,27 @@ export class BrsFile {
this.srcPath = value;
}

/**
* Should this file be written to disk
TwitchBronBron marked this conversation as resolved.
Show resolved Hide resolved
*/
public get isPublishable() {
let isPublishable = false;
this.ast.walk(createVisitor({
FunctionStatement: () => {
isPublishable = true;
},
MethodStatement: () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Methods can't exist on their own, so we can probably remove this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed 👍

isPublishable = true;
},
ClassStatement: () => {
isPublishable = true;
}
}), {
walkMode: WalkMode.visitStatementsRecursive
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably don't need the recursive part (that steps into function bodies which we don't care about for this)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wasn't sure if perhaps something like a namespace could contain functions so perhaps I needed the recursive version? Happy to change this and the methods version though. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

});
return isPublishable;
}

/**
* The parseMode used for the parser for this file
*/
Expand Down
59 changes: 59 additions & 0 deletions src/files/XmlFile.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,65 @@ describe('XmlFile', () => {
const code = file.transpile().code;
expect(code.endsWith(`<!--//# sourceMappingURL=./SimpleScene.xml.map -->`)).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`
<?xml version="1.0" encoding="utf-8" ?>
<component
name="SimpleScene" extends="Scene"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://devtools.web.roku.com/schema/RokuSceneGraph.xsd"
>
<script type="text/brightscript" uri="SimpleScene.bs"/>
</component>
`, trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="SimpleScene" extends="Scene" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://devtools.web.roku.com/schema/RokuSceneGraph.xsd">
<script type="text/brightscript" uri="pkg:/source/bslib.brs" />
</component>
`, 'none', 'components/SimpleScene.xml');
});

it('removes extra imports found via dependencies if given file is not publishable', () => {
program.options.publishEmptyFiles = false;
program.setFile(`source/simplescenetypes.bs`, `
enum SimpleSceneTypes
world = "world"
end enum
`);
program.setFile(`components/SimpleScene.bs`, `
import "pkg:/source/simplescenetypes.bs"

sub init()
? "Hello " + SimpleSceneTypes.world
end sub
`);

testTranspile(trim`
<?xml version="1.0" encoding="utf-8" ?>
<component
name="SimpleScene" extends="Scene"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://devtools.web.roku.com/schema/RokuSceneGraph.xsd"
>
<script type="text/brightscript" uri="SimpleScene.bs"/>
</component>
`, trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="SimpleScene" extends="Scene" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="https://devtools.web.roku.com/schema/RokuSceneGraph.xsd">
<script type="text/brightscript" uri="SimpleScene.brs" />
<script type="text/brightscript" uri="pkg:/source/bslib.brs" />
</component>
`, 'none', 'components/SimpleScene.xml');
});
});

describe('Transform plugins', () => {
Expand Down
48 changes: 41 additions & 7 deletions src/files/XmlFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ export class XmlFile {

public commentFlags = [] as CommentFlag[];

/**
* should this file be written to disk
*/
TwitchBronBron marked this conversation as resolved.
Show resolved Hide resolved
readonly isPublishable = true;

/**
* The list of script imports delcared in the XML of this file.
* This excludes parent imports and auto codebehind imports
Expand Down Expand Up @@ -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));

Expand Down
6 changes: 6 additions & 0 deletions src/util.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,12 @@ describe('util', () => {
expect(util.normalizeConfig(<any>{ emitDefinitions: 'true' }).emitDefinitions).to.be.false;
});

it('sets publishEmptyFiles to true by default, or false if explicitly false', () => {
expect(util.normalizeConfig({}).publishEmptyFiles).to.be.true;
expect(util.normalizeConfig({ publishEmptyFiles: true }).publishEmptyFiles).to.be.true;
expect(util.normalizeConfig({ publishEmptyFiles: false }).publishEmptyFiles).to.be.false;
});

it('loads project from disc', () => {
fsExtra.outputFileSync(s`${tempDir}/rootDir/bsconfig.json`, `{ "outFile": "customOutDir/pkg.zip" }`);
let config = util.normalizeAndResolveConfig({
Expand Down
1 change: 1 addition & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down