Skip to content

Commit

Permalink
Improve perf (WIP)
Browse files Browse the repository at this point in the history
  • Loading branch information
TwitchBronBron committed Jul 23, 2022
1 parent fde94a9 commit d5a99c1
Show file tree
Hide file tree
Showing 9 changed files with 144 additions and 92 deletions.
2 changes: 1 addition & 1 deletion src/LanguageServer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ describe('LanguageServer', () => {
});
});

describe('sendDiagnostics', () => {
describe.skip('sendDiagnostics', () => {
it('waits for program to finish loading before sending diagnostics', async () => {
server.onInitialize({
capabilities: {
Expand Down
1 change: 0 additions & 1 deletion src/SymbolTable.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { Range } from 'vscode-languageserver';
import type { BscType } from './types/BscType';


/**
* Stores the types associated with variables and functions in the Brighterscript code
* Can be part of a hierarchy, so lookups can reference parent scopes
Expand Down
181 changes: 97 additions & 84 deletions src/bscPlugin/validation/ScopeValidator.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { URI } from 'vscode-uri';
import { isBrsFile, isCallExpression, isLiteralExpression, isNewExpression, isXmlScope } from '../../astUtils/reflection';
import { isBrsFile, isLiteralExpression, isXmlScope } from '../../astUtils/reflection';
import { Cache } from '../../Cache';
import { DiagnosticMessages } from '../../DiagnosticMessages';
import type { BrsFile } from '../../files/BrsFile';
import type { BscFile, BsDiagnostic, OnScopeValidateEvent } from '../../interfaces';
import type { Expression } from '../../parser/Expression';
import type { DottedGetExpression, Expression, VariableExpression } from '../../parser/Expression';
import type { EnumStatement } from '../../parser/Statement';
import util from '../../util';
import { nodes, components } from '../../roku-types';
Expand Down Expand Up @@ -52,97 +52,104 @@ export class ScopeValidator {
});
}

private expressionsByFile = new Cache<BrsFile, Readonly<ExpressionInfo>[]>();
private iterateFileExpressions(file: BrsFile) {
const { scope } = this.event;
const expressions = [
...file.parser.references.expressions,
//all class "extends <whatever>" expressions
...file.parser.references.classStatements.map(x => x.parentClassName?.expression),
//all interface "extends <whatever>" expressions
...file.parser.references.interfaceStatements.map(x => x.parentInterfaceName?.expression)
];
//build an expression collection ONCE per file
const expressionInfos = this.expressionsByFile.getOrAdd(file, () => {
const result: DeepWriteable<ExpressionInfo[]> = [];
const expressions = [
...file.parser.references.expressions,
//all class "extends <whatever>" expressions
...file.parser.references.classStatements.map(x => x.parentClassName?.expression),
//all interface "extends <whatever>" expressions
...file.parser.references.interfaceStatements.map(x => x.parentInterfaceName?.expression)
];
for (let expression of expressions) {
if (!expression) {
continue;
}

//walk left-to-right on every expression, only keep the ones that start with VariableExpression, and then keep subsequent DottedGet parts
const parts = util.getDottedGetPath(expression);

if (parts.length > 0) {
result.push({
parts: parts,
expression: expression
});
}
}
return result as unknown as Readonly<ExpressionInfo>[];
});

outer:
for (let referenceExpression of expressions) {
if (!referenceExpression) {
for (const info of expressionInfos) {
const symbolTable = info.expression.getSymbolTable();
const firstPart = info.parts[0];
//flag all unknown left-most variables
if (!symbolTable.hasSymbol(firstPart.name?.text)) {
this.addMultiScopeDiagnostic({
file: file as BscFile,
...DiagnosticMessages.cannotFindName(firstPart.name?.text),
range: firstPart.name.range
});
//skip to the next expression
continue;
}
let expression: Expression;
//lift the callee from call expressions to handle namespaced function calls
if (isCallExpression(referenceExpression)) {
expression = referenceExpression.callee;
} else if (isNewExpression(referenceExpression)) {
expression = referenceExpression.call.callee;
} else {
expression = referenceExpression;

const firstNamespacePart = info.parts[0].name.text;
const firstNamespacePartLower = firstNamespacePart?.toLowerCase();
const namespaceContainer = scope.namespaceLookup.get(firstNamespacePartLower);
const enumStatement = scope.getEnum(firstNamespacePartLower);
//if this isn't a namespace, skip it
if (!namespaceContainer && !enumStatement) {
continue;
}
const tokens = util.getAllDottedGetParts(expression);
if (tokens?.length > 0) {
const symbolTable = expression.getSymbolTable() ?? scope.symbolTable;
//flag all unknown left-most variables
if (!symbolTable.hasSymbol(tokens[0]?.text)) {
this.addMultiScopeDiagnostic({
file: file as BscFile,
...DiagnosticMessages.cannotFindName(tokens[0].text),
range: tokens[0].range
});
//skip to the next expression
continue;
}
//at this point, we know the first item is a known symbol. find unknown namespace parts after the first part
if (tokens.length > 1) {
const firstNamespacePart = tokens.shift().text;
const firstNamespacePartLower = firstNamespacePart?.toLowerCase();
const namespaceContainer = scope.namespaceLookup.get(firstNamespacePartLower);
const enumStatement = scope.getEnum(firstNamespacePartLower);
//if this isn't a namespace, skip it
if (!namespaceContainer && !enumStatement) {
continue;
}
//catch unknown namespace items
const processedNames: string[] = [firstNamespacePart];
for (const token of tokens ?? []) {
processedNames.push(token.text);
const entityName = processedNames.join('.');
const entityNameLower = entityName.toLowerCase();
//catch unknown namespace items
const processedNames: string[] = [firstNamespacePart];
for (let i = 1; i < info.parts.length; i++) {
const part = info.parts[i];
processedNames.push(part.name.text);
const entityName = processedNames.join('.');
const entityNameLower = entityName.toLowerCase();

//if this is an enum member, stop validating here to prevent errors further down the chain
if (scope.getEnumMemberMap().has(entityNameLower)) {
break;
}
//if this is an enum member, stop validating here to prevent errors further down the chain
if (scope.getEnumMemberMap().has(entityNameLower)) {
break;
}

if (
!scope.getEnumMap().has(entityNameLower) &&
!scope.getClassMap().has(entityNameLower) &&
!scope.getConstMap().has(entityNameLower) &&
!scope.getCallableByName(entityNameLower) &&
!scope.namespaceLookup.has(entityNameLower)
) {
//if this looks like an enum, provide a nicer error message
const theEnum = this.getEnum(scope, entityNameLower)?.item;
if (theEnum) {
this.addMultiScopeDiagnostic({
file: file,
...DiagnosticMessages.unknownEnumValue(token.text?.split('.').pop(), theEnum.fullName),
range: tokens[tokens.length - 1].range,
relatedInformation: [{
message: 'Enum declared here',
location: util.createLocation(
URI.file(file.srcPath).toString(),
theEnum.tokens.name.range
)
}]
});
} else {
this.addMultiScopeDiagnostic({
...DiagnosticMessages.cannotFindName(token.text, entityName),
range: token.range,
file: file
});
}
//no need to add another diagnostic for future unknown items
continue outer;
}
if (
!scope.getEnumMap().has(entityNameLower) &&
!scope.getClassMap().has(entityNameLower) &&
!scope.getConstMap().has(entityNameLower) &&
!scope.getCallableByName(entityNameLower) &&
!scope.namespaceLookup.has(entityNameLower)
) {
//if this looks like an enum, provide a nicer error message
const theEnum = this.getEnum(scope, entityNameLower)?.item;
if (theEnum) {
this.addMultiScopeDiagnostic({
file: file,
...DiagnosticMessages.unknownEnumValue(part.name.text?.split('.').pop(), theEnum.fullName),
range: part.name.range,
relatedInformation: [{
message: 'Enum declared here',
location: util.createLocation(
URI.file(file.srcPath).toString(),
theEnum.tokens.name.range
)
}]
});
} else {
this.addMultiScopeDiagnostic({
...DiagnosticMessages.cannotFindName(part.name.text, entityName),
range: part.name.range,
file: file
});
}
//no need to add another diagnostic for future unknown items
continue outer;
}
}
}
Expand Down Expand Up @@ -331,3 +338,9 @@ export class ScopeValidator {

private multiScopeCache = new Cache<string, BsDiagnostic>();
}

interface ExpressionInfo {
parts: Readonly<[VariableExpression, ...DottedGetExpression[]]>;
expression: Readonly<Expression>;
}
type DeepWriteable<T> = { -readonly [P in keyof T]: DeepWriteable<T[P]> };
2 changes: 1 addition & 1 deletion src/files/BrsFile.Class.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ describe('BrsFile BrighterScript classes', () => {
`, undefined, 'source/main.bs');
});

it('works with namespaces', () => {
it.only('works with namespaces', () => {
testTranspile(`
namespace Birds.WaterFowl
class Duck
Expand Down
4 changes: 2 additions & 2 deletions src/parser/Expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export abstract class Expression {
/**
* Get the closest symbol table for this node. Should be overridden in children that directly contain a symbol table
*/
public getSymbolTable() {
public getSymbolTable(): SymbolTable {
return this.parent?.getSymbolTable();
}
}
Expand Down Expand Up @@ -823,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
9 changes: 7 additions & 2 deletions src/parser/Statement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export abstract class Statement {
/**
* Get the closest symbol table for this node. Should be overridden in children that directly contain a symbol table
*/
public getSymbolTable() {
public getSymbolTable(): SymbolTable {
return this.parent?.getSymbolTable();
}
}
Expand Down Expand Up @@ -79,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
34 changes: 34 additions & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1394,6 +1394,7 @@ export class Util {
while (nextPart) {
if (isDottedGetExpression(nextPart) || isIndexedGetExpression(nextPart) || isXmlAttributeGetExpression(nextPart)) {
nextPart = nextPart.obj;

} else if (isCallExpression(nextPart) || isCallfuncExpression(nextPart)) {
nextPart = nextPart.callee;

Expand All @@ -1407,6 +1408,39 @@ export class Util {
return parts;
}

/**
* Break an expression into each part, and return any VariableExpression or DottedGet expresisons from left-to-right.
*/
public getDottedGetPath(expression: Expression): [VariableExpression, ...DottedGetExpression[]] {
let parts: Expression[] = [];
let nextPart = expression;
while (nextPart) {
if (isDottedGetExpression(nextPart)) {
parts.unshift(nextPart);
nextPart = nextPart.obj;

} else if (isIndexedGetExpression(nextPart) || isXmlAttributeGetExpression(nextPart)) {
nextPart = nextPart.obj;
parts = [];

} else if (isCallExpression(nextPart) || isCallfuncExpression(nextPart)) {
nextPart = nextPart.callee;
parts = [];

} else if (isNamespacedVariableNameExpression(nextPart)) {
nextPart = nextPart.expression;

} else if (isVariableExpression(nextPart)) {
parts.unshift(nextPart);
break;
} else {
parts = [];
break;
}
}
return parts as any;
}

/**
* Returns an integer if valid, or undefined. Eliminates checking for NaN
*/
Expand Down
1 change: 1 addition & 0 deletions vscode-profile-2022-07-23-00-30-27.cpuprofile

Large diffs are not rendered by default.

0 comments on commit d5a99c1

Please sign in to comment.