Skip to content

Commit 2ea4d21

Browse files
markwpearcemarkwpearce
authored
Fixes getting types from pocket tables in Try/Catch (#1602)
2 parents a65ebfc + 549ba37 commit 2ea4d21

File tree

5 files changed

+290
-16
lines changed

5 files changed

+290
-16
lines changed

‎src/AstValidationSegmenter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export class AstValidationSegmenter {
8181
if (isArrayType(typeInTypeExpression)) {
8282
typeInTypeExpression = typeInTypeExpression.defaultType;
8383
}
84-
if (typeInTypeExpression.isResolvable()) {
84+
if (typeInTypeExpression?.isResolvable()) {
8585
return this.handleTypeCastTypeExpression(segment, expression);
8686
}
8787
}

‎src/Scope.spec.ts‎

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4400,6 +4400,238 @@ describe('Scope', () => {
44004400
let lhs2 = (ifStmts[1].condition as BinaryExpression).left as DottedGetExpression;
44014401
expectTypeToBe(lhs2.obj.getType({ flags: SymbolTypeFlag.runtime }), ObjectType);
44024402
});
4403+
4404+
it('should understand assignment within try/catch blocks', () => {
4405+
const testFile = program.setFile<BrsFile>('source/test.bs', `
4406+
sub test()
4407+
data = "hello"
4408+
print data ' printStmt 0 - should be string
4409+
try
4410+
data = 123
4411+
print data ' printStmt 1 - should be int
4412+
catch error
4413+
print error ' printStmt 2 - (ignored)
4414+
end try
4415+
print data ' printStmt 3 - should be (string or int)
4416+
end sub
4417+
`);
4418+
program.validate();
4419+
expectZeroDiagnostics(program);
4420+
const printStmts = testFile.ast.findChildren<PrintStatement>(isPrintStatement);
4421+
let dataVar = printStmts[0].expressions[0];
4422+
expectTypeToBe(dataVar.getType({ flags: SymbolTypeFlag.runtime }), StringType);
4423+
dataVar = printStmts[1].expressions[0];
4424+
expectTypeToBe(dataVar.getType({ flags: SymbolTypeFlag.runtime }), IntegerType);
4425+
dataVar = printStmts[3].expressions[0];
4426+
let dataVarType = dataVar.getType({ flags: SymbolTypeFlag.runtime });
4427+
expectTypeToBe(dataVarType, UnionType);
4428+
expect((dataVarType as UnionType).types).to.include(StringType.instance);
4429+
expect((dataVarType as UnionType).types).to.include(IntegerType.instance);
4430+
});
4431+
4432+
it('should understand assignment in if/then in try/catch blocks', () => {
4433+
const testFile = program.setFile<BrsFile>('source/test.bs', `
4434+
sub test()
4435+
data = "hello"
4436+
print data ' printStmt 0 - should be string
4437+
try
4438+
if data = "hello"
4439+
data = "goodbye"
4440+
end if
4441+
print data ' printStmt 1 - should be string
4442+
catch error
4443+
print error ' printStmt 2 - (ignored)
4444+
end try
4445+
print data ' printStmt 3 - should be (string)
4446+
end sub
4447+
`);
4448+
program.validate();
4449+
expectZeroDiagnostics(program);
4450+
const printStmts = testFile.ast.findChildren<PrintStatement>(isPrintStatement);
4451+
let dataVar = printStmts[0].expressions[0];
4452+
expectTypeToBe(dataVar.getType({ flags: SymbolTypeFlag.runtime }), StringType);
4453+
dataVar = printStmts[1].expressions[0];
4454+
expectTypeToBe(dataVar.getType({ flags: SymbolTypeFlag.runtime }), StringType);
4455+
dataVar = printStmts[3].expressions[0];
4456+
let dataVarType = dataVar.getType({ flags: SymbolTypeFlag.runtime });
4457+
expectTypeToBe(dataVarType, StringType);
4458+
});
4459+
4460+
it('should understand assignment that changes types in if/then in try/catch blocks', () => {
4461+
const testFile = program.setFile<BrsFile>('source/test.bs', `
4462+
sub test()
4463+
data = "hello"
4464+
print data ' printStmt 0 - should be string
4465+
try
4466+
if data = "hello"
4467+
data = 123
4468+
end if
4469+
print data ' printStmt 1 - should be union (string or int)
4470+
catch error
4471+
print error ' printStmt 2 - (ignored)
4472+
end try
4473+
print data ' printStmt 3 - should be union (string or int)
4474+
end sub
4475+
`);
4476+
program.validate();
4477+
expectZeroDiagnostics(program);
4478+
const printStmts = testFile.ast.findChildren<PrintStatement>(isPrintStatement);
4479+
let dataVar = printStmts[0].expressions[0];
4480+
expectTypeToBe(dataVar.getType({ flags: SymbolTypeFlag.runtime }), StringType);
4481+
dataVar = printStmts[1].expressions[0];
4482+
let dataVarType = dataVar.getType({ flags: SymbolTypeFlag.runtime });
4483+
expectTypeToBe(dataVarType, UnionType);
4484+
expect((dataVarType as UnionType).types).to.include(StringType.instance);
4485+
expect((dataVarType as UnionType).types).to.include(IntegerType.instance);
4486+
dataVar = printStmts[3].expressions[0];
4487+
dataVarType = dataVar.getType({ flags: SymbolTypeFlag.runtime });
4488+
expectTypeToBe(dataVarType, UnionType);
4489+
expect((dataVarType as UnionType).types).to.include(StringType.instance);
4490+
expect((dataVarType as UnionType).types).to.include(IntegerType.instance);
4491+
});
4492+
4493+
it('should understand changing the type of a param in try/catch', () => {
4494+
const testFile = program.setFile<BrsFile>('source/test.bs', `
4495+
sub testPocket1(msg as string)
4496+
try
4497+
if msg = "" then
4498+
msg = "hello!"
4499+
end if
4500+
msg = 123
4501+
catch e
4502+
end try
4503+
print msg
4504+
print msg.toStr() ' confirming msg is string|int, and not uninitialized
4505+
end sub
4506+
`);
4507+
program.validate();
4508+
expectZeroDiagnostics(program);
4509+
const printStmts = testFile.ast.findChildren<PrintStatement>(isPrintStatement);
4510+
let msgVar = printStmts[0].expressions[0];
4511+
let msgVarType = msgVar.getType({ flags: SymbolTypeFlag.runtime });
4512+
expectTypeToBe(msgVarType, UnionType);
4513+
expect((msgVarType as UnionType).types).to.include(StringType.instance);
4514+
expect((msgVarType as UnionType).types).to.include(IntegerType.instance);
4515+
});
4516+
4517+
it('should understand changing the type of a param in try/catch in non-first function', () => {
4518+
const testFile = program.setFile<BrsFile>('source/test.bs', `
4519+
function foo() as string
4520+
msg = "test"
4521+
return msg
4522+
end function
4523+
4524+
sub testPocket1(msg as string)
4525+
try
4526+
if msg = "" then
4527+
msg = "hello!"
4528+
end if
4529+
msg = 123
4530+
catch e
4531+
end try
4532+
print msg
4533+
print msg.toStr() ' confirming msg is string|int, and not uninitialized
4534+
end sub
4535+
`);
4536+
program.validate();
4537+
expectZeroDiagnostics(program);
4538+
const printStmts = testFile.ast.findChildren<PrintStatement>(isPrintStatement);
4539+
let msgVar = printStmts[0].expressions[0];
4540+
let msgVarType = msgVar.getType({ flags: SymbolTypeFlag.runtime });
4541+
expectTypeToBe(msgVarType, UnionType);
4542+
expect((msgVarType as UnionType).types).to.include(StringType.instance);
4543+
expect((msgVarType as UnionType).types).to.include(IntegerType.instance);
4544+
});
4545+
4546+
4547+
it('should allow redefinition of function param and immediate use', () => {
4548+
const testFile = program.setFile<BrsFile>('source/test.bs', `
4549+
sub testPocket1(data as string)
4550+
data = 123
4551+
data += 1
4552+
print data
4553+
end sub
4554+
`);
4555+
program.validate();
4556+
expectZeroDiagnostics(program);
4557+
const printStmts = testFile.ast.findChildren<PrintStatement>(isPrintStatement);
4558+
let dataType = printStmts[0].expressions[0];
4559+
let dataTypeType = dataType.getType({ flags: SymbolTypeFlag.runtime });
4560+
expectTypeToBe(dataTypeType, IntegerType);
4561+
});
4562+
4563+
it('handles this long function from Rooibos', () => {
4564+
program.setFile<BrsFile>('source/test.bs', `
4565+
function assertAAHasKeys(aa as dynamic, keys as dynamic, msg = "" as string) as boolean
4566+
if m.currentResult.isFail then
4567+
return false
4568+
end if
4569+
4570+
try
4571+
if not isAA(aa) then
4572+
if msg = "" then
4573+
msg = "expected to be an AssociativeArray"
4574+
end if
4575+
fail(msg, "", "", true)
4576+
return false
4577+
end if
4578+
4579+
if not isArray(keys) then
4580+
if msg = "" then
4581+
msg = "expected to be an Array"
4582+
end if
4583+
fail(msg, "", "", true)
4584+
return false
4585+
end if
4586+
4587+
foundKeys = []
4588+
missingKeys = []
4589+
for each key in keys
4590+
if not aa.ifAssociativeArray.DoesExist(key) then
4591+
missingKeys.push(key)
4592+
else
4593+
foundKeys.push(key)
4594+
end if
4595+
end for
4596+
4597+
if missingKeys.count() > 0 then
4598+
actual = "blah"
4599+
expected = "blah"
4600+
if msg = "" then
4601+
msg = "expected to have properties"
4602+
end if
4603+
fail(msg, actual, expected, true)
4604+
return false
4605+
end if
4606+
return true
4607+
catch error
4608+
failCrash(error, msg)
4609+
end try
4610+
return false
4611+
end function
4612+
4613+
4614+
function isAA(value as dynamic) as boolean
4615+
return type(value) = "roAssociativeArray"
4616+
end function
4617+
4618+
4619+
function isArray(value as dynamic) as boolean
4620+
return type(value) = "roArray"
4621+
end function
4622+
4623+
4624+
sub fail(msg as string, actual as dynamic, expected as dynamic, isSoftFail as boolean)
4625+
print "fail"
4626+
end sub
4627+
4628+
sub failCrash(error as dynamic, msg as string)
4629+
print "fail crash"
4630+
end sub
4631+
`);
4632+
program.validate();
4633+
expectZeroDiagnostics(program);
4634+
});
44034635
});
44044636

44054637
describe('unlinkSymbolTable', () => {

‎src/SymbolTable.ts‎

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ export class SymbolTable implements SymbolTypeGetter {
101101
public pocketTables = new Array<PocketTable>();
102102

103103
public addPocketTable(pocketTable: PocketTable) {
104+
pocketTable.table.isPocketTable = true;
104105
this.pocketTables.push(pocketTable);
105106
return () => {
106107
const index = this.pocketTables.findIndex(pt => pt === pocketTable);
@@ -110,6 +111,18 @@ export class SymbolTable implements SymbolTypeGetter {
110111
};
111112
}
112113

114+
private isPocketTable = false;
115+
116+
private getCurrentPocketTableDepth() {
117+
let depth = 0;
118+
let currentTable: SymbolTable = this;
119+
while (currentTable.isPocketTable) {
120+
depth++;
121+
currentTable = currentTable.parent;
122+
}
123+
return depth;
124+
}
125+
113126
public getStatementIndexOfPocketTable(symbolTable: SymbolTable) {
114127
return this.pocketTables.find(pt => pt.table === symbolTable)?.index ?? -1;
115128
}
@@ -202,27 +215,41 @@ export class SymbolTable implements SymbolTypeGetter {
202215
}
203216

204217
// look in our map first
205-
result = currentTable.symbolMap.get(key);
206-
if (result) {
218+
let currentResults = currentTable.symbolMap.get(key);
219+
if (currentResults) {
207220
// eslint-disable-next-line no-bitwise
208-
result = result.filter(symbol => symbol.flags & bitFlags).filter(this.getSymbolLookupFilter(currentTable, maxStatementIndex, memberOfAncestor));
221+
currentResults = currentResults.filter(symbol => symbol.flags & bitFlags)
222+
.filter(this.getSymbolLookupFilter(currentTable, maxStatementIndex, memberOfAncestor));
209223
}
210224

211225
let precedingAssignmentIndex = -1;
212-
if (result?.length > 0 && currentTable.isOrdered && maxStatementIndex >= 0) {
213-
this.sortSymbolsByAssignmentOrderInPlace(result);
214-
const lastResult = result[result.length - 1];
215-
result = [lastResult];
226+
if (currentResults?.length > 0 && currentTable.isOrdered && maxStatementIndex >= 0) {
227+
this.sortSymbolsByAssignmentOrderInPlace(currentResults);
228+
const lastResult = currentResults[currentResults.length - 1];
229+
currentResults = [lastResult];
216230
precedingAssignmentIndex = lastResult.data?.definingNode?.statementIndex ?? -1;
217231
}
218232

219-
result = currentTable.augmentSymbolResultsWithPocketTableResults(name, bitFlags, result, {
233+
if (result?.length > 0) {
234+
// we already have results from a deeper pocketTable
235+
if (currentResults?.length > 0) {
236+
result.push(...currentResults);
237+
}
238+
} else if (currentResults?.length > 0) {
239+
result = currentResults;
240+
}
241+
242+
let depth = additionalOptions?.depth ?? currentTable.getCurrentPocketTableDepth();
243+
const augmentationResult = currentTable.augmentSymbolResultsWithPocketTableResults(name, bitFlags, result, {
220244
...additionalOptions,
245+
depth: depth,
221246
maxStatementIndex: maxStatementIndex,
222247
precedingAssignmentIndex: precedingAssignmentIndex
223248
});
249+
result = augmentationResult.symbols;
250+
const needCheckParent = (!augmentationResult.exhaustive && depth > 0);
224251

225-
if (result?.length > 0) {
252+
if (result?.length > 0 && !needCheckParent) {
226253
result = result.map(addAncestorInfo);
227254
break;
228255
}
@@ -244,7 +271,7 @@ export class SymbolTable implements SymbolTypeGetter {
244271
return result;
245272
}
246273

247-
private augmentSymbolResultsWithPocketTableResults(name: string, bitFlags: SymbolTypeFlag, result: BscSymbol[], additionalOptions: { precedingAssignmentIndex?: number } & GetSymbolAdditionalOptions = {}): BscSymbol[] {
274+
private augmentSymbolResultsWithPocketTableResults(name: string, bitFlags: SymbolTypeFlag, result: BscSymbol[], additionalOptions: { precedingAssignmentIndex?: number } & GetSymbolAdditionalOptions = {}): { symbols: BscSymbol[]; exhaustive: boolean } {
248275
let pocketTableResults: BscSymbol[] = [];
249276
let pocketTablesWeFoundSomethingIn = this.getSymbolDataFromPocketTables(name, bitFlags, additionalOptions);
250277
let pocketTablesAreExhaustive = false;
@@ -307,7 +334,9 @@ export class SymbolTable implements SymbolTypeGetter {
307334
result.push(...pocketTableResults);
308335
}
309336
}
310-
return result;
337+
// Do the results cover all possible execution paths?
338+
const areResultsExhaustive = pocketTablesAreExhaustive || pocketTablesWeFoundSomethingIn.length === 0;
339+
return { symbols: result, exhaustive: areResultsExhaustive };
311340
}
312341

313342
private getSymbolDataFromPocketTables(name: string, bitFlags: SymbolTypeFlag, additionalOptions: { precedingAssignmentIndex?: number } & GetSymbolAdditionalOptions = {}): Array<{ pocketTable: PocketTable; results: BscSymbol[] }> {
@@ -650,6 +679,7 @@ export class SymbolTable implements SymbolTypeGetter {
650679
// order doesn't matter for current table
651680
return true;
652681
}
682+
653683
if (maxAllowedStatementIndex >= 0 && t.data?.definingNode) {
654684
if (memberOfAncestor || t.data.canUseInDefinedAstNode) {
655685
// if we've already gone up a level, it's possible to have a variable assigned and used

‎src/bscPlugin/validation/BrsFileValidator.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,16 @@ export class BrsFileValidator {
205205
// add param symbol at expression level, so it can be used as default value in other params
206206
const funcExpr = node.findAncestor<FunctionExpression>(isFunctionExpression);
207207
const funcSymbolTable = funcExpr?.getSymbolTable();
208-
funcSymbolTable?.addSymbol(paramName, { definingNode: node, isInstance: true, isFromDocComment: data.isFromDocComment, description: data.description }, nodeType, SymbolTypeFlag.runtime);
208+
const extraSymbolData: ExtraSymbolData = {
209+
definingNode: node,
210+
isInstance: true,
211+
isFromDocComment: data.isFromDocComment,
212+
description: data.description
213+
};
214+
funcSymbolTable?.addSymbol(paramName, extraSymbolData, nodeType, SymbolTypeFlag.runtime);
209215

210216
//also add param symbol at block level, as it may be redefined, and if so, should show a union
211-
funcExpr.body.getSymbolTable()?.addSymbol(paramName, { definingNode: node, isInstance: true, isFromDocComment: data.isFromDocComment }, nodeType, SymbolTypeFlag.runtime);
217+
funcExpr.body.getSymbolTable()?.addSymbol(paramName, extraSymbolData, nodeType, SymbolTypeFlag.runtime);
212218
},
213219
InterfaceStatement: (node) => {
214220
if (!node.tokens.name) {
@@ -342,7 +348,8 @@ export class BrsFileValidator {
342348
// this block is in a function. order matters!
343349
blockSymbolTable.isOrdered = true;
344350
}
345-
if (!isFunctionExpression(node.parent)) {
351+
if (!isFunctionExpression(node.parent) && node.parent) {
352+
node.symbolTable.name = `Block-${node.parent.kind}@${node.location?.range?.start?.line}`;
346353
// we're a block inside another block (or body). This block is a pocket in the bigger block
347354
node.parent.getSymbolTable().addPocketTable({
348355
index: node.parent.statementIndex,

0 commit comments

Comments
 (0)