Compute node bounds once at construction

docs/plugins.md

@@ -606,6 +606,8 @@ Build the line/character `Location` on demand with `util.getLocation()`:
 const location = util.getLocation(funcStmt.tokens.name); // { uri, range }
 ```
 
+A node's bounds are computed once, when it's constructed, from the children and tokens it's given. They describe where the node came from in the original source, so they don't change if a plugin edits the AST afterwards (i.e. adding statements to a block doesn't move the block).
+
 `util.getLocation()` returns `undefined` for synthetic tokens and nodes (ones that were created in code rather than parsed from a file), since they have no position. A synthetic node won't get a source map entry, and diagnostics can't point at it.
 
 A position describes where the code came from in the original source, so it's used as-is for source maps and diagnostics. Don't change the position of a parsed token or node. Only give one to a synthetic token or node you're creating:

src/parser/AstNode.spec.ts

@@ -7,12 +7,12 @@ import type { AAIndexedMemberExpression, AALiteralExpression, AAMemberExpression
 import { expectZeroDiagnostics } from '../testHelpers.spec';
 import { tempDir, rootDir, outDir } from '../testHelpers.spec';
 import { isAAIndexedMemberExpression, isAALiteralExpression, isAAMemberExpression, isAnnotationExpression, isArrayLiteralExpression, isAssignmentStatement, isBinaryExpression, isBlock, isCallExpression, isCallfuncExpression, isCatchStatement, isClassStatement, isConstStatement, isDimStatement, isDottedGetExpression, isDottedSetStatement, isEnumMemberStatement, isEnumStatement, isExpressionStatement, isForEachStatement, isForStatement, isFunctionExpression, isFunctionStatement, isGroupingExpression, isIfStatement, isIncrementStatement, isIndexedGetExpression, isIndexedSetStatement, isInterfaceFieldStatement, isInterfaceMethodStatement, isInterfaceStatement, isMethodStatement, isNamespaceStatement, isNewExpression, isNullCoalescingExpression, isPrintStatement, isReturnStatement, isTaggedTemplateStringExpression, isTemplateStringExpression, isTemplateStringQuasiExpression, isTernaryExpression, isThrowStatement, isTryCatchStatement, isTypecastExpression, isUnaryExpression, isVariableExpression, isLiteralExpression, isWhileStatement, isXmlAttributeGetExpression } from '../astUtils/reflection';
-import type { ClassStatement, FunctionStatement, InterfaceFieldStatement, InterfaceMethodStatement, MethodStatement, InterfaceStatement, CatchStatement, ThrowStatement, EnumStatement, EnumMemberStatement, ConstStatement, Block, PrintStatement, DimStatement, ForStatement, WhileStatement, IndexedSetStatement, NamespaceStatement, TryCatchStatement, DottedSetStatement, ExpressionStatement } from './Statement';
+import type { ConditionalCompileStatement, ClassStatement, FunctionStatement, InterfaceFieldStatement, InterfaceMethodStatement, MethodStatement, InterfaceStatement, CatchStatement, ThrowStatement, EnumStatement, EnumMemberStatement, ConstStatement, Block, PrintStatement, DimStatement, ForStatement, WhileStatement, IndexedSetStatement, NamespaceStatement, TryCatchStatement, DottedSetStatement, ExpressionStatement } from './Statement';
 import { AssignmentStatement, EmptyStatement } from './Statement';
 import { ParseMode, Parser } from './Parser';
 import type { AstNode } from './AstNode';
 import { WalkMode } from '../astUtils/visitors';
-import { isStatement } from '../astUtils/reflection';
+import { isConditionalCompileStatement, isStatement } from '../astUtils/reflection';
 import { createAssignmentStatement, createBlock, createIfStatement, createToken, createVariableExpression } from '../astUtils/creators';
 import { TokenKind } from '../lexer/TokenKind';
 
@@ -305,6 +305,115 @@ describe('AstNode', () => {
         });
     });
 
+    describe('bounds', () => {
+        it('keeps the bounds from construction when the AST is changed afterwards', () => {
+            const { ast } = Parser.parse('sub main()\n    a = 1\nend sub');
+            const block = ast.findChild<Block>(isBlock);
+            const bodyLocation = util.getLocation(ast);
+            const blockLocation = util.getLocation(block);
+            //a plugin moves a statement with a real position from elsewhere into this AST
+            const other = Parser.parse('\n\n\n\n\nsub other()\n    b = 2\nend sub').ast;
+            ast.statements.push(other.statements[0]);
+            block.statements.push(other.findChild<AssignmentStatement>(isAssignmentStatement));
+            expect(util.getLocation(ast)).to.eql(bodyLocation);
+            expect(util.getLocation(block)).to.eql(blockLocation);
+        });
+
+        it('computes the body bounds from its statements', () => {
+            const { ast } = Parser.parse('\n\nsub main()\nend sub\n\nsub other()\nend sub\n');
+            expect(util.getLocation(ast).range).to.eql(util.createRange(2, 0, 6, 7));
+        });
+
+        it('includes the annotation call in the annotation bounds', () => {
+            const { ast } = Parser.parse('@annotation(1, 2)\nsub main()\nend sub', { mode: ParseMode.BrighterScript });
+            const annotation = (ast.statements[0] as FunctionStatement).annotations[0];
+            expect(util.getLocation(annotation).range).to.eql(util.createRange(0, 0, 0, 17));
+        });
+
+        describe('empty blocks', () => {
+            function getEmptyBlockRanges(code: string) {
+                const { ast } = Parser.parse(code, { mode: ParseMode.BrighterScript });
+                return ast.findChildren<Block>(isBlock, { walkMode: WalkMode.visitAllRecursive }).map(x => util.getLocation(x)?.range);
+            }
+
+            it('function', () => {
+                expect(getEmptyBlockRanges('sub main(a)\nend sub')).to.eql([
+                    util.createRange(0, 11, 1, 0)
+                ]);
+            });
+
+            it('if/else', () => {
+                expect(getEmptyBlockRanges('sub main()\n    if true then\n    else\n    end if\nend sub')).to.eql([
+                    //the function body is not empty, so it's bounded by its statements
+                    util.createRange(1, 4, 3, 10),
+                    //then branch
+                    util.createRange(1, 16, 2, 4),
+                    //else branch
+                    util.createRange(2, 8, 3, 4)
+                ]);
+            });
+
+            it('for', () => {
+                expect(getEmptyBlockRanges('sub main()\n    for i = 0 to 1\n    end for\nend sub')[1]).to.eql(
+                    util.createRange(1, 18, 2, 4)
+                );
+            });
+
+            it('for each', () => {
+                expect(getEmptyBlockRanges('sub main()\n    for each x in y\n    end for\nend sub')[1]).to.eql(
+                    util.createRange(1, 19, 2, 4)
+                );
+            });
+
+            it('while', () => {
+                expect(getEmptyBlockRanges('sub main()\n    while true\n    end while\nend sub')[1]).to.eql(
+                    util.createRange(1, 14, 2, 4)
+                );
+            });
+
+            it('try/catch', () => {
+                expect(getEmptyBlockRanges('sub main()\n    try\n    catch e\n    end try\nend sub').slice(1)).to.eql([
+                    //try branch
+                    util.createRange(1, 7, 2, 4),
+                    //catch branch
+                    util.createRange(2, 11, 3, 4)
+                ]);
+            });
+
+            it('#if/#else', () => {
+                //the walker skips inactive conditional compile branches, so read both branches off the statement directly
+                const { ast } = Parser.parse('#const DEBUG = true\n#if DEBUG\n#else\n#end if', { mode: ParseMode.BrighterScript });
+                const statement = ast.findChild<ConditionalCompileStatement>(isConditionalCompileStatement);
+                expect([statement.thenBranch, statement.elseBranch].map(x => util.getLocation(x)?.range)).to.eql([
+                    //then branch
+                    util.createRange(1, 9, 2, 0),
+                    //else branch
+                    util.createRange(2, 5, 3, 0)
+                ]);
+            });
+
+            it('select case', () => {
+                expect(getEmptyBlockRanges('sub main()\n    select case x\n        case 1\n        case else\n    end select\nend sub').slice(1)).to.eql([
+                    //`case 1` body ends at the next case
+                    util.createRange(2, 14, 3, 8),
+                    //`case else` body ends at `end select`
+                    util.createRange(3, 17, 4, 4)
+                ]);
+            });
+        });
+
+        it('spans the gap between the directives for #if/#else branches that have statements', () => {
+            const { ast } = Parser.parse('#const DEBUG = true\n#if DEBUG\n    print 1\n#else\n    print 2\n#end if', { mode: ParseMode.BrighterScript });
+            const statement = ast.findChild<ConditionalCompileStatement>(isConditionalCompileStatement);
+            expect([statement.thenBranch, statement.elseBranch].map(x => util.getLocation(x)?.range)).to.eql([
+                //then branch
+                util.createRange(1, 9, 3, 0),
+                //else branch
+                util.createRange(3, 5, 5, 0)
+            ]);
+        });
+    });
+
     describe('findChild', () => {
         it('finds a child that matches the matcher', () => {
             const file = program.setFile<BrsFile>('source/main.brs', `

