From 8782a0dff7de63b4669da7d4e7b97be27cb97e09 Mon Sep 17 00:00:00 2001 From: Philippe Date: Mon, 21 Jun 2021 11:53:31 +0100 Subject: [PATCH] Fix incorrect Block range for inline if/then branch (#424) * Fix incorrect Block range for inline if/then branch * Added tests Fix other tests Co-authored-by: Bronley Plumb --- src/lexer/Token.ts | 4 ++-- src/parser/Parser.ts | 5 ++-- src/parser/tests/Parser.spec.ts | 21 ++++++++++------- src/parser/tests/controlFlow/For.spec.ts | 4 ++-- src/parser/tests/controlFlow/If.spec.ts | 23 ++++++++++++++++++- src/parser/tests/expression/Call.spec.ts | 8 +++---- .../tests/statement/ReturnStatement.spec.ts | 2 +- 7 files changed, 46 insertions(+), 21 deletions(-) diff --git a/src/lexer/Token.ts b/src/lexer/Token.ts index 37166e03e..42e814123 100644 --- a/src/lexer/Token.ts +++ b/src/lexer/Token.ts @@ -10,13 +10,13 @@ export interface Token { /** The text found in the original BrightScript source, if any. */ text: string; /** True if this token's `text` is a reserved word, otherwise `false`. */ - isReserved: boolean; + isReserved?: boolean; /** Where the token was found. */ range: Range; /** * Any leading whitespace found prior to this token. Excludes newline characters. */ - leadingWhitespace: string; + leadingWhitespace?: string; } /** diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts index b9be0de98..ed0d08f14 100644 --- a/src/parser/Parser.ts +++ b/src/parser/Parser.ts @@ -200,8 +200,6 @@ export class Parser { /** * Static wrapper around creating a new parser and parsing a list of tokens */ - public static parse(source: string, options?: ParseOptions): Parser; - public static parse(tokens: Token[], options?: ParseOptions): Parser; public static parse(toParse: Token[] | string, options?: ParseOptions): Parser { let tokens: Token[]; if (typeof toParse === 'string') { @@ -1637,6 +1635,7 @@ export class Parser { return undefined; } statements.push(statement); + const startingRange = statement.range; //look for colon statement separator let foundColon = false; @@ -1662,7 +1661,7 @@ export class Parser { }); } } - return new Block(statements, this.peek().range); + return new Block(statements, startingRange); } private expressionStatement(expr: Expression): ExpressionStatement | IncrementStatement { diff --git a/src/parser/tests/Parser.spec.ts b/src/parser/tests/Parser.spec.ts index d900e5a64..ce2369acb 100644 --- a/src/parser/tests/Parser.spec.ts +++ b/src/parser/tests/Parser.spec.ts @@ -1,15 +1,12 @@ import type { Token } from '../../lexer'; import { TokenKind, ReservedWords } from '../../lexer'; import { interpolatedRange } from '../../astUtils/creators'; +import type { Range } from '../../astUtils'; /* A set of utilities to be used while writing tests for the BRS parser. */ /** * Creates a token with the given `kind` and (optional) `literal` value. - * @param {TokenKind} kind the tokenKind the produced token should represent. - * @param {string} text the text represented by this token. - * @param {*} [literal] the literal value that the produced token should contain, if any - * @returns {object} a token of `kind` representing `text` with value `literal`. */ export function token(kind: TokenKind, text?: string): Token { return { @@ -23,11 +20,19 @@ export function token(kind: TokenKind, text?: string): Token { /** * Creates an Identifier token with the given `text`. - * @param {string} text - * @returns {object} a token with the provided `text`. */ -export function identifier(text) { - return exports.token(TokenKind.Identifier, text); +export function identifier(text: string) { + return token(TokenKind.Identifier, text); +} + +/** + * Test whether a range matches a group of elements with a `range` + */ +export function rangeMatch(range: Range, elements: ({ range: Range })[]): boolean { + return range.start.line === elements[0].range.start.line && + range.start.character === elements[0].range.start.character && + range.end.line === elements[elements.length - 1].range.end.line && + range.end.character === elements[elements.length - 1].range.end.character; } /** An end-of-file token. */ diff --git a/src/parser/tests/controlFlow/For.spec.ts b/src/parser/tests/controlFlow/For.spec.ts index c86305c96..684cbd198 100644 --- a/src/parser/tests/controlFlow/For.spec.ts +++ b/src/parser/tests/controlFlow/For.spec.ts @@ -128,8 +128,8 @@ describe('parser for loops', () => { text: 'to', isReserved: false, range: { - start: { line: 0, column: 10 }, - end: { start: 0, column: 12 } + start: { line: 0, character: 10 }, + end: { line: 0, character: 12 } } }, { diff --git a/src/parser/tests/controlFlow/If.spec.ts b/src/parser/tests/controlFlow/If.spec.ts index 53d249fcf..d3252cafb 100644 --- a/src/parser/tests/controlFlow/If.spec.ts +++ b/src/parser/tests/controlFlow/If.spec.ts @@ -3,8 +3,9 @@ import * as assert from 'assert'; import { Parser } from '../../Parser'; import { TokenKind, Lexer } from '../../../lexer'; -import { EOF, identifier, token } from '../Parser.spec'; +import { EOF, identifier, rangeMatch, token } from '../Parser.spec'; import { isBlock, isCommentStatement, isIfStatement } from '../../../astUtils'; +import type { Block, IfStatement } from '../../Statement'; describe('parser if statements', () => { it('allows empty if blocks', () => { @@ -602,4 +603,24 @@ describe('parser if statements', () => { expect(diagnostics).to.be.lengthOf(0); expect(statements).to.be.length.greaterThan(0); }); + + it('single-line if block statements have correct range', () => { + let { tokens } = Lexer.scan(` + if false then print "true" + if false then print "true": a = 10 + if false then print "true" else print "false" + if false then print "true" else print "false": a = 20 + `); + let { statements, diagnostics } = Parser.parse(tokens); + expect(diagnostics).to.be.lengthOf(0); + + const then1 = (statements[0] as IfStatement).thenBranch; + expect(rangeMatch(then1.range, then1.statements)).to.be.true; + const then2 = (statements[1] as IfStatement).thenBranch; + expect(rangeMatch(then2.range, then2.statements)).to.be.true; + const else1 = (statements[2] as IfStatement).elseBranch as Block; + expect(rangeMatch(else1.range, else1.statements)).to.be.true; + const else2 = (statements[3] as IfStatement).elseBranch as Block; + expect(rangeMatch(else2.range, else2.statements)).to.be.true; + }); }); diff --git a/src/parser/tests/expression/Call.spec.ts b/src/parser/tests/expression/Call.spec.ts index 770a06ab1..136188673 100644 --- a/src/parser/tests/expression/Call.spec.ts +++ b/src/parser/tests/expression/Call.spec.ts @@ -9,7 +9,7 @@ describe('parser call expressions', () => { it('parses named function calls', () => { const { statements, diagnostics } = Parser.parse([ identifier('RebootSystem'), - { kind: TokenKind.LeftParen, text: '(', line: 1 }, + { kind: TokenKind.LeftParen, text: '(', range: null }, token(TokenKind.RightParen, ')'), EOF ]); @@ -57,7 +57,7 @@ describe('parser call expressions', () => { it('allows closing parentheses on separate line', () => { const { statements, diagnostics } = Parser.parse([ identifier('RebootSystem'), - { kind: TokenKind.LeftParen, text: '(', line: 1 }, + { kind: TokenKind.LeftParen, text: '(', range: null }, token(TokenKind.Newline, '\\n'), token(TokenKind.Newline, '\\n'), token(TokenKind.RightParen, ')'), @@ -71,9 +71,9 @@ describe('parser call expressions', () => { it('accepts arguments', () => { const { statements, diagnostics } = Parser.parse([ identifier('add'), - { kind: TokenKind.LeftParen, text: '(', line: 1 }, + { kind: TokenKind.LeftParen, text: '(', range: null }, token(TokenKind.IntegerLiteral, '1'), - { kind: TokenKind.Comma, text: ',', line: 1 }, + { kind: TokenKind.Comma, text: ',', range: null }, token(TokenKind.IntegerLiteral, '2'), token(TokenKind.RightParen, ')'), EOF diff --git a/src/parser/tests/statement/ReturnStatement.spec.ts b/src/parser/tests/statement/ReturnStatement.spec.ts index e6123b45e..9974418b2 100644 --- a/src/parser/tests/statement/ReturnStatement.spec.ts +++ b/src/parser/tests/statement/ReturnStatement.spec.ts @@ -51,7 +51,7 @@ describe('parser return statements', () => { token(TokenKind.Newline, '\\n'), token(TokenKind.Return, 'return'), identifier('RebootSystem'), - { kind: TokenKind.LeftParen, text: '(', line: 2 }, + { kind: TokenKind.LeftParen, text: '(', range: null }, token(TokenKind.RightParen, ')'), token(TokenKind.Newline, '\\n'), token(TokenKind.EndFunction, 'end function'),