From ceee0706dcdc5f70d424ac4bd05cee7682fc6d66 Mon Sep 17 00:00:00 2001
From: Sean Barag <sean.barag@hulu.com>
Date: Fri, 24 Jul 2020 17:08:24 -0700
Subject: [PATCH] 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
---
 src/Error.ts                                  | 19 +++++-
 src/componentprocessor/index.ts               | 68 ++++++++++++++++---
 src/index.ts                                  |  7 +-
 .../componentprocessor.test.js                | 11 ++-
 .../resources/extendedComponent.xml           |  2 +-
 5 files changed, 85 insertions(+), 22 deletions(-)

diff --git a/src/Error.ts b/src/Error.ts
index d6c40c200..037f86a4a 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,10 +39,12 @@ 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`;
     }
 }
 
+export function formatError(location: Location, message: string) {}
+
 /** Wraps up the metadata associated with a type mismatch error. */
 export interface TypeMismatchMetadata {
     /**
diff --git a/src/componentprocessor/index.ts b/src/componentprocessor/index.ts
index beb4e754e..152ebaaa9 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;
+}
diff --git a/src/index.ts b/src/index.ts
index 1c19dd6b5..368c9574f 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -38,7 +38,12 @@ export async function execute(filenames: string[], options: Partial<ExecutionOpt
     const executionOptions = Object.assign(defaultExecutionOptions, options);
 
     let manifest = await PP.getManifest(executionOptions.root);
-    let componentDefinitions = await getComponentDefinitionMap(executionOptions.root);
+    let componentDefinitions: Map<string, ComponentDefinition>;
+    try {
+        componentDefinitions = await getComponentDefinitionMap(executionOptions.root);
+    } catch (err) {
+        throw err;
+    }
 
     componentDefinitions.forEach((component: ComponentDefinition) => {
         if (component.scripts.length < 1) return;
diff --git a/test/componentprocessor/componentprocessor.test.js b/test/componentprocessor/componentprocessor.test.js
index a3686063e..7e13803eb 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 604769e8c..69f4f97ed 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" />