Skip to content

Commit

Permalink
bslib ropm support (#334)
Browse files Browse the repository at this point in the history
* Add support for `bslib` installing from ropm.

* remove local install

* auto-detect bslib from program.
remove config option.

* update readme.

* simplify program logic and fix tests

* fix duplicate bslib import

* address PR concerns.

* remove redundant path standardize
  • Loading branch information
TwitchBronBron authored Feb 27, 2021
1 parent 0c6b7f4 commit bcdf02a
Show file tree
Hide file tree
Showing 11 changed files with 159 additions and 65 deletions.
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,22 @@ end sub
The primary motivation for this feature was to provide a stopgap measure to hide incorrectly-thrown errors on legitimate BrightScript code due to parser bugs. This is still a new project and it is likely to be missing support for certain BrightScript syntaxes. It is recommended that you only use these comments when absolutely necessary.
## ropm support
In order for BrighterScript-transpiled projects to work as ropm modules, they need a reference to [bslib](https://github.com/rokucommunity/bslib/blob/master/source/bslib.brs) (the BrightScript runtime library for BrighterScript features) in their package. As `ropm` and `brighterscript` become more popular, this could result in many duplicate copies of `bslib.brs`.
To encourage reducing code duplication, BrighterScript has built-in support for loading `bslib` from [ropm](https://github.com/rokucommunity/ropm). Here's how it works:
1. if your program does not use ropm, or _does_ use ropm but does not directly reference bslib, then the BrighterScript compiler will copy bslib to `"pkg:/source/bslib.brs"` at transpile-time.
2. if your program uses ropm and has installed `bslib` as a dependency, then the BrighterScript compiler will _not_ emit a copy of bslib at `pkg:/source/bslib.brs`, and will instead use the path to the version from ropm `pkg:/source/roku_modules/bslib/bslib.brs`.



### Installing bslib in your ropm-enabled project
bslib is actually published to npm under the name [@rokucommunity/bslib](https://npmjs.com/package/@rokucommunity/bslib). However, to keep the bslib function names short, BrighterScript requires that you install @rokucommunity/bslib with the `bslib` alias. Here's the command to do that using the ropm CLI.
```bash
ropm install bslib@npm:@rokucommunity/bslib
```
## Language Server Protocol
This project also contributes a class that aligns with Microsoft's [Language Server Protocol](https://microsoft.github.io/language-server-protocol/), which makes it easy to integrate `BrightScript` and `BrighterScript` with any IDE that supports the protocol. We won't go into more detail here, but you can use the `LanguageServer` class from this project to integrate into your IDE. The [vscode-BrightScript-language](https://github.com/rokucommunity/vscode-BrightScript-language) extension uses this LanguageServer class to bring `BrightScript` and `BrighterScript` language support to Visual Studio Code.
Expand Down
47 changes: 0 additions & 47 deletions bslib.brs

This file was deleted.

5 changes: 5 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
"dependencies": {
"@xml-tools/parser": "^1.0.7",
"array-flat-polyfill": "^1.0.1",
"bslib": "npm:@rokucommunity/bslib@^0.1.1",
"chalk": "^2.4.2",
"chokidar": "^3.0.2",
"clear": "^0.1.0",
Expand Down
14 changes: 12 additions & 2 deletions src/Program.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1638,6 +1638,18 @@ describe('Program', () => {
});

describe('transpile', () => {
it('copies bslib.brs when no ropm version was found', async () => {
await program.transpile([], stagingFolderPath);
expect(fsExtra.pathExistsSync(`${stagingFolderPath}/source/bslib.brs`)).to.be.true;
});

it('does not copy bslib.brs when found in roku_modules', async () => {
program.addOrReplaceFile('source/roku_modules/bslib/bslib.brs', '');
await program.transpile([], stagingFolderPath);
expect(fsExtra.pathExistsSync(`${stagingFolderPath}/source/bslib.brs`)).to.be.false;
expect(fsExtra.pathExistsSync(`${stagingFolderPath}/source/roku_modules/bslib/bslib.brs`)).to.be.true;
});

it('transpiles in-memory-only files', async () => {
program.addOrReplaceFile('source/logger.bs', trim`
sub logInfo()
Expand Down Expand Up @@ -2241,7 +2253,5 @@ describe('Program', () => {
expect(signatureHelp[0].index, `failed on col ${col}`).to.equal(2);
}
});


});
});
26 changes: 17 additions & 9 deletions src/Program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { ParseMode } from './parser';
import { TokenKind } from './lexer';
import { BscPlugin } from './bscPlugin/BscPlugin';
const startOfSourcePkgPath = `source${path.sep}`;
const bslibRokuModulesPkgPath = s`source/roku_modules/bslib/bslib.brs`;

export interface SourceObj {
pathAbsolute: string;
Expand Down Expand Up @@ -121,6 +122,18 @@ export class Program {
*/
private diagnostics = [] as BsDiagnostic[];

/**
* The path to bslib.brs (the BrightScript runtime for certain BrighterScript features)
*/
public get bslibPkgPath() {
//if there's a version of bslib from roku_modules loaded into the program, use that
if (this.getFileByPkgPath(bslibRokuModulesPkgPath)) {
return bslibRokuModulesPkgPath;
} else {
return `source${path.sep}bslib.brs`;
}
}

/**
* A map of every file loaded into this program, indexed by its original file location
*/
Expand Down Expand Up @@ -1141,15 +1154,10 @@ export class Program {
this.plugins.emit('afterFileTranspile', entry);
});

//copy the brighterscript stdlib to the output directory
promises.push(
fsExtra.ensureDir(s`${stagingFolderPath}/source`).then(() => {
return fsExtra.copyFile(
s`${__dirname}/../bslib.brs`,
s`${stagingFolderPath}/source/bslib.brs`
);
})
);
//if there's no bslib file already loaded into the program, copy it to the staging directory
if (!this.getFileByPkgPath(bslibRokuModulesPkgPath) && !this.getFileByPkgPath(s`source/bslib.brs`)) {
promises.push(util.copyBslibToStaging(stagingFolderPath));
}
await Promise.all(promises);

this.plugins.emit('afterProgramTranspile', this, entries);
Expand Down
5 changes: 5 additions & 0 deletions src/Scope.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { CodeAction, CompletionItem, Position, Range } from 'vscode-languageserver';
import * as path from 'path';
import { CompletionItemKind, Location } from 'vscode-languageserver';
import chalk from 'chalk';
import type { DiagnosticInfo } from './DiagnosticMessages';
Expand Down Expand Up @@ -835,6 +836,10 @@ export class Scope {
let referencedFile = this.getFileByRelativePath(scriptImport.pkgPath);
//if we can't find the file
if (!referencedFile) {
//skip the default bslib file, it will exist at transpile time but should not show up in the program during validation cycle
if (scriptImport.pkgPath === `source${path.sep}bslib.brs`) {
continue;
}
let dInfo: DiagnosticInfo;
if (scriptImport.text.trim().length === 0) {
dInfo = DiagnosticMessages.scriptSrcCannotBeEmpty();
Expand Down
61 changes: 60 additions & 1 deletion src/files/XmlFile.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { BrsFile } from './BrsFile';
import { XmlFile } from './XmlFile';
import util, { standardizePath as s } from '../util';
import { getTestTranspile } from './BrsFile.spec';
import { expectCodeActions, trim, trimMap } from '../testHelpers.spec';
import { expectCodeActions, expectZeroDiagnostics, trim, trimMap } from '../testHelpers.spec';
import { URI } from 'vscode-uri';

describe('XmlFile', () => {
Expand Down Expand Up @@ -626,6 +626,65 @@ describe('XmlFile', () => {
});

describe('transpile', () => {
it('includes bslib script', () => {
testTranspile(trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
</component>
`, trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<script type="text/brightscript" uri="pkg:/source/bslib.brs" />
</component>
`, 'none', 'components/Comp.xml');
});

it('does not include additional bslib script if already there ', () => {
testTranspile(trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<script type="text/brightscript" uri="pkg:/source/bslib.brs" />
</component>
`, trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<script type="text/brightscript" uri="pkg:/source/bslib.brs" />
</component>
`, 'none', 'components/child.xml');
});

it('does not include bslib script if already there from ropm', () => {
program.addOrReplaceFile('source/roku_modules/bslib/bslib.brs', ``);
program.addOrReplaceFile('source/lib.bs', ``);
//include a bs file to force transpile for the xml file
testTranspile(trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<script type="text/brightscript" uri="pkg:/source/lib.bs" />
<script type="text/brightscript" uri="pkg:/source/roku_modules/bslib/bslib.brs" />
</component>
`, trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<script type="text/brightscript" uri="pkg:/source/lib.brs" />
<script type="text/brightscript" uri="pkg:/source/roku_modules/bslib/bslib.brs" />
</component>
`, 'none', 'components/child.xml');
});

it('does not transpile xml file when bslib script is already present', () => {
const file = program.addOrReplaceFile('components/comp.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="Comp" extends="Group">
<script type="text/brightscript" uri="pkg:/source/bslib.brs" />
</component>
`);
program.validate();
expectZeroDiagnostics(program);
expect(file.needsTranspiled).to.be.false;
});


/**
* There was a bug that would incorrectly replace one of the script paths on the second or third transpile, so this test verifies it doesn't do that anymore
*/
Expand Down
17 changes: 11 additions & 6 deletions src/files/XmlFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,33 +413,38 @@ export class XmlFile {
*/
private getMissingImportsForTranspile() {
let ownImports = this.getAvailableScriptImports();
//add the bslib path to ownImports, it'll get filtered down below
ownImports.push(this.program.bslibPkgPath);

let parentImports = this.parentComponent?.getAvailableScriptImports() ?? [];

let parentMap = parentImports.reduce((map, pkgPath) => {
map[pkgPath] = true;
map[pkgPath.toLowerCase()] = true;
return map;
}, {});

//if the XML already has this import, skip this one
let alreadyThereScriptImportMap = this.scriptTagImports.reduce((map, fileReference) => {
map[fileReference.pkgPath] = true;
map[fileReference.pkgPath.toLowerCase()] = true;
return map;
}, {});

let resultMap = {};
let result = [] as string[];
for (let ownImport of ownImports) {
const ownImportLower = ownImport.toLowerCase();
if (
//if the parent doesn't have this import
!parentMap[ownImport] &&
!parentMap[ownImportLower] &&
//the XML doesn't already have a script reference for this
!alreadyThereScriptImportMap[ownImport]
!alreadyThereScriptImportMap[ownImportLower] &&
//the result doesn't already have this reference
!resultMap[ownImportLower]
) {
result.push(ownImport);
resultMap[ownImportLower] = true;
}
}

result.push('source/bslib.brs');
return result;
}

Expand Down
7 changes: 7 additions & 0 deletions src/util.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,4 +630,11 @@ describe('util', () => {
expect(stub.callCount).to.equal(0);
});
});

describe('copyBslibToStaging', () => {
it('copies from local bslib dependency', async () => {
await util.copyBslibToStaging(tempDir);
expect(fsExtra.pathExistsSync(`${tempDir}/source/bslib.brs`)).to.be.true;
});
});
});
25 changes: 25 additions & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1175,6 +1175,31 @@ export class Util {
range: attr.range
} as SGAttribute;
}

/**
* Copy the version of bslib from local node_modules to the staging folder
*/
public async copyBslibToStaging(stagingDir: string) {
//copy bslib to the output directory
await fsExtra.ensureDir(standardizePath(`${stagingDir}/source`));
// eslint-disable-next-line
const bslib = require('bslib');
let source = bslib.source as string;

//apply the `bslib_` prefix to the functions
let match: RegExpExecArray;
const positions = [] as number[];
// eslint-disable-next-line no-cond-assign
while (match = /^(\s*(?:function|sub)\s+)([a-z0-9_]+)/.exec(source)) {
positions.push(match.index + match[1].length);
}

for (let i = positions.length - 1; i >= 0; i--) {
const position = positions[i];
source = source.slice(0, position) + 'bslib_' + source.slice(position);
}
await fsExtra.writeFile(`${stagingDir}/source/bslib.brs`, source);
}
}

/**
Expand Down

0 comments on commit bcdf02a

Please sign in to comment.