Skip to content

Commit

Permalink
Removed isInline property from IfStatement (#1227)
Browse files Browse the repository at this point in the history 
* Added tests for inline conditional statements

* Fix lint issue

* Renamed variable to better refelct what it means
  • Loading branch information
@markwpearce
markwpearce authored Jun 12, 2024
1 parent c9a0a13 commit 0bf5758
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 21 deletions.
18 changes: 11 additions & 7 deletions src/parser/Parser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1844,6 +1846,9 @@ export class Parser {

if (isInlineIfThen) {
/*** PARSE INLINE IF STATEMENT ***/
if (!incrementNestedCount) {
this.nestedInlineConditionalCount++;
}

thenBranch = this.inlineConditionalBranch(TokenKind.Else, TokenKind.EndIf);

Expand All @@ -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) {
Expand Down Expand Up @@ -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();
Expand All @@ -1914,7 +1919,7 @@ export class Parser {
});
}
}

this.nestedInlineConditionalCount--;
} else {
/*** PARSE MULTI-LINE IF STATEMENT ***/

Expand Down Expand Up @@ -1960,8 +1965,7 @@ export class Parser {
else: elseToken,
condition: condition,
thenBranch: thenBranch,
elseBranch: elseBranch,
isInline: isInlineIfThen
elseBranch: elseBranch
});
}

Expand Down
19 changes: 16 additions & 3 deletions src/parser/Statement.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand Down
143 changes: 142 additions & 1 deletion src/parser/tests/controlFlow/If.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
10 changes: 0 additions & 10 deletions src/testHelpers.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 0bf5758

Please sign in to comment.