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.

Sign up for GitHub

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 incorrectly nested statements #747

Merged
merged 1 commit into from
Nov 29, 2022
Merged

Flag incorrectly nested statements #747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
234 changes: 234 additions & 0 deletions src/bscPlugin/validation/BrsFileValidator.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { DiagnosticMessages } from '../../DiagnosticMessages';
import { expectDiagnostics, expectZeroDiagnostics } from '../../testHelpers.spec';
import { Program } from '../../Program';
import { isClassStatement, isNamespaceStatement } from '../../astUtils/reflection';
import util from '../../util';

describe('BrsFileValidator', () => {
let program: Program;
Expand Down Expand Up @@ -85,4 +86,237 @@ describe('BrsFileValidator', () => {
]);
});
});

it('allows classes in correct locations', () => {
program.setFile('source/main.bs', `
class Alpha
end class
namespace Beta
class Charlie
end class
namespace Delta
class Echo
end class
end namespace
end namespace
`);
program.validate();
expectZeroDiagnostics(program);
});

it('flags classes in wrong locations', () => {
program.setFile('source/main.bs', `
function test()
class Alpha
end class
if true then
class Beta
end class
end if
end function
`);
program.validate();
expectDiagnostics(program, [{
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('class'),
range: util.createRange(2, 16, 2, 27)
}, {
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('class'),
range: util.createRange(5, 20, 5, 30)
}]);
});

it('allows enums in correct locations', () => {
program.setFile('source/main.bs', `
enum Alpha
value1
end enum
namespace Beta
enum Charlie
value1
end enum
namespace Delta
enum Echo
value1
end enum
end namespace
end namespace
`);
program.validate();
expectZeroDiagnostics(program);
});

it('flags enums in wrong locations', () => {
program.setFile('source/main.bs', `
function test()
enum Alpha
value1
end enum
if true then
enum Beta
value1
end enum
end if
end function
`);
program.validate();
expectDiagnostics(program, [{
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('enum'),
range: util.createRange(2, 16, 2, 26)
}, {
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('enum'),
range: util.createRange(6, 20, 6, 29)
}]);
});

it('allows functions in correct locations', () => {
program.setFile('source/main.bs', `
function Alpha()
end function
namespace Beta
function Charlie()
end function
namespace Delta
function Echo()
end function
end namespace
end namespace
`);
program.validate();
expectZeroDiagnostics(program);
});

it('flags functions in wrong locations', () => {
program.setFile('source/main.bs', `
function test()
function Alpha()
end function
if true then
function Beta()
end function
end if
end function
`);
program.validate();
expectDiagnostics(program, [{
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('function'),
range: util.createRange(2, 16, 2, 30)
}, {
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('function'),
range: util.createRange(5, 20, 5, 33)
}]);
});

it('allows namespaces in correct locations', () => {
program.setFile('source/main.bs', `
namespace Alpha
end namespace
namespace Beta
namespace Charlie
end namespace
namespace Delta
namespace Echo
end namespace
end namespace
end namespace
`);
program.validate();
expectZeroDiagnostics(program);
});

it('flags classes in wrong locations', () => {
program.setFile('source/main.bs', `
function test()
namespace Alpha
end namespace
if true then
namespace Beta
end namespace
end if
end function
`);
program.validate();
expectDiagnostics(program, [{
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('namespace'),
range: util.createRange(2, 16, 2, 31)
}, {
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('namespace'),
range: util.createRange(5, 20, 5, 34)
}]);
});

it('allows interfaces in correct locations', () => {
program.setFile('source/main.bs', `
interface Alpha
prop as string
end interface
namespace Beta
interface Charlie
prop as string
end interface
namespace Delta
interface Echo
prop as string
end interface
end namespace
end namespace
`);
program.validate();
expectZeroDiagnostics(program);
});

it('flags interfaces in wrong locations', () => {
program.setFile('source/main.bs', `
function test()
interface Alpha
prop as string
end interface
if true then
interface Beta
prop as string
end interface
end if
end function
`);
program.validate();
expectDiagnostics(program, [{
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('interface'),
range: util.createRange(2, 16, 2, 31)
}, {
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('interface'),
range: util.createRange(6, 20, 6, 34)
}]);
});

it('allows consts in correct locations', () => {
program.setFile('source/main.bs', `
const Alpha = 1
namespace Beta
const Charlie = 2
namespace Delta
const Echo = 3
end namespace
end namespace
`);
program.validate();
expectZeroDiagnostics(program);
});

