Skip to content

Commit

Permalink
Feat/enum completion (#485)
Browse files Browse the repository at this point in the history 
* adds completions for enum types

* add tests to prove there are no enum member collisions

* Formatting fixes

* small var naming tweaks

* remove unnecessary eslint disable.

* clean up test a little

* adds validation for enums

* adds completions for enum types

* add tests to prove there are no enum member collisions

* Formatting fixes

* small var naming tweaks

* remove unnecessary eslint disable.

* clean up test a little

* changes namespace.enumStatements to map

* changes namespace.enumStatements to map

* fixes broken imports

* uses visitor to ascertain dotted get statements

* fix circular ref issues.

* clean up some tests

Co-authored-by: Bronley Plumb <bronley@gmail.com>
  • Loading branch information
@georgejecook @TwitchBronBron
georgejecook and TwitchBronBron committed Feb 1, 2022
1 parent 5330964 commit c2b3aa8
Show file tree
Hide file tree
Showing 7 changed files with 416 additions and 16 deletions.
1 change: 0 additions & 1 deletion package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/DiagnosticMessages.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,13 @@ export let DiagnosticMessages = {
message: `Value is required for ${expectedType} enum`,
code: 1125,
severity: DiagnosticSeverity.Error
}),
unknownEnumValue: (name: string, enumName: string) => ({
message: `Enum value ${name} is not found in enum ${enumName}`,
code: 1126,
severity: DiagnosticSeverity.Error
})

};

export const DiagnosticCodeMap = {} as Record<keyof (typeof DiagnosticMessages), number>;
Expand Down
109 changes: 109 additions & 0 deletions src/Scope.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -845,4 +845,113 @@ describe('Scope', () => {
program['scopes']['source'].buildNamespaceLookup();
});
});

describe('buildEnumLookup', () => {

it('builds enum lookup', () => {
const sourceScope = program.getScopeByName('source');
//eslint-disable-next-line @typescript-eslint/no-floating-promises
program.addOrReplaceFile('source/main.bs', `
enum foo
bar1
bar2
end enum
namespace test
function fooFace2()
end function
class fooClass2
end class
enum foo2
bar2_1
bar2_2
end enum
end namespace
function fooFace()
end function
class fooClass
end class
enum foo3
bar3_1
bar3_2
end enum
`);
// program.validate();
let lookup = sourceScope.enumLookup;

expect(
[...lookup.keys()]
).to.eql([
'foo',
'foo.bar1',
'foo.bar2',
'test.foo2',
'test.foo2.bar2_1',
'test.foo2.bar2_2',
'foo3',
'foo3.bar3_1',
'foo3.bar3_2'
]);
});
});
describe('enums', () => {
it('gets enum completions', () => {
//eslint-disable-next-line @typescript-eslint/no-floating-promises
program.addOrReplaceFile('source/main.bs', `
enum foo
bar1
bar2
end enum
sub Main()
g1 = foo.bar1
g2 = test.foo2.bar2_1
g3 = test.foo2.bar2_1
g4 = test.nested.foo3.bar3_1
b1 = foo.bad1
b2 = test.foo2.bad2
b4 = test.nested.foo3.bad3
'unknown namespace
b3 = test.foo3.bar3_1
end sub
namespace test
function fooFace2()
end function
class fooClass2
end class
enum foo2
bar2_1
bar2_2
end enum
end namespace
function fooFace()
end function
class fooClass
end class
namespace test.nested
enum foo3
bar3_1
bar3_2
end enum
end namespace
`);
program.validate();
expectDiagnostics(program, [
DiagnosticMessages.unknownEnumValue('bad1', 'foo'),
DiagnosticMessages.unknownEnumValue('bad2', 'test.foo2'),
DiagnosticMessages.unknownEnumValue('bad3', 'test.nested.foo3')
]);
});
});
});
119 changes: 115 additions & 4 deletions src/Scope.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@ import { DiagnosticMessages } from './DiagnosticMessages';
import type { CallableContainer, BsDiagnostic, FileReference, BscFile, CallableContainerMap } from './interfaces';
import type { FileLink, Program } from './Program';
import { BsClassValidator } from './validators/ClassValidator';
import type { NamespaceStatement, Statement, FunctionStatement, ClassStatement } from './parser/Statement';
import type { NewExpression } from './parser/Expression';
import type { NamespaceStatement, Statement, FunctionStatement, ClassStatement, EnumStatement } from './parser/Statement';
import type { DottedGetExpression, NewExpression } from './parser/Expression';
import { ParseMode } from './parser/Parser';
import { standardizePath as s, util } from './util';
import { globalCallableMap } from './globalCallables';
import { Cache } from './Cache';
import { URI } from 'vscode-uri';
import { LogLevel } from './Logger';
import { isBrsFile, isClassStatement, isFunctionStatement, isFunctionType, isXmlFile, isCustomType, isClassMethodStatement } from './astUtils/reflection';
import type { BrsFile } from './files/BrsFile';
import type { DependencyGraph, DependencyChangedEvent } from './DependencyGraph';
import { isBrsFile, isClassMethodStatement, isClassStatement, isCustomType, isDottedGetExpression, isEnumStatement, isFunctionStatement, isFunctionType, isVariableExpression, isXmlFile } from './astUtils/reflection';
import { createVisitor, WalkMode } from './astUtils/visitors';

