Skip to content

Commit

Permalink
Use Symbol Tables for Completions (#874)
Browse files Browse the repository at this point in the history 
* Adds non-statement expressions to AST

* WIP to verify namespace symbol table changes

* Fix broken semantic tokens test

* Solving more tests

* Refactored Symbol Tables to keep track of teh defining AST node of a symbol

* Completes refactor of completions to use symbol tables

* Fixes namespace hovers

* Cache data after loop in BuiltInInterfaceAdder

* Comment documentaation gets a little formatting help - jsdoc tags are respected

* Revert caching of symbol add calls in BuiltInInterfaceAdder

* removed parser in body hack

* fix for #881

* Use leadingTrivia for comments and fix #883

* Solves #882

---------

Co-authored-by: Bronley Plumb <bronley@gmail.com>
  • Loading branch information
@markwpearce @TwitchBronBron
markwpearce and TwitchBronBron authored Sep 13, 2023
1 parent 5d54673 commit 69d3037
Show file tree
Hide file tree
Showing 30 changed files with 1,260 additions and 946 deletions.
17 changes: 10 additions & 7 deletions src/LanguageServer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -689,10 +689,15 @@ export class LanguageServer {
//ensure programs are initialized
await this.waitAllProjectFirstRuns();


let filePath = util.uriToPath(params.textDocument.uri);

//wait until the file has settled
await this.keyedThrottler.onIdleOnce(filePath, true);
// make sure validation is complete
await this.validateAllThrottled();
//wait for the validation cycle to settle
await this.onValidateSettled();

let completions = this
.getProjects()
Expand Down Expand Up @@ -965,23 +970,21 @@ export class LanguageServer {
public async handleFileChanges(project: Project, changes: { type: FileChangeType; srcPath: string }[]) {
//this loop assumes paths are both file paths and folder paths, which eliminates the need to detect.
//All functions below can handle being given a file path AND a folder path, and will only operate on the one they are looking for
let consumeCount = 0;
await Promise.all(changes.map(async (change) => {
await this.keyedThrottler.run(change.srcPath, async () => {
consumeCount += await this.handleFileChange(project, change) ? 1 : 0;
if (await this.handleFileChange(project, change)) {
await this.validateAllThrottled();
}
});
}));

if (consumeCount > 0) {
await this.validateAllThrottled();
}
}

/**
* This only operates on files that match the specified files globs, so it is safe to throw
* any file changes you receive with no unexpected side-effects
* @returns true if the file was handled by this project, false if it was not
*/
private async handleFileChange(project: Project, change: { type: FileChangeType; srcPath: string }) {
private async handleFileChange(project: Project, change: { type: FileChangeType; srcPath: string }): Promise<boolean> {
const { program, options, rootDir } = project.builder;

//deleted
Expand Down
16 changes: 16 additions & 0 deletions src/Scope.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2529,6 +2529,22 @@ describe('Scope', () => {
expectZeroDiagnostics(program);
});

it('should find things in current namespace', () => {
program.setFile('source/utils.bs', `
namespace sgnode
sub speak(message)
print message
end sub
sub sayHello()
sgnode.speak("Hello")
end sub
end namespace`
);
program.validate();
expectZeroDiagnostics(program);
});

});

describe('const values', () => {
Expand Down
20 changes: 12 additions & 8 deletions src/Scope.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,8 @@ export class Scope {
enumStatements: new Map(),
constStatements: new Map(),
statements: [],
symbolTable: new SymbolTable(`Namespace Aggregate: '${loopName}'`, () => this.symbolTable)
// the aggregate symbol table should have no parent. It should include just the symbols of the namespace.
symbolTable: new SymbolTable(`Namespace Aggregate: '${loopName}'`)
});
}
}
Expand Down Expand Up @@ -790,7 +791,6 @@ export class Scope {
*/
public linkSymbolTable() {
SymbolTable.cacheVerifier.generateToken();

const allNamespaces: NamespaceStatement[] = [];
for (const file of this.getAllFiles()) {
if (isBrsFile(file)) {
Expand All @@ -800,7 +800,7 @@ export class Scope {
allNamespaces.push(...file.parser.references.namespaceStatements);
}
}

const aggregatesAdded = new Set<string>();
//Add namespace aggregates to namespace member tables
for (const namespace of allNamespaces) {
//link each NamespaceType member table with the aggregate NamespaceLookup SymbolTable
Expand Down Expand Up @@ -834,21 +834,25 @@ export class Scope {
: new NamespaceType(nameSoFar);
if (previousNSType) {
// adding as a member of existing NS
previousNSType.addMember(nsNamePart, namespace.range, currentNSType, getTypeOptions.flags);
previousNSType.addMember(nsNamePart, { definingNode: namespace }, currentNSType, getTypeOptions.flags);
} else {
symbolTable.addSymbol(nsNamePart, namespace.range, currentNSType, getTypeOptions.flags);
symbolTable.addSymbol(nsNamePart, { definingNode: namespace }, currentNSType, getTypeOptions.flags);
}
} else {
break;
}
}

// Now that the namespace type is built, add the aggregate as a sibling
const lowerNameSpace = nameSoFar.toLowerCase();
let aggregateNSSymbolTable = this.namespaceLookup.get(nameSoFar.toLowerCase()).symbolTable;
this.linkSymbolTableDisposables.push(
currentNSType.memberTable.addSibling(aggregateNSSymbolTable)
);

if (!aggregatesAdded.has(lowerNameSpace)) {
this.linkSymbolTableDisposables.push(
currentNSType.memberTable.addSibling(aggregateNSSymbolTable)
);
aggregatesAdded.add(lowerNameSpace);
}
if (isFinalNamespace) {
this.linkSymbolTableDisposables.push(
namespace.getSymbolTable().addSibling(aggregateNSSymbolTable)
Expand Down
74 changes: 42 additions & 32 deletions src/SymbolTable.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { Range } from 'vscode-languageserver';
import type { BscType } from './types/BscType';
import type { GetTypeOptions } from './interfaces';
import type { ExtraSymbolData, GetTypeOptions } from './interfaces';
import { CacheVerifier } from './CacheVerifier';
import type { ReferenceType } from './types/ReferenceType';
import type { UnionType } from './types/UnionType';
Expand Down Expand Up @@ -37,7 +36,7 @@ export class SymbolTable implements SymbolTypeGetter {

private cacheToken: string;

private typeCache: Array<Map<string, BscType>>;
private typeCache: Array<Map<string, TypeCacheEntry>>;

/**
* Used to invalidate the cache for all symbol tables.
Expand Down Expand Up @@ -165,52 +164,52 @@ export class SymbolTable implements SymbolTypeGetter {
/**
* Adds a new symbol to the table
*/
addSymbol(name: string, range: Range, type: BscType, bitFlags: SymbolTypeFlag) {
addSymbol(name: string, data: ExtraSymbolData, type: BscType, bitFlags: SymbolTypeFlag) {
const key = name.toLowerCase();
if (!this.symbolMap.has(key)) {
this.symbolMap.set(key, []);
}
this.symbolMap.get(key).push({
name: name,
range: range,
data: data,
type: type,
flags: bitFlags
});
}

public getSymbolTypes(name: string, bitFlags: SymbolTypeFlag): BscType[] {
const symbolArray = this.getSymbol(name, bitFlags);
public getSymbolTypes(name: string, options: GetSymbolTypeOptions): TypeCacheEntry[] {
const symbolArray = this.getSymbol(name, options.flags);
if (!symbolArray) {
return undefined;
}
return symbolArray.map(symbol => symbol.type);
return symbolArray?.map(symbol => ({ type: symbol.type, data: symbol.data }));
}

getSymbolType(name: string, options: GetSymbolTypeOptions): BscType {
let resolvedType = this.getCachedType(name, options);
const cacheEntry = this.getCachedType(name, options);
let resolvedType = cacheEntry?.type;
const doSetCache = !resolvedType;
const originalIsReferenceType = isAnyReferenceType(resolvedType);
let data = {} as ExtraSymbolData;
if (!resolvedType || originalIsReferenceType) {
resolvedType = getUniqueType(this.getSymbolTypes(name, options.flags), SymbolTable.unionTypeFactory);
const symbolTypes = this.getSymbolTypes(name, options);
data = symbolTypes?.[0]?.data;
resolvedType = getUniqueType(symbolTypes?.map(symbol => symbol.type), SymbolTable.unionTypeFactory);
}
if (!resolvedType && options.fullName && options.tableProvider) {
resolvedType = SymbolTable.referenceTypeFactory(name, options.fullName, options.flags, options.tableProvider);
}
const newNonReferenceType = originalIsReferenceType && !isAnyReferenceType(resolvedType);
if (doSetCache || newNonReferenceType || resolvedType) {
this.setCachedType(name, resolvedType, options);
this.setCachedType(name, { type: resolvedType, data: data }, options);
}
return resolvedType;
}

setSymbolTypeCache(name: string, resolvedType: BscType, options: GetSymbolTypeOptions) {
if (resolvedType) {
this.setCachedType(name, resolvedType, options);
if (options.data) {
options.data.definingNode = data?.definingNode;
options.data.description = data?.description;
}
return resolvedType;
}


/**
* Adds all the symbols from another table to this one
* It will overwrite any existing symbols in this table
Expand All @@ -220,7 +219,7 @@ export class SymbolTable implements SymbolTypeGetter {
for (const symbol of value) {
this.addSymbol(
symbol.name,
symbol.range,
symbol.data,
symbol.type,
symbol.flags
);
Expand All @@ -231,15 +230,21 @@ export class SymbolTable implements SymbolTypeGetter {
/**
* Get list of symbols declared directly in this SymbolTable (excludes parent SymbolTable).
*/
public getOwnSymbols(): BscSymbol[] {
return [].concat(...this.symbolMap.values());
public getOwnSymbols(bitFlags: SymbolTypeFlag): BscSymbol[] {
let symbols: BscSymbol[] = [].concat(...this.symbolMap.values());
// eslint-disable-next-line no-bitwise
symbols = symbols.filter(symbol => symbol.flags & bitFlags);
if (this.parent) {
symbols = symbols.concat(this.parent.getOwnSymbols(bitFlags));
}
return symbols;
}

/**
* Get list of all symbols declared in this SymbolTable (includes parent SymbolTable).
*/
public getAllSymbols(bitFlags: SymbolTypeFlag): BscSymbol[] {
let symbols = this.getOwnSymbols();
let symbols = [].concat(...this.symbolMap.values());
//look through any sibling maps next
for (let sibling of this.siblings) {
symbols = symbols.concat(sibling.getAllSymbols(bitFlags));
Expand All @@ -254,14 +259,14 @@ export class SymbolTable implements SymbolTypeGetter {
private resetTypeCache() {
this.typeCache = [
undefined,
new Map<string, BscType>(), //SymbolTypeFlags.runtime
new Map<string, BscType>(), //SymbolTypeFlags.typetime
new Map<string, BscType>() //SymbolTypeFlags.runtime & SymbolTypeFlags.typetime
new Map<string, TypeCacheEntry>(), // SymbolTypeFlags.runtime
new Map<string, TypeCacheEntry>(), // SymbolTypeFlags.typetime
new Map<string, TypeCacheEntry>() // SymbolTypeFlags.runtime & SymbolTypeFlags.typetime
];
this.cacheToken = SymbolTable.cacheVerifier?.getToken();
}

getCachedType(name: string, options: GetTypeOptions): BscType {
getCachedType(name: string, options: GetTypeOptions): TypeCacheEntry {
if (SymbolTable.cacheVerifier) {
if (!SymbolTable.cacheVerifier?.checkToken(this.cacheToken)) {
// we have a bad token
Expand All @@ -275,8 +280,8 @@ export class SymbolTable implements SymbolTypeGetter {
return this.typeCache[options.flags]?.get(name.toLowerCase());
}

setCachedType(name: string, type: BscType, options: GetTypeOptions) {
if (!type) {
setCachedType(name: string, cacheEntry: TypeCacheEntry, options: GetTypeOptions) {
if (!cacheEntry) {
return;
}
if (SymbolTable.cacheVerifier) {
Expand All @@ -289,11 +294,11 @@ export class SymbolTable implements SymbolTypeGetter {
return;
}
let existingCachedValue = this.typeCache[options.flags]?.get(name.toLowerCase());
if (isReferenceType(type) && !isReferenceType(existingCachedValue)) {
if (isReferenceType(cacheEntry.type) && !isReferenceType(existingCachedValue)) {
// No need to overwrite a non-referenceType with a referenceType
return;
}
return this.typeCache[options.flags]?.set(name.toLowerCase(), type);
return this.typeCache[options.flags]?.set(name.toLowerCase(), cacheEntry);
}

/**
Expand All @@ -319,14 +324,14 @@ export class SymbolTable implements SymbolTypeGetter {

export interface BscSymbol {
name: string;
range: Range;
data: ExtraSymbolData;
type: BscType;
flags: SymbolTypeFlag;
}

export interface SymbolTypeGetter {
getSymbolType(name: string, options: GetSymbolTypeOptions): BscType;
setCachedType(name: string, resolvedType: BscType, options: GetSymbolTypeOptions);
setCachedType(name: string, cacheEntry: TypeCacheEntry, options: GetSymbolTypeOptions);
}

/**
Expand All @@ -344,3 +349,8 @@ export interface GetSymbolTypeOptions extends GetTypeOptions {
fullName?: string;
tableProvider?: SymbolTableProvider;
}

export interface TypeCacheEntry {
type: BscType;
data?: ExtraSymbolData;
}
Loading

0 comments on commit 69d3037

Please sign in to comment.