Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flag functions with keyword names. #247

Merged
merged 1 commit into from
Nov 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 68 additions & 4 deletions src/lexer/TokenKind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ export enum TokenKind {
True = 'True',
Type = 'Type',
While = 'While',
Try = 'Try',
Catch = 'Catch',
EndTry = 'EndTry',
Throw = 'Throw',

//misc
Library = 'Library',
Expand Down Expand Up @@ -211,6 +215,7 @@ export const ReservedWords = new Set([
'sub',
'tab',
'then',
'throw',
'to',
'true',
'type',
Expand Down Expand Up @@ -297,7 +302,10 @@ export const Keywords: Record<string, TokenKind> = {
'source_function_name': TokenKind.SourceFunctionNameLiteral,
'source_location': TokenKind.SourceLocationLiteral,
'pkg_path': TokenKind.PkgPathLiteral,
'pkg_location': TokenKind.PkgLocationLiteral
'pkg_location': TokenKind.PkgLocationLiteral,
try: TokenKind.Try,
catch: TokenKind.Catch,
endtry: TokenKind.EndTry
};
//hide the constructor prototype method because it causes issues
Keywords.constructor = undefined;
Expand Down Expand Up @@ -412,7 +420,11 @@ export const AllowedProperties = [
TokenKind.SourceFunctionNameLiteral,
TokenKind.SourceLocationLiteral,
TokenKind.PkgPathLiteral,
TokenKind.PkgLocationLiteral
TokenKind.PkgLocationLiteral,
TokenKind.Try,
TokenKind.Catch,
TokenKind.EndTry,
TokenKind.Throw
];

/** List of TokenKind that are allowed as local var identifiers. */
Expand Down Expand Up @@ -441,7 +453,10 @@ export const AllowedLocalIdentifiers = [
TokenKind.Override,
TokenKind.Namespace,
TokenKind.EndNamespace,
TokenKind.Import
TokenKind.Import,
TokenKind.Try,
TokenKind.Catch,
TokenKind.EndTry
];

export const BrighterScriptSourceLiterals = [
Expand Down Expand Up @@ -508,10 +523,59 @@ export const DisallowedLocalIdentifiers = [
TokenKind.SourceFunctionNameLiteral,
TokenKind.SourceLocationLiteral,
TokenKind.PkgPathLiteral,
TokenKind.PkgLocationLiteral
TokenKind.PkgLocationLiteral,
TokenKind.Throw
];

export const DisallowedLocalIdentifiersText = new Set([
'run',
...DisallowedLocalIdentifiers.map(x => x.toLowerCase())
]);

/**
* List of string versions of TokenKind and various globals that are NOT allowed as scope function names.
* Used to throw more helpful "you can't use a reserved word as a function name" errors.
*/
export const DisallowedFunctionIdentifiers = [
TokenKind.And,
TokenKind.CreateObject,
TokenKind.Dim,
TokenKind.Each,
TokenKind.Else,
TokenKind.ElseIf,
TokenKind.End,
TokenKind.EndFunction,
TokenKind.EndIf,
TokenKind.EndSub,
TokenKind.EndWhile,
TokenKind.Exit,
TokenKind.ExitWhile,
TokenKind.False,
TokenKind.For,
TokenKind.Function,
TokenKind.Goto,
TokenKind.If,
TokenKind.Invalid,
TokenKind.Let,
TokenKind.Next,
TokenKind.Not,
TokenKind.ObjFun,
TokenKind.Or,
TokenKind.Print,
TokenKind.Rem,
TokenKind.Return,
TokenKind.Step,
TokenKind.Sub,
TokenKind.Tab,
TokenKind.Then,
TokenKind.To,
TokenKind.True,
TokenKind.Type,
TokenKind.While,
TokenKind.Throw
];

export const DisallowedFunctionIdentifiersText = new Set([
'run',
...DisallowedFunctionIdentifiers.map(x => x.toLowerCase())
]);
70 changes: 69 additions & 1 deletion src/parser/Parser.Class.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('parser class', () => {
end class
`);
let { statements, diagnostics } = Parser.parse(tokens, { mode: ParseMode.BrighterScript });
expect(diagnostics).to.be.lengthOf(0);
expect(diagnostics[0]?.message).not.to.exist;
expect(statements[0]).instanceof(ClassStatement);
});
}
Expand All @@ -70,6 +70,74 @@ describe('parser class', () => {
});
}

it('does not allow "throw" to be defined as a local var', () => {
const parser = Parser.parse(`
sub main()
'not allowed to define throw as local var
throw = true
end sub
`);

expect(parser.diagnostics[0]?.message).to.eql(DiagnosticMessages.cannotUseReservedWordAsIdentifier('throw').message);
});

it('does not allow function named "throw"', () => {
const parser = Parser.parse(`
'not allowed to define a function called "throw"
sub throw()
end sub
`);

expect(parser.diagnostics[0]?.message).to.eql(DiagnosticMessages.cannotUseReservedWordAsIdentifier('throw').message);
});

it('supports the try/catch keywords in various places', () => {
const parser = Parser.parse(`
sub main()
'allowed to be local vars
try = true
catch = true
endTry = true
'not allowed to use throw as local variable
'throw = true

'allowed to be object props
person = {
try: true,
catch: true,
endTry: true,
throw: true
}

person.try = true
person.catch = true
person.endTry = true
person.throw = true

'allowed as object property reference
print person.try
print person.catch
print person.endTry
print person.throw
end sub

sub try()
end sub

sub catch()
end sub

sub endTry()
end sub

'not allowed to define a function called "throw"
' sub throw()
' end sub
`);

expect(parser.diagnostics[0]?.message).not.to.exist;
});

it('parses empty class', () => {
let { tokens } = Lexer.scan(`
class Person
Expand Down
17 changes: 13 additions & 4 deletions src/parser/Parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
AllowedLocalIdentifiers,
AssignmentOperators,
DisallowedLocalIdentifiersText,
DisallowedFunctionIdentifiersText,
AllowedProperties,
Lexer,
BrighterScriptSourceLiterals,
Expand Down Expand Up @@ -404,7 +405,7 @@ export class Parser {

//methods (function/sub keyword OR identifier followed by opening paren)
if (this.checkAny(TokenKind.Function, TokenKind.Sub) || (this.checkAny(TokenKind.Identifier, ...AllowedProperties) && this.checkNext(TokenKind.LeftParen))) {
let funcDeclaration = this.functionDeclaration(false);
let funcDeclaration = this.functionDeclaration(false, false);

//remove this function from the lists because it's not a callable
const functionStatement = this._references.functionStatements.pop();
Expand Down Expand Up @@ -542,9 +543,9 @@ export class Parser {
*/
private callExpressions = [];

private functionDeclaration(isAnonymous: true): FunctionExpression;
private functionDeclaration(isAnonymous: false): FunctionStatement;
private functionDeclaration(isAnonymous: boolean) {
private functionDeclaration(isAnonymous: true, checkIdentifier?: boolean): FunctionExpression;
private functionDeclaration(isAnonymous: false, checkIdentifier?: boolean): FunctionStatement;
private functionDeclaration(isAnonymous: boolean, checkIdentifier = true) {
let previousCallExpressions = this.callExpressions;
this.callExpressions = [];
try {
Expand Down Expand Up @@ -600,6 +601,14 @@ export class Parser {
range: name.range
});
}

//flag functions with keywords for names (only for standard functions)
if (checkIdentifier && DisallowedFunctionIdentifiersText.has(name.text.toLowerCase())) {
this.diagnostics.push({
...DiagnosticMessages.cannotUseReservedWordAsIdentifier(name.text),
range: name.range
});
}
}

let params = [] as FunctionParameterExpression[];
Expand Down