From 2f32d2835967950fb99f83e591fed147176463c2 Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Fri, 23 Sep 2022 14:54:29 -0300 Subject: [PATCH 1/6] Finds and includes more deeply embedded expressions --- src/files/BrsFile.spec.ts | 19 ++ src/files/BrsFile.ts | 2 +- src/parser/Expression.ts | 35 ++-- src/parser/Parser.ts | 166 +++++++++++------- .../tests/expression/ArrayLiterals.spec.ts | 52 ++++++ .../AssociativeArrayLiterals.spec.ts | 68 ++++++- src/parser/tests/expression/Call.spec.ts | 126 ++++++++++++- src/parser/tests/expression/Indexing.spec.ts | 74 +++++++- src/testHelpers.spec.ts | 87 ++++++--- 9 files changed, 515 insertions(+), 114 deletions(-) diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 5e6c1207b..17545ab1f 100644 --- a/src/files/BrsFile.spec.ts +++ b/src/files/BrsFile.spec.ts @@ -1311,6 +1311,25 @@ describe('BrsFile', () => { expect(file.functionCalls[1].nameRange).to.eql(Range.create(5, 20, 5, 23)); }); + it('finds function calls that are unfinished', () => { + let file = new BrsFile('absolute_path/file.brs', 'relative_path/file.brs', program); + file.parse(` + function DoA() + DoB("a" + end function + function DoB(a as string) + DoC( + end function + `); + expect(file.functionCalls.length).to.equal(2); + + expect(file.functionCalls[0].range).to.eql(Range.create(2, 20, 2, 27)); + expect(file.functionCalls[0].nameRange).to.eql(Range.create(2, 20, 2, 23)); + + expect(file.functionCalls[1].range).to.eql(Range.create(5, 20, 5, 24)); + expect(file.functionCalls[1].nameRange).to.eql(Range.create(5, 20, 5, 23)); + }); + it('sanitizes brs errors', () => { let file = new BrsFile('absolute_path/file.brs', 'relative_path/file.brs', program); file.parse(` diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index 204900cf4..72d30ebe1 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -692,7 +692,7 @@ export class BrsFile { } } let functionCall: FunctionCall = { - range: util.createRangeFromPositions(expression.range.start, expression.closingParen.range.end), + range: expression.range, //util.(expression.range.start, expression.closingParen.range.end), functionScope: this.getFunctionScopeAtPosition(callee.range.start), file: this, name: functionName, diff --git a/src/parser/Expression.ts b/src/parser/Expression.ts index 07564f031..5bac6389b 100644 --- a/src/parser/Expression.ts +++ b/src/parser/Expression.ts @@ -62,7 +62,7 @@ export class CallExpression extends Expression { readonly args: Expression[] ) { super(); - this.range = util.createRangeFromPositions(this.callee.range.start, this.closingParen.range.end); + this.range = util.createBoundingRange(this.callee, this.openingParen, ...args, this.closingParen); } public readonly range: Range; @@ -88,9 +88,11 @@ export class CallExpression extends Expression { let arg = this.args[i]; result.push(...arg.transpile(state)); } - result.push( - state.transpileToken(this.closingParen) - ); + if (this.closingParen) { + result.push( + state.transpileToken(this.closingParen) + ); + } return result; } @@ -429,8 +431,8 @@ export class IndexedGetExpression extends Expression { ...this.obj.transpile(state), this.questionDotToken ? state.transpileToken(this.questionDotToken) : '', state.transpileToken(this.openingSquare), - ...this.index.transpile(state), - state.transpileToken(this.closingSquare) + ...(this.index ? this.index.transpile(state) : []), + this.closingSquare ? state.transpileToken(this.closingSquare) : '' ]; } @@ -546,7 +548,7 @@ export class ArrayLiteralExpression extends Expression { readonly hasSpread = false ) { super(); - this.range = util.createRangeFromPositions(this.open.range.start, this.close.range.end); + this.range = util.createBoundingRange(this.open, ...this.elements, this.close); } public readonly range: Range; @@ -591,10 +593,11 @@ export class ArrayLiteralExpression extends Expression { result.push('\n'); result.push(state.indent()); } - - result.push( - state.transpileToken(this.close) - ); + if (this.close) { + result.push( + state.transpileToken(this.close) + ); + } return result; } @@ -637,7 +640,7 @@ export class AALiteralExpression extends Expression { readonly close: Token ) { super(); - this.range = util.createRangeFromPositions(this.open.range.start, this.close.range.end); + this.range = util.createBoundingRange(this.open, ...this.elements, this.close); } public readonly range: Range; @@ -704,9 +707,11 @@ export class AALiteralExpression extends Expression { result.push(state.indent()); } //close curly - result.push( - state.transpileToken(this.close) - ); + if (this.close) { + result.push( + state.transpileToken(this.close) + ); + } return result; } diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts index c68586f2f..d6a31e67b 100644 --- a/src/parser/Parser.ts +++ b/src/parser/Parser.ts @@ -2357,12 +2357,17 @@ export class Parser { private indexedGet(expr: Expression) { let openingSquare = this.previous(); let questionDotToken = this.getMatchingTokenAtOffset(-2, TokenKind.QuestionDot); + let index: Expression; + let closingSquare: Token; while (this.match(TokenKind.Newline)) { } - - let index = this.expression(); + try { + index = this.expression(); + } catch (error) { + this.rethrowNonDiagnosticError(error); + } while (this.match(TokenKind.Newline)) { } - let closingSquare = this.consume( + closingSquare = this.tryConsume( DiagnosticMessages.expectedRightSquareBraceAfterArrayOrObjectIndex(), TokenKind.RightSquareBracket ); @@ -2429,11 +2434,14 @@ export class Parser { expr = this.indexedGet(expr); } else { let dot = this.previous(); - let name = this.consume( + let name = this.tryConsume( DiagnosticMessages.expectedPropertyNameAfterPeriod(), TokenKind.Identifier, ...AllowedProperties ); + if (!name) { + break; + } // force it into an identifier so the AST makes some sense name.kind = TokenKind.Identifier; @@ -2444,7 +2452,7 @@ export class Parser { } else if (this.checkAny(TokenKind.At, TokenKind.QuestionAt)) { let dot = this.advance(); - let name = this.consume( + let name = this.tryConsume( DiagnosticMessages.expectedAttributeNameAfterAtSymbol(), TokenKind.Identifier, ...AllowedProperties @@ -2452,7 +2460,9 @@ export class Parser { // force it into an identifier so the AST makes some sense name.kind = TokenKind.Identifier; - + if (!name) { + break; + } expr = new XmlAttributeGetExpression(expr, name as Identifier, dot); //only allow a single `@` expression break; @@ -2483,13 +2493,19 @@ export class Parser { }); throw this.lastDiagnosticAsError(); } - args.push(this.expression()); + try { + args.push(this.expression()); + } catch (error) { + this.rethrowNonDiagnosticError(error); + // we were unable to get an expression, so don't continue + break; + } } while (this.match(TokenKind.Comma)); } while (this.match(TokenKind.Newline)) { } - const closingParen = this.consume( + const closingParen = this.tryConsume( DiagnosticMessages.expectedRightParenAfterFunctionCallArguments(), TokenKind.RightParen ); @@ -2614,34 +2630,39 @@ export class Parser { while (this.match(TokenKind.Newline)) { } + let closingSquare: Token; if (!this.match(TokenKind.RightSquareBracket)) { - elements.push(this.expression()); + try { + elements.push(this.expression()); - while (this.matchAny(TokenKind.Comma, TokenKind.Newline, TokenKind.Comment)) { - if (this.checkPrevious(TokenKind.Comment) || this.check(TokenKind.Comment)) { - let comment = this.check(TokenKind.Comment) ? this.advance() : this.previous(); - elements.push(new CommentStatement([comment])); - } - while (this.match(TokenKind.Newline)) { + while (this.matchAny(TokenKind.Comma, TokenKind.Newline, TokenKind.Comment)) { + if (this.checkPrevious(TokenKind.Comment) || this.check(TokenKind.Comment)) { + let comment = this.check(TokenKind.Comment) ? this.advance() : this.previous(); + elements.push(new CommentStatement([comment])); + } + while (this.match(TokenKind.Newline)) { - } + } - if (this.check(TokenKind.RightSquareBracket)) { - break; - } + if (this.check(TokenKind.RightSquareBracket)) { + break; + } - elements.push(this.expression()); + elements.push(this.expression()); + } + } catch (error: any) { + this.rethrowNonDiagnosticError(error); } - this.consume( + closingSquare = this.tryConsume( DiagnosticMessages.unmatchedLeftSquareBraceAfterArrayLiteral(), TokenKind.RightSquareBracket ); + } else { + closingSquare = this.previous(); } - let closingSquare = this.previous(); - //this.consume("Expected newline or ':' after array literal", TokenKind.Newline, TokenKind.Colon, TokenKind.Eof); return new ArrayLiteralExpression(elements, openingSquare, closingSquare); } @@ -2677,47 +2698,14 @@ export class Parser { }; while (this.match(TokenKind.Newline)) { } - + let closingBrace: Token; if (!this.match(TokenKind.RightCurlyBrace)) { let lastAAMember: AAMemberExpression; - if (this.check(TokenKind.Comment)) { - lastAAMember = null; - members.push(new CommentStatement([this.advance()])); - } else { - let k = key(); - let expr = this.expression(); - lastAAMember = new AAMemberExpression( - k.keyToken, - k.colonToken, - expr - ); - members.push(lastAAMember); - } - - while (this.matchAny(TokenKind.Comma, TokenKind.Newline, TokenKind.Colon, TokenKind.Comment)) { - // collect comma at end of expression - if (lastAAMember && this.checkPrevious(TokenKind.Comma)) { - lastAAMember.commaToken = this.previous(); - } - - //check for comment at the end of the current line - if (this.check(TokenKind.Comment) || this.checkPrevious(TokenKind.Comment)) { - let token = this.checkPrevious(TokenKind.Comment) ? this.previous() : this.advance(); - members.push(new CommentStatement([token])); + try { + if (this.check(TokenKind.Comment)) { + lastAAMember = null; + members.push(new CommentStatement([this.advance()])); } else { - this.consumeStatementSeparators(true); - - //check for a comment on its own line - if (this.check(TokenKind.Comment) || this.checkPrevious(TokenKind.Comment)) { - let token = this.checkPrevious(TokenKind.Comment) ? this.previous() : this.advance(); - lastAAMember = null; - members.push(new CommentStatement([token])); - continue; - } - - if (this.check(TokenKind.RightCurlyBrace)) { - break; - } let k = key(); let expr = this.expression(); lastAAMember = new AAMemberExpression( @@ -2727,16 +2715,53 @@ export class Parser { ); members.push(lastAAMember); } + + while (this.matchAny(TokenKind.Comma, TokenKind.Newline, TokenKind.Colon, TokenKind.Comment)) { + // collect comma at end of expression + if (lastAAMember && this.checkPrevious(TokenKind.Comma)) { + lastAAMember.commaToken = this.previous(); + } + + //check for comment at the end of the current line + if (this.check(TokenKind.Comment) || this.checkPrevious(TokenKind.Comment)) { + let token = this.checkPrevious(TokenKind.Comment) ? this.previous() : this.advance(); + members.push(new CommentStatement([token])); + } else { + this.consumeStatementSeparators(true); + + //check for a comment on its own line + if (this.check(TokenKind.Comment) || this.checkPrevious(TokenKind.Comment)) { + let token = this.checkPrevious(TokenKind.Comment) ? this.previous() : this.advance(); + lastAAMember = null; + members.push(new CommentStatement([token])); + continue; + } + + if (this.check(TokenKind.RightCurlyBrace)) { + break; + } + let k = key(); + let expr = this.expression(); + lastAAMember = new AAMemberExpression( + k.keyToken, + k.colonToken, + expr + ); + members.push(lastAAMember); + } + } + } catch (error: any) { + this.rethrowNonDiagnosticError(error); } - this.consume( + closingBrace = this.tryConsume( DiagnosticMessages.unmatchedLeftCurlyAfterAALiteral(), TokenKind.RightCurlyBrace ); + } else { + closingBrace = this.previous(); } - let closingBrace = this.previous(); - const aaExpr = new AALiteralExpression(members, openingBrace, closingBrace); this.addPropertyHints(aaExpr); return aaExpr; @@ -2911,6 +2936,19 @@ export class Parser { return this.tokens[this.current - 1]; } + /** + * There are sometimes we get catch an error that is a diagnostic + * If that's the case, we want to continue parsing + * Otherwise, re-throw the error + * + * @param error error caught in a try/catch + */ + private rethrowNonDiagnosticError(error) { + if (!error.isDiagnostic) { + throw error; + } + } + /** * Get the token that is {offset} indexes away from {this.current} * @param offset the number of index steps away from current index to fetch diff --git a/src/parser/tests/expression/ArrayLiterals.spec.ts b/src/parser/tests/expression/ArrayLiterals.spec.ts index d50367136..f5d8d7a1e 100644 --- a/src/parser/tests/expression/ArrayLiterals.spec.ts +++ b/src/parser/tests/expression/ArrayLiterals.spec.ts @@ -4,6 +4,11 @@ import { Parser } from '../../Parser'; import { TokenKind } from '../../../lexer/TokenKind'; import { EOF, identifier, token } from '../Parser.spec'; import { Range } from 'vscode-languageserver'; +import { expectDiagnostics, expectDiagnosticsIncludes } from '../../../testHelpers.spec'; +import { DiagnosticMessages } from '../../../DiagnosticMessages'; +import { isArrayLiteralExpression, isAssignmentStatement, isDottedGetExpression, isLiteralExpression } from '../../../astUtils/reflection'; +import type { AssignmentStatement } from '../../Statement'; +import type { ArrayLiteralExpression } from '../../Expression'; describe('parser array literals', () => { describe('empty arrays', () => { @@ -169,6 +174,53 @@ describe('parser array literals', () => { }); }); + describe('unfinished', () => { + it('will still be parsed', () => { + let { statements, diagnostics } = Parser.parse([ + identifier('_'), + token(TokenKind.Equal, '='), + token(TokenKind.LeftSquareBracket, '['), + token(TokenKind.IntegerLiteral, '1'), + token(TokenKind.Comma, ','), + identifier('data'), + token(TokenKind.Dot, '.'), + identifier('foo'), + EOF + ]); + + expectDiagnostics(diagnostics, [DiagnosticMessages.unmatchedLeftSquareBraceAfterArrayLiteral()]); + expect(statements).to.be.lengthOf(1); + expect(isAssignmentStatement(statements[0])).to.be.true; + const assignStmt = statements[0] as AssignmentStatement; + expect(isArrayLiteralExpression(assignStmt.value)); + const arryLitExpr = assignStmt.value as ArrayLiteralExpression; + expect(isLiteralExpression(arryLitExpr.elements[0])).to.be.true; + expect(isDottedGetExpression(arryLitExpr.elements[1])).to.be.true; + }); + + it('gets correct diagnostic for missing square brace without elements', () => { + let { diagnostics } = Parser.parse(` + sub setData() + m.data = [ + end sub + `); + expectDiagnosticsIncludes(diagnostics, [ + DiagnosticMessages.unmatchedLeftSquareBraceAfterArrayLiteral() + ]); + }); + + it('gets correct diagnostic for missing curly brace with elements', () => { + let { diagnostics } = Parser.parse(` + sub setData() + m.data = [1,2,3 + end sub + `); + expectDiagnosticsIncludes(diagnostics, [ + DiagnosticMessages.unmatchedLeftSquareBraceAfterArrayLiteral() + ]); + }); + }); + it('location tracking', () => { /** * 0 0 0 1 diff --git a/src/parser/tests/expression/AssociativeArrayLiterals.spec.ts b/src/parser/tests/expression/AssociativeArrayLiterals.spec.ts index da50c6620..71c16dff3 100644 --- a/src/parser/tests/expression/AssociativeArrayLiterals.spec.ts +++ b/src/parser/tests/expression/AssociativeArrayLiterals.spec.ts @@ -5,8 +5,10 @@ import { TokenKind } from '../../../lexer/TokenKind'; import { EOF, identifier, token } from '../Parser.spec'; import { Range } from 'vscode-languageserver'; import type { AssignmentStatement } from '../../Statement'; -import type { AALiteralExpression } from '../../Expression'; -import { isCommentStatement } from '../../../astUtils/reflection'; +import type { AALiteralExpression, AAMemberExpression } from '../../Expression'; +import { isAALiteralExpression, isAssignmentStatement, isCommentStatement, isDottedGetExpression, isLiteralExpression } from '../../../astUtils/reflection'; +import { expectDiagnostics, expectDiagnosticsIncludes } from '../../../testHelpers.spec'; +import { DiagnosticMessages } from '../../../DiagnosticMessages'; describe('parser associative array literals', () => { describe('empty associative arrays', () => { @@ -205,6 +207,68 @@ describe('parser associative array literals', () => { ]); }); + describe('unfinished', () => { + it('will still be parsed', () => { + // No closing brace: _ = {name: "john", age: 42, address: data.address + let { statements, diagnostics } = Parser.parse([ + identifier('_'), + token(TokenKind.Equal, '='), + token(TokenKind.LeftCurlyBrace, '{'), + identifier('name'), + token(TokenKind.Colon, ':'), + token(TokenKind.StringLiteral, '"john"'), + token(TokenKind.Comma, ','), + identifier('age'), + token(TokenKind.Colon, ':'), + token(TokenKind.IntegerLiteral, '42'), + token(TokenKind.Comma, ','), + identifier('address'), + token(TokenKind.Colon, ':'), + identifier('data'), + token(TokenKind.Dot, '.'), + identifier('address'), + EOF + ]); + + expectDiagnostics(diagnostics, [DiagnosticMessages.unmatchedLeftCurlyAfterAALiteral()]); + expect(statements).to.be.lengthOf(1); + expect(isAssignmentStatement(statements[0])).to.be.true; + const assignStmt = statements[0] as AssignmentStatement; + expect(isAALiteralExpression(assignStmt.value)); + const aaLitExpr = assignStmt.value as AALiteralExpression; + expect(aaLitExpr.elements).to.be.lengthOf(3); + const memberExprs = aaLitExpr.elements as AAMemberExpression[]; + expect(isLiteralExpression(memberExprs[0].value)).to.be.true; + expect(isLiteralExpression(memberExprs[1].value)).to.be.true; + expect(isDottedGetExpression(memberExprs[2].value)).to.be.true; + }); + + it('gets correct diagnostic for missing curly brace without final value', () => { + let { diagnostics } = Parser.parse(` + + sub setData() + m.data = {hello: + end sub + `); + expectDiagnostics(diagnostics, [ + DiagnosticMessages.unexpectedToken('\n'), + DiagnosticMessages.unmatchedLeftCurlyAfterAALiteral() + ]); + }); + + it('gets correct diagnostic for missing curly brace with final value', () => { + let { diagnostics } = Parser.parse(` + + sub setData() + m.data = {hello: "world" + end sub + `); + expectDiagnosticsIncludes(diagnostics, [ + DiagnosticMessages.unmatchedLeftCurlyAfterAALiteral() + ]); + }); + }); + it('location tracking', () => { /** * 0 0 0 1 diff --git a/src/parser/tests/expression/Call.spec.ts b/src/parser/tests/expression/Call.spec.ts index b8497148b..303ec7042 100644 --- a/src/parser/tests/expression/Call.spec.ts +++ b/src/parser/tests/expression/Call.spec.ts @@ -5,6 +5,10 @@ import { Lexer } from '../../../lexer/Lexer'; import { TokenKind } from '../../../lexer/TokenKind'; import { EOF, identifier, token } from '../Parser.spec'; import { Range } from 'vscode-languageserver'; +import type { ExpressionStatement, FunctionStatement } from '../../Statement'; +import { DiagnosticMessages } from '../../../DiagnosticMessages'; +import { expectDiagnostics, expectDiagnosticsIncludes } from '../../../testHelpers.spec'; +import { isAssignmentStatement, isCallExpression, isDottedGetExpression, isDottedSetStatement, isExpressionStatement, isIndexedGetExpression, isReturnStatement } from '../../../astUtils/reflection'; describe('parser call expressions', () => { it('parses named function calls', () => { @@ -49,7 +53,10 @@ describe('parser call expressions', () => { `); const { statements, diagnostics } = Parser.parse(tokens); //there should only be 1 error - expect(diagnostics).to.be.lengthOf(1); + expectDiagnostics(diagnostics, [ + DiagnosticMessages.unexpectedToken(':'), + DiagnosticMessages.expectedRightParenAfterFunctionCallArguments() + ]); expect(statements).to.be.length.greaterThan(0); //the error should be BEFORE the `name = "bob"` statement expect(diagnostics[0].range.end.character).to.be.lessThan(25); @@ -69,6 +76,55 @@ describe('parser call expressions', () => { expect(statements).to.be.length.greaterThan(0); }); + it('includes partial statements and expressions', () => { + const { statements, diagnostics } = Parser.parse(` + function processData(data) + data.foo.bar. = "hello" + data.foo.func(). + result = data.foo. + return result. + end function + `); + + expect(diagnostics).to.be.lengthOf(4); + expectDiagnostics(diagnostics, [ + DiagnosticMessages.expectedPropertyNameAfterPeriod(), + DiagnosticMessages.expectedPropertyNameAfterPeriod(), + DiagnosticMessages.expectedPropertyNameAfterPeriod(), + DiagnosticMessages.expectedPropertyNameAfterPeriod() + ]); + expect(statements).to.be.lengthOf(1); + const bodyStatements = (statements[0] as FunctionStatement).func.body.statements; + expect(bodyStatements).to.be.lengthOf(4); // each line is a statement + + // first should be: data.foo.bar = "hello" + expect(isDottedSetStatement(bodyStatements[0])).to.be.true; + const setStmt = bodyStatements[0] as any; + expect(setStmt.name.text).to.equal('bar'); + expect(setStmt.obj.name.text).to.equal('foo'); + expect(setStmt.obj.obj.name.text).to.equal('data'); + expect(setStmt.value.token.text).to.equal('"hello"'); + + // 2nd should be: data.foo.func() + expect(isExpressionStatement(bodyStatements[1])).to.be.true; + expect(isCallExpression((bodyStatements[1] as any).expression)).to.be.true; + const callExpr = (bodyStatements[1] as any).expression; + expect(callExpr.callee.name.text).to.be.equal('func'); + expect(callExpr.callee.obj.name.text).to.be.equal('foo'); + expect(callExpr.callee.obj.obj.name.text).to.be.equal('data'); + + // 3rd should be: result = data.foo + expect(isAssignmentStatement(bodyStatements[2])).to.be.true; + const assignStmt = (bodyStatements[2] as any); + expect(assignStmt.name.text).to.equal('result'); + expect(assignStmt.value.name.text).to.equal('foo'); + + // 4th should be: return result + expect(isReturnStatement(bodyStatements[3])).to.be.true; + const returnStmt = (bodyStatements[3] as any); + expect(returnStmt.value.name.text).to.equal('result'); + }); + it('accepts arguments', () => { const { statements, diagnostics } = Parser.parse([ identifier('add'), @@ -143,4 +199,72 @@ describe('parser call expressions', () => { Range.create(0, 0, 0, 17) ); }); + + describe('unfinished', () => { + it('continues parsing inside unfinished function calls', () => { + const { statements, diagnostics } = Parser.parse(` + sub doSomething(data) + otherFunc(data.foo, data.bar[0] + end sub + `); + + expect(diagnostics).to.be.lengthOf(2); + expectDiagnostics(diagnostics, [ + DiagnosticMessages.expectedRightParenAfterFunctionCallArguments(), + DiagnosticMessages.expectedNewlineOrColon() + ]); + expect(statements).to.be.lengthOf(1); + const bodyStatements = (statements[0] as FunctionStatement).func.body.statements; + expect(bodyStatements).to.be.lengthOf(1); + + // Function statement should still be parsed + expect(isExpressionStatement(bodyStatements[0])).to.be.true; + expect(isCallExpression((bodyStatements[0] as ExpressionStatement).expression)).to.be.true; + const callExpr = (bodyStatements[0] as ExpressionStatement).expression as any; + expect(callExpr.callee.name.text).to.equal('otherFunc'); + + // args should still be parsed, as well! + expect(callExpr.args).to.be.lengthOf(2); + expect(isDottedGetExpression(callExpr.args[0])).to.be.true; + expect(isIndexedGetExpression(callExpr.args[1])).to.be.true; + }); + + it('gets correct diagnostic for missing close paren without args', () => { + let { diagnostics } = Parser.parse(` + sub process() + someFunc( + end sub + `); + expectDiagnosticsIncludes(diagnostics, [ + DiagnosticMessages.expectedRightParenAfterFunctionCallArguments() + ]); + }); + + it('gets correct diagnostic for missing close paren with args', () => { + let { diagnostics } = Parser.parse(` + sub process() + someFunc("hello" + end sub + `); + expectDiagnosticsIncludes(diagnostics, [ + DiagnosticMessages.expectedRightParenAfterFunctionCallArguments() + ]); + }); + + it('gets correct diagnostic for missing close paren with invalid expression as arg', () => { + let { diagnostics, statements } = Parser.parse(` + sub process(data) + someFunc(data.name. , + end sub + `); + expectDiagnosticsIncludes(diagnostics, [ + DiagnosticMessages.expectedRightParenAfterFunctionCallArguments() + ]); + expect(statements).to.be.lengthOf(1); + const bodyStatements = (statements[0] as FunctionStatement).func.body.statements; + expect(bodyStatements).to.be.lengthOf(1); + + }); + }); + }); diff --git a/src/parser/tests/expression/Indexing.spec.ts b/src/parser/tests/expression/Indexing.spec.ts index ca8f64dbe..744695d71 100644 --- a/src/parser/tests/expression/Indexing.spec.ts +++ b/src/parser/tests/expression/Indexing.spec.ts @@ -6,6 +6,10 @@ import { EOF, identifier, token } from '../Parser.spec'; import { Range } from 'vscode-languageserver'; import { DiagnosticMessages } from '../../../DiagnosticMessages'; import { AssignmentStatement } from '../../Statement'; +import { expectDiagnostics, expectDiagnosticsIncludes } from '../../../testHelpers.spec'; +import { isAssignmentStatement, isDottedGetExpression, isIndexedGetExpression, isVariableExpression } from '../../../astUtils/reflection'; +import type { DottedGetExpression, IndexedGetExpression, VariableExpression } from '../../Expression'; + describe('parser indexing', () => { describe('one level', () => { @@ -58,7 +62,7 @@ describe('parser indexing', () => { }); it('multiple dots', () => { - let { diagnostics } = Parser.parse([ + let { diagnostics, statements } = Parser.parse([ identifier('_'), token(TokenKind.Equal, '='), identifier('foo'), @@ -68,15 +72,18 @@ describe('parser indexing', () => { token(TokenKind.LeftSquareBracket, '['), token(TokenKind.Integer, '2'), token(TokenKind.RightSquareBracket, ']'), + token(TokenKind.Newline), EOF ]); - expect(diagnostics.length).to.equal(1); - expect( - diagnostics[0]?.message - ).to.exist.and.to.equal( - DiagnosticMessages.expectedPropertyNameAfterPeriod().message - ); + expect(diagnostics.length).to.equal(3); + expectDiagnostics(diagnostics, [ + DiagnosticMessages.expectedPropertyNameAfterPeriod(), // expected name after first dot + DiagnosticMessages.expectedNewlineOrColon(), // expected newline after "_ = foo" statement + DiagnosticMessages.unexpectedToken('.') // everything after the 2nd dot is ignored + ]); + // expect statement "_ = foo" to still be included + expect(statements.length).to.equal(1); }); }); @@ -233,4 +240,57 @@ describe('parser indexing', () => { expect(statements).to.be.length.greaterThan(0); }); }); + + describe('unfinished brackets', () => { + it('parses expression inside of brackets', () => { + let { statements, diagnostics } = Parser.parse([ + identifier('_'), + token(TokenKind.Equal, '='), + identifier('foo'), + token(TokenKind.LeftSquareBracket, '['), + token(TokenKind.Identifier, 'bar'), + token(TokenKind.Dot), + token(TokenKind.Identifier, 'baz'), + token(TokenKind.Dot), + EOF + ]); + + // Parses as `_ = foo[bar.baz]` + + expect(diagnostics.length).to.be.greaterThan(0); + expect(statements).to.be.lengthOf(1); + expect(isAssignmentStatement(statements[0])).to.be.true; + const assignStmt = statements[0] as AssignmentStatement; + expect(assignStmt.name.text).to.equal('_'); + expect(isIndexedGetExpression(assignStmt.value)).to.be.true; + const indexedGetExpr = assignStmt.value as IndexedGetExpression; + expect((indexedGetExpr.obj as VariableExpression).name.text).to.equal('foo'); + expect(isDottedGetExpression(indexedGetExpr.index)).to.be.true; + const dottedGetExpr = indexedGetExpr.index as DottedGetExpression; + expect(dottedGetExpr.name.text).to.equal('baz'); + expect(isVariableExpression(dottedGetExpr.obj)).to.be.true; + }); + + it('gets correct diagnostic for missing square brace without index', () => { + let { diagnostics } = Parser.parse(` + sub setData(obj) + m.data = obj[ + end sub + `); + expectDiagnosticsIncludes(diagnostics, [ + DiagnosticMessages.expectedRightSquareBraceAfterArrayOrObjectIndex() + ]); + }); + + it('gets correct diagnostic for missing square brace with index', () => { + let { diagnostics } = Parser.parse(` + sub setData(obj) + m.data = obj[1 + end sub + `); + expectDiagnosticsIncludes(diagnostics, [ + DiagnosticMessages.expectedRightSquareBraceAfterArrayOrObjectIndex() + ]); + }); + }); }); diff --git a/src/testHelpers.spec.ts b/src/testHelpers.spec.ts index e342de737..7556a55ec 100644 --- a/src/testHelpers.spec.ts +++ b/src/testHelpers.spec.ts @@ -67,6 +67,35 @@ interface PartialDiagnostic { file?: Partial; } + +function cloneDiagnostic(actualDiagnosticInput: BsDiagnostic, expectedDiagnostic: BsDiagnostic) { + const actualDiagnostic = cloneObject( + actualDiagnosticInput, + expectedDiagnostic, + ['message', 'code', 'range', 'severity', 'relatedInformation'] + ); + //deep clone relatedInformation if available + if (actualDiagnostic.relatedInformation) { + for (let j = 0; j < actualDiagnostic.relatedInformation.length; j++) { + actualDiagnostic.relatedInformation[j] = cloneObject( + actualDiagnostic.relatedInformation[j], + expectedDiagnostic?.relatedInformation[j], + ['location', 'message'] + ) as any; + } + } + //deep clone file info if available + if (actualDiagnostic.file) { + actualDiagnostic.file = cloneObject( + actualDiagnostic.file, + expectedDiagnostic?.file, + ['srcPath', 'pkgPath'] + ) as any; + } + return actualDiagnostic; +} + + /** * Ensure the DiagnosticCollection exactly contains the data from expected list. * @param arg - any object that contains diagnostics (such as `Program`, `Scope`, or even an array of diagnostics) @@ -91,35 +120,45 @@ export function expectDiagnostics(arg: DiagnosticCollection, expected: Array) { + const actualDiagnostics = getDiagnostics(arg); + const expectedDiagnostics = + expected.map(x => { + let result = x; + if (typeof x === 'string') { + result = { message: x }; + } else if (typeof x === 'number') { + result = { code: x }; + } + return result as unknown as BsDiagnostic; + }); + + let expectedFound = 0; + + for (const expectedDiagnostic of expectedDiagnostics) { + const foundDiag = actualDiagnostics.find((actualDiag) => { + const actualDiagnosticClone = cloneDiagnostic(actualDiag, expectedDiagnostic); + return JSON.stringify(actualDiagnosticClone) === JSON.stringify(expectedDiagnostic); + }); + if (foundDiag) { + expectedFound++; + } + + } + expect(expectedFound).to.eql(expectedDiagnostics.length); +} + /** * Test that the given object has zero diagnostics. If diagnostics are found, they are printed to the console in a pretty fashion. */ From 97cc82daf43d6bd46bbc1ebaa1277ecaaa716bbd Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Fri, 23 Sep 2022 15:05:00 -0300 Subject: [PATCH 2/6] Removed commented out code --- src/files/BrsFile.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts index 72d30ebe1..ef1f3587e 100644 --- a/src/files/BrsFile.ts +++ b/src/files/BrsFile.ts @@ -692,7 +692,7 @@ export class BrsFile { } } let functionCall: FunctionCall = { - range: expression.range, //util.(expression.range.start, expression.closingParen.range.end), + range: expression.range, functionScope: this.getFunctionScopeAtPosition(callee.range.start), file: this, name: functionName, From 08512821c9001526cf87dc8af607993f27259663 Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Fri, 23 Sep 2022 15:33:39 -0300 Subject: [PATCH 3/6] Update src/parser/Parser.ts Co-authored-by: Bronley Plumb --- src/parser/Parser.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts index d6a31e67b..0d0007173 100644 --- a/src/parser/Parser.ts +++ b/src/parser/Parser.ts @@ -2937,8 +2937,8 @@ export class Parser { } /** - * There are sometimes we get catch an error that is a diagnostic - * If that's the case, we want to continue parsing + * Sometimes we catch an error that is a diagnostic. + * If that's the case, we want to continue parsing. * Otherwise, re-throw the error * * @param error error caught in a try/catch From 9cf4f8eefbf28f804a947c33fcdc6bff606f8a74 Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Fri, 23 Sep 2022 15:33:49 -0300 Subject: [PATCH 4/6] Update src/parser/Expression.ts Co-authored-by: Bronley Plumb --- src/parser/Expression.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parser/Expression.ts b/src/parser/Expression.ts index 5bac6389b..5a1e1ab28 100644 --- a/src/parser/Expression.ts +++ b/src/parser/Expression.ts @@ -431,7 +431,7 @@ export class IndexedGetExpression extends Expression { ...this.obj.transpile(state), this.questionDotToken ? state.transpileToken(this.questionDotToken) : '', state.transpileToken(this.openingSquare), - ...(this.index ? this.index.transpile(state) : []), + ...(this.index?.transpile(state) ?? []), this.closingSquare ? state.transpileToken(this.closingSquare) : '' ]; } From a498cbc6fc88ebeec82cb846535e5dd46b6300c3 Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Fri, 23 Sep 2022 15:46:02 -0300 Subject: [PATCH 5/6] Updated tests to use direct string parsing --- src/files/BrsFile.spec.ts | 7 ++++++ .../tests/expression/ArrayLiterals.spec.ts | 13 ++-------- .../AssociativeArrayLiterals.spec.ts | 24 ++----------------- src/parser/tests/expression/Call.spec.ts | 1 - src/parser/tests/expression/Indexing.spec.ts | 14 +---------- 5 files changed, 12 insertions(+), 47 deletions(-) diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts index 17545ab1f..41fcc3657 100644 --- a/src/files/BrsFile.spec.ts +++ b/src/files/BrsFile.spec.ts @@ -1321,6 +1321,13 @@ describe('BrsFile', () => { DoC( end function `); + expectDiagnostics(file.parser.diagnostics, [ + DiagnosticMessages.expectedRightParenAfterFunctionCallArguments(), + DiagnosticMessages.expectedNewlineOrColon(), + DiagnosticMessages.unexpectedToken('end function'), + DiagnosticMessages.expectedRightParenAfterFunctionCallArguments(), + DiagnosticMessages.expectedNewlineOrColon() + ]); expect(file.functionCalls.length).to.equal(2); expect(file.functionCalls[0].range).to.eql(Range.create(2, 20, 2, 27)); diff --git a/src/parser/tests/expression/ArrayLiterals.spec.ts b/src/parser/tests/expression/ArrayLiterals.spec.ts index f5d8d7a1e..f52e90cab 100644 --- a/src/parser/tests/expression/ArrayLiterals.spec.ts +++ b/src/parser/tests/expression/ArrayLiterals.spec.ts @@ -176,17 +176,8 @@ describe('parser array literals', () => { describe('unfinished', () => { it('will still be parsed', () => { - let { statements, diagnostics } = Parser.parse([ - identifier('_'), - token(TokenKind.Equal, '='), - token(TokenKind.LeftSquareBracket, '['), - token(TokenKind.IntegerLiteral, '1'), - token(TokenKind.Comma, ','), - identifier('data'), - token(TokenKind.Dot, '.'), - identifier('foo'), - EOF - ]); + // no closing brace: + let { statements, diagnostics } = Parser.parse(`_ = [1, data.foo`); expectDiagnostics(diagnostics, [DiagnosticMessages.unmatchedLeftSquareBraceAfterArrayLiteral()]); expect(statements).to.be.lengthOf(1); diff --git a/src/parser/tests/expression/AssociativeArrayLiterals.spec.ts b/src/parser/tests/expression/AssociativeArrayLiterals.spec.ts index 71c16dff3..6be4efff1 100644 --- a/src/parser/tests/expression/AssociativeArrayLiterals.spec.ts +++ b/src/parser/tests/expression/AssociativeArrayLiterals.spec.ts @@ -209,27 +209,8 @@ describe('parser associative array literals', () => { describe('unfinished', () => { it('will still be parsed', () => { - // No closing brace: _ = {name: "john", age: 42, address: data.address - let { statements, diagnostics } = Parser.parse([ - identifier('_'), - token(TokenKind.Equal, '='), - token(TokenKind.LeftCurlyBrace, '{'), - identifier('name'), - token(TokenKind.Colon, ':'), - token(TokenKind.StringLiteral, '"john"'), - token(TokenKind.Comma, ','), - identifier('age'), - token(TokenKind.Colon, ':'), - token(TokenKind.IntegerLiteral, '42'), - token(TokenKind.Comma, ','), - identifier('address'), - token(TokenKind.Colon, ':'), - identifier('data'), - token(TokenKind.Dot, '.'), - identifier('address'), - EOF - ]); - + // No closing brace: + let { statements, diagnostics } = Parser.parse(`_ = {name: "john", age: 42, address: data.address`); expectDiagnostics(diagnostics, [DiagnosticMessages.unmatchedLeftCurlyAfterAALiteral()]); expect(statements).to.be.lengthOf(1); expect(isAssignmentStatement(statements[0])).to.be.true; @@ -245,7 +226,6 @@ describe('parser associative array literals', () => { it('gets correct diagnostic for missing curly brace without final value', () => { let { diagnostics } = Parser.parse(` - sub setData() m.data = {hello: end sub diff --git a/src/parser/tests/expression/Call.spec.ts b/src/parser/tests/expression/Call.spec.ts index 303ec7042..3f21c0e1d 100644 --- a/src/parser/tests/expression/Call.spec.ts +++ b/src/parser/tests/expression/Call.spec.ts @@ -263,7 +263,6 @@ describe('parser call expressions', () => { expect(statements).to.be.lengthOf(1); const bodyStatements = (statements[0] as FunctionStatement).func.body.statements; expect(bodyStatements).to.be.lengthOf(1); - }); }); diff --git a/src/parser/tests/expression/Indexing.spec.ts b/src/parser/tests/expression/Indexing.spec.ts index 744695d71..8765b9139 100644 --- a/src/parser/tests/expression/Indexing.spec.ts +++ b/src/parser/tests/expression/Indexing.spec.ts @@ -243,19 +243,7 @@ describe('parser indexing', () => { describe('unfinished brackets', () => { it('parses expression inside of brackets', () => { - let { statements, diagnostics } = Parser.parse([ - identifier('_'), - token(TokenKind.Equal, '='), - identifier('foo'), - token(TokenKind.LeftSquareBracket, '['), - token(TokenKind.Identifier, 'bar'), - token(TokenKind.Dot), - token(TokenKind.Identifier, 'baz'), - token(TokenKind.Dot), - EOF - ]); - - // Parses as `_ = foo[bar.baz]` + let { statements, diagnostics } = Parser.parse(`_ = foo[bar.baz.`); expect(diagnostics.length).to.be.greaterThan(0); expect(statements).to.be.lengthOf(1); From 8518fb7f43bfcbb144656dfeee326c6cf917515c Mon Sep 17 00:00:00 2001 From: Mark Pearce Date: Fri, 23 Sep 2022 15:50:10 -0300 Subject: [PATCH 6/6] Fixed some function comments --- src/testHelpers.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/testHelpers.spec.ts b/src/testHelpers.spec.ts index 7556a55ec..e9a8f4a4f 100644 --- a/src/testHelpers.spec.ts +++ b/src/testHelpers.spec.ts @@ -67,7 +67,9 @@ interface PartialDiagnostic { file?: Partial; } - +/** + * Helper function to clone a Diagnostic so it will give partial data that has the same properties as the expected + */ function cloneDiagnostic(actualDiagnosticInput: BsDiagnostic, expectedDiagnostic: BsDiagnostic) { const actualDiagnostic = cloneObject( actualDiagnosticInput, @@ -127,7 +129,7 @@ export function expectDiagnostics(arg: DiagnosticCollection, expected: Array