From 94dcefcae299aff4a768358a20a617b39736addd Mon Sep 17 00:00:00 2001 From: Sean Barag Date: Sat, 9 Mar 2019 15:15:04 -0800 Subject: [PATCH] fix(parser): Allow complex expressions on RHS of set statements (#191) The parser was a little bit too greedy when attempting to detect set statements, which I should have noticed with the weird workarounds in that function. Now that we're _not_ trying to read an entire expression as the left-hand side of a set statement (e.g. `foo + bar *= 3`), that complication can be removed. Arbitrarily complex expressions are now supported on the right-hand side of a set statement, e.g. `foo.bar = true and false or 3 > 4` fixes #156 --- src/parser/Parser.ts | 70 ++-- test/e2e/Iterables.test.js | 5 +- test/e2e/resources/associative-arrays.brs | 6 +- test/parser/expression/Function.test.js | 2 +- test/parser/statement/Set.test.js | 43 ++- .../statement/__snapshots__/Set.test.js.snap | 326 ++++++++++++++---- 6 files changed, 340 insertions(+), 112 deletions(-) diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts index 1ed73f87f..5780be641 100644 --- a/src/parser/Parser.ts +++ b/src/parser/Parser.ts @@ -632,50 +632,34 @@ export class Parser { ); } - let expr = expression(); - - let left: Expr.Expression; - let operator: Token; - let right: Expr.Expression; - - if (expr instanceof Expr.Binary) { - // Simple assignments (e.g. `foo.bar = "baz"`) are parsed as binary expressions - // because of the ambiguity of the `=` operator without context - left = expr.left; - operator = expr.token; - right = expr.right; - } else if (check(...assignmentOperators)) { - // Otherwise it appears to be a set statement - left = expr; - operator = advance(); - let parsedRhs = expression(); - - // De-sugar assignment operators into binary expressions that operate on the - // left- and right-hand sides of the operator - right = new Expr.Binary(left, operator, parsedRhs); - } else { - return _expressionStatement(); - } - - - // Create a dotted or indexed "set" based on the left-hand side's type - if (left instanceof Expr.IndexedGet) { - consume("Expected newline or ':' after indexed 'set' statement", Lexeme.Newline, Lexeme.Colon, Lexeme.Eof); - - return new Stmt.IndexedSet( - left.obj, - left.index, - right, - left.closingSquare, - ); - } else if (left instanceof Expr.DottedGet) { - consume("Expected newline or ':' after dotted 'set' statement", Lexeme.Newline, Lexeme.Colon, Lexeme.Eof); + let expr = call(); + if (check(...assignmentOperators) && !(expr instanceof Expr.Call)) { + let left = expr; + let operator = advance(); + let right = expression(); + + + // Create a dotted or indexed "set" based on the left-hand side's type + if (left instanceof Expr.IndexedGet) { + consume("Expected newline or ':' after indexed 'set' statement", Lexeme.Newline, Lexeme.Colon, Lexeme.Eof); + + return new Stmt.IndexedSet( + left.obj, + left.index, + operator.kind === Lexeme.Equal ? right : new Expr.Binary(left, operator, right), + left.closingSquare, + ); + } else if (left instanceof Expr.DottedGet) { + consume("Expected newline or ':' after dotted 'set' statement", Lexeme.Newline, Lexeme.Colon, Lexeme.Eof); - return new Stmt.DottedSet( - left.obj, - left.name, - right, - ); + return new Stmt.DottedSet( + left.obj, + left.name, + operator.kind === Lexeme.Equal ? right : new Expr.Binary(left, operator, right), + ); + } else { + return _expressionStatement(); + } } else { return _expressionStatement(); } diff --git a/test/e2e/Iterables.test.js b/test/e2e/Iterables.test.js index 651f96c4a..5d7018713 100644 --- a/test/e2e/Iterables.test.js +++ b/test/e2e/Iterables.test.js @@ -47,7 +47,10 @@ describe("end to end iterables", () => { "3", // modify twoDimensional.secondLayer.level to sanity-check *= and friends - "6" + "6", + + // add `false` via expression to `empty` + "false" ]); }); }); diff --git a/test/e2e/resources/associative-arrays.brs b/test/e2e/resources/associative-arrays.brs index a8ed5ac85..498a257d8 100644 --- a/test/e2e/resources/associative-arrays.brs +++ b/test/e2e/resources/associative-arrays.brs @@ -19,6 +19,10 @@ twoDimensional.secondLayer.thirdLayer = { level: 3 } print twoDimensional.secondLayer.thirdLayer.level -' modify the top for an as +' modify the top for a silly example twoDimensional.secondLayer.level *= 3 print twoDimensional.secondLayer.level + +' add property to `empty` to be really silly +empty.isEmpty = oneDimensional.isOneDimensional and false +print empty.isEmpty \ No newline at end of file diff --git a/test/parser/expression/Function.test.js b/test/parser/expression/Function.test.js index f3fb7a33c..6ef0d5814 100644 --- a/test/parser/expression/Function.test.js +++ b/test/parser/expression/Function.test.js @@ -392,7 +392,7 @@ describe("parser", () => { it("allows sub expressions in call arguments", () => { const { statements, errors } = parser.parse([ identifier("acceptsCallback"), - { kind: Lexeme.LeftParen, text: "(", line: 1 }, + token(Lexeme.LeftParen, "("), token(Lexeme.Newline, "\\n"), token(Lexeme.Function, "function"), diff --git a/test/parser/statement/Set.test.js b/test/parser/statement/Set.test.js index 54d791193..eed2ed286 100644 --- a/test/parser/statement/Set.test.js +++ b/test/parser/statement/Set.test.js @@ -11,7 +11,7 @@ describe("parser indexed assignment", () => { }); describe("dotted", () => { - test("assignment", () => { + it("assigns anonymous functions", () => { let { statements, errors } = parser.parse([ identifier("foo"), token(Lexeme.Dot, "."), @@ -31,6 +31,25 @@ describe("parser indexed assignment", () => { expect(statements).toMatchSnapshot(); }); + it("assigns boolean expressions", () => { + let { statements, errors } = parser.parse([ + identifier("foo"), + token(Lexeme.Dot, "."), + identifier("bar"), + token(Lexeme.Equal, "="), + token(Lexeme.True, "true"), + token(Lexeme.And, "and"), + token(Lexeme.False, "false"), + token(Lexeme.Newline, "\\n"), + EOF + ]); + + expect(errors).toEqual([]) + expect(statements).toBeDefined(); + expect(statements).not.toBeNull(); + expect(statements).toMatchSnapshot(); + }); + test("assignment operator", () => { let { statements, errors } = parser.parse([ identifier("foo"), @@ -50,7 +69,7 @@ describe("parser indexed assignment", () => { }); describe("bracketed", () => { - it("assignment", () => { + it("assigns anonymous functions", () => { let { statements, errors } = parser.parse([ identifier("someArray"), token(Lexeme.LeftSquare, "["), @@ -71,6 +90,26 @@ describe("parser indexed assignment", () => { expect(statements).toMatchSnapshot(); }); + it("assigns boolean expressions", () => { + let { statements, errors } = parser.parse([ + identifier("someArray"), + token(Lexeme.LeftSquare, "["), + token(Lexeme.Integer, "0", new Int32(0)), + token(Lexeme.RightSquare, "]"), + token(Lexeme.Equal, "="), + token(Lexeme.True, "true"), + token(Lexeme.And, "and"), + token(Lexeme.False, "false"), + token(Lexeme.Newline, "\\n"), + EOF + ]); + + expect(errors).toEqual([]) + expect(statements).toBeDefined(); + expect(statements).not.toBeNull(); + expect(statements).toMatchSnapshot(); + }); + it("assignment operator", () => { let { statements, errors } = parser.parse([ identifier("someArray"), diff --git a/test/parser/statement/__snapshots__/Set.test.js.snap b/test/parser/statement/__snapshots__/Set.test.js.snap index 378a0379f..4c855acd5 100644 --- a/test/parser/statement/__snapshots__/Set.test.js.snap +++ b/test/parser/statement/__snapshots__/Set.test.js.snap @@ -1,6 +1,149 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`parser indexed assignment bracketed assignment 1`] = ` +exports[`parser indexed assignment bracketed assignment operator 1`] = ` +Array [ + IndexedSet { + "closingSquare": Object { + "isReserved": false, + "kind": 3, + "literal": undefined, + "location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "text": "]", + }, + "index": Literal { + "_location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "value": Int32 { + "kind": 3, + "value": 0, + }, + }, + "obj": Variable { + "name": Object { + "isReserved": false, + "kind": 28, + "literal": undefined, + "location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "text": "someArray", + }, + }, + "value": Binary { + "left": IndexedGet { + "closingSquare": Object { + "isReserved": false, + "kind": 3, + "literal": undefined, + "location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "text": "]", + }, + "index": Literal { + "_location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "value": Int32 { + "kind": 3, + "value": 0, + }, + }, + "obj": Variable { + "name": Object { + "isReserved": false, + "kind": 28, + "literal": undefined, + "location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "text": "someArray", + }, + }, + }, + "right": Literal { + "_location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "value": Int32 { + "kind": 3, + "value": 3, + }, + }, + "token": Object { + "isReserved": false, + "kind": 17, + "literal": undefined, + "location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "text": "*=", + }, + }, + }, +] +`; + +exports[`parser indexed assignment bracketed assigns anonymous functions 1`] = ` Array [ IndexedSet { "closingSquare": Object { @@ -106,7 +249,7 @@ Array [ ] `; -exports[`parser indexed assignment bracketed assignment operator 1`] = ` +exports[`parser indexed assignment bracketed assigns boolean expressions 1`] = ` Array [ IndexedSet { "closingSquare": Object { @@ -160,10 +303,101 @@ Array [ }, }, "value": Binary { - "left": IndexedGet { - "closingSquare": Object { + "left": Literal { + "_location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "value": BrsBoolean { + "kind": 1, + "value": true, + }, + }, + "right": Literal { + "_location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "value": BrsBoolean { + "kind": 1, + "value": false, + }, + }, + "token": Object { + "isReserved": true, + "kind": 45, + "literal": undefined, + "location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "text": "and", + }, + }, + }, +] +`; + +exports[`parser indexed assignment dotted assignment operator 1`] = ` +Array [ + DottedSet { + "name": Object { + "isReserved": false, + "kind": 28, + "literal": undefined, + "location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "text": "bar", + }, + "obj": Variable { + "name": Object { + "isReserved": false, + "kind": 28, + "literal": undefined, + "location": Object { + "end": Object { + "column": -9, + "line": -9, + }, + "start": Object { + "column": -9, + "line": -9, + }, + }, + "text": "foo", + }, + }, + "value": Binary { + "left": DottedGet { + "name": Object { "isReserved": false, - "kind": 3, + "kind": 28, "literal": undefined, "location": Object { "end": Object { @@ -175,23 +409,7 @@ Array [ "line": -9, }, }, - "text": "]", - }, - "index": Literal { - "_location": Object { - "end": Object { - "column": -9, - "line": -9, - }, - "start": Object { - "column": -9, - "line": -9, - }, - }, - "value": Int32 { - "kind": 3, - "value": 0, - }, + "text": "bar", }, "obj": Variable { "name": Object { @@ -208,7 +426,7 @@ Array [ "line": -9, }, }, - "text": "someArray", + "text": "foo", }, }, }, @@ -225,7 +443,7 @@ Array [ }, "value": Int32 { "kind": 3, - "value": 3, + "value": 5, }, }, "token": Object { @@ -249,7 +467,7 @@ Array [ ] `; -exports[`parser indexed assignment dotted assignment 1`] = ` +exports[`parser indexed assignment dotted assigns anonymous functions 1`] = ` Array [ DottedSet { "name": Object { @@ -339,7 +557,7 @@ Array [ ] `; -exports[`parser indexed assignment dotted assignment operator 1`] = ` +exports[`parser indexed assignment dotted assigns boolean expressions 1`] = ` Array [ DottedSet { "name": Object { @@ -377,41 +595,21 @@ Array [ }, }, "value": Binary { - "left": DottedGet { - "name": Object { - "isReserved": false, - "kind": 28, - "literal": undefined, - "location": Object { - "end": Object { - "column": -9, - "line": -9, - }, - "start": Object { - "column": -9, - "line": -9, - }, + "left": Literal { + "_location": Object { + "end": Object { + "column": -9, + "line": -9, }, - "text": "bar", - }, - "obj": Variable { - "name": Object { - "isReserved": false, - "kind": 28, - "literal": undefined, - "location": Object { - "end": Object { - "column": -9, - "line": -9, - }, - "start": Object { - "column": -9, - "line": -9, - }, - }, - "text": "foo", + "start": Object { + "column": -9, + "line": -9, }, }, + "value": BrsBoolean { + "kind": 1, + "value": true, + }, }, "right": Literal { "_location": Object { @@ -424,14 +622,14 @@ Array [ "line": -9, }, }, - "value": Int32 { - "kind": 3, - "value": 5, + "value": BrsBoolean { + "kind": 1, + "value": false, }, }, "token": Object { - "isReserved": false, - "kind": 17, + "isReserved": true, + "kind": 45, "literal": undefined, "location": Object { "end": Object { @@ -443,7 +641,7 @@ Array [ "line": -9, }, }, - "text": "*=", + "text": "and", }, }, },