Skip to content

Commit

Permalink
Fixes issue with multiple namespace symbol tables (#825)
Browse files Browse the repository at this point in the history 
Co-authored-by: Bronley Plumb <bronley@gmail.com>
  • Loading branch information
@markwpearce @TwitchBronBron
markwpearce and TwitchBronBron committed Jun 10, 2023
1 parent 3386eb8 commit c754382
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 52 deletions.
62 changes: 62 additions & 0 deletions src/Scope.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2468,6 +2468,68 @@ describe('Scope', () => {
expectTypeToBe(deltaType, NamespaceType);
expectTypeToBe(deltaValueType, IntegerType);
});

it('should be able to define deep namespaces in multiple files', () => {
program.setFile('source/util.bs', `
function getData()
return alpha.beta.gamma.sayHello(alpha.values.value1)
end function
namespace alpha.beta.gamma
function sayHello(num as integer)
return "hello " + num.toStr()
end function
end namespace
`);
program.setFile('source/values.bs', `
namespace alpha.values
const value1 = 300
end namespace
`);
program.validate();
expectZeroDiagnostics(program);
});

it('should allow access to underscored version of namespace members in different file', () => {
program.setFile('source/main.bs', `
sub printPi()
print alpha_util_getPi().toStr()
end sub
`);
program.setFile('source/util.bs', `
namespace alpha.util
function getPi() as float
return 3.14
end function
end namespace
`);
program.validate();
expectZeroDiagnostics(program);
});

it('resolves deep namespaces defined in different locations', () => {
program.setFile(`source/main.bs`, `
sub main()
print NameA.NameB.makeClassB().value
end sub
namespace NameA
namespace NameB
function makeClassB() as SomeKlass
return new SomeKlass()
end function
end namespace
end namespace
namespace NameA.NameB
class SomeKlass
value = 3.14
end class
end namespace
`);
program.validate();
expectZeroDiagnostics(program);
});

});
});
});
57 changes: 49 additions & 8 deletions src/Scope.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ import { URI } from 'vscode-uri';
import { LogLevel } from './Logger';
import type { BrsFile } from './files/BrsFile';
import type { DependencyGraph, DependencyChangedEvent } from './DependencyGraph';
import { isBrsFile, isMethodStatement, isClassStatement, isConstStatement, isEnumStatement, isFunctionStatement, isFunctionType, isXmlFile, isEnumMemberStatement, isNamespaceStatement } from './astUtils/reflection';
import { SymbolTable } from './SymbolTable';
import { isBrsFile, isMethodStatement, isClassStatement, isConstStatement, isEnumStatement, isFunctionStatement, isFunctionType, isXmlFile, isEnumMemberStatement, isNamespaceStatement, isNamespaceType, isReferenceType } from './astUtils/reflection';
import { SymbolTable, SymbolTypeFlag } from './SymbolTable';
import type { Statement } from './parser/AstNode';
import type { BscType } from './types/BscType';
import { NamespaceType } from './types/NamespaceType';

/**
* A class to keep track of all declarations within a given scope (like source scope, component scope)
Expand Down Expand Up @@ -764,19 +766,58 @@ export class Scope {
* This will only rebuilt if the symbol table has not been built before
*/
public linkSymbolTable() {
const allNameSpaces: NamespaceStatement[] = [];
for (const file of this.getAllFiles()) {
if (isBrsFile(file)) {
file.parser.symbolTable.pushParentProvider(() => this.symbolTable);
allNameSpaces.push(...file.parser.references.namespaceStatements);
}
}

//link each NamespaceStatement's SymbolTable with the aggregate NamespaceLookup SymbolTable
for (const namespace of file.parser.references.namespaceStatements) {
const namespaceNameLower = namespace.getName(ParseMode.BrighterScript).toLowerCase();
namespace.getSymbolTable().addSibling(
this.namespaceLookup.get(namespaceNameLower).symbolTable
);
//Add namespace aggregates to namespace member tables
for (const namespace of allNameSpaces) {
//link each NamespaceType member table with the aggregate NamespaceLookup SymbolTable
let fullNamespaceName = namespace.getName(ParseMode.BrighterScript);
let namespaceParts = fullNamespaceName.split('.');

// eslint-disable-next-line no-bitwise
let getSymbolFlags = { flags: SymbolTypeFlag.runtime | SymbolTypeFlag.typetime };
let currentNSType: BscType = null;
let nameSoFar = '';

for (const nsNamePart of namespaceParts) {
// for each section of the namespace name, add it as either a top level symbol (if it is the first part)
// or as a member to the containing namespace.
let previousNSType = currentNSType;
currentNSType = currentNSType === null
? this.symbolTable.getSymbolType(nsNamePart, getSymbolFlags)
: currentNSType.getMemberType(nsNamePart, getSymbolFlags);
nameSoFar = nameSoFar === '' ? nsNamePart : `${nameSoFar}.${nsNamePart}`;
let isFinalNamespace = nameSoFar.toLowerCase() === fullNamespaceName.toLowerCase();
if (!isNamespaceType(currentNSType)) {
if (!currentNSType || isReferenceType(currentNSType)) {
currentNSType = isFinalNamespace
? namespace.getType(getSymbolFlags)
: new NamespaceType(nameSoFar);
if (previousNSType) {
// adding as a member of existing NS
previousNSType.addMember(nsNamePart, namespace.range, currentNSType, getSymbolFlags.flags);
} else {
this.symbolTable.addSymbol(nsNamePart, namespace.range, currentNSType, getSymbolFlags.flags);
}
} else {
break;
}
}
// Now the namespace type is built, add the aggregate as a sibling
let aggregateNSSymbolTable = this.namespaceLookup.get(nameSoFar.toLowerCase()).symbolTable;
currentNSType.memberTable.addSibling(aggregateNSSymbolTable);
if (isFinalNamespace) {
namespace.body.getSymbolTable().addSibling(aggregateNSSymbolTable);
}
}
}

this.program.typeCacheVerifier.generateToken();
}

Expand Down
4 changes: 3 additions & 1 deletion src/SymbolTable.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ export class SymbolTable implements SymbolTypeGetter {
* Add a sibling symbol table (which will be inspected first before walking upward to the parent
*/
public addSibling(sibling: SymbolTable) {
this.siblings.add(sibling);
if (!this.siblings.has(sibling)) {
this.siblings.add(sibling);
}
}

/**
Expand Down
45 changes: 2 additions & 43 deletions src/bscPlugin/validation/BrsFileValidator.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isBody, isClassStatement, isCommentStatement, isConstStatement, isDottedGetExpression, isDottedSetStatement, isEnumStatement, isForEachStatement, isForStatement, isFunctionStatement, isImportStatement, isIndexedGetExpression, isIndexedSetStatement, isInterfaceStatement, isLibraryStatement, isLiteralExpression, isNamespaceStatement, isNamespaceType, isUnaryExpression, isWhileStatement } from '../../astUtils/reflection';
import { isBody, isClassStatement, isCommentStatement, isConstStatement, isDottedGetExpression, isDottedSetStatement, isEnumStatement, isForEachStatement, isForStatement, isFunctionStatement, isImportStatement, isIndexedGetExpression, isIndexedSetStatement, isInterfaceStatement, isLibraryStatement, isLiteralExpression, isNamespaceStatement, isUnaryExpression, isWhileStatement } from '../../astUtils/reflection';
import { createVisitor, WalkMode } from '../../astUtils/visitors';
import { DiagnosticMessages } from '../../DiagnosticMessages';
import type { BrsFile } from '../../files/BrsFile';
Expand All @@ -11,7 +11,6 @@ import type { ContinueStatement, EnumMemberStatement, EnumStatement, ForEachStat
import { SymbolTypeFlag } from '../../SymbolTable';
import type { BscType } from '../../types/BscType';
import { DynamicType } from '../../types/DynamicType';
import { NamespaceType } from '../../types/NamespaceType';
import util from '../../util';
import type { Range } from 'vscode-languageserver';

Expand Down Expand Up @@ -95,47 +94,7 @@ export class BrsFileValidator {
},
NamespaceStatement: (node) => {
this.validateDeclarationLocations(node, 'namespace', () => util.createBoundingRange(node.keyword, node.nameExpression));

// Since namespaces are accessed via a DottedGetExpression, we need to build the types for
// the top level namespace, all the way down.
const namespaceParts = node.name.split('.');
const topLevel = namespaceParts[0];
if (!topLevel) {
return;
}
const parentSymbolTable = node.parent.getSymbolTable();
// eslint-disable-next-line no-bitwise
const namespaceTypeFlags = SymbolTypeFlag.runtime | SymbolTypeFlag.typetime;

//Build namespace types, part by part, if they don't exist
//Last one can use the node's `getType()` method
let nameSoFar = '';
let nextNamespaceType: BscType;
let currentSymbolTable = parentSymbolTable;
for (let i = 0; i < namespaceParts.length; i++) {
const part = namespaceParts[i];
nameSoFar += (i > 0 ? '.' : '') + part;

nextNamespaceType = currentSymbolTable.getSymbolType(part, { flags: SymbolTypeFlag.typetime });

if (!isNamespaceType(nextNamespaceType)) {
// if it is the last one, ie, the part that represents this node/namespace.
// make sure the namespace's symboltable is a provider for the previous types' member table
nextNamespaceType = (i === namespaceParts.length - 1) ? node.getType({ flags: SymbolTypeFlag.typetime }) : new NamespaceType(nameSoFar);
currentSymbolTable.addSymbol(part, node.nameExpression.range, nextNamespaceType, namespaceTypeFlags);

}
// this type already exists, and is a NameSpaceType
if (i === namespaceParts.length - 1) {
// Since functions/consts, etc defined in this namespace are added to
// the NamespaceStatement's symbol table, we need to make sure
// that symbol table is a sibling of this type's member table
// so member table lookups will find them
nextNamespaceType.memberTable.addSibling(node.getSymbolTable());
}

currentSymbolTable = nextNamespaceType.memberTable;
}
//Namespace Types are added at the Scope level - This is handled when the SymbolTables get linked
},
FunctionStatement: (node) => {
this.validateDeclarationLocations(node, 'function', () => util.createBoundingRange(node.func.functionType, node.name));
Expand Down

0 comments on commit c754382

Please sign in to comment.