Skip to content

Commit

Permalink
Link all brs AST nodes to parent onFileValidate (#650)
Browse files Browse the repository at this point in the history
  • Loading branch information
TwitchBronBron authored Aug 2, 2022
1 parent 13df25b commit 2695401
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 36 deletions.
2 changes: 2 additions & 0 deletions src/Program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,9 @@ export class Program {
this.logger.time(LogLevel.info, ['Validate all scopes'], () => {
for (let scopeName in this.scopes) {
let scope = this.scopes[scopeName];
scope.linkSymbolTable();
scope.validate();
scope.unlinkSymbolTable();
}
});

Expand Down
86 changes: 54 additions & 32 deletions src/bscPlugin/validation/BrsFileValidator.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { isClassStatement, isCommentStatement, isConstStatement, isEnumStatement, isFunctionStatement, isImportStatement, isInterfaceStatement, isLibraryStatement, isLiteralExpression, isNamespaceStatement } from '../../astUtils/reflection';
import { WalkMode } from '../../astUtils/visitors';
import { DiagnosticMessages } from '../../DiagnosticMessages';
import type { BrsFile } from '../../files/BrsFile';
import type { BsDiagnostic, OnFileValidateEvent } from '../../interfaces';
import type { OnFileValidateEvent } from '../../interfaces';
import { TokenKind } from '../../lexer/TokenKind';
import type { LiteralExpression } from '../../parser/Expression';
import type { EnumMemberStatement } from '../../parser/Statement';
import type { EnumMemberStatement, EnumStatement } from '../../parser/Statement';

export class BrsFileValidator {
constructor(
Expand All @@ -13,43 +14,66 @@ export class BrsFileValidator {
}

public process() {
this.validateEnumDeclarations();
this.walk();
this.flagTopLevelStatements();
}

private validateEnumDeclarations() {
const diagnostics = [] as BsDiagnostic[];
for (const stmt of this.event.file.parser.references.enumStatements) {
const members = stmt.getMembers();
//the enum data type is based on the first member value
const enumValueKind = (members.find(x => x.value)?.value as LiteralExpression)?.token?.kind ?? TokenKind.IntegerLiteral;
const memberNames = new Set<string>();
for (const member of members) {
const memberNameLower = member.name?.toLowerCase();
/**
* Walk the full AST
*/
private walk() {
this.event.file.ast.walk((node, parent) => {
// link every child with its parent
node.parent = parent;

/**
* flag duplicate member names
*/
if (memberNames.has(memberNameLower)) {
diagnostics.push({
...DiagnosticMessages.duplicateIdentifier(member.name),
file: this.event.file,
range: member.range
});
} else {
memberNames.add(memberNameLower);
//do some file-based validations
if (isEnumStatement(node)) {
this.validateEnumDeclaration(node);
} else if (isNamespaceStatement(node)) {
//namespace names shouldn't be walked, but it needs its parent assigned
node.nameExpression.parent = parent;
} else if (isClassStatement(node)) {
//class extends names don't get walked, but it needs its parent
if (node.parentClassName) {
node.parentClassName.parent = parent;
}
} else if (isInterfaceStatement(node)) {
//class extends names don't get walked, but it needs its parent
if (node.parentInterfaceName) {
node.parentInterfaceName.parent = parent;
}
}
}, {
walkMode: WalkMode.visitAllRecursive
});
}

//Enforce all member values are the same type
this.validateEnumValueTypes(diagnostics, member, enumValueKind);
private validateEnumDeclaration(stmt: EnumStatement) {
const members = stmt.getMembers();
//the enum data type is based on the first member value
const enumValueKind = (members.find(x => x.value)?.value as LiteralExpression)?.token?.kind ?? TokenKind.IntegerLiteral;
const memberNames = new Set<string>();
for (const member of members) {
const memberNameLower = member.name?.toLowerCase();

/**
* flag duplicate member names
*/
if (memberNames.has(memberNameLower)) {
this.event.file.addDiagnostic({
...DiagnosticMessages.duplicateIdentifier(member.name),
range: member.range
});
} else {
memberNames.add(memberNameLower);
}

//Enforce all member values are the same type
this.validateEnumValueTypes(member, enumValueKind);
}
this.event.file.addDiagnostics(diagnostics);
}

private validateEnumValueTypes(diagnostics: BsDiagnostic[], member: EnumMemberStatement, enumValueKind: TokenKind) {
private validateEnumValueTypes(member: EnumMemberStatement, enumValueKind: TokenKind) {
const memberValueKind = (member.value as LiteralExpression)?.token?.kind;

if (
Expand All @@ -58,8 +82,7 @@ export class BrsFileValidator {
//has value, that value is not a literal
(member.value && !isLiteralExpression(member.value))
) {
diagnostics.push({
file: this.event.file,
this.event.file.addDiagnostic({
...DiagnosticMessages.enumValueMustBeType(
enumValueKind.replace(/literal$/i, '').toLowerCase()
),
Expand All @@ -73,8 +96,7 @@ export class BrsFileValidator {
if (memberValueKind) {
//member value is same as enum
if (memberValueKind !== enumValueKind) {
diagnostics.push({
file: this.event.file,
this.event.file.addDiagnostic({
...DiagnosticMessages.enumValueMustBeType(
enumValueKind.replace(/literal$/i, '').toLowerCase()
),
Expand All @@ -84,7 +106,7 @@ export class BrsFileValidator {

//default value missing
} else {
diagnostics.push({
this.event.file.addDiagnostic({
file: this.event.file,
...DiagnosticMessages.enumValueIsRequired(
enumValueKind.replace(/literal$/i, '').toLowerCase()
Expand Down
43 changes: 41 additions & 2 deletions src/parser/Expression.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable no-bitwise */
import type { Token, Identifier } from '../lexer/Token';
import { TokenKind } from '../lexer/TokenKind';
import type { Block, CommentStatement, FunctionStatement } from './Statement';
import type { Block, CommentStatement, FunctionStatement, Statement } from './Statement';
import type { Range } from 'vscode-languageserver';
import util from '../util';
import type { BrsTranspileState } from './BrsTranspileState';
Expand Down Expand Up @@ -33,6 +33,17 @@ export abstract class Expression {
public visitMode = InternalWalkMode.visitExpressions;

public abstract walk(visitor: WalkVisitor, options: WalkOptions);
/**
* The parent node for this expression. This is set dynamically during `onFileValidate`, and should not be set directly.
*/
public parent?: Statement | Expression;

/**
* Get the closest symbol table for this node. Should be overridden in children that directly contain a symbol table
*/
public getSymbolTable(): SymbolTable {
return this.parent?.getSymbolTable();
}
}

export class BinaryExpression extends Expression {
Expand Down Expand Up @@ -155,6 +166,10 @@ export class FunctionExpression extends Expression implements TypedefProvider {

public symbolTable: SymbolTable;

public getSymbolTable() {
return this.symbolTable;
}

/**
* The type this function returns
*/
Expand Down Expand Up @@ -333,6 +348,18 @@ export class NamespacedVariableNameExpression extends Expression {
}
range: Range;

// @ts-expect-error override the property
public get parent() {
return this._parent;
}
public set parent(value) {
if (this.expression) {
this.expression.parent = value;
}
this._parent = value;
}
private _parent: Expression | Statement;

transpile(state: BrsTranspileState) {
return [
state.sourceNode(this, this.getName(ParseMode.BrightScript))
Expand Down Expand Up @@ -386,6 +413,18 @@ export class DottedGetExpression extends Expression {

public readonly range: Range;

// @ts-expect-error override the property
public get parent() {
return this._parent;
}
public set parent(value) {
if (this.obj) {
this.obj.parent = value;
}
this._parent = value;
}
private _parent: Expression | Statement;

transpile(state: BrsTranspileState) {
//if the callee starts with a namespace name, transpile the name
if (state.file.calleeStartsWithNamespace(this)) {
Expand Down Expand Up @@ -784,7 +823,7 @@ export class VariableExpression extends Expression {
public readonly range: Range;

public getName(parseMode: ParseMode) {
return parseMode === ParseMode.BrightScript ? this.name.text : this.name.text;
return this.name.text;
}

transpile(state: BrsTranspileState) {
Expand Down
2 changes: 1 addition & 1 deletion src/parser/Parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ export class Parser {
private body() {
const parentAnnotations = this.enterAnnotationBlock();

let body = new Body([]);
let body = new Body([], this.symbolTable);
if (this.tokens.length > 0) {
this.consumeStatementSeparators(true);

Expand Down
23 changes: 22 additions & 1 deletion src/parser/Statement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,18 @@ export abstract class Statement {
public visitMode = InternalWalkMode.visitStatements;

public abstract walk(visitor: WalkVisitor, options: WalkOptions);

/**
* The parent node for this statement. This is set dynamically during `onFileValidate`, and should not be set directly.
*/
public parent?: Statement | Expression;

/**
* Get the closest symbol table for this node. Should be overridden in children that directly contain a symbol table
*/
public getSymbolTable(): SymbolTable {
return this.parent?.getSymbolTable();
}
}

export class EmptyStatement extends Statement {
Expand All @@ -67,11 +79,16 @@ export class EmptyStatement extends Statement {
*/
export class Body extends Statement implements TypedefProvider {
constructor(
public statements: Statement[] = []
public statements: Statement[] = [],
public symbolTable = new SymbolTable()
) {
super();
}

public getSymbolTable() {
return this.symbolTable;
}

public get range() {
return util.createRangeFromPositions(
this.statements[0]?.range.start ?? Position.create(0, 0),
Expand Down Expand Up @@ -1110,6 +1127,10 @@ export class NamespaceStatement extends Statement implements TypedefProvider {

public symbolTable: SymbolTable;

public getSymbolTable() {
return this.symbolTable;
}

/**
* The string name for this namespace
*/
Expand Down

0 comments on commit 2695401

Please sign in to comment.