Skip to content

Commit

Permalink
prevent js proto collision by storing namespaceLookup in Map
Browse files Browse the repository at this point in the history
  • Loading branch information
TwitchBronBron committed Nov 23, 2021
1 parent 0631a9d commit 85a110d
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 29 deletions.
9 changes: 9 additions & 0 deletions src/Scope.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@ describe('Scope', () => {
expect(program.getDiagnostics()[0]?.message).not.to.exist;
});

it('handles variables with javascript prototype names', () => {
program.addOrReplaceFile('source/main.brs', `
sub main()
constructor = true
end sub
`);
program.validate();
});

it('flags parameter with same name as namespace', () => {
program.addOrReplaceFile('source/main.bs', `
namespace NameA.NameB
Expand Down
38 changes: 20 additions & 18 deletions src/Scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ export class Scope {
* Builds a tree of namespace objects
*/
public buildNamespaceLookup() {
let namespaceLookup = {} as Record<string, NamespaceContainer>;
let namespaceLookup = new Map<string, NamespaceContainer>();
this.enumerateBrsFiles((file) => {
for (let namespace of file.parser.references.namespaceStatements) {
//TODO should we handle non-brighterscript?
Expand All @@ -350,18 +350,20 @@ export class Scope {
for (let part of nameParts) {
loopName = loopName === null ? part : `${loopName}.${part}`;
let lowerLoopName = loopName.toLowerCase();
namespaceLookup[lowerLoopName] = namespaceLookup[lowerLoopName] ?? {
file: file,
fullName: loopName,
nameRange: namespace.nameExpression.range,
lastPartName: part,
namespaces: {},
classStatements: {},
functionStatements: {},
statements: []
};
if (!namespaceLookup.has(lowerLoopName)) {
namespaceLookup.set(lowerLoopName, {
file: file,
fullName: loopName,
nameRange: namespace.nameExpression.range,
lastPartName: part,
namespaces: new Map<string, NamespaceContainer>(),
classStatements: {},
functionStatements: {},
statements: []
});
}
}
let ns = namespaceLookup[name.toLowerCase()];
let ns = namespaceLookup.get(name.toLowerCase());
ns.statements.push(...namespace.body.statements);
for (let statement of namespace.body.statements) {
if (isClassStatement(statement) && statement.name) {
Expand All @@ -373,15 +375,15 @@ export class Scope {
}

//associate child namespaces with their parents
for (let key in namespaceLookup) {
let ns = namespaceLookup[key];
for (let [, ns] of namespaceLookup) {
let parts = ns.fullName.split('.');

if (parts.length > 1) {
//remove the last part
parts.pop();
let parentName = parts.join('.');
namespaceLookup[parentName.toLowerCase()].namespaces[ns.lastPartName.toLowerCase()] = ns;
const parent = namespaceLookup.get(parentName.toLowerCase());
parent.namespaces.set(ns.lastPartName.toLowerCase(), ns);
}
}
});
Expand Down Expand Up @@ -481,7 +483,7 @@ export class Scope {
for (let func of file.parser.references.functionExpressions) {
for (let param of func.parameters) {
let lowerParamName = param.name.text.toLowerCase();
let namespace = this.namespaceLookup[lowerParamName];
let namespace = this.namespaceLookup.get(lowerParamName);
//see if the param matches any starting namespace part
if (namespace) {
this.diagnostics.push({
Expand All @@ -502,7 +504,7 @@ export class Scope {

for (let assignment of file.parser.references.assignmentStatements) {
let lowerAssignmentName = assignment.name.text.toLowerCase();
let namespace = this.namespaceLookup[lowerAssignmentName];
let namespace = this.namespaceLookup.get(lowerAssignmentName);
//see if the param matches any starting namespace part
if (namespace) {
this.diagnostics.push({
Expand Down Expand Up @@ -1008,7 +1010,7 @@ interface NamespaceContainer {
statements: Statement[];
classStatements: Record<string, ClassStatement>;
functionStatements: Record<string, FunctionStatement>;
namespaces: Record<string, NamespaceContainer>;
namespaces: Map<string, NamespaceContainer>;
}

interface AugmentedNewExpression extends NewExpression {
Expand Down
15 changes: 5 additions & 10 deletions src/files/BrsFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -922,19 +922,16 @@ export class BrsFile {
let closestParentNamespaceName = completionName.replace(/\.([a-z0-9_]*)?$/gi, '');
let newToken = this.getTokenBefore(currentToken, TokenKind.New);

let namespaceLookup = scope.namespaceLookup;
let result = new Map<string, CompletionItem>();
for (let key in namespaceLookup) {
let namespace = namespaceLookup[key.toLowerCase()];
for (let [, namespace] of scope.namespaceLookup) {
//completionName = "NameA."
//completionName = "NameA.Na
//NameA
//NameA.NameB
//NameA.NameB.NameC
if (namespace.fullName.toLowerCase() === closestParentNamespaceName.toLowerCase()) {
//add all of this namespace's immediate child namespaces, bearing in mind if we are after a new keyword
for (let childKey in namespace.namespaces) {
const ns = namespace.namespaces[childKey];
for (let [, ns] of namespace.namespaces) {
if (!newToken || ns.statements.find((s) => isClassStatement(s))) {
if (!result.has(ns.lastPartName)) {
result.set(ns.lastPartName, {
Expand Down Expand Up @@ -1107,7 +1104,7 @@ export class BrsFile {
//find the first scope that contains this namespace
let scopes = this.program.getScopesForFile(this);
for (let scope of scopes) {
if (scope.namespaceLookup[lowerName]) {
if (scope.namespaceLookup.has(lowerName)) {
return true;
}
}
Expand All @@ -1125,7 +1122,7 @@ export class BrsFile {
if (lowerCalleeName) {
let scopes = this.program.getScopesForFile(this);
for (let scope of scopes) {
let namespace = scope.namespaceLookup[namespaceName.toLowerCase()];
let namespace = scope.namespaceLookup.get(namespaceName.toLowerCase());
if (namespace.functionStatements[lowerCalleeName]) {
return true;
}
Expand Down Expand Up @@ -1480,10 +1477,8 @@ export class BrsFile {
if (!dottedGetText) {
return [];
}
let namespaceLookup = scope.namespaceLookup;
let resultsMap = new Map<string, SignatureInfoObj>();
for (let key in namespaceLookup) {
let namespace = namespaceLookup[key.toLowerCase()];
for (let [, namespace] of scope.namespaceLookup) {
//completionName = "NameA."
//completionName = "NameA.Na
//NameA
Expand Down
2 changes: 1 addition & 1 deletion src/files/XmlFile.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,7 @@ describe('XmlFile', () => {
program.validate();
expect(program.getDiagnostics()[0]?.message).not.to.exist;
const scope = program.getComponentScope('ChildComponent');
expect(Object.keys(scope.namespaceLookup).sort()).to.eql([
expect([...scope.namespaceLookup.keys()].sort()).to.eql([
'lib',
'parent'
]);
Expand Down

0 comments on commit 85a110d

Please sign in to comment.