Repository: rokucommunity/brighterscript
Pull request #514 (2022-02-13 10:12:09)

Refactor try-catch statement
Split the catch part into its a child statement of the try statement in order to facilitate source walking (for linting).
Add missing reflection methods.

TwitchBronBron: @elsassph when you get a chance, could you review my suggestions? 

Patch:
diff --git a/src/astUtils/reflection.spec.ts b/src/astUtils/reflection.spec.ts
index df4d059d..1bc859dd 100644
--- a/src/astUtils/reflection.spec.ts
+++ b/src/astUtils/reflection.spec.ts
@@ -1,10 +1,10 @@
 /* eslint-disable no-multi-spaces */
 import { expect } from 'chai';
-import { PrintStatement, Block, Body, AssignmentStatement, CommentStatement, ExitForStatement, ExitWhileStatement, ExpressionStatement, FunctionStatement, IfStatement, IncrementStatement, GotoStatement, LabelStatement, ReturnStatement, EndStatement, StopStatement, ForStatement, ForEachStatement, WhileStatement, DottedSetStatement, IndexedSetStatement, LibraryStatement, NamespaceStatement, ImportStatement, ClassStatement, EmptyStatement } from '../parser/Statement';
+import { PrintStatement, Block, Body, AssignmentStatement, CommentStatement, ExitForStatement, ExitWhileStatement, ExpressionStatement, FunctionStatement, IfStatement, IncrementStatement, GotoStatement, LabelStatement, ReturnStatement, EndStatement, StopStatement, ForStatement, ForEachStatement, WhileStatement, DottedSetStatement, IndexedSetStatement, LibraryStatement, NamespaceStatement, ImportStatement, ClassStatement, EmptyStatement, TryCatchStatement, CatchStatement } from '../parser/Statement';
 import { FunctionExpression, NamespacedVariableNameExpression, BinaryExpression, CallExpression, DottedGetExpression, IndexedGetExpression, GroupingExpression, EscapedCharCodeLiteralExpression, ArrayLiteralExpression, AALiteralExpression, UnaryExpression, VariableExpression, SourceLiteralExpression, NewExpression, CallfuncExpression, TemplateStringQuasiExpression, XmlAttributeGetExpression, TemplateStringExpression, TaggedTemplateStringExpression, AnnotationExpression } from '../parser/Expression';
 import type { Token } from '../lexer/Token';
 import { TokenKind } from '../lexer/TokenKind';
-import { isPrintStatement, isIfStatement, isBody, isAssignmentStatement, isBlock, isExpressionStatement, isCommentStatement, isExitForStatement, isExitWhileStatement, isFunctionStatement, isIncrementStatement, isGotoStatement, isLabelStatement, isReturnStatement, isEndStatement, isStopStatement, isForStatement, isForEachStatement, isWhileStatement, isDottedSetStatement, isIndexedSetStatement, isLibraryStatement, isNamespaceStatement, isImportStatement, isExpression, isBinaryExpression, isCallExpression, isFunctionExpression, isNamespacedVariableNameExpression, isDottedGetExpression, isXmlAttributeGetExpression, isIndexedGetExpression, isGroupingExpression, isLiteralExpression, isEscapedCharCodeLiteralExpression, isArrayLiteralExpression, isAALiteralExpression, isUnaryExpression, isVariableExpression, isSourceLiteralExpression, isNewExpression, isCallfuncExpression, isTemplateStringQuasiExpression, isTemplateStringExpression, isTaggedTemplateStringExpression, isBrsFile, isXmlFile, isClassStatement, isStatement, isAnnotationExpression } from './reflection';
+import { isPrintStatement, isIfStatement, isBody, isAssignmentStatement, isBlock, isExpressionStatement, isCommentStatement, isExitForStatement, isExitWhileStatement, isFunctionStatement, isIncrementStatement, isGotoStatement, isLabelStatement, isReturnStatement, isEndStatement, isStopStatement, isForStatement, isForEachStatement, isWhileStatement, isDottedSetStatement, isIndexedSetStatement, isLibraryStatement, isNamespaceStatement, isImportStatement, isExpression, isBinaryExpression, isCallExpression, isFunctionExpression, isNamespacedVariableNameExpression, isDottedGetExpression, isXmlAttributeGetExpression, isIndexedGetExpression, isGroupingExpression, isLiteralExpression, isEscapedCharCodeLiteralExpression, isArrayLiteralExpression, isAALiteralExpression, isUnaryExpression, isVariableExpression, isSourceLiteralExpression, isNewExpression, isCallfuncExpression, isTemplateStringQuasiExpression, isTemplateStringExpression, isTaggedTemplateStringExpression, isBrsFile, isXmlFile, isClassStatement, isStatement, isAnnotationExpression, isTryCatchStatement, isCatchStatement } from './reflection';
 import { createToken, createStringLiteral, interpolatedRange as range } from './creators';
 import { Program } from '../Program';
 import { BrsFile } from '../files/BrsFile';
