Repository: sjbarag/brs (js)

Issue:
Add support for relative URIs in `<script>` tags
SGDEX uses relative paths for the `uri` attribute of its `<script>` tags, which is perfectly valid in RBI!  We should support that here as well.

Example error, from https://github.com/hulu/roca/issues/35

```
Interpreter found an error:  Error: Encountered an error when parsing component ButtonBar: TypeError [ERR_INVALID_URL]: Invalid URL: ButtonBar.brs
    at componentDefinitions.forEach (/Users/user/Projects/roca-test-roku/nod_modules/brs/lib/index.js:66:19)
```

Parsing through https://github.com/rokudev/SceneGraphDeveloperExtensions/blob/master/extensions/SGDEX/ButtonBar/ButtonBar.xml with https://github.com/rokudev/SceneGraphDeveloperExtensions/blob/master/extensions/SGDEX/ButtonBar/ButtonBar.brs next to it should be enough to repro this bug

Pull request:
fix(rsg): Support relative paths in <script/> tags
RBI allows relative paths in <script/> tags, resolved relative to the XML file that contains the element.  We should support that too, because SGDEX makes heavy use of relative paths (also it's really handy :D)

fixes #471

Patch:
diff --git a/.github/workflows/nightly-build.yaml b/.github/workflows/nightly-build.yaml
index 878994b..e718940 100644
--- a/.github/workflows/nightly-build.yaml
+++ b/.github/workflows/nightly-build.yaml
@@ -1,9 +1,9 @@
 {
   name: "Nightly build",
   on: {
-      schedule: [
-          { cron: "05 7 * * *" }
-      ]
+    schedule: [
+      { cron: "05 16 * * *" }
+    ]
   },
   jobs: {
     publish: {
@@ -14,17 +14,16 @@
           name: "Set up node 12",
           uses: "actions/setup-node@v1",
           with: {
-            registry: "https://registry.npmjs.com",
-            scope: "",
+            registry-url: "https://registry.npmjs.org",
             node-version: "12"
           }
         },
         { run: "yarn" },
         {
-            run: "yarn publish --no-git-tag-version --new-version `node scripts/getNightlyVersion.js` --tag nightly",
-            env: {
-                NODE_AUTH_TOKEN: "${{ secrets.NPM_PUBLISH_TOKEN }}"
-            }
+          run: "yarn publish --no-git-tag-version --new-version `node scripts/getNightlyVersion.js` --tag nightly",
+          env: {
+            NODE_AUTH_TOKEN: "${{ secrets.NPM_PUBLISH_TOKEN }}"
+          }
         },
       ],
       env: {
diff --git a/src/Error.ts b/src/Error.ts
index d6c40c2..74079a3 100644
--- a/src/Error.ts
+++ b/src/Error.ts
@@ -11,11 +11,22 @@ export class BrsError extends Error {
      * and column, and the message associated with the error, e.g.:
      *
      * `lorem.brs(1,1-3): Expected '(' after sub name`
-     * ```
+     * @see BrsError#format
      */
     format() {
-        let location = this.location;
+        return BrsError.format(this.message, this.location);
+    }
 
+    /**
+     * Formats a location and message into a human-readable string including filename, starting
+     * and ending line and column, and the message associated with the error, e.g.:
+     *
+     * `lorem.brs(1,1-3): Expected '(' after sub name`
+     *
+     * @param message a string describing the error
+     * @param location where the error occurred
+     */
+    static format(message: string, location: Location): string {
         let formattedLocation: string;
 
         if (location.start.line === location.end.line) {
@@ -28,7 +39,7 @@ export class BrsError extends Error {
             formattedLocation = `${location.file}(${location.start.line},${location.start.column},${location.end.line},${location.end.line})`;
         }
 
-        return `${formattedLocation}: ${this.message}\n`;
+        return `${formattedLocation}: ${message}\n`;
     }
 }
 
diff --git a/src/componentprocessor/index.ts b/src/componentprocessor/index.ts
index beb4e75..152ebaa 100644
--- a/src/componentprocessor/index.ts
+++ b/src/componentprocessor/index.ts
@@ -6,7 +6,7 @@ import pSettle = require("p-settle");
 const readFile = promisify(fs.readFile);
 import * as fg from "fast-glob";
 import { Environment } from "../interpreter/Environment";
-import { BrsComponentName } from "../brsTypes";
+import { BrsError } from "../Error";
 
 interface FieldAttributes {
     id: string;
@@ -89,11 +89,12 @@ export async function getComponentDefinitionMap(rootDir: string) {
     let defs = xmlFiles.map((file) => new ComponentDefinition(file));
     let parsedPromises = defs.map(async (def) => def.parse());
 
-    return processXmlTree(pSettle(parsedPromises));
+    return processXmlTree(pSettle(parsedPromises), rootDir);
 }
 
 async function processXmlTree(
-    settledPromises: Promise<pSettle.SettledResult<ComponentDefinition>[]>
+    settledPromises: Promise<pSettle.SettledResult<ComponentDefinition>[]>,
+    rootDir: string
 ) {
     let nodeDefs = await settledPromises;
     let nodeDefMap = new Map<string, ComponentDefinition>();
@@ -151,13 +152,13 @@ async function processXmlTree(
         }
     });
 
-    nodeDefMap.forEach((nodeDef) => {
+    for (let nodeDef of nodeDefMap.values()) {
         let xmlNode = nodeDef.xmlNode;
         if (xmlNode) {
             nodeDef.children = getChildren(xmlNode);
-            nodeDef.scripts = getScripts(xmlNode);
+            nodeDef.scripts = await getScripts(xmlNode, nodeDef.xmlPath, rootDir);
         }
-    });
+    }
 
     return nodeDefMap;
 }
@@ -241,19 +242,64 @@ function parseChildren(element: XmlElement, children: ComponentNode[]): void {
     });
 }
 
-function getScripts(node: XmlDocument): ComponentScript[] {
+async function getScripts(
+    node: XmlDocument,
+    xmlPath: string,
+    rootDir: string
+): Promise<ComponentScript[]> {
     let scripts = node.childrenNamed("script");
     let componentScripts: ComponentScript[] = [];
 
-    // TODO: Verify if uri is valid
-    scripts.map((script) => {
+    for (let script of scripts) {
+        let absoluteUri: URL;
+        try {
+            absoluteUri = new URL(script.attr.uri, `pkg:/${path.posix.relative(rootDir, xmlPath)}`);
+        } catch (err) {
+            let file = await readFile(xmlPath, "utf-8");
+
+            let tag = file.substring(script.startTagPosition, script.position);
+            let tagLines = tag.split("\n");
+            let leadingLines = file.substring(0, script.startTagPosition).split("\n");
+            let start = {
+                line: leadingLines.length,
+                column: columnsInLastLine(leadingLines),
+            };
+
+            return Promise.reject({
+                message: BrsError.format(
+                    `Invalid path '${script.attr.uri}' found in <script/> tag`,
+                    {
+                        file: xmlPath,
+                        start: start,
+                        end: {
+                            line: start.line + tagLines.length - 1,
+                            column: start.column + columnsInLastLine(tagLines),
+                        },
+                    }
+                ).trim(),
+            });
+        }
+
         if (script.attr) {
             componentScripts.push({
                 type: script.attr.type,
-                uri: script.attr.uri,
+                uri: absoluteUri.href,
             });
         }
-    });
+    }
 
     return componentScripts;
 }
+
+/**
+ * Returns the number of columns occupied by the final line in an array of lines as parsed by `xmldoc`.
+ * xmldoc parses positions to ignore `\n` characters, which is pretty confusing.  This function
+ * compensates for that.
+ *
+ * @param lines an array of strings, where each is a line from an XML document
+ *
+ * @return the corrected column number for the last line of text as parsed by `xmlDoc`
+ */
+function columnsInLastLine(lines: string[]): number {
+    return lines[lines.length - 1].length + lines.length - 1;
+}


Test patch:
diff --git a/test/componentprocessor/componentprocessor.test.js b/test/componentprocessor/componentprocessor.test.js
index a368606..7e13803 100644
--- a/test/componentprocessor/componentprocessor.test.js
+++ b/test/componentprocessor/componentprocessor.test.js
@@ -8,7 +8,7 @@ const fs = require("fs");
 
 const realFs = jest.requireActual("fs");
 
-describe.only("component parsing support", () => {
+describe("component parsing support", () => {
     afterEach(() => {
         fg.sync.mockRestore();
         fs.readFile.mockRestore();
@@ -65,11 +65,10 @@ describe.only("component parsing support", () => {
                         "scripts/baseComp.brs",
                         "scripts/extendedComp.brs",
                         "scripts/utility.brs",
-                    ];
+                    ].map((f) => path.join(__dirname, "resources", f));
                 });
                 fs.readFile.mockImplementation((filename, _, cb) => {
-                    resourcePath = path.join(__dirname, "resources", filename);
-                    realFs.readFile(resourcePath, (err, contents) => {
+                    realFs.readFile(filename, (err, contents) => {
                         cb(/* no error */ null, contents);
                     });
                 });
@@ -90,7 +89,7 @@ describe.only("component parsing support", () => {
             });
 
             it("adds all scripts into node in correct order", async () => {
-                let map = await getComponentDefinitionMap("/doesnt/matter");
+                let map = await getComponentDefinitionMap(process.cwd());
                 let parsedExtendedComp = map.get("ExtendedComponent");
                 expect(parsedExtendedComp).not.toBeUndefined();
                 expect(parsedExtendedComp.scripts).not.toBeUndefined();
@@ -106,7 +105,7 @@ describe.only("component parsing support", () => {
 
             it("ignores extensions of unknown node subtypes", async () => {
                 fg.sync.mockImplementation(() => {
-                    return ["unknownExtensionComponent.xml"];
+                    return [path.join(__dirname, "resources", "unknownExtensionComponent.xml")];
                 });
 
                 jest.spyOn(global.console, "error").mockImplementation();
diff --git a/test/componentprocessor/resources/extendedComponent.xml b/test/componentprocessor/resources/extendedComponent.xml
index 604769e..69f4f97 100644
--- a/test/componentprocessor/resources/extendedComponent.xml
+++ b/test/componentprocessor/resources/extendedComponent.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="utf-8" ?>
 <component name="ExtendedComponent" extends="BaseComponent">
-<script type="text/brightscript" uri="pkg:/test/componentprocessor/resources/scripts/extendedComponent.brs" />
+<script type="text/brightscript" uri="scripts/extendedComponent.brs" />
 <script type="text/brightscript" uri="pkg:/test/componentprocessor/resources/scripts/utility.brs" />
 <children>
         <Label name="label_b" />