Repository: sjbarag/brs (js)

Issue:
Parsing errors in files executed with `Run` silently ignored
Consider two files:

#### main.brs
```brightscript
sub main()
    print "main.brs start"
    run("pkg:/foo.brs")
    print "main.brs end"
end sub
```

#### foo.brs
```brightscript
sub main()
    print "foo.brs start"

    m = {
        it: __it,
        assert: {
            isInvalid: sub(a, b)
                if a <> invalid then print b
            end sub
        }
    }

    m.it("is a test" sub()
    '              ~~~ note the missing `,`
        m.assert.isInvalid(12345, "TEST TEST TEST")
    end sub)
    print "foo.brs end"
end sub

sub __it(a as string, b as object)
    b()
end sub
```

Executing `foo.brs` directly results in parse errors:

```
$ brs foo.brs
foo.brs(13,21-24): Expected ')' after function call arguments
foo.brs(16,11-12): Found unexpected token ')'
foo.brs(18,0-7): Found unexpected token 'end sub'
Error occurred parsing
```

But executing `main.brs` executes… weirdly:

```
$ brs main.brs
main.brs start
foo.brs end
foo.brs start
main.brs end
```

We'll need to figure out what's happening here!

Pull request:
fix(parser,stdlib) Run errors ignored
- Added error listener to lexer-parser process (`lexParseSync`) to send errors to stderr
-  `lexParseSync` now throws an error stopping execution when parsing fails. This fixes `run` and `RunInScope` functions which use `lexParseSync`
- Removed `try`/`catch` block from RunInScope` to let the error bubble up to main error handler.

Fixes #383 

Patch:
diff --git a/src/extensions/RunInScope.ts b/src/extensions/RunInScope.ts
index 1ac2594..f736d91 100644
--- a/src/extensions/RunInScope.ts
+++ b/src/extensions/RunInScope.ts
@@ -37,18 +37,12 @@ function runFilesInScope(interpreter: Interpreter, filenames: BrsString[], args:
         return BrsInvalid.Instance;
     }
 
-    try {
-        let ast = brs.lexParseSync(pathsToFiles, interpreter.options);
-        return interpreter.inSubEnv(subInterpreter => {
-            // remove the original `main` function so we can execute the new file
-            subInterpreter.environment.remove("main", Scope.Module);
-            return subInterpreter.exec(ast, ...args)[0] || BrsInvalid.Instance;
-        });
-    } catch (err) {
-        // swallow errors and just return invalid; RBI returns invalid for "file doesn't exist" errors,
-        // syntax errors, etc.
-        return BrsInvalid.Instance;
-    }
+    let ast = brs.lexParseSync(pathsToFiles, interpreter.options);
+    return interpreter.inSubEnv(subInterpreter => {
+        // remove the original `main` function so we can execute the new file
+        subInterpreter.environment.remove("main", Scope.Module);
+        return subInterpreter.exec(ast, ...args)[0] || BrsInvalid.Instance;
+    });
 }
 
 export const RunInScope = new Callable(
diff --git a/src/index.ts b/src/index.ts
index 622811d..f7d579c 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -83,11 +83,23 @@ export function lexParseSync(filenames: string[], options: Partial<ExecutionOpti
 
     return filenames
         .map(filename => {
-            let contents = fs.readFileSync(filename, "utf8");
-            let scanResults = Lexer.scan(contents, filename);
+            let lexer = new Lexer();
             let preprocessor = new PP.Preprocessor();
+            let parser = new Parser();
+            [lexer, preprocessor, parser].forEach(emitter =>
+                emitter.onError(BrsError.getLoggerUsing(executionOptions.stderr))
+            );
+
+            let contents = fs.readFileSync(filename, "utf8");
+            let scanResults = lexer.scan(contents, filename);
             let preprocessorResults = preprocessor.preprocess(scanResults.tokens, manifest);
-            return Parser.parse(preprocessorResults.processedTokens).statements;
+            let parseResults = parser.parse(preprocessorResults.processedTokens);
+
+            if (parseResults.errors.length > 0) {
+                throw "Error occurred parsing";
+            }
+
+            return parseResults.statements;
         })
         .reduce((allStatements, statements) => [...allStatements, ...statements], []);
 }


Test patch:
diff --git a/test/extensions/RunInScope.test.js b/test/extensions/RunInScope.test.js
index 617f6fc..8381bae 100644
--- a/test/extensions/RunInScope.test.js
+++ b/test/extensions/RunInScope.test.js
@@ -33,18 +33,22 @@ describe("global Run function", () => {
         );
     });
 
-    it("returns invalid for lexing errors", () => {
+    it("returns invalid for lexing errors", async () => {
         fs.readFileSync.mockImplementation(() => `can't lex this`);
-        expect(RunInScope.call(interpreter, new BrsString("pkg:/errors/lex.brs"))).toBe(
-            BrsInvalid.Instance
-        );
+        try {
+            await RunInScope.call(interpreter, new BrsString("pkg:/errors/lex.brs"));
+        } catch (e) {
+            expect(e).toEqual("Error occurred parsing");
+        }
     });
 
-    it("returns invalid for parse errors", () => {
+    it("returns invalid for parse errors", async () => {
         fs.readFileSync.mockImplementationOnce(() => `if return "parse error" exit while`);
-        expect(RunInScope.call(interpreter, new BrsString("pkg:/errors/parse.brs"))).toBe(
-            BrsInvalid.Instance
-        );
+        try {
+            await RunInScope.call(interpreter, new BrsString("pkg:/errors/parse.brs"));
+        } catch (e) {
+            expect(e).toEqual("Error occurred parsing");
+        }
     });
 
     it("returns invalid for runtime errors", () => {