@@ -53,6 +53,8 @@ describe('reflection', () => {
         const namespace = new NamespaceStatement(token, new NamespacedVariableNameExpression(createVariableExpression('a', range)), body, token);
         const cls = new ClassStatement(token, ident, [], token);
         const imports = new ImportStatement(token, token);
+        const catchStmt = new CatchStatement({ catchToken: token }, ident, block);
+        const tryCatch = new TryCatchStatement({ tryToken: token }, block, catchStmt);
 
         it('isStatement', () => {
             expect(isStatement(library)).to.be.true;
@@ -165,6 +167,14 @@ describe('reflection', () => {
             expect(isImportStatement(imports)).to.be.true;
             expect(isImportStatement(body)).to.be.false;
         });
+        it('isTryCatchStatement', () => {
+            expect(isTryCatchStatement(tryCatch)).to.be.true;
+            expect(isTryCatchStatement(body)).to.be.false;
+        });
+        it('isCatchStatement', () => {
+            expect(isCatchStatement(catchStmt)).to.be.true;
+            expect(isCatchStatement(body)).to.be.false;
+        });
     });
 
     describe('Expressions', () => {
diff --git a/src/astUtils/reflection.ts b/src/astUtils/reflection.ts
index cb0e5006..bb1c6344 100644
--- a/src/astUtils/reflection.ts
+++ b/src/astUtils/reflection.ts
@@ -1,4 +1,4 @@
-import type { Body, AssignmentStatement, Block, ExpressionStatement, CommentStatement, ExitForStatement, ExitWhileStatement, FunctionStatement, IfStatement, IncrementStatement, PrintStatement, GotoStatement, LabelStatement, ReturnStatement, EndStatement, StopStatement, ForStatement, ForEachStatement, WhileStatement, DottedSetStatement, IndexedSetStatement, LibraryStatement, NamespaceStatement, ImportStatement, ClassFieldStatement, ClassMethodStatement, ClassStatement, Statement, InterfaceFieldStatement, InterfaceMethodStatement, InterfaceStatement, EnumStatement, EnumMemberStatement } from '../parser/Statement';
+import type { Body, AssignmentStatement, Block, ExpressionStatement, CommentStatement, ExitForStatement, ExitWhileStatement, FunctionStatement, IfStatement, IncrementStatement, PrintStatement, GotoStatement, LabelStatement, ReturnStatement, EndStatement, StopStatement, ForStatement, ForEachStatement, WhileStatement, DottedSetStatement, IndexedSetStatement, LibraryStatement, NamespaceStatement, ImportStatement, ClassFieldStatement, ClassMethodStatement, ClassStatement, Statement, InterfaceFieldStatement, InterfaceMethodStatement, InterfaceStatement, EnumStatement, EnumMemberStatement, TryCatchStatement, CatchStatement } from '../parser/Statement';
 import type { LiteralExpression, Expression, BinaryExpression, CallExpression, FunctionExpression, NamespacedVariableNameExpression, DottedGetExpression, XmlAttributeGetExpression, IndexedGetExpression, GroupingExpression, EscapedCharCodeLiteralExpression, ArrayLiteralExpression, AALiteralExpression, UnaryExpression, VariableExpression, SourceLiteralExpression, NewExpression, CallfuncExpression, TemplateStringQuasiExpression, TemplateStringExpression, TaggedTemplateStringExpression, AnnotationExpression, FunctionParameterExpression, AAMemberExpression } from '../parser/Expression';
 import type { BrsFile } from '../files/BrsFile';
 import type { XmlFile } from '../files/XmlFile';
@@ -144,6 +144,12 @@ export function isEnumStatement(element: Statement | Expression | undefined): el
 export function isEnumMemberStatement(element: Statement | Expression | undefined): element is EnumMemberStatement {
     return element?.constructor.name === 'EnumMemberStatement';
 }
+export function isTryCatchStatement(element: Statement | Expression | undefined): element is TryCatchStatement {
+    return element?.constructor.name === 'TryCatchStatement';
+}
+export function isCatchStatement(element: Statement | Expression | undefined): element is CatchStatement {
+    return element?.constructor.name === 'CatchStatement';
+}
 
 // Expressions reflection
 /**
@@ -296,4 +302,3 @@ export function isLiteralString(e: any): e is LiteralExpression & { type: String
 export function isLiteralNumber(e: any): e is LiteralExpression & { type: IntegerType | LongIntegerType | FloatType | DoubleType } {
     return isLiteralExpression(e) && isNumberType(e.type);
 }
-
diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts
index d02b5c18..8d022d74 100644
--- a/src/parser/Parser.ts
+++ b/src/parser/Parser.ts
@@ -23,6 +23,7 @@ import {
     AssignmentStatement,
     Block,
     Body,
+    CatchStatement,
     ClassFieldStatement,
     ClassMethodStatement,
     ClassStatement,
@@ -1579,7 +1580,7 @@ export class Parser {
     private tryCatchStatement(): TryCatchStatement {
         const tryToken = this.advance();
         const statement = new TryCatchStatement(
-            tryToken
+            { tryToken: tryToken }
         );
 
         //ensure statement separator
@@ -1595,24 +1596,24 @@ export class Parser {
             });
             //gracefully handle end-try
             if (peek.kind === TokenKind.EndTry) {
-                statement.endTryToken = this.advance();
+                statement.tokens.endTryToken = this.advance();
             }
             return statement;
-        } else {
-            statement.catchToken = this.advance();
         }
+        const catchStmt = new CatchStatement({ catchToken: this.advance() });
+        statement.catchStatement = catchStmt;
 
         const exceptionVarToken = this.tryConsume(DiagnosticMessages.missingExceptionVarToFollowCatch(), TokenKind.Identifier, ...this.allowedLocalIdentifiers);
         if (exceptionVarToken) {
             // force it into an identifier so the AST makes some sense
             exceptionVarToken.kind = TokenKind.Identifier;
-            statement.exceptionVariable = exceptionVarToken as Identifier;
+            catchStmt.exceptionVariable = exceptionVarToken as Identifier;
         }
 
         //ensure statement sepatator
         this.consumeStatementSeparators();
 
-        statement.catchBranch = this.block(TokenKind.EndTry);
+        catchStmt.catchBranch = this.block(TokenKind.EndTry);
 
         if (this.peek().kind !== TokenKind.EndTry) {
             this.diagnostics.push({
@@ -1620,7 +1621,7 @@ export class Parser {
                 range: this.peek().range
             });
         } else {
-            statement.endTryToken = this.advance();
+            statement.tokens.endTryToken = this.advance();
         }
         return statement;
     }
diff --git a/src/parser/Statement.ts b/src/parser/Statement.ts
index a508da44..f33bbe2f 100644
--- a/src/parser/Statement.ts
+++ b/src/parser/Statement.ts
@@ -2097,42 +2097,73 @@ export type ClassMemberStatement = ClassFieldStatement | ClassMethodStatement;
 
 export class TryCatchStatement extends Statement {
     constructor(
-        public tryToken: Token,
+        public tokens: {
+            tryToken: Token;
+            endTryToken?: Token;
+        },
         public tryBranch?: Block,
-        public catchToken?: Token,
-        public exceptionVariable?: Identifier,
-        public catchBranch?: Block,
-        public endTryToken?: Token
+        public catchStatement?: CatchStatement
     ) {
         super();
     }
 
     public get range() {
         return util.createRangeFromPositions(
-            this.tryToken.range.start,
-            (this.endTryToken ?? this.catchBranch ?? this.exceptionVariable ?? this.catchToken ?? this.tryBranch ?? this.tryToken).range.end
+            this.tokens.tryToken.range.start,
+            (this.tokens.endTryToken ?? this.catchStatement ?? this.tryBranch ?? this.tokens.tryToken).range.end
         );
     }
 
     public transpile(state: BrsTranspileState): TranspileResult {
         return [
-            state.transpileToken(this.tryToken),
+            state.transpileToken(this.tokens.tryToken),
             ...this.tryBranch.transpile(state),
             state.newline,
             state.indent(),
-            state.transpileToken(this.catchToken),
-            ' ',
-            state.transpileToken(this.exceptionVariable),
-            ...this.catchBranch.transpile(state),
+            ...(this.catchStatement?.transpile(state) ?? ['catch']),
             state.newline,
             state.indent(),
-            state.transpileToken(this.endTryToken)
+            state.transpileToken(this.tokens.endTryToken)
         ];
     }
 
     public walk(visitor: WalkVisitor, options: WalkOptions) {
         if (this.tryBranch && options.walkMode & InternalWalkMode.walkStatements) {
             walk(this, 'tryBranch', visitor, options);
+            walk(this, 'catchStatement', visitor, options);
+        }
+    }
+}
+
+export class CatchStatement extends Statement {
+    constructor(
+        public tokens: {
+            catchToken: Token;
+        },
+        public exceptionVariable?: Identifier,
+        public catchBranch?: Block
+    ) {
+        super();
+    }
+
+    public get range() {
+        return util.createRangeFromPositions(
+            this.tokens.catchToken.range.start,
+            (this.catchBranch ?? this.exceptionVariable ?? this.tokens.catchToken).range.end
+        );
+    }
+
+    public transpile(state: BrsTranspileState): TranspileResult {
+        return [
+            state.transpileToken(this.tokens.catchToken),
+            ' ',
+            this.exceptionVariable?.text ?? 'e',
+            ...(this.catchBranch?.transpile(state) ?? [])
+        ];
+    }
+
+    public walk(visitor: WalkVisitor, options: WalkOptions) {
+        if (this.catchBranch && options.walkMode & InternalWalkMode.walkStatements) {
             walk(this, 'catchBranch', visitor, options);
         }
     }


Test patch:
diff --git a/src/parser/tests/statement/TryCatch.spec.ts b/src/parser/tests/statement/TryCatch.spec.ts
index 0c034b69..84074496 100644
--- a/src/parser/tests/statement/TryCatch.spec.ts
+++ b/src/parser/tests/statement/TryCatch.spec.ts
@@ -18,9 +18,11 @@ describe('parser try/catch', () => {
         expect(stmt).to.be.instanceof(TryCatchStatement);
         expect(stmt.tryToken?.text).to.eql('try');
         expect(stmt.tryBranch).to.exist.and.ownProperty('statements').to.be.lengthOf(1);
-        expect(stmt.catchToken?.text).to.eql('catch');
-        expect(stmt.exceptionVariable.text).to.eql('e');
-        expect(stmt.catchBranch).to.exist.and.ownProperty('statements').to.be.lengthOf(1);
+        expect(stmt.catchStatement).to.exist;
+        const cstmt = stmt.catchStatement;
+        expect(cstmt.catchToken?.text).to.eql('catch');
+        expect(cstmt.exceptionVariable.text).to.eql('e');
+        expect(cstmt.catchBranch).to.exist.and.ownProperty('statements').to.be.lengthOf(1);
         expect(stmt.endTryToken?.text).to.eql('end try');
     });