From cb0ae477074234ad4702724d8fe47ae3957768ac Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Sat, 13 Jan 2018 08:39:49 -0800 Subject: [PATCH 1/8] (grammar) Move assignments to lower precedence in grammar Since assignments in BrightScript take the place of declarations in most other languages, they need to be lower precedence (i.e. appear in the top-down grammar before statements). Since this grammar isn't being validated or used anywhere this is quite likely wrong and quite likely a moot effort, but I'm sticking with it for now gosh darnit! --- src/grammar/BrightscriptGrammar.ebnf | 37 +++++++++++++++++----------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/grammar/BrightscriptGrammar.ebnf b/src/grammar/BrightscriptGrammar.ebnf index 712079ec2..f0a741075 100644 --- a/src/grammar/BrightscriptGrammar.ebnf +++ b/src/grammar/BrightscriptGrammar.ebnf @@ -17,7 +17,29 @@ (* Start from the top, a brightscript script will have a number of "statements" before reaching the end of the file *) program - : statementList? EOF + : declarationList? EOF + ; + +declarationList + : multiDeclaration+ + ; + +(* brightscript supports multiple declarations on a line if they're separated bu ':' literals*) +multiDeclaration + : singleAssignment (":" singleDeclaration)* NEWLINE + ; + +singleAssignment + : assignment + | statementList + ; + +assignment + : IDENTIFIER assignmentOperator expression + ; + +assignmentOperator + : ("=" | "*=" | "/=" | "\=" | "+=" | "-=" | "<<=" | ">>=") ; statementList @@ -50,20 +72,7 @@ expressionStatement ; expression - : assignmentExpression - ; - -assignmentExpression : conditionalExpression - | assignment - ; - -assignment - : IDENTIFIER assignmentOperator expression - ; - -assignmentOperator - : ("=" | "*=" | "/=" | "\=" | "+=" | "-=" | "<<=" | ">>=") ; (* From 3d5013244295ac830ae7c266e2be07324f90ded3 Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Sat, 13 Jan 2018 09:40:33 -0800 Subject: [PATCH 2/8] (parse) Start wiring up variable references and assignments --- src/parser/Statement.ts | 9 +++++++++ src/parser/index.ts | 30 ++++++++++++++++++++++++++++-- src/visitor/Executioner.ts | 4 ++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/parser/Statement.ts b/src/parser/Statement.ts index 4450cfc13..ba3764414 100644 --- a/src/parser/Statement.ts +++ b/src/parser/Statement.ts @@ -2,6 +2,7 @@ import * as Expr from "./Expression"; import { Token } from "../Token"; export interface Visitor { + visitAssignment(statement: Assignment): T; visitExpression(statement: Expression): T; visitPrint(statement: Print): T; } @@ -10,6 +11,14 @@ export interface Statement { accept (visitor: Visitor): R; } +export class Assignment implements Statement { + constructor(readonly name: Token, readonly value: Expr.Expression) {} + + accept(visitor: Visitor): R { + return visitor.visitAssignment(this); + } +} + export class Block implements Statement { constructor(readonly statements: ReadonlyArray) {} diff --git a/src/parser/index.ts b/src/parser/index.ts index 32d564b3f..6b82d6518 100644 --- a/src/parser/index.ts +++ b/src/parser/index.ts @@ -13,11 +13,14 @@ export function parse(toParse: ReadonlyArray) { current = 0; tokens = toParse; - let statements = []; + let statements: Statement[] = []; try { while (!isAtEnd()) { - statements.push(statement()); + let dec = declaration(); + if (dec) { + statements.push(dec); + } } return statements; @@ -26,6 +29,27 @@ export function parse(toParse: ReadonlyArray) { } } +function declaration(): Statement | undefined { + try { + if (match(Lexeme.Identifier)) { + return assignment(); + } + + return statement(); + } catch (error) { + return; + } +} + +function assignment(): Statement { + let name = previous(); + consume("Expected '=' after idenfifier", Lexeme.Equal); + // TODO: support +=, -=, >>=, etc. + + let value = expression(); + return new Stmt.Assignment(name, value); +} + function statement(): Statement { if (match(Lexeme.Print)) { return printStatement(); @@ -145,6 +169,8 @@ function primary(): Expression { let p = previous(); let lit = new Expr.Literal(p.literal); return lit; + case match(Lexeme.Identifier): + return new Expr.Variable(previous()); case match(Lexeme.LeftParen): let expr = expression(); consume("Unmatched '(' - expected ')' after expression", Lexeme.RightParen); diff --git a/src/visitor/Executioner.ts b/src/visitor/Executioner.ts index 8227bce29..ea7069af3 100644 --- a/src/visitor/Executioner.ts +++ b/src/visitor/Executioner.ts @@ -24,6 +24,10 @@ export class Executioner implements Expr.Visitor, Stmt.Visitor this.execute(statement)); } + visitAssignment(statement: Stmt.Assignment): TokenLiteral { + return undefined; + } + visitExpression(statement: Stmt.Expression): TokenLiteral { return this.evaluate(statement.expression); } From 08f48f5604b5649611735c1081f85847fd0d862d Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Sun, 14 Jan 2018 14:59:53 -0800 Subject: [PATCH 3/8] feat(parse,exec): Implement global-scope variables This includes assignment *and* retrieval from global scope! BRS is now stateful. --- src/Error.ts | 11 ++++++++--- src/index.ts | 3 ++- src/parser/index.ts | 13 +++++++++++-- src/visitor/Environment.ts | 19 +++++++++++++++++++ src/visitor/Executioner.ts | 14 ++++++++++---- 5 files changed, 50 insertions(+), 10 deletions(-) create mode 100644 src/visitor/Environment.ts diff --git a/src/Error.ts b/src/Error.ts index 6378ca84a..cb7b1d443 100644 --- a/src/Error.ts +++ b/src/Error.ts @@ -1,12 +1,17 @@ let foundError = false; export function make(message: string, line: number, file?: string) { + foundError = true; + return runtime(message, line, file); +} + +export function runtime(message: string, line: number, file?: string) { let location = file ? `${file}: ${line}` : `line ${line}`; + let error = new Error(`[${location}] ${message}`); if (process.env.NODE_ENV !== "test") { - console.error(`[${location}] ${message}`); + console.error(error); } - - foundError = true; + return error; } export function found() { diff --git a/src/index.ts b/src/index.ts index 309f73513..ee84855b2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,6 +9,8 @@ import { Executioner, isLong } from "./visitor/Executioner"; import { stringify } from "./Stringify"; import * as BrsError from "./Error"; +const executioner = new Executioner(); + export function execute(filename: string) { fs.readFile(filename, "utf-8", (err, contents) => { run(contents); @@ -47,7 +49,6 @@ function run(contents: string) { return; } - const executioner = new Executioner(); if (!statements) { return; } return executioner.exec(statements); diff --git a/src/parser/index.ts b/src/parser/index.ts index 6b82d6518..7f8ca1f0e 100644 --- a/src/parser/index.ts +++ b/src/parser/index.ts @@ -31,7 +31,7 @@ export function parse(toParse: ReadonlyArray) { function declaration(): Statement | undefined { try { - if (match(Lexeme.Identifier)) { + if (check(Lexeme.Identifier) && checkNext(Lexeme.Equal)) { return assignment(); } @@ -42,7 +42,7 @@ function declaration(): Statement | undefined { } function assignment(): Statement { - let name = previous(); + let name = advance(); consume("Expected '=' after idenfifier", Lexeme.Equal); // TODO: support +=, -=, >>=, etc. @@ -212,10 +212,19 @@ function check(lexeme: Lexeme) { return peek().kind === lexeme; } +function checkNext(lexeme: Lexeme) { + return peekNext().kind === lexeme; +} + function isAtEnd() { return peek().kind === Lexeme.Eof; } +function peekNext() { + if (isAtEnd()) { return peek(); } + return tokens[current + 1]; +} + function peek() { return tokens[current]; } diff --git a/src/visitor/Environment.ts b/src/visitor/Environment.ts new file mode 100644 index 000000000..70cc89a10 --- /dev/null +++ b/src/visitor/Environment.ts @@ -0,0 +1,19 @@ +import { Token, Literal as TokenLiteral } from "../Token"; +import { Lexeme } from "../Lexeme"; +import * as BrsError from "../Error"; + +export default class Environment { + private values = new Map(); + + public define(name: string, value: TokenLiteral): void { + this.values.set(name, value); + } + + public get(name: Token): TokenLiteral { + if (this.values.has(name.text!)) { + return this.values.get(name.text!); + } + + throw BrsError.runtime(`Undefined variable ${name.text}`, name.line); + } +} \ No newline at end of file diff --git a/src/visitor/Executioner.ts b/src/visitor/Executioner.ts index ea7069af3..7ab67d32f 100644 --- a/src/visitor/Executioner.ts +++ b/src/visitor/Executioner.ts @@ -7,6 +7,8 @@ import { Lexeme } from "../Lexeme"; import { stringify } from "../Stringify"; import * as BrsError from "../Error"; +import Environment from "./Environment"; + export function isLong(arg: TokenLiteral): arg is Long { return Long.isLong(arg); } @@ -20,11 +22,13 @@ function isString(arg: TokenLiteral): arg is string { } export class Executioner implements Expr.Visitor, Stmt.Visitor { + private environment = new Environment(); + exec(statements: Stmt.Statement[]) { return statements.map((statement) => this.execute(statement)); } - visitAssignment(statement: Stmt.Assignment): TokenLiteral { + visitAssign(statement: Expr.Assign): TokenLiteral { return undefined; } @@ -38,8 +42,10 @@ export class Executioner implements Expr.Visitor, Stmt.Visitor, Stmt.Visitor Date: Mon, 15 Jan 2018 08:42:45 -0800 Subject: [PATCH 4/8] docs(parser): Justify looking at two tokens to find declarations --- src/parser/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/parser/index.ts b/src/parser/index.ts index 7f8ca1f0e..885e5f714 100644 --- a/src/parser/index.ts +++ b/src/parser/index.ts @@ -31,6 +31,9 @@ export function parse(toParse: ReadonlyArray) { function declaration(): Statement | undefined { try { + // BrightScript is like python, in that variables can be declared without a `var`, + // `let`, (...) keyword. As such, we must check the token *after* an identifier to figure + // out what to do with it. if (check(Lexeme.Identifier) && checkNext(Lexeme.Equal)) { return assignment(); } From bc13d590b106d8f5de03a0867b1db336199e8d99 Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Mon, 15 Jan 2018 09:54:24 -0800 Subject: [PATCH 5/8] test(parse): Add tests for variable declarations --- test/parser/ParserTests.js | 13 ++++ test/parser/statement/Declaration.test.js | 53 ++++++++++++++++ .../__snapshots__/Declaration.test.js.snap | 60 +++++++++++++++++++ 3 files changed, 126 insertions(+) create mode 100644 test/parser/statement/Declaration.test.js create mode 100644 test/parser/statement/__snapshots__/Declaration.test.js.snap diff --git a/test/parser/ParserTests.js b/test/parser/ParserTests.js index 6356a267e..2d18af8db 100644 --- a/test/parser/ParserTests.js +++ b/test/parser/ParserTests.js @@ -16,5 +16,18 @@ exports.token = function(kind, literal) { }; } +/** + * Creates an Identifier token with the given `text`. + * @param {string} text + * @returns {object} a token with the provided `text`. + */ +exports.identifier = function(text) { + return { + kind: Lexeme.Identifier, + text: text, + line: 1 + }; +} + /** An end-of-file token. */ exports.EOF = exports.token(Lexeme.Eof); diff --git a/test/parser/statement/Declaration.test.js b/test/parser/statement/Declaration.test.js new file mode 100644 index 000000000..83ad64d66 --- /dev/null +++ b/test/parser/statement/Declaration.test.js @@ -0,0 +1,53 @@ +const Parser = require("../../../lib/parser"); +const { Lexeme } = require("../../../lib/Lexeme"); +const BrsError = require("../../../lib/Error"); + +const { token, identifier, EOF } = require("../ParserTests"); + + +describe("parser", () => { + afterEach(() => BrsError.reset()); + + describe("variable declarations", () => { + it("parses literal value assignments", () => { + let parsed = Parser.parse([ + identifier("foo"), + token(Lexeme.Equal), + token(Lexeme.Integer, 5), + EOF + ]); + + expect(parsed).toBeDefined(); + expect(parsed).not.toBeNull(); + expect(parsed).toMatchSnapshot(); + }); + + it("parses evaluated value assignments", () => { + let parsed = Parser.parse([ + identifier("bar"), + token(Lexeme.Equal), + token(Lexeme.Integer, 5), + token(Lexeme.Caret), + token(Lexeme.Integer, 3), + EOF + ]); + + expect(parsed).toBeDefined(); + expect(parsed).not.toBeNull(); + expect(parsed).toMatchSnapshot(); + }); + + it("parses variable aliasing", () => { + let parsed = Parser.parse([ + identifier("baz"), + token(Lexeme.Equal), + identifier("foo"), + EOF + ]); + + expect(parsed).toBeDefined(); + expect(parsed).not.toBeNull(); + expect(parsed).toMatchSnapshot(); + }); + }); +}); \ No newline at end of file diff --git a/test/parser/statement/__snapshots__/Declaration.test.js.snap b/test/parser/statement/__snapshots__/Declaration.test.js.snap new file mode 100644 index 000000000..73c1492d8 --- /dev/null +++ b/test/parser/statement/__snapshots__/Declaration.test.js.snap @@ -0,0 +1,60 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`parser variable declarations parses evaluated value assignments 1`] = ` +Array [ + Assignment { + "name": Object { + "kind": 21, + "line": 1, + "text": "bar", + }, + "value": Binary { + "left": Literal { + "value": 5, + }, + "right": Literal { + "value": 3, + }, + "token": Object { + "kind": 6, + "line": 1, + "literal": undefined, + }, + }, + }, +] +`; + +exports[`parser variable declarations parses literal value assignments 1`] = ` +Array [ + Assignment { + "name": Object { + "kind": 21, + "line": 1, + "text": "foo", + }, + "value": Literal { + "value": 5, + }, + }, +] +`; + +exports[`parser variable declarations parses variable aliasing 1`] = ` +Array [ + Assignment { + "name": Object { + "kind": 21, + "line": 1, + "text": "baz", + }, + "value": Variable { + "name": Object { + "kind": 21, + "line": 1, + "text": "foo", + }, + }, + }, +] +`; From 1fbd8c04a8f38a8135c481fa7738a64399a0921e Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Mon, 15 Jan 2018 10:08:46 -0800 Subject: [PATCH 6/8] test(exec): Add execution tests for global variables Global variables currently take a copy-value approach, e.g.: ```brightscript a = 10 b = a a = "foo" print a ' "foo" print b ' 10 print a = b ' false ``` This will likely need to change once we implement associative arrays in BRS, but IIRC "copy-value" is correct for primitive values. --- test/executioner/Variables.test.js | 52 ++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 test/executioner/Variables.test.js diff --git a/test/executioner/Variables.test.js b/test/executioner/Variables.test.js new file mode 100644 index 000000000..4f189d817 --- /dev/null +++ b/test/executioner/Variables.test.js @@ -0,0 +1,52 @@ +const BrsError = require("../../lib/Error"); +const Expr = require("../../lib/parser/Expression"); +const Stmt = require("../../lib/parser/Statement"); +const { identifier, token } = require("../parser/ParserTests"); +const { Lexeme } = require("../../lib/Lexeme"); +const { Executioner } = require("../../lib/visitor/Executioner"); + +let executioner; + +describe("executioner", () => { + beforeEach(() => { + BrsError.reset(); + executioner = new Executioner(); + }); + + it("returns 'invalid' for assignments", () => { + let ast = new Stmt.Assignment( + identifier("foo"), + new Expr.Literal(5) + ); + + let result = executioner.exec([ast]); + expect(result).toEqual([undefined]); + }); + + it("stores assigned values in global scope", () => { + let ast = new Stmt.Assignment( + identifier("bar"), + new Expr.Literal(6) + ); + executioner.exec([ast]); + expect( + executioner.environment.get( + identifier("bar") + ) + ).toBe(6); + }); + + it("retrieves variables from global scope", () => { + let assign = new Stmt.Assignment( + identifier("baz"), + new Expr.Literal(7) + ); + let retrieve = new Stmt.Expression( + new Expr.Variable( + identifier("baz") + ) + ); + let results = executioner.exec([ assign, retrieve ]); + expect(results).toEqual([undefined, 7]); + }); +}); \ No newline at end of file From 5c9405a373cef031874c50da7e3317b3225fe557 Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Mon, 15 Jan 2018 10:26:09 -0800 Subject: [PATCH 7/8] fix(parse): Allow newlines and ':' after assignments This language isn't very useful if you only get a REPL! --- src/parser/index.ts | 1 + test/parser/statement/Declaration.test.js | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/src/parser/index.ts b/src/parser/index.ts index 885e5f714..60a8e8e8f 100644 --- a/src/parser/index.ts +++ b/src/parser/index.ts @@ -50,6 +50,7 @@ function assignment(): Statement { // TODO: support +=, -=, >>=, etc. let value = expression(); + consume("Expected newline or ':' after assignment", Lexeme.Newline, Lexeme.Colon, Lexeme.Eof); return new Stmt.Assignment(name, value); } diff --git a/test/parser/statement/Declaration.test.js b/test/parser/statement/Declaration.test.js index 83ad64d66..3fab6f89f 100644 --- a/test/parser/statement/Declaration.test.js +++ b/test/parser/statement/Declaration.test.js @@ -9,6 +9,19 @@ describe("parser", () => { afterEach(() => BrsError.reset()); describe("variable declarations", () => { + it("allows newlines after assignments", () => { + let parsed = Parser.parse([ + identifier("hasNewlines"), + token(Lexeme.Equal), + token(Lexeme.True), + token(Lexeme.Newline), + EOF + ]); + + expect(parsed).toBeDefined(); + expect(parsed).not.toBeNull(); + }); + it("parses literal value assignments", () => { let parsed = Parser.parse([ identifier("foo"), From 6a67e98f9e389e28462790493d874e4f63c395b0 Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Mon, 15 Jan 2018 10:31:18 -0800 Subject: [PATCH 8/8] fix(parse): Synchronize after statement parse errors Rather than infinitely reporting an error at the same location (not very helpful), we should move on to the next statement boundary (only a ':' or newline so far) and continue parsing. --- src/parser/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/parser/index.ts b/src/parser/index.ts index 60a8e8e8f..be7dda060 100644 --- a/src/parser/index.ts +++ b/src/parser/index.ts @@ -40,6 +40,7 @@ function declaration(): Statement | undefined { return statement(); } catch (error) { + synchronize(); return; } }