src/parser/Expression.ts

@@ -292,15 +292,16 @@ export class FunctionExpression extends Expression implements TypedefProvider {
             this.body.parent = this;
         }
         this.symbolTable = new SymbolTable('FunctionExpression', () => this.parent?.getSymbolTable());
-        util.setBounds(this,
+        const signature = util.setBounds({} as Locatable,
             this.tokens.functionType,
             this.tokens.leftParen,
             ...this.parameters ?? [],
             this.tokens.rightParen,
             this.tokens.as,
-            this.returnTypeExpression,
-            this.tokens.endFunctionType
+            this.returnTypeExpression
         );
+        util.setBounds(this, signature, this.tokens.endFunctionType);
+        this.body?.setBoundsBetween(signature, this.tokens.endFunctionType);
     }
 
     public readonly kind = AstNodeKind.FunctionExpression;
@@ -1053,6 +1054,7 @@ export class LiteralExpression extends Expression {
         this.tokens = {
             value: options.value
         };
+        util.setBounds(this, this.tokens.value);
     }
 
     public readonly tokens: {
@@ -1061,15 +1063,9 @@ export class LiteralExpression extends Expression {
 
     public readonly kind = AstNodeKind.LiteralExpression;
 
-    public get startPosition() {
-        return this.tokens.value.startPosition;
-    }
-    public get endPosition() {
-        return this.tokens.value.endPosition;
-    }
-    public get sourceInfo() {
-        return this.tokens.value.sourceInfo;
-    }
+    public readonly startPosition: number;
+    public readonly endPosition: number;
+    public readonly sourceInfo: SourceInfo | undefined;
 
     public getType(options?: GetTypeOptions) {
         return util.tokenToBscType(this.tokens.value);
@@ -2441,6 +2437,8 @@ export class AnnotationExpression extends Expression {
         };
         this.call = options.call;
         this.name = this.tokens.name.text;
+        //the parser assigns `call` after construction (since the call needs this annotation as its callee), and updates the bounds then
+        util.setBounds(this, this.tokens.at, this.tokens.name, this.call);
     }
 
     public readonly kind = AstNodeKind.AnnotationExpression;
@@ -2450,19 +2448,9 @@ export class AnnotationExpression extends Expression {
         readonly name: Token;
     };
 
-    public get startPosition() {
-        return this.getBounds().startPosition;
-    }
-    public get endPosition() {
-        return this.getBounds().endPosition;
-    }
-    public get sourceInfo() {
-        return this.getBounds().sourceInfo;
-    }
-    private getBounds() {
-        //this needs to be computed on demand because `call` is assigned after construction
-        return util.setBounds({} as Locatable, this.tokens.at, this.tokens.name, this.call);
-    }
+    public readonly startPosition: number;
+    public readonly endPosition: number;
+    public readonly sourceInfo: SourceInfo | undefined;
 
     public readonly name: string;
 
@@ -2775,22 +2763,17 @@ export class RegexLiteralExpression extends Expression {
         this.tokens = {
             regexLiteral: options.regexLiteral
         };
+        util.setBounds(this, this.tokens.regexLiteral);
     }
 
     public readonly kind = AstNodeKind.RegexLiteralExpression;
     public readonly tokens: {
         readonly regexLiteral: Token;
     };
 
-    public get startPosition() {
-        return this.tokens?.regexLiteral?.startPosition;
-    }
-    public get endPosition() {
-        return this.tokens?.regexLiteral?.endPosition;
-    }
-    public get sourceInfo() {
-        return this.tokens?.regexLiteral?.sourceInfo;
-    }
+    public readonly startPosition: number;
+    public readonly endPosition: number;
+    public readonly sourceInfo: SourceInfo | undefined;
 
     public transpile(state: BrsTranspileState): TranspileResult {
         let text = this.tokens.regexLiteral?.text ?? '';

src/parser/Parser.ts

@@ -256,7 +256,7 @@ export class Parser {
     private body() {
         const parentAnnotations = this.enterAnnotationBlock();
 
-        let body = new Body({ statements: [] });
+        const statements: Statement[] = [];
         if (this.tokens.length > 0) {
             this.consumeStatementSeparators(true);
 
@@ -271,7 +271,7 @@ export class Parser {
                     if (dec) {
                         if (!isAnnotationExpression(dec)) {
                             this.consumePendingAnnotations(dec);
-                            body.statements.push(dec);
+                            statements.push(dec);
                             //ensure statement separator
                             this.consumeStatementSeparators(false);
                         } else {
@@ -286,7 +286,8 @@ export class Parser {
         }
 
         this.exitAnnotationBlock(parentAnnotations);
-        return body;
+        //construct the body once all of its statements are known, so its bounds are computed from them
+        return new Body({ statements: statements });
     }
 
     private sanitizeParseOptions(options: ParseOptions) {
@@ -1796,6 +1797,8 @@ export class Parser {
         if (this.check(TokenKind.LeftParen)) {
             let leftParen = this.advance();
             annotation.call = this.finishCall(leftParen, annotation, false);
+            //the call needs the annotation as its callee, so it can't be passed to the constructor. Include it in the bounds now that it exists
+            util.setBounds(annotation, annotation.tokens.at, annotation.tokens.name, annotation.call);
         }
         return annotation;
     }