Skip to content

Commit

Permalink
the exceptionVariable is now an expression instead of a token, and is…
Browse files Browse the repository at this point in the history
… optional for bs
  • Loading branch information
TwitchBronBron committed Sep 26, 2024
1 parent c610b9e commit 0230758
Show file tree
Hide file tree
Showing 12 changed files with 166 additions and 29 deletions.
25 changes: 25 additions & 0 deletions docs/exceptions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Exceptions
## Omitting the exception variable
There are times where you don't need to use the exception variable in a try/catch. BrighterScript allows you to omit the variable. At transpile time, we'll inject a variable so that your output is still valid brightscript

```BrighterScript
function init()
try
somethingDangerous()
catch
print "I live on the edge"
end try
end function
```

transpiles to

```BrighterScript
function init()
try
somethingDangerous()
catch e
print "I live on the edge"
end try
end function
```
4 changes: 2 additions & 2 deletions src/DiagnosticMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -612,8 +612,8 @@ export let DiagnosticMessages = {
code: 1116,
severity: DiagnosticSeverity.Error
}),
missingExceptionVarToFollowCatch: () => ({
message: `Missing exception variable after 'catch' keyword`,
expectedExceptionVarToFollowCatch: () => ({
message: `Expected exception variable after 'catch' keyword`,
code: 1117,
severity: DiagnosticSeverity.Error
}),
Expand Down
2 changes: 1 addition & 1 deletion src/astUtils/reflection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ describe('reflection', () => {
const namespace = new NamespaceStatement({ namespace: token, nameExpression: createVariableExpression('a'), body: body, endNamespace: token });
const cls = new ClassStatement({ class: token, name: ident, body: [], endClass: token });
const imports = new ImportStatement({ import: token, path: token });
const catchStmt = new CatchStatement({ catch: token, exceptionVariable: ident, catchBranch: block });
const catchStmt = new CatchStatement({ catch: token, exceptionVariableExpression: createVariableExpression('e'), catchBranch: block });
const tryCatch = new TryCatchStatement({ try: token, tryBranch: block, catchStatement: catchStmt });
const throwSt = new ThrowStatement({ throw: createToken(TokenKind.Throw) });

Expand Down
40 changes: 40 additions & 0 deletions src/bscPlugin/validation/BrsFileValidator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1211,7 +1211,47 @@ describe('BrsFileValidator', () => {
const varType = returnStmt.getSymbolTable().getSymbolType('someDate', { flags: SymbolTypeFlag.runtime, data: data });
expectTypeToBe(varType, StringType);
});
});
});

describe('try/catch', () => {
it('allows omitting the exception variable in standard brightscript mode', () => {
program.setFile('source/main.brs', `
sub new()
try
print "hello"
catch
print "error"
end try
end sub
`);
expectZeroDiagnostics(program);
});

it('shows diagnostic when omitting the exception variable in standard brightscript mode', () => {
program.setFile('source/main.brs', `
sub new()
try
print "hello"
catch
print "error"
end try
end sub
`);
expectDiagnostics(program, []);
});

it('shows diagnostics when using when omitting the exception variable in standard brightscript mode', () => {
program.setFile('source/main.brs', `
sub new()
try
print "hello"
catch
print "error"
end try
end sub
`);
expectDiagnostics(program, []);
});
});
});
34 changes: 31 additions & 3 deletions src/bscPlugin/validation/BrsFileValidator.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isAliasStatement, isArrayType, isBlock, isBody, isClassStatement, isConditionalCompileConstStatement, isConditionalCompileErrorStatement, isConditionalCompileStatement, isConstStatement, isDottedGetExpression, isDottedSetStatement, isEnumStatement, isForEachStatement, isForStatement, isFunctionExpression, isFunctionStatement, isImportStatement, isIndexedGetExpression, isIndexedSetStatement, isInterfaceStatement, isLibraryStatement, isLiteralExpression, isMethodStatement, isNamespaceStatement, isTypecastStatement, isUnaryExpression, isVariableExpression, isWhileStatement } from '../../astUtils/reflection';
import { isAliasStatement, isArrayType, isBlock, isBody, isClassStatement, isConditionalCompileConstStatement, isConditionalCompileErrorStatement, isConditionalCompileStatement, isConstStatement, isDottedGetExpression, isDottedSetStatement, isEnumStatement, isForEachStatement, isForStatement, isFunctionExpression, isFunctionStatement, isImportStatement, isIndexedGetExpression, isIndexedSetStatement, isInterfaceStatement, isLibraryStatement, isLiteralExpression, isMethodStatement, isNamespaceStatement, isTypecastExpression, isTypecastStatement, isUnaryExpression, isVariableExpression, isWhileStatement } from '../../astUtils/reflection';
import { createVisitor, WalkMode } from '../../astUtils/visitors';
import { DiagnosticMessages } from '../../DiagnosticMessages';
import type { BrsFile } from '../../files/BrsFile';
Expand Down Expand Up @@ -54,7 +54,7 @@ export class BrsFileValidator {
* Walk the full AST
*/
private walk() {

const isBrighterscript = this.event.file.parser.options.mode === ParseMode.BrighterScript;

const visitor = createVisitor({
MethodStatement: (node) => {
Expand Down Expand Up @@ -199,7 +199,35 @@ export class BrsFileValidator {
node.parent.getSymbolTable().addSymbol(node.tokens.name.text, { definingNode: node, isInstance: true }, nodeType, SymbolTypeFlag.runtime);
},
CatchStatement: (node) => {
node.parent.getSymbolTable().addSymbol(node.tokens.exceptionVariable.text, { definingNode: node, isInstance: true }, DynamicType.instance, SymbolTypeFlag.runtime);
//brs and bs both support variableExpression for the exception variable
if (isVariableExpression(node.exceptionVariableExpression)) {
node.parent.getSymbolTable().addSymbol(
node.exceptionVariableExpression.getName(),
{ definingNode: node, isInstance: true },
//TODO I think we can produce a slightly more specific type here (like an AA but with the known exception properties)
DynamicType.instance,
SymbolTypeFlag.runtime
);
//brighterscript allows catch without an exception variable
} else if (isBrighterscript && !node.exceptionVariableExpression) {
//this is fine

//brighterscript allows a typecast expression here
} else if (isBrighterscript && isTypecastExpression(node.exceptionVariableExpression) && isVariableExpression(node.exceptionVariableExpression.obj)) {
node.parent.getSymbolTable().addSymbol(
node.exceptionVariableExpression.obj.getName(),
{ definingNode: node, isInstance: true },
node.exceptionVariableExpression.getType({ flags: SymbolTypeFlag.runtime }),
SymbolTypeFlag.runtime
);

//no other expressions are allowed here
} else {
this.event.program.diagnostics.register({
...DiagnosticMessages.expectedExceptionVarToFollowCatch(),
location: node.exceptionVariableExpression?.location ?? node.tokens.catch?.location
});
}
},
DimStatement: (node) => {
if (node.tokens.name) {
Expand Down
35 changes: 34 additions & 1 deletion src/files/BrsFile.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2175,11 +2175,44 @@ describe('BrsFile', () => {
try
print m.b.c
catch e
print e
print "crash"
end try
end sub
`);
});

it('recognizes the exception variable', async () => {
await testTranspile(`
sub main()
try
print m.b.c
catch e
message = e.message
print message
end try
end sub
`);
});

it('supports omitting the exception variable in brighterscript mode (we auto-add it at transpile time)', async () => {
await testTranspile(`
sub new()
try
print "hello"
catch
print "error"
end try
end sub
`, `
sub new()
try
print "hello"
catch e
print "error"
end try
end sub
`, undefined, 'source/main.bs');
});
});

describe('namespaces', () => {
Expand Down
18 changes: 11 additions & 7 deletions src/parser/Parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1751,17 +1751,21 @@ export class Parser {
});
} else {
const catchToken = this.advance();
const exceptionVarToken = this.tryConsume(DiagnosticMessages.missingExceptionVarToFollowCatch(), TokenKind.Identifier, ...this.allowedLocalIdentifiers) as Identifier;
if (exceptionVarToken) {
// force it into an identifier so the AST makes some sense
exceptionVarToken.kind = TokenKind.Identifier;

//get the exception variable as an expression
let exceptionVariableExpression: Expression;
//if we consumed any statement separators, that means we don't have an exception variable
if (this.consumeStatementSeparators(true)) {
//no exception variable. That's fine in BrighterScript but not in brightscript. But that'll get caught by the validator later...
} else {
exceptionVariableExpression = this.expression(true);
this.consumeStatementSeparators();
}
//ensure statement sepatator
this.consumeStatementSeparators();

const catchBranch = this.block(TokenKind.EndTry);
catchStmt = new CatchStatement({
catch: catchToken,
exceptionVariable: exceptionVarToken,
exceptionVariableExpression: exceptionVariableExpression,
catchBranch: catchBranch
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/parser/Statement.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ describe('Statement', () => {
const namespace = new NamespaceStatement({ namespace: token, nameExpression: createVariableExpression('a'), body: body, endNamespace: token });
const cls = new ClassStatement({ class: token, name: ident, body: [], endClass: token });
const imports = new ImportStatement({ import: token, path: token });
const catchStmt = new CatchStatement({ catch: token, exceptionVariable: ident, catchBranch: block });
const catchStmt = new CatchStatement({ catch: token, exceptionVariableExpression: createVariableExpression('e'), catchBranch: block });
const tryCatch = new TryCatchStatement({ try: token, tryBranch: block, catchStatement: catchStmt });
const throwSt = new ThrowStatement({ throw: createToken(TokenKind.Throw) });

Expand Down
15 changes: 8 additions & 7 deletions src/parser/Statement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ export class Block extends Statement {
} else if (isCatchStatement(this.parent) && isTryCatchStatement(this.parent?.parent)) {
lastBitBefore = util.createBoundingLocation(
this.parent.tokens.catch,
this.parent.tokens.exceptionVariable
this.parent.exceptionVariableExpression
);
firstBitAfter = this.parent.parent.tokens.endTry?.location;
}
Expand Down Expand Up @@ -3109,27 +3109,28 @@ export class TryCatchStatement extends Statement {
export class CatchStatement extends Statement {
constructor(options?: {
catch?: Token;
exceptionVariable?: Identifier;
exceptionVariableExpression?: Expression;
catchBranch?: Block;
}) {
super();
this.tokens = {
catch: options?.catch,
exceptionVariable: options?.exceptionVariable
catch: options?.catch
};
this.exceptionVariableExpression = options?.exceptionVariableExpression;
this.catchBranch = options?.catchBranch;
this.location = util.createBoundingLocation(
this.tokens.catch,
this.tokens.exceptionVariable,
this.exceptionVariableExpression,
this.catchBranch
);
}

public readonly tokens: {
readonly catch?: Token;
readonly exceptionVariable?: Identifier;
};

public readonly exceptionVariableExpression?: Expression;

public readonly catchBranch?: Block;

public readonly kind = AstNodeKind.CatchStatement;
Expand All @@ -3140,7 +3141,7 @@ export class CatchStatement extends Statement {
return [
state.transpileToken(this.tokens.catch, 'catch'),
' ',
this.tokens.exceptionVariable?.text ?? 'e',
this.exceptionVariableExpression?.transpile(state) ?? ['e'],
...(this.catchBranch?.transpile(state) ?? [])
];
}
Expand Down
8 changes: 6 additions & 2 deletions src/parser/tests/statement/Throw.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@ import type { TryCatchStatement, ThrowStatement } from '../../Statement';
import { DiagnosticMessages } from '../../../DiagnosticMessages';
import { LiteralExpression } from '../../Expression';
import { Parser } from '../../Parser';
import { expectDiagnostics, expectZeroDiagnostics } from '../../../testHelpers.spec';

describe('parser ThrowStatement', () => {
it('parses properly', () => {
const parser = Parser.parse(`
try
throw "some message"
catch
catch e
end try
`);
expectZeroDiagnostics(parser);
const throwStatement = (parser.ast.statements[0] as TryCatchStatement).tryBranch!.statements[0] as ThrowStatement;
//the statement should still exist and have null expression
expect(throwStatement).to.exist;
Expand All @@ -25,7 +27,9 @@ describe('parser ThrowStatement', () => {
catch
end try
`);
expect(parser.diagnostics[0]?.message).to.eql(DiagnosticMessages.missingExceptionExpressionAfterThrowKeyword().message);
expectDiagnostics(parser, [
DiagnosticMessages.missingExceptionExpressionAfterThrowKeyword().message
]);
const throwStatement = (parser.ast.statements[0] as TryCatchStatement).tryBranch!.statements[0] as ThrowStatement;
//the statement should still exist and have null expression
expect(throwStatement).to.exist;
Expand Down
4 changes: 2 additions & 2 deletions src/parser/tests/statement/TryCatch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { expect } from '../../../chai-config.spec';
import { Parser } from '../../Parser';
import { TryCatchStatement } from '../../Statement';
import { isFunctionExpression } from '../../../astUtils/reflection';
import type { FunctionExpression } from '../../Expression';
import type { FunctionExpression, VariableExpression } from '../../Expression';
import { expectDiagnosticsIncludes } from '../../../testHelpers.spec';
import { DiagnosticMessages } from '../../../DiagnosticMessages';

Expand All @@ -27,7 +27,7 @@ describe('parser try/catch', () => {
expect(stmt.catchStatement).to.exist;
const cstmt = stmt.catchStatement;
expect(cstmt!.tokens.catch?.text).to.eql('catch');
expect(cstmt!.tokens.exceptionVariable!.text).to.eql('e');
expect((cstmt!.exceptionVariableExpression as VariableExpression).getName()).to.eql('e');
expect(cstmt!.catchBranch).to.exist.and.ownProperty('statements').to.be.lengthOf(1);
expect(stmt.tokens.endTry?.text).to.eql('end try');
});
Expand Down
8 changes: 5 additions & 3 deletions src/testHelpers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,10 @@ function cloneDiagnostic(actualDiagnosticInput: BsDiagnostic, expectedDiagnostic
['message', 'code', 'severity', 'relatedInformation']
);
//clone Location if available
if (expectedDiagnostic.location) {
actualDiagnostic.location = cloneObject(actualDiagnosticInput.location, expectedDiagnostic.location, ['uri', 'range']) as any;
if (expectedDiagnostic?.location) {
actualDiagnostic.location = actualDiagnosticInput.location
? cloneObject(actualDiagnosticInput.location, expectedDiagnostic.location, ['uri', 'range']) as any
: undefined;
}
//deep clone relatedInformation if available
if (actualDiagnostic.relatedInformation) {
Expand Down Expand Up @@ -173,7 +175,7 @@ export function expectZeroDiagnostics(arg: DiagnosticCollection) {
for (const diagnostic of diagnostics) {
//escape any newlines
diagnostic.message = diagnostic.message.replace(/\r/g, '\\r').replace(/\n/g, '\\n');
message += `\n • bs${diagnostic.code} "${diagnostic.message}" at ${diagnostic.location?.uri ?? ''}#(${diagnostic.location.range?.start.line}:${diagnostic.location.range?.start.character})-(${diagnostic.location.range?.end.line}:${diagnostic.location.range?.end.character})`;
message += `\n • bs${diagnostic.code} "${diagnostic.message}" at ${diagnostic.location?.uri ?? ''}#(${diagnostic.location?.range?.start.line}:${diagnostic.location?.range?.start.character})-(${diagnostic.location?.range?.end.line}:${diagnostic.location?.range?.end.character})`;
//print the line containing the error (if we can find it)srcPath
if (arg instanceof Program) {
const file = arg.getFile(diagnostic.location.uri);
Expand Down

0 comments on commit 0230758

Please sign in to comment.