Repository: rokucommunity/brighterscript
Pull request #1224 (2024-06-10 14:57:33)

Remove block starting range
Removes the `startingRange` property on Blocks.

If a block is not empty, the range of a Block is the range of its statements

If the block is empty, the Block looks at its parent to see what the range should be, with specific logic for all the different types of nodes that can contain a block.


Patch:
diff --git a/src/astUtils/visitors.spec.ts b/src/astUtils/visitors.spec.ts
index 85df9bc9..20e37311 100644
--- a/src/astUtils/visitors.spec.ts
+++ b/src/astUtils/visitors.spec.ts
@@ -1,6 +1,6 @@
 /* eslint-disable no-multi-spaces */
 import type { CancellationToken } from 'vscode-languageserver';
-import { CancellationTokenSource, Range } from 'vscode-languageserver';
+import { CancellationTokenSource } from 'vscode-languageserver';
 import { expect } from '../chai-config.spec';
 import * as sinon from 'sinon';
 import { Program } from '../Program';
@@ -252,7 +252,7 @@ describe('astUtils visitors', () => {
                 print: createToken(TokenKind.Print),
                 expressions: []
             });
-            const blockStatement = new Block({ statements: [], startingRange: Range.create(0, 0, 0, 0) });
+            const blockStatement = new Block({ statements: [] });
             visitor(printStatement, undefined);
             visitor(blockStatement, undefined);
             expect(printHandler.callCount).to.equal(1);
@@ -276,8 +276,7 @@ describe('astUtils visitors', () => {
                 statements: [
                     printStatement1,
                     new ReturnStatement({ return: createToken(TokenKind.Return) })
-                ],
-                startingRange: Range.create(0, 0, 0, 0)
+                ]
             });
             const visitor = createVisitor({
                 PrintStatement: () => printStatement2
@@ -300,8 +299,7 @@ describe('astUtils visitors', () => {
             });
 
             const block = new Block({
-                statements: [printStatement1],
-                startingRange: Range.create(0, 0, 0, 0)
+                statements: [printStatement1]
             });
 
             block.walk(createVisitor({
diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts
index d9556a7d..9bf8814f 100644
--- a/src/parser/Parser.ts
+++ b/src/parser/Parser.ts
@@ -915,11 +915,7 @@ export class Parser {
             }
 
             if (!body) {
-                body = new Block({
-                    statements: [],
-                    startingRange: util.createBoundingRange(
-                        functionType, name, leftParen, ...params, rightParen, asToken, typeExpression, endFunctionType)
-                });
+                body = new Block({ statements: [] });
             }
 
             let func = new FunctionExpression({
@@ -1829,7 +1825,6 @@ export class Parser {
         }
 
         const ifToken = this.advance();
-        const startingRange = ifToken.range;
 
         const condition = this.expression();
         let thenBranch: Block;
@@ -1951,7 +1946,7 @@ export class Parser {
                 } else {
                     //missing endif
                     this.diagnostics.push({
-                        ...DiagnosticMessages.expectedEndIfToCloseIfStatement(startingRange.start),
+                        ...DiagnosticMessages.expectedEndIfToCloseIfStatement(ifToken.range.start),
                         range: ifToken.range
                     });
                 }
@@ -1998,7 +1993,6 @@ export class Parser {
 
     private conditionalCompileStatement(): ConditionalCompileStatement {
         const hashIfToken = this.advance();
-        const startingRange = hashIfToken.range;
         let notToken: Token | undefined;
 
         if (this.check(TokenKind.Not)) {
@@ -2062,7 +2056,7 @@ export class Parser {
             } else {
                 //missing #endif
                 this.diagnostics.push({
-                    ...DiagnosticMessages.expectedHashEndIfToCloseHashIf(startingRange.start.line),
+                    ...DiagnosticMessages.expectedHashEndIfToCloseHashIf(hashIfToken.range.start.line),
                     range: hashIfToken.range
                 });
             }
@@ -2112,7 +2106,6 @@ export class Parser {
         const parentAnnotations = this.enterAnnotationBlock();
 
         this.consumeStatementSeparators(true);
-        let startingToken = this.peek();
         const unsafeTerminators = BlockTerminators;
         const conditionalEndTokens = [TokenKind.HashElse, TokenKind.HashElseIf, TokenKind.HashEndIf];
         const terminators = [...conditionalEndTokens, ...unsafeTerminators];
@@ -2182,7 +2175,7 @@ export class Parser {
             }
         }
         this.exitAnnotationBlock(parentAnnotations);
-        return new Block({ statements: statements, startingRange: startingToken.range });
+        return new Block({ statements: statements });
     }
 
     private conditionalCompileConstStatement() {
@@ -2289,7 +2282,6 @@ export class Parser {
             return undefined;
         }
         statements.push(statement);
-        const startingRange = statement.range;
 
         //look for colon statement separator
         let foundColon = false;
@@ -2315,7 +2307,7 @@ export class Parser {
                 });
             }
         }
-        return new Block({ statements: statements, startingRange: startingRange });
+        return new Block({ statements: statements });
     }
 
     private expressionStatement(expr: Expression): ExpressionStatement | IncrementStatement {
@@ -2527,8 +2519,6 @@ export class Parser {
         const parentAnnotations = this.enterAnnotationBlock();
 
         this.consumeStatementSeparators(true);
-        let startingToken = this.peek();
-
         const statements: Statement[] = [];
         const flatGlobalTerminators = this.globalTerminators.flat().flat();
         while (!this.isAtEnd() && !this.checkAny(TokenKind.EndSub, TokenKind.EndFunction, ...terminators, ...flatGlobalTerminators)) {
@@ -2577,7 +2567,7 @@ export class Parser {
         }
 
         this.exitAnnotationBlock(parentAnnotations);
-        return new Block({ statements: statements, startingRange: startingToken.range });
+        return new Block({ statements: statements });
     }
 
     /**
diff --git a/src/parser/Statement.ts b/src/parser/Statement.ts
index 226f9b35..51d1a72f 100644
--- a/src/parser/Statement.ts
+++ b/src/parser/Statement.ts
@@ -4,12 +4,12 @@ import { TokenKind } from '../lexer/TokenKind';
 import type { DottedGetExpression, FunctionExpression, FunctionParameterExpression, LiteralExpression, TypeExpression, TypecastExpression } from './Expression';
 import { CallExpression, VariableExpression } from './Expression';
 import { util } from '../util';
-import type { Range } from 'vscode-languageserver';
+import type { Position, Range } from 'vscode-languageserver';
 import type { BrsTranspileState } from './BrsTranspileState';
 import { ParseMode } from './Parser';
 import type { WalkVisitor, WalkOptions } from '../astUtils/visitors';
 import { InternalWalkMode, walk, createVisitor, WalkMode, walkArray } from '../astUtils/visitors';
-import { isCallExpression, isConditionalCompileStatement, isEnumMemberStatement, isExpression, isExpressionStatement, isFieldStatement, isFunctionStatement, isIfStatement, isInterfaceFieldStatement, isInterfaceMethodStatement, isInvalidType, isLiteralExpression, isMethodStatement, isNamespaceStatement, isTypedefProvider, isUnaryExpression, isVoidType } from '../astUtils/reflection';
+import { isCallExpression, isCatchStatement, isConditionalCompileStatement, isEnumMemberStatement, isExpression, isExpressionStatement, isFieldStatement, isForEachStatement, isForStatement, isFunctionExpression, isFunctionStatement, isIfStatement, isInterfaceFieldStatement, isInterfaceMethodStatement, isInvalidType, isLiteralExpression, isMethodStatement, isNamespaceStatement, isTryCatchStatement, isTypedefProvider, isUnaryExpression, isVoidType, isWhileStatement } from '../astUtils/reflection';
 import { TypeChainEntry, type GetTypeOptions, type TranspileResult, type TypedefProvider } from '../interfaces';
 import { createInvalidLiteral, createMethodStatement, createToken } from '../astUtils/creators';
 import { DynamicType } from '../types/DynamicType';
@@ -253,23 +253,105 @@ export class AugmentedAssignmentStatement extends Statement {
 export class Block extends Statement {
     constructor(options: {
         statements: Statement[];
-        startingRange?: Range;
     }) {
         super();
         this.statements = options.statements;
-        this.startingRange = options.startingRange;
-        this.range = util.createBoundingRange(
-            this.startingRange,
-            ...(this.statements ?? [])
-        );
     }
 
     public readonly statements: Statement[];
-    public readonly startingRange?: Range;
 
     public readonly kind = AstNodeKind.Block;
 
-    public readonly range: Range | undefined;
+    get range(): Range {
+        if (this.statements.length > 0) {
+            return util.createBoundingRange(...this.statements);
+        }
+        let lastBitBefore: Position;
+        let firstBitAfter: Position;
+
+        if (isFunctionExpression(this.parent)) {
+            lastBitBefore = util.createBoundingRange(
+                this.parent.tokens.functionType,
+                this.parent.tokens.leftParen,
+                ...(this.parent.parameters ?? []),
+                this.parent.tokens.rightParen,
+                this.parent.tokens.as,
+                this.parent.returnTypeExpression
+            )?.end;
+            firstBitAfter = this.parent.tokens.endFunctionType?.range?.start;
+        } else if (isIfStatement(this.parent)) {
+            if (this.parent.thenBranch === this) {
+                lastBitBefore = util.createBoundingRange(
+                    this.parent.tokens.then,
+                    this.parent.condition
+                )?.end;
+                firstBitAfter = util.createBoundingRange(
+                    this.parent.tokens.else,
+                    this.parent.elseBranch,
+                    this.parent.tokens.endIf
+                )?.start;
+            } else if (this.parent.elseBranch === this) {
+                lastBitBefore = this.parent.tokens.else?.range.end;
+                firstBitAfter = this.parent.tokens.endIf?.range.start;
+            }
+        } else if (isConditionalCompileStatement(this.parent)) {
+            if (this.parent.thenBranch === this) {
+                lastBitBefore = util.createBoundingRange(
+                    this.parent.tokens.condition,
+                    this.parent.tokens.not,
+                    this.parent.tokens.hashIf
+                )?.end;
+                firstBitAfter = util.createBoundingRange(
+                    this.parent.tokens.hashElse,
+                    this.parent.elseBranch,
+                    this.parent.tokens.hashEndIf
+                )?.start;
+            } else if (this.parent.elseBranch === this) {
+                lastBitBefore = this.parent.tokens.hashElse?.range.end;
+                firstBitAfter = this.parent.tokens.hashEndIf?.range.start;
+            }
+        } else if (isForStatement(this.parent)) {
+            lastBitBefore = util.createBoundingRange(
+                this.parent.increment,
+                this.parent.tokens.step,
+                this.parent.finalValue,
+                this.parent.tokens.to,
+                this.parent.counterDeclaration,
+                this.parent.tokens.for
+            )?.end;
+            firstBitAfter = this.parent.tokens.endFor?.range.start;
+        } else if (isForEachStatement(this.parent)) {
+            lastBitBefore = util.createBoundingRange(
+                this.parent.target,
+                this.parent.tokens.in,
+                this.parent.tokens.item,
+                this.parent.tokens.forEach
+            )?.end;
+            firstBitAfter = this.parent.tokens.endFor?.range.start;
+        } else if (isWhileStatement(this.parent)) {
+            lastBitBefore = util.createBoundingRange(
+                this.parent.condition,
+                this.parent.tokens.while
+            )?.end;
+            firstBitAfter = this.parent.tokens.endWhile?.range.start;
+        } else if (isTryCatchStatement(this.parent)) {
+            lastBitBefore = util.createBoundingRange(
+                this.parent.tokens.try
+            )?.end;
+            firstBitAfter = util.createBoundingRange(
+                this.parent.tokens.endTry,
+                this.parent.catchStatement
+            )?.start;
+        } else if (isCatchStatement(this.parent) && isTryCatchStatement(this.parent?.parent)) {
+            lastBitBefore = util.createBoundingRange(
+                this.parent.tokens.catch,
+                this.parent.tokens.exceptionVariable
+            )?.end;
+            firstBitAfter = this.parent.parent.tokens.endTry?.range?.start;
+        }
+        return util.createRangeFromPositions(lastBitBefore, firstBitAfter);
+
+    }
 
     transpile(state: BrsTranspileState) {
         state.blockDepth++;
diff --git a/src/util.spec.ts b/src/util.spec.ts
index 8751d86c..f59e6ce5 100644
--- a/src/util.spec.ts
+++ b/src/util.spec.ts
@@ -6,7 +6,7 @@ import type { BsConfig } from './BsConfig';
 import * as fsExtra from 'fs-extra';
 import { createSandbox } from 'sinon';
 import { DiagnosticMessages } from './DiagnosticMessages';
-import { tempDir, rootDir, expectTypeToBe } from './testHelpers.spec';
+import { tempDir, rootDir, expectTypeToBe, expectRangeToBe } from './testHelpers.spec';
 import { Program } from './Program';
 import type { BsDiagnostic } from './interfaces';
 import { TypeChainEntry } from './interfaces';
@@ -651,6 +651,29 @@ describe('util', () => {
         });
     });
 
+    describe('range creation', () => {
+        it('createRangeFromPositions', () => {
+            const pos11 = { line: 1, character: 1 };
+            const pos99 = { line: 9, character: 9 };
+
+            expectRangeToBe(util.createRangeFromPositions(pos11, pos99), util.createRange(1, 1, 9, 9));
+            expectRangeToBe(util.createRangeFromPositions(null, pos99), util.createRange(9, 9, 9, 9));
+            expectRangeToBe(util.createRangeFromPositions(pos11, null), util.createRange(1, 1, 1, 1));
+        });
+
+
+        it('createBoundingRange', () => {
+            const range1 = util.createRange(1, 1, 2, 2);
+            const range2 = util.createRange(2, 2, 3, 3);
+            const range3 = util.createRange(100, 100, 100, 100);
+
+            expectRangeToBe(util.createBoundingRange(range1), util.createRange(1, 1, 2, 2));
+            expectRangeToBe(util.createBoundingRange(range1, range2), util.createRange(1, 1, 3, 3));
+            expectRangeToBe(util.createBoundingRange(range2, range1), util.createRange(1, 1, 3, 3));
+            expectRangeToBe(util.createBoundingRange(range2, range3, range1), util.createRange(1, 1, 100, 100));
+        });
+    });
+
     describe('rangesIntersect', () => {
         it('does not crash on undefined range', () => {
             expect(
diff --git a/src/util.ts b/src/util.ts
index c1c136fc..f4497d34 100644
--- a/src/util.ts
+++ b/src/util.ts
@@ -1088,7 +1088,12 @@ export class Util {
     /**
      * Create a `Range` from two `Position`s
      */
-    public createRangeFromPositions(startPosition: Position, endPosition: Position): Range {
+    public createRangeFromPositions(startPosition: Position, endPosition: Position): Range | undefined {
+        startPosition = startPosition ?? endPosition;
+        endPosition = endPosition ?? startPosition;
+        if (!startPosition && !endPosition) {
+            return undefined;
+        }
         return this.createRange(startPosition.line, startPosition.character, endPosition.line, endPosition.character);
     }
 


Test patch:
diff --git a/src/testHelpers.spec.ts b/src/testHelpers.spec.ts
index fdebd84a..b90244f8 100644
--- a/src/testHelpers.spec.ts
+++ b/src/testHelpers.spec.ts
@@ -383,6 +383,16 @@ export function expectTypeToBe(someType: BscType, expectedTypeClass: any) {
     expect(someType?.constructor?.name).to.eq(expectedTypeClass.name);
 }
 
+/**
+ * Test that a range is waht was expected
+ */
+export function expectRangeToBe(actual: Range, expected: Range) {
+    expect(actual.start.line, 'start line').to.eq(expected.start.line);
+    expect(actual.start.character, 'start character').to.eq(expected.start.character);
+    expect(actual.end.line, 'end line').to.eq(expected.end.line);
+    expect(actual.end.character, 'end character').to.eq(expected.end.character);
+}
+
 export function stripConsoleColors(inputString) {
     // Regular expression to match ANSI escape codes for colors
     // eslint-disable-next-line no-control-regex