it('flags consts in wrong locations', () => {
program.setFile('source/main.bs', `
function test()
const Alpha = 1
if true then
const Beta = 2
end if
end function
`);
program.validate();
expectDiagnostics(program, [{
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('const'),
range: util.createRange(2, 16, 2, 27)
}, {
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('const'),
range: util.createRange(4, 20, 4, 30)
}]);
});
});
52 changes: 33 additions & 19 deletions src/bscPlugin/validation/BrsFileValidator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import { DiagnosticMessages } from '../../DiagnosticMessages';
import type { BrsFile } from '../../files/BrsFile';
import type { OnFileValidateEvent } from '../../interfaces';
import { TokenKind } from '../../lexer/TokenKind';
import type { Expression } from '../../parser/AstNode';
import type { Expression, Statement } from '../../parser/AstNode';
import type { LiteralExpression } from '../../parser/Expression';
import { ParseMode } from '../../parser/Parser';
import type { ContinueStatement, EnumMemberStatement, EnumStatement, ForEachStatement, ForStatement, ImportStatement, LibraryStatement, NamespaceStatement, WhileStatement } from '../../parser/Statement';
import type { ContinueStatement, EnumMemberStatement, EnumStatement, ForEachStatement, ForStatement, ImportStatement, LibraryStatement, WhileStatement } from '../../parser/Statement';
import { DynamicType } from '../../types/DynamicType';
import util from '../../util';
import type { Range } from 'vscode-languageserver';

export class BrsFileValidator {
constructor(
Expand Down Expand Up @@ -37,12 +38,16 @@ export class BrsFileValidator {
node.func.body.symbolTable.addSymbol('super', undefined, DynamicType.instance);
},
EnumStatement: (node) => {
this.validateDeclarationLocations(node, 'enum', () => util.createBoundingRange(node.tokens.enum, node.tokens.name));

this.validateEnumDeclaration(node);

//register this enum declaration
node.parent.getSymbolTable()?.addSymbol(node.tokens.name.text, node.tokens.name.range, DynamicType.instance);
},
ClassStatement: (node) => {
this.validateDeclarationLocations(node, 'class', () => util.createBoundingRange(node.classKeyword, node.name));

//register this class
node.parent.getSymbolTable()?.addSymbol(node.name.text, node.name.range, DynamicType.instance);
},
Expand All @@ -55,7 +60,7 @@ export class BrsFileValidator {
node.parent.getSymbolTable()?.addSymbol(node.item.text, node.item.range, DynamicType.instance);
},
NamespaceStatement: (node) => {
this.validateNamespaceStatement(node);
this.validateDeclarationLocations(node, 'namespace', () => util.createBoundingRange(node.keyword, node.nameExpression));

node.parent.getSymbolTable().addSymbol(
node.name.split('.')[0],
Expand All @@ -64,6 +69,8 @@ export class BrsFileValidator {
);
},
FunctionStatement: (node) => {
this.validateDeclarationLocations(node, 'function', () => util.createBoundingRange(node.func.functionType, node.name));

if (node.name?.text) {
node.parent.getSymbolTable().addSymbol(
node.name.text,
Expand Down Expand Up @@ -97,7 +104,12 @@ export class BrsFileValidator {
const symbolTable = node.getSymbolTable();
symbolTable?.addSymbol(paramName, node.name.range, node.type);
},
InterfaceStatement: (node) => {
this.validateDeclarationLocations(node, 'interface', () => util.createBoundingRange(node.tokens.interface, node.tokens.name));
},
ConstStatement: (node) => {
this.validateDeclarationLocations(node, 'const', () => util.createBoundingRange(node.tokens.const, node.tokens.name));

node.parent.getSymbolTable().addSymbol(node.tokens.name.text, node.tokens.name.range, DynamicType.instance);
},
CatchStatement: (node) => {
Expand All @@ -120,6 +132,24 @@ export class BrsFileValidator {
});
}

/**
* Validate that a statement is defined in one of these specific locations
* - the root of the AST
* - inside a namespace
* This is applicable to things like FunctionStatement, ClassStatement, NamespaceStatement, EnumStatement, InterfaceStatement
*/
private validateDeclarationLocations(statement: Statement, keyword: string, rangeFactory?: () => Range) {
//if nested inside a namespace, or defined at the root of the AST (i.e. in a body that has no parent)
if (isNamespaceStatement(statement.parent?.parent) || (isBody(statement.parent) && !statement.parent?.parent)) {
return;
}
//the statement was defined in the wrong place. Flag it.
this.event.file.addDiagnostic({
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel(keyword),
range: rangeFactory?.() ?? statement.range
});
}

private validateEnumDeclaration(stmt: EnumStatement) {
const members = stmt.getMembers();
//the enum data type is based on the first member value
Expand Down Expand Up @@ -197,22 +227,6 @@ export class BrsFileValidator {
}
}

private validateNamespaceStatement(stmt: NamespaceStatement) {
let parentNode = stmt.parent;

while (parentNode) {
if (!isNamespaceStatement(parentNode) && !isBody(parentNode)) {
this.event.file.addDiagnostic({
...DiagnosticMessages.keywordMustBeDeclaredAtNamespaceLevel('namespace'),
range: stmt.range
});
break;
}

parentNode = parentNode.parent;
}
}

/**
* Find statements defined at the top level (or inside a namespace body) that are not allowed to be there
*/
Expand Down