AST tweaks

[TwitchBronBron]

Here are a few tweaks that should be made to the AST while we're making breaking changes:

• [x] Block.startingRange - I think we can eliminate this if we instead include whitespace or newlines in the leadingTrivia of the first statement's first token.
• [x] IfStatement.isInline - probably remove? what's the purpose of this? Does it actually matter in the context of syntax validation and transpiling?
• [x] standardize equal or equals for tokens in statements and expressions
• ~do we need a dedicated Identifier token type?~
• [x] for FunctionType.tokens.end, should we call it endFunctionType instead?
• [x] ImportStatement.filePath -> rename to .path

[TwitchBronBron]

Added another suggestion:

• ImportStatement.filePath -> rename to .path

[markwpearce]

re: Block.startingRange

Removing this causes problems with "empty" blocks...

consider:

function noop(x)

end function

if there are no statements in that block, it effectively has no range, even though in reality there are characters (whitespace, etc), inside it.

Any searches based on range position will not work, because the block has no range, so util.comparePositionToRange() will not work.

Any ideas on how to deal with that?

[TwitchBronBron]

Can we look at this.parent.range to determine our range?

[markwpearce]

@TwitchBronBron

this.parent.range will not work for determining empty block range.

consider:

if condition
 ' this is a then block
 ' that has no statements
else 
 ' this is an else block that has no statements
end if

In this case, it's difficult to know what the range of those blocks are. The problem is that the BrsFile.getClosestExpression(position) function walks the tree, assuming that each child node will have a range inside the range of the parent, basically, if we use this.parent.range, then getClosestExpression walks too far down the tree.

I thought it might be possible to introduce a new method on "block-containing nodes" (like FunctionExpression, etc). that would give the "innerRange", eg. the range where a block could go... however, it becomes difficult with IfStatement because there are 2 potential blocks.

It basically is just getting uglier to deal with the more I dig into it.

[markwpearce]

An alternate solution here is to make a giant if statement in the Block.range getter:

    get range(): Range {
        if (this.statements.length > 0) {
            return util.createBoundingRange(...(this.statements));
        }
        if (isFunctionExpression(this.parent)) {
            const 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;
            return util.createRangeFromPositions(lastBitBefore, this.parent.tokens.endFunctionType?.range?.start);
        } else if (isIfStatement(this.parent)) {
            if (this.parent.thenBranch === this) {
                const lastBitBefore = util.createBoundingRange(
                    this.parent.tokens.then,
                    this.parent.condition
                )?.end;
                const firstBitAfter = util.createBoundingRange(
                    this.parent.tokens.else,
                    this.parent.elseBranch,
                    this.parent.tokens.endIf
                )?.start;
                return util.createRangeFromPositions(lastBitBefore, firstBitAfter);
            } else if (this.parent.elseBranch === this) {
                const lastBitBefore = util.createBoundingRange(
                    this.parent.tokens.else
                )?.end;
                const firstBitAfter = util.createBoundingRange(
                    this.parent.tokens.endIf
                )?.start;
                return util.createRangeFromPositions(lastBitBefore, firstBitAfter);
            }
        } else if (isForStatement(this.parent)) {

        } else if (isForEachStatement(this.parent)) {

        }else if (isWhileStatement(this.parent)) {

        }
        return util.createRange(0, 0, 0, 0);
    }

this is what we ended up using

[markwpearce]

@TwitchBronBron What is the alternative of not using TokenKind.Identifier ?

[TwitchBronBron]

@TwitchBronBron What is the alternative of not using TokenKind.Identifier ?

I think I was talking about this thing.

https://github.com/rokucommunity/brighterscript/blob/42db50190171f83dfac6d501a9c797a97fa00d3a/src/lexer/Token.ts#L33

But the more I look at it, it's probably good to be more specific, so let's just ignore that point.

[markwpearce]

Great. That means this issue is complete