diff --git a/src/AstValidationSegmenter.ts b/src/AstValidationSegmenter.ts index d8ba968f2..c05f02ac2 100644 --- a/src/AstValidationSegmenter.ts +++ b/src/AstValidationSegmenter.ts @@ -81,7 +81,7 @@ export class AstValidationSegmenter { if (isArrayType(typeInTypeExpression)) { typeInTypeExpression = typeInTypeExpression.defaultType; } - if (typeInTypeExpression.isResolvable()) { + if (typeInTypeExpression?.isResolvable()) { return this.handleTypeCastTypeExpression(segment, expression); } } diff --git a/src/Scope.spec.ts b/src/Scope.spec.ts index 39345e1ad..70d0e8538 100644 --- a/src/Scope.spec.ts +++ b/src/Scope.spec.ts @@ -4400,6 +4400,238 @@ describe('Scope', () => { let lhs2 = (ifStmts[1].condition as BinaryExpression).left as DottedGetExpression; expectTypeToBe(lhs2.obj.getType({ flags: SymbolTypeFlag.runtime }), ObjectType); }); + + it('should understand assignment within try/catch blocks', () => { + const testFile = program.setFile('source/test.bs', ` + sub test() + data = "hello" + print data ' printStmt 0 - should be string + try + data = 123 + print data ' printStmt 1 - should be int + catch error + print error ' printStmt 2 - (ignored) + end try + print data ' printStmt 3 - should be (string or int) + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + const printStmts = testFile.ast.findChildren(isPrintStatement); + let dataVar = printStmts[0].expressions[0]; + expectTypeToBe(dataVar.getType({ flags: SymbolTypeFlag.runtime }), StringType); + dataVar = printStmts[1].expressions[0]; + expectTypeToBe(dataVar.getType({ flags: SymbolTypeFlag.runtime }), IntegerType); + dataVar = printStmts[3].expressions[0]; + let dataVarType = dataVar.getType({ flags: SymbolTypeFlag.runtime }); + expectTypeToBe(dataVarType, UnionType); + expect((dataVarType as UnionType).types).to.include(StringType.instance); + expect((dataVarType as UnionType).types).to.include(IntegerType.instance); + }); + + it('should understand assignment in if/then in try/catch blocks', () => { + const testFile = program.setFile('source/test.bs', ` + sub test() + data = "hello" + print data ' printStmt 0 - should be string + try + if data = "hello" + data = "goodbye" + end if + print data ' printStmt 1 - should be string + catch error + print error ' printStmt 2 - (ignored) + end try + print data ' printStmt 3 - should be (string) + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + const printStmts = testFile.ast.findChildren(isPrintStatement); + let dataVar = printStmts[0].expressions[0]; + expectTypeToBe(dataVar.getType({ flags: SymbolTypeFlag.runtime }), StringType); + dataVar = printStmts[1].expressions[0]; + expectTypeToBe(dataVar.getType({ flags: SymbolTypeFlag.runtime }), StringType); + dataVar = printStmts[3].expressions[0]; + let dataVarType = dataVar.getType({ flags: SymbolTypeFlag.runtime }); + expectTypeToBe(dataVarType, StringType); + }); + + it('should understand assignment that changes types in if/then in try/catch blocks', () => { + const testFile = program.setFile('source/test.bs', ` + sub test() + data = "hello" + print data ' printStmt 0 - should be string + try + if data = "hello" + data = 123 + end if + print data ' printStmt 1 - should be union (string or int) + catch error + print error ' printStmt 2 - (ignored) + end try + print data ' printStmt 3 - should be union (string or int) + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + const printStmts = testFile.ast.findChildren(isPrintStatement); + let dataVar = printStmts[0].expressions[0]; + expectTypeToBe(dataVar.getType({ flags: SymbolTypeFlag.runtime }), StringType); + dataVar = printStmts[1].expressions[0]; + let dataVarType = dataVar.getType({ flags: SymbolTypeFlag.runtime }); + expectTypeToBe(dataVarType, UnionType); + expect((dataVarType as UnionType).types).to.include(StringType.instance); + expect((dataVarType as UnionType).types).to.include(IntegerType.instance); + dataVar = printStmts[3].expressions[0]; + dataVarType = dataVar.getType({ flags: SymbolTypeFlag.runtime }); + expectTypeToBe(dataVarType, UnionType); + expect((dataVarType as UnionType).types).to.include(StringType.instance); + expect((dataVarType as UnionType).types).to.include(IntegerType.instance); + }); + + it('should understand changing the type of a param in try/catch', () => { + const testFile = program.setFile('source/test.bs', ` + sub testPocket1(msg as string) + try + if msg = "" then + msg = "hello!" + end if + msg = 123 + catch e + end try + print msg + print msg.toStr() ' confirming msg is string|int, and not uninitialized + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + const printStmts = testFile.ast.findChildren(isPrintStatement); + let msgVar = printStmts[0].expressions[0]; + let msgVarType = msgVar.getType({ flags: SymbolTypeFlag.runtime }); + expectTypeToBe(msgVarType, UnionType); + expect((msgVarType as UnionType).types).to.include(StringType.instance); + expect((msgVarType as UnionType).types).to.include(IntegerType.instance); + }); + + it('should understand changing the type of a param in try/catch in non-first function', () => { + const testFile = program.setFile('source/test.bs', ` + function foo() as string + msg = "test" + return msg + end function + + sub testPocket1(msg as string) + try + if msg = "" then + msg = "hello!" + end if + msg = 123 + catch e + end try + print msg + print msg.toStr() ' confirming msg is string|int, and not uninitialized + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + const printStmts = testFile.ast.findChildren(isPrintStatement); + let msgVar = printStmts[0].expressions[0]; + let msgVarType = msgVar.getType({ flags: SymbolTypeFlag.runtime }); + expectTypeToBe(msgVarType, UnionType); + expect((msgVarType as UnionType).types).to.include(StringType.instance); + expect((msgVarType as UnionType).types).to.include(IntegerType.instance); + }); + + + it('should allow redefinition of function param and immediate use', () => { + const testFile = program.setFile('source/test.bs', ` + sub testPocket1(data as string) + data = 123 + data += 1 + print data + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + const printStmts = testFile.ast.findChildren(isPrintStatement); + let dataType = printStmts[0].expressions[0]; + let dataTypeType = dataType.getType({ flags: SymbolTypeFlag.runtime }); + expectTypeToBe(dataTypeType, IntegerType); + }); + + it('handles this long function from Rooibos', () => { + program.setFile('source/test.bs', ` + function assertAAHasKeys(aa as dynamic, keys as dynamic, msg = "" as string) as boolean + if m.currentResult.isFail then + return false + end if + + try + if not isAA(aa) then + if msg = "" then + msg = "expected to be an AssociativeArray" + end if + fail(msg, "", "", true) + return false + end if + + if not isArray(keys) then + if msg = "" then + msg = "expected to be an Array" + end if + fail(msg, "", "", true) + return false + end if + + foundKeys = [] + missingKeys = [] + for each key in keys + if not aa.ifAssociativeArray.DoesExist(key) then + missingKeys.push(key) + else + foundKeys.push(key) + end if + end for + + if missingKeys.count() > 0 then + actual = "blah" + expected = "blah" + if msg = "" then + msg = "expected to have properties" + end if + fail(msg, actual, expected, true) + return false + end if + return true + catch error + failCrash(error, msg) + end try + return false + end function + + + function isAA(value as dynamic) as boolean + return type(value) = "roAssociativeArray" + end function + + + function isArray(value as dynamic) as boolean + return type(value) = "roArray" + end function + + + sub fail(msg as string, actual as dynamic, expected as dynamic, isSoftFail as boolean) + print "fail" + end sub + + sub failCrash(error as dynamic, msg as string) + print "fail crash" + end sub + `); + program.validate(); + expectZeroDiagnostics(program); + }); }); describe('unlinkSymbolTable', () => { diff --git a/src/SymbolTable.ts b/src/SymbolTable.ts index 28a8f6886..ea38707d2 100644 --- a/src/SymbolTable.ts +++ b/src/SymbolTable.ts @@ -101,6 +101,7 @@ export class SymbolTable implements SymbolTypeGetter { public pocketTables = new Array(); public addPocketTable(pocketTable: PocketTable) { + pocketTable.table.isPocketTable = true; this.pocketTables.push(pocketTable); return () => { const index = this.pocketTables.findIndex(pt => pt === pocketTable); @@ -110,6 +111,18 @@ export class SymbolTable implements SymbolTypeGetter { }; } + private isPocketTable = false; + + private getCurrentPocketTableDepth() { + let depth = 0; + let currentTable: SymbolTable = this; + while (currentTable.isPocketTable) { + depth++; + currentTable = currentTable.parent; + } + return depth; + } + public getStatementIndexOfPocketTable(symbolTable: SymbolTable) { return this.pocketTables.find(pt => pt.table === symbolTable)?.index ?? -1; } @@ -202,27 +215,41 @@ export class SymbolTable implements SymbolTypeGetter { } // look in our map first - result = currentTable.symbolMap.get(key); - if (result) { + let currentResults = currentTable.symbolMap.get(key); + if (currentResults) { // eslint-disable-next-line no-bitwise - result = result.filter(symbol => symbol.flags & bitFlags).filter(this.getSymbolLookupFilter(currentTable, maxStatementIndex, memberOfAncestor)); + currentResults = currentResults.filter(symbol => symbol.flags & bitFlags) + .filter(this.getSymbolLookupFilter(currentTable, maxStatementIndex, memberOfAncestor)); } let precedingAssignmentIndex = -1; - if (result?.length > 0 && currentTable.isOrdered && maxStatementIndex >= 0) { - this.sortSymbolsByAssignmentOrderInPlace(result); - const lastResult = result[result.length - 1]; - result = [lastResult]; + if (currentResults?.length > 0 && currentTable.isOrdered && maxStatementIndex >= 0) { + this.sortSymbolsByAssignmentOrderInPlace(currentResults); + const lastResult = currentResults[currentResults.length - 1]; + currentResults = [lastResult]; precedingAssignmentIndex = lastResult.data?.definingNode?.statementIndex ?? -1; } - result = currentTable.augmentSymbolResultsWithPocketTableResults(name, bitFlags, result, { + if (result?.length > 0) { + // we already have results from a deeper pocketTable + if (currentResults?.length > 0) { + result.push(...currentResults); + } + } else if (currentResults?.length > 0) { + result = currentResults; + } + + let depth = additionalOptions?.depth ?? currentTable.getCurrentPocketTableDepth(); + const augmentationResult = currentTable.augmentSymbolResultsWithPocketTableResults(name, bitFlags, result, { ...additionalOptions, + depth: depth, maxStatementIndex: maxStatementIndex, precedingAssignmentIndex: precedingAssignmentIndex }); + result = augmentationResult.symbols; + const needCheckParent = (!augmentationResult.exhaustive && depth > 0); - if (result?.length > 0) { + if (result?.length > 0 && !needCheckParent) { result = result.map(addAncestorInfo); break; } @@ -244,7 +271,7 @@ export class SymbolTable implements SymbolTypeGetter { return result; } - private augmentSymbolResultsWithPocketTableResults(name: string, bitFlags: SymbolTypeFlag, result: BscSymbol[], additionalOptions: { precedingAssignmentIndex?: number } & GetSymbolAdditionalOptions = {}): BscSymbol[] { + private augmentSymbolResultsWithPocketTableResults(name: string, bitFlags: SymbolTypeFlag, result: BscSymbol[], additionalOptions: { precedingAssignmentIndex?: number } & GetSymbolAdditionalOptions = {}): { symbols: BscSymbol[]; exhaustive: boolean } { let pocketTableResults: BscSymbol[] = []; let pocketTablesWeFoundSomethingIn = this.getSymbolDataFromPocketTables(name, bitFlags, additionalOptions); let pocketTablesAreExhaustive = false; @@ -307,7 +334,9 @@ export class SymbolTable implements SymbolTypeGetter { result.push(...pocketTableResults); } } - return result; + // Do the results cover all possible execution paths? + const areResultsExhaustive = pocketTablesAreExhaustive || pocketTablesWeFoundSomethingIn.length === 0; + return { symbols: result, exhaustive: areResultsExhaustive }; } private getSymbolDataFromPocketTables(name: string, bitFlags: SymbolTypeFlag, additionalOptions: { precedingAssignmentIndex?: number } & GetSymbolAdditionalOptions = {}): Array<{ pocketTable: PocketTable; results: BscSymbol[] }> { @@ -650,6 +679,7 @@ export class SymbolTable implements SymbolTypeGetter { // order doesn't matter for current table return true; } + if (maxAllowedStatementIndex >= 0 && t.data?.definingNode) { if (memberOfAncestor || t.data.canUseInDefinedAstNode) { // if we've already gone up a level, it's possible to have a variable assigned and used diff --git a/src/bscPlugin/validation/BrsFileValidator.ts b/src/bscPlugin/validation/BrsFileValidator.ts index 0ca91b6f1..2c7bc0872 100644 --- a/src/bscPlugin/validation/BrsFileValidator.ts +++ b/src/bscPlugin/validation/BrsFileValidator.ts @@ -205,10 +205,16 @@ export class BrsFileValidator { // add param symbol at expression level, so it can be used as default value in other params const funcExpr = node.findAncestor(isFunctionExpression); const funcSymbolTable = funcExpr?.getSymbolTable(); - funcSymbolTable?.addSymbol(paramName, { definingNode: node, isInstance: true, isFromDocComment: data.isFromDocComment, description: data.description }, nodeType, SymbolTypeFlag.runtime); + const extraSymbolData: ExtraSymbolData = { + definingNode: node, + isInstance: true, + isFromDocComment: data.isFromDocComment, + description: data.description + }; + funcSymbolTable?.addSymbol(paramName, extraSymbolData, nodeType, SymbolTypeFlag.runtime); //also add param symbol at block level, as it may be redefined, and if so, should show a union - funcExpr.body.getSymbolTable()?.addSymbol(paramName, { definingNode: node, isInstance: true, isFromDocComment: data.isFromDocComment }, nodeType, SymbolTypeFlag.runtime); + funcExpr.body.getSymbolTable()?.addSymbol(paramName, extraSymbolData, nodeType, SymbolTypeFlag.runtime); }, InterfaceStatement: (node) => { if (!node.tokens.name) { @@ -342,7 +348,8 @@ export class BrsFileValidator { // this block is in a function. order matters! blockSymbolTable.isOrdered = true; } - if (!isFunctionExpression(node.parent)) { + if (!isFunctionExpression(node.parent) && node.parent) { + node.symbolTable.name = `Block-${node.parent.kind}@${node.location?.range?.start?.line}`; // we're a block inside another block (or body). This block is a pocket in the bigger block node.parent.getSymbolTable().addPocketTable({ index: node.parent.statementIndex, diff --git a/src/parser/AstNode.ts b/src/parser/AstNode.ts index 4f2234a39..34cb2a970 100644 --- a/src/parser/AstNode.ts +++ b/src/parser/AstNode.ts @@ -11,7 +11,7 @@ import util from '../util'; import { DynamicType } from '../types/DynamicType'; import type { BscType } from '../types/BscType'; import type { Token } from '../lexer/Token'; -import { isBlock, isBody } from '../astUtils/reflection'; +import { isBlock, isBody, isFunctionParameterExpression } from '../astUtils/reflection'; /** * A BrightScript AST node @@ -248,8 +248,13 @@ export abstract class AstNode { return -1; } let currentNode: AstNode = this; + if (isFunctionParameterExpression(currentNode)) { + // function parameters are not part of statement lists + return -1; + } while (currentNode && !(isBlock(currentNode?.parent) || isBody(currentNode?.parent))) { currentNode = currentNode.parent; + } if (isBlock(currentNode?.parent) || isBody(currentNode?.parent)) { return currentNode.parent.statements.indexOf(currentNode);