/**
* A class to keep track of all declarations within a given scope (like source scope, component scope)
Expand Down Expand Up @@ -53,6 +54,12 @@ export class Scope {
public get namespaceLookup() {
return this.cache.getOrAdd('namespaceLookup', () => this.buildNamespaceLookup());
}
/**
* A dictionary of enums, indexed by the lower case full name of each enum.
*/
public get enumLookup() {
return this.cache.getOrAdd('enumLookup', () => this.buildEnumLookup());
}

/**
* Get the class with the specified name.
Expand Down Expand Up @@ -113,6 +120,28 @@ export class Scope {
});
}

/**
* A dictionary of all enums in this scope. This includes namespaced enums always with their full name.
* The key is stored in lower case
*/
public getEnumMap(): Map<string, FileLink<EnumStatement>> {
return this.cache.getOrAdd('enumMap', () => {
const map = new Map<string, FileLink<EnumStatement>>();
this.enumerateBrsFiles((file) => {
if (isBrsFile(file)) {
for (let enumStmt of file.parser.references.enumStatements) {
const lowerEnumName = enumStmt.fullName.toLowerCase();
//only track enums with a defined name (i.e. exclude nameless malformed enums)
if (lowerEnumName) {
map.set(lowerEnumName, { item: enumStmt, file: file });
}
}
}
});
return map;
});
}

/**
* The list of diagnostics found specifically for this scope. Individual file diagnostics are stored on the files themselves.
*/
Expand Down Expand Up @@ -360,7 +389,8 @@ export class Scope {
namespaces: new Map<string, NamespaceContainer>(),
classStatements: {},
functionStatements: {},
statements: []
statements: [],
enumStatements: new Map<string, EnumStatement>()
});
}
}
Expand All @@ -371,6 +401,8 @@ export class Scope {
ns.classStatements[statement.name.text.toLowerCase()] = statement;
} else if (isFunctionStatement(statement) && statement.name) {
ns.functionStatements[statement.name.text.toLowerCase()] = statement;
} else if (isEnumStatement(statement) && statement.fullName) {
ns.enumStatements.set(statement.fullName.toLowerCase(), statement);
}
}
}
Expand All @@ -391,6 +423,35 @@ export class Scope {
return namespaceLookup;
}

public buildEnumLookup() {
let lookup = new Map<string, EnumContainer>();
this.enumerateBrsFiles((file) => {
for (let [key, es] of file.parser.references.enumStatementLookup) {
if (!lookup.has(key)) {
lookup.set(key, {
file: file,
fullName: key,
nameRange: es.range,
lastPartName: es.name,
statement: es
});
for (const ems of es.getMembers()) {
const fullMemberName = `${key}.${ems.name.toLowerCase()}`;
lookup.set(fullMemberName, {
file: file,
fullName: fullMemberName,
nameRange: ems.range,
lastPartName: ems.name,
statement: es
});
}
}
}
});
return lookup;
}


public getAllNamespaceStatements() {
let result = [] as NamespaceStatement[];
this.enumerateBrsFiles((file) => {
Expand Down Expand Up @@ -467,6 +528,7 @@ export class Scope {
this.diagnosticDetectFunctionCollisions(file);
this.detectVariableNamespaceCollisions(file);
this.diagnosticDetectInvalidFunctionExpressionTypes(file);
this.detectUnknownEnumMembers(file);
});
}

Expand Down Expand Up @@ -1001,6 +1063,46 @@ export class Scope {
}
return items;
}

private detectUnknownEnumMembers(file: BrsFile) {
if (!isBrsFile(file)) {
return;
}
file.parser.ast.walk(createVisitor({
DottedGetExpression: (dge) => {
let nameParts = this.getAllDottedGetParts(dge);
let name = nameParts.pop();
let parentPath = nameParts.join('.');
let ec = this.enumLookup.get(parentPath);
if (ec && !this.enumLookup.has(`${parentPath}.${name}`)) {
this.diagnostics.push({
file: file,
...DiagnosticMessages.unknownEnumValue(name, ec.fullName),
range: dge.range,
relatedInformation: [{
message: 'Enum declared here',
location: Location.create(
URI.file(ec.file.pathAbsolute).toString(),
ec.statement.range
)
}]
});

}
}
}), { walkMode: WalkMode.visitAllRecursive });
}

private getAllDottedGetParts(dg: DottedGetExpression) {
let parts = [dg?.name?.text];
let nextPart = dg.obj;
while (isDottedGetExpression(nextPart) || isVariableExpression(nextPart)) {
parts.push(nextPart?.name?.text);
nextPart = isDottedGetExpression(nextPart) ? nextPart.obj : undefined;
}
return parts.reverse();
}

}

interface NamespaceContainer {
Expand All @@ -1011,9 +1113,18 @@ interface NamespaceContainer {
statements: Statement[];
classStatements: Record<string, ClassStatement>;
functionStatements: Record<string, FunctionStatement>;
enumStatements: Map<string, EnumStatement>;
namespaces: Map<string, NamespaceContainer>;
}

interface EnumContainer {
file: BscFile;
fullName: string;
nameRange: Range;
lastPartName: string;
statement: EnumStatement;
}

interface AugmentedNewExpression extends NewExpression {
file: BscFile;
}
Loading

0 comments on commit c2b3aa8

Please sign in to comment.