Repository: rokucommunity/brighterscript
Pull request #1227 (2024-06-11 19:36:53)

Removed `isInline` property from `IfStatement` 
- It was changed to a getter, so that proper diagnostic could still be added if there was a problem.
- Added tests for inline conditional statements
- Added support for nested inline conditionals. (People must have a deathwish!)


Patch:
diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts
index 9bf8814f..f234f719 100644
--- a/src/parser/Parser.ts
+++ b/src/parser/Parser.ts
@@ -1810,12 +1810,14 @@ export class Parser {
         });
     }
 
-    private ifStatement(): IfStatement {
+    private nestedInlineConditionalCount = 0;
+
+    private ifStatement(incrementNestedCount = true): IfStatement {
         // colon before `if` is usually not allowed, unless it's after `then`
         if (this.current > 0) {
             const prev = this.previous();
             if (prev.kind === TokenKind.Colon) {
-                if (this.current > 1 && this.tokens[this.current - 2].kind !== TokenKind.Then) {
+                if (this.current > 1 && this.tokens[this.current - 2].kind !== TokenKind.Then && this.nestedInlineConditionalCount === 0) {
                     this.diagnostics.push({
                         ...DiagnosticMessages.unexpectedColonBeforeIfStatement(),
                         range: prev.range
@@ -1844,6 +1846,9 @@ export class Parser {
 
         if (isInlineIfThen) {
             /*** PARSE INLINE IF STATEMENT ***/
+            if (!incrementNestedCount) {
+                this.nestedInlineConditionalCount++;
+            }
 
             thenBranch = this.inlineConditionalBranch(TokenKind.Else, TokenKind.EndIf);
 
@@ -1863,7 +1868,7 @@ export class Parser {
 
                 if (this.check(TokenKind.If)) {
                     // recurse-read `else if`
-                    elseBranch = this.ifStatement();
+                    elseBranch = this.ifStatement(false);
 
                     //no multi-line if chained with an inline if
                     if (!elseBranch.isInline) {
@@ -1901,7 +1906,7 @@ export class Parser {
             if (!elseBranch || !isIfStatement(elseBranch)) {
                 //enforce newline at the end of the inline if statement
                 const peek = this.peek();
-                if (peek.kind !== TokenKind.Newline && peek.kind !== TokenKind.Comment && !this.isAtEnd()) {
+                if (peek.kind !== TokenKind.Newline && peek.kind !== TokenKind.Comment && peek.kind !== TokenKind.Else && !this.isAtEnd()) {
                     //ignore last error if it was about a colon
                     if (this.previous().kind === TokenKind.Colon) {
                         this.diagnostics.pop();
@@ -1914,7 +1919,7 @@ export class Parser {
                     });
                 }
             }
-
+            this.nestedInlineConditionalCount--;
         } else {
             /*** PARSE MULTI-LINE IF STATEMENT ***/
 
@@ -1960,8 +1965,7 @@ export class Parser {
             else: elseToken,
             condition: condition,
             thenBranch: thenBranch,
-            elseBranch: elseBranch,
-            isInline: isInlineIfThen
+            elseBranch: elseBranch
         });
     }
 
diff --git a/src/parser/Statement.ts b/src/parser/Statement.ts
index 51d1a72f..1844674d 100644
--- a/src/parser/Statement.ts
+++ b/src/parser/Statement.ts
@@ -386,6 +386,10 @@ export class Block extends Statement {
         return results;
     }
 
+    public getLeadingTrivia(): Token[] {
+        return this.statements[0]?.getLeadingTrivia() ?? [];
+    }
+
     walk(visitor: WalkVisitor, options: WalkOptions) {
         if (options.walkMode & InternalWalkMode.walkStatements) {
             walkArray(this.statements, visitor, options, this);
@@ -587,13 +591,11 @@ export class IfStatement extends Statement {
         condition: Expression;
         thenBranch: Block;
         elseBranch?: IfStatement | Block;
-        isInline?: boolean;
     }) {
         super();
         this.condition = options.condition;
         this.thenBranch = options.thenBranch;
         this.elseBranch = options.elseBranch;
-        this.isInline = options.isInline;
 
         this.tokens = {
             if: options.if,
@@ -619,12 +621,23 @@ export class IfStatement extends Statement {
     public readonly condition: Expression;
     public readonly thenBranch: Block;
     public readonly elseBranch?: IfStatement | Block;
-    public readonly isInline?: boolean;
 
     public readonly kind = AstNodeKind.IfStatement;
 
     public readonly range: Range | undefined;
 
+    get isInline() {
+        const allLeadingTrivia = [
+            ...this.thenBranch.getLeadingTrivia(),
+            ...this.thenBranch.statements.map(s => s.getLeadingTrivia()).flat(),
+            ...(this.tokens.else?.leadingTrivia ?? []),
+            ...(this.tokens.endIf?.leadingTrivia ?? [])
+        ];
+
+        const hasNewline = allLeadingTrivia.find(t => t.kind === TokenKind.Newline);
+        return !hasNewline;
+    }
+
     transpile(state: BrsTranspileState) {
         let results = [] as TranspileResult;
         //if   (already indented by block)


Test patch:
diff --git a/src/parser/tests/controlFlow/If.spec.ts b/src/parser/tests/controlFlow/If.spec.ts
index 73ceee30..bc6815d2 100644
--- a/src/parser/tests/controlFlow/If.spec.ts
+++ b/src/parser/tests/controlFlow/If.spec.ts
@@ -6,6 +6,8 @@ import { TokenKind } from '../../../lexer/TokenKind';
 import { EOF, identifier, rangeMatch, token } from '../Parser.spec';
 import { isBlock, isFunctionStatement, isIfStatement } from '../../../astUtils/reflection';
 import type { Block, FunctionStatement, IfStatement } from '../../Statement';
+import { expectDiagnosticsIncludes, expectZeroDiagnostics } from '../../../testHelpers.spec';
+import { DiagnosticMessages } from '../../../DiagnosticMessages';
 
 describe('parser if statements', () => {
     it('allows empty if blocks', () => {
@@ -233,9 +235,148 @@ describe('parser if statements', () => {
                 if true then return else print 1
             `);
 
-            expect(diagnostics).to.be.lengthOf(0);
+            expectZeroDiagnostics(diagnostics);
             expect(statements).to.be.length.greaterThan(0);
         });
+
+        it('colon used between inline else statements', () => {
+            const { statements, diagnostics } = Parser.parse(`
+                if x print 1 else print 2 : print 3
+            `);
+
+            expectZeroDiagnostics(diagnostics);
+            expect(statements).to.be.lengthOf(1);
+            const ifStatement = statements[0] as IfStatement;
+            expect(ifStatement.thenBranch.statements).to.be.lengthOf(1);
+            expect((ifStatement.elseBranch as Block).statements).to.be.lengthOf(2);
+        });
+
+        it('colon used between inline else if statements', () => {
+            const { statements, diagnostics } = Parser.parse(`
+                if x print 1 else if y print 2 : print 3
+            `);
+
+            expectZeroDiagnostics(diagnostics);
+            expect(statements).to.be.lengthOf(1);
+            const ifStatement = statements[0] as IfStatement;
+            expect(ifStatement.thenBranch.statements).to.be.lengthOf(1);
+            expect((ifStatement.elseBranch as IfStatement).thenBranch.statements).to.be.lengthOf(2);
+        });
+
+        it('colon used between all kinds of statements', () => {
+            const { statements, diagnostics } = Parser.parse(`
+                if x print 1 : print 1 else if y print 2 : print 2 : print 2 else print 3 : print 3: print 3: print 3
+            `);
+
+            expectZeroDiagnostics(diagnostics);
+            expect(statements).to.be.lengthOf(1);
+            const ifStatement = statements[0] as IfStatement;
+            expect(ifStatement.thenBranch.statements).to.be.lengthOf(2);
+            const elseIf = ifStatement.elseBranch as IfStatement;
+            expect(elseIf.thenBranch.statements).to.be.lengthOf(3);
+            expect((elseIf.elseBranch as Block).statements).to.be.lengthOf(4);
+        });
+
+        it('has diagnostic with extra else', () => {
+            const { diagnostics } = Parser.parse(`
+                if x=1 print 1 else print 2 else print 3
+            `);
+
+            expectDiagnosticsIncludes(diagnostics, [
+                DiagnosticMessages.expectedNewlineOrColon().message
+            ]);
+        });
+
+        it('nested inline if statements', () => {
+            const { statements, diagnostics } = Parser.parse(`
+                if x=1 print 1 else if x=2 print 2 : if y=1 print "y is 1" else print "y is not 1" else print 3: print 3: print 3
+            `);
+
+            expectZeroDiagnostics(diagnostics);
+            expect(statements).to.be.lengthOf(1);
+            const ifStatement = statements[0] as IfStatement;
+            expect(ifStatement.thenBranch.statements).to.be.lengthOf(1);
+            const elseIf = ifStatement.elseBranch as IfStatement;
+            expect(elseIf.thenBranch.statements).to.be.lengthOf(2);
+            expect(isIfStatement(elseIf.thenBranch.statements[1])).to.be.true;
+            const nestedInlineIf = elseIf.thenBranch.statements[1] as IfStatement;
+            expect(nestedInlineIf.thenBranch.statements).to.be.lengthOf(1);
+            expect((nestedInlineIf.elseBranch as Block).statements).to.be.lengthOf(1);
+            expect((elseIf.elseBranch as Block).statements).to.be.lengthOf(3);
+        });
+
+        describe('expected inline if', () => {
+
+            it('non-inline statement used in inline if', () => {
+                const { statements, diagnostics } = Parser.parse(`
+                    if true print 1 else
+                        print 1
+                    end if
+                `);
+                expectDiagnosticsIncludes(diagnostics, [
+                    DiagnosticMessages.expectedInlineIfStatement().message
+                ]);
+                expect(statements.length).to.be.greaterThan(0);
+            });
+
+            it('non inline statement used in inline else if', () => {
+                const { statements, diagnostics } = Parser.parse(`
+                    if x print 1 else if y
+                        print 2
+                    end if
+                `);
+                expectDiagnosticsIncludes(diagnostics, [
+                    DiagnosticMessages.expectedInlineIfStatement().message
+                ]);
+                expect(statements.length).to.be.greaterThan(0);
+            });
+
+            it('colon used after inline else', () => {
+                const { statements, diagnostics } = Parser.parse(`
+                    if x print 1 else : print 2 : print 3: end if
+                `);
+                expectDiagnosticsIncludes(diagnostics, [
+                    DiagnosticMessages.expectedInlineIfStatement().message
+                ]);
+                expect(statements.length).to.be.greaterThan(0);
+            });
+
+            it('new line used in inline else', () => {
+                const { statements, diagnostics } = Parser.parse(`
+                    if x print 1 else
+                        print 2
+                        print 3
+                    end if
+                `);
+                expectDiagnosticsIncludes(diagnostics, [
+                    DiagnosticMessages.expectedInlineIfStatement().message
+                ]);
+                expect(statements.length).to.be.greaterThan(0);
+            });
+
+
+            it('colon used after inline else if', () => {
+                const { diagnostics } = Parser.parse(`
+                    if x print 1 else if y : print 2
+                `);
+                expectDiagnosticsIncludes(diagnostics, [
+                    DiagnosticMessages.expectedEndIfElseIfOrElseToTerminateThenBlock().message
+                ]);
+            });
+
+            it('new line used in inline else if', () => {
+                const { statements, diagnostics } = Parser.parse(`
+                    if x print 1 else if y
+                        print 2
+                    end if
+                `);
+                expectDiagnosticsIncludes(diagnostics, [
+                    DiagnosticMessages.expectedInlineIfStatement().message
+                ]);
+                expect(statements.length).to.be.greaterThan(0);
+            });
+
+        });
     });
 
     describe('block if', () => {
diff --git a/src/testHelpers.spec.ts b/src/testHelpers.spec.ts
index b90244f8..fdebd84a 100644
--- a/src/testHelpers.spec.ts
+++ b/src/testHelpers.spec.ts
@@ -383,16 +383,6 @@ 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