Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes completions when file in multiple scopes #1238

Merged
merged 2 commits into from
Jun 25, 2024
Merged

Fixes completions when file in multiple scopes #1238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
627a20e
Fixes completions when file in multiple scopes
markwpearce Jun 24, 2024
202e420
Adds log timing message for completions, moves global completions out…
markwpearce Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions src/bscPlugin/completions/CompletionsProcessor.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ describe('CompletionsProcessor', () => {
expect(
// message = alpha|
processor['getBrsFileCompletions'](util.createPosition(2, 39), file)
).to.eql([]);
).to.eql({ global: [], scoped: [] });
});

it('does not crash when functionExpression.body is undefined (somehow...)', () => {
Expand All @@ -718,7 +718,7 @@ describe('CompletionsProcessor', () => {
expect(
// message = alpha|
processor['getBrsFileCompletions'](util.createPosition(2, 39), file)
).to.eql([]);
).to.eql({ global: [], scoped: [] });
});
});

Expand Down Expand Up @@ -791,7 +791,7 @@ describe('CompletionsProcessor', () => {
});
program.validate();
//get the name of all global completions
const globalCompletions = program.globalScope.getAllFiles().flatMap(x => completionProcessor.getBrsFileCompletions(position, x as BrsFile)).map(x => x.label);
const globalCompletions = program.globalScope.getAllFiles().flatMap(x => completionProcessor.getBrsFileCompletions(position, x as BrsFile).global).map(x => x.label);
//filter out completions from global scope
completions = completions.filter(x => !globalCompletions.includes(x.label));
expect(completions).to.be.empty;
Expand All @@ -811,6 +811,31 @@ describe('CompletionsProcessor', () => {

expect(labels).to.deep.include({ label: 'count' });
});

it('finds parameters when file is in multiple scopes', () => {
program.setFile('source/main.bs', `
sub Main(count = 1)
firstName = "bob"
age = 21
shoeSize = 10
end sub
`);

program.setFile<XmlFile>('components/MyNode.xml',
trim`<?xml version="1.0" encoding="utf-8" ?>
<component name="Component2" extends="Component1">
<script type="text/brightscript" uri="pkg:/source/main.bs" />
<interface>
<function name="sayHello3"/>
<function name="sayHello4"/>
</interface>
</component>`);
program.validate();
let completions = program.getCompletions(`${rootDir}/source/main.bs`, Position.create(2, 10));
let labels = completions.map(x => pick(x, 'label'));

expect(labels).to.deep.include({ label: 'count' });
});
});
});

Expand Down
134 changes: 70 additions & 64 deletions src/bscPlugin/completions/CompletionsProcessor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type { ClassStatement, FunctionStatement, NamespaceStatement, AliasStatem
import type { Token } from '../../lexer/Token';
import { createIdentifier } from '../../astUtils/creators';
import type { FunctionExpression } from '../../parser/Expression';
import { LogLevel } from '../../Logger';

export class CompletionsProcessor {
constructor(
Expand All @@ -32,38 +33,45 @@ export class CompletionsProcessor {

public process() {
let file = this.event.file;

//find the scopes for this file
let scopesForFile = this.event.program.getScopesForFile(file);

//if there are no scopes, include the global scope so we at least get the built-in functions
scopesForFile = scopesForFile.length > 0 ? scopesForFile : [this.event.program.globalScope];

//get the completions from all scopes for this file
let completionResults: CompletionItem[] = [];
if (isXmlFile(file)) {
completionResults = this.getXmlFileCompletions(this.event.position, file);
} else if (isBrsFile(file)) {
//handle script import completions
let scriptImport = util.getScriptImportAtPosition(file.ownScriptImports, this.event.position);
if (scriptImport) {
this.event.completions.push(...this.getScriptImportCompletions(file.program, file.pkgPath, scriptImport));
return;
this.event.program.logger.time(LogLevel.log, ['Processing completions'], () => {
//find the scopes for this file
let scopesForFile = this.event.program.getScopesForFile(file);

//if there are no scopes, include the global scope so we at least get the built-in functions
scopesForFile = scopesForFile.length > 0 ? scopesForFile : [this.event.program.globalScope];

//get the completions from all scopes for this file
let completionResults: CompletionItem[] = [];
let globalResults: CompletionItem[] = [];

if (isXmlFile(file)) {
completionResults = this.getXmlFileCompletions(this.event.position, file);
} else if (isBrsFile(file)) {
//handle script import completions
let scriptImport = util.getScriptImportAtPosition(file.ownScriptImports, this.event.position);
if (scriptImport) {
this.event.completions.push(...this.getScriptImportCompletions(file.program, file.pkgPath, scriptImport));
return;
}
const results = this.getBrsFileCompletions(this.event.position, file);
completionResults = results.scoped;
globalResults = results.global;
}
completionResults = this.getBrsFileCompletions(this.event.position, file);
}

let allCompletions = completionResults.flat();
this.event.completions.push(...globalResults);

//only keep completions common to every scope for this file
let keyCounts = new Map<string, number>();
for (let completion of allCompletions) {
let key = `${completion.label}-${completion.kind}`;
keyCounts.set(key, keyCounts.has(key) ? keyCounts.get(key) + 1 : 1);
if (keyCounts.get(key) === scopesForFile.length) {
this.event.completions.push(completion);
let allCompletions = completionResults.flat();

//only keep completions common to every scope for this file
let keyCounts = new Map<string, number>();
for (let completion of allCompletions) {
let key = `${completion.label}-${completion.kind}`;
keyCounts.set(key, keyCounts.has(key) ? keyCounts.get(key) + 1 : 1);
if (keyCounts.get(key) === scopesForFile.length) {
this.event.completions.push(completion);
}
}
}
});
}


Expand Down Expand Up @@ -143,18 +151,21 @@ export class CompletionsProcessor {
/**
* Get completions available at the given cursor. This aggregates all values from this file and the current scope.
*/
public getBrsFileCompletions(position: Position, file: BrsFile): CompletionItem[] {
public getBrsFileCompletions(position: Position, file: BrsFile): { global: CompletionItem[]; scoped: CompletionItem[] } {
let result = [] as CompletionItem[];
const currentTokenByFilePosition = file.getTokenAt(position);
const currentToken = currentTokenByFilePosition ?? file.getTokenAt(file.getClosestExpression(position)?.location?.range.start);

const emptyResult = { global: [], scoped: [] };

if (!currentToken) {
return [];
return emptyResult;
}
const tokenKind = currentToken?.kind;

//if cursor is after a comment, disable completions
if (this.isPostionInComment(file, position)) {
return [];
return emptyResult;
}

let expression: AstNode;
Expand All @@ -165,7 +176,7 @@ export class CompletionsProcessor {

if (file.tokenFollows(currentToken, TokenKind.Goto)) {
let functionScope = file.getFunctionScopeAtPosition(position);
return this.getLabelCompletion(functionScope);
return { global: [], scoped: this.getLabelCompletion(functionScope) };
}

if (this.isTokenAdjacentTo(file, currentToken, TokenKind.Dot)) {
Expand All @@ -180,7 +191,7 @@ export class CompletionsProcessor {
shouldLookForCallFuncMembers = true;
} else if (this.isTokenAdjacentTo(file, currentToken, TokenKind.As)) {
if (file.parseMode === ParseMode.BrightScript) {
return NativeTypeCompletions;
return { global: NativeTypeCompletions, scoped: [] };
}
expression = file.getClosestExpression(this.event.position);
symbolTableLookupFlag = SymbolTypeFlag.typetime;
Expand All @@ -201,7 +212,7 @@ export class CompletionsProcessor {
}

if (!expression) {
return [];
return emptyResult;
}

const tokenBefore = file.getTokenBefore(file.getClosestToken(expression.location?.range?.start));
Expand Down Expand Up @@ -234,14 +245,25 @@ export class CompletionsProcessor {
return symbolTableToUse;
}

let gotSymbolsFromThisFile = false;
let gotSymbolsFromGlobal = false;
const shouldLookInNamespace: NamespaceStatement = !(shouldLookForMembers || shouldLookForCallFuncMembers) && expression.findAncestor(isNamespaceStatement);
const areMembers = (shouldLookForMembers || shouldLookForCallFuncMembers);
const notMembers = !areMembers;
const isAfterNew = tokenBefore?.kind === TokenKind.New;
const shouldLookInNamespace: NamespaceStatement = notMembers && expression.findAncestor(isNamespaceStatement);

const containingClassStmt = expression.findAncestor<ClassStatement>(isClassStatement);
const containingNamespace = expression.findAncestor<NamespaceStatement>(isNamespaceStatement);
const containingNamespaceName = containingNamespace?.getName(ParseMode.BrighterScript);
const containingFunctionExpression = expression.findAncestor<FunctionExpression>(isFunctionExpression);
const tokenIsLiteralString = (tokenKind === TokenKind.StringLiteral || tokenKind === TokenKind.TemplateStringQuasi);

const globalCompletions: CompletionItem[] = [];
if (!tokenIsLiteralString && notMembers && !isAfterNew && this.event.program?.globalScope) {
let globalSymbols: BscSymbol[] = this.event.program.globalScope.symbolTable.getOwnSymbols(symbolTableLookupFlag) ?? [];
if (symbolTableLookupFlag === SymbolTypeFlag.runtime) {
globalSymbols.push(...this.getGlobalValues());
}
globalCompletions.push(...this.getSymbolsCompletion(globalSymbols));
}

for (const scope of this.event.scopes) {
if (tokenKind === TokenKind.StringLiteral || tokenKind === TokenKind.TemplateStringQuasi) {
Expand All @@ -252,7 +274,7 @@ export class CompletionsProcessor {
const symbolTable = getSymbolTableForLookups();
let currentSymbols: BscSymbol[] = [];

if (shouldLookForMembers || shouldLookForCallFuncMembers) {
if (areMembers) {
currentSymbols = symbolTable?.getAllSymbols(symbolTableLookupFlag) ?? [];
const tokenType = expression.getType({ flags: SymbolTypeFlag.runtime });
if (isClassType(tokenType)) {
Expand All @@ -265,15 +287,12 @@ export class CompletionsProcessor {
});
}
} else {
// get symbols directly from current symbol table and scope
if (!gotSymbolsFromThisFile) {
currentSymbols = symbolTable?.getOwnSymbols(symbolTableLookupFlag) ?? [];
currentSymbols = symbolTable?.getOwnSymbols(symbolTableLookupFlag) ?? [];

if (containingFunctionExpression) {
currentSymbols.push(...containingFunctionExpression.getSymbolTable().getOwnSymbols(symbolTableLookupFlag));
}
gotSymbolsFromThisFile = true;
if (containingFunctionExpression) {
currentSymbols.push(...containingFunctionExpression.getSymbolTable().getOwnSymbols(symbolTableLookupFlag));
}

if (shouldLookInNamespace) {
const nsNameParts = shouldLookInNamespace.getNameParts();
let nameSpaceTypeSofar: BscType;
Expand All @@ -290,31 +309,18 @@ export class CompletionsProcessor {
currentSymbols.push(...nameSpaceTypeSofar.getMemberTable().getAllSymbols(symbolTableLookupFlag));
}
}

currentSymbols.push(...this.getScopeSymbolCompletions(file, scope, symbolTableLookupFlag));

// get global symbols
if (!gotSymbolsFromGlobal) {
currentSymbols.push(...this.event.program.globalScope.symbolTable.getOwnSymbols(symbolTableLookupFlag));
if (symbolTableLookupFlag === SymbolTypeFlag.runtime) {
currentSymbols.push(...this.getGlobalValues());
}
gotSymbolsFromGlobal = true;
}
}

let ignoreAllPropertyNames = false;

// eslint-disable-next-line @typescript-eslint/switch-exhaustiveness-check
switch (tokenBefore?.kind) {
case TokenKind.New:
//we are after a new keyword; so we can only be namespaces that have a class or classes at this point
currentSymbols = currentSymbols.filter(symbol => isClassType(symbol.type) || this.isNamespaceTypeWithMemberType(symbol.type, isClassType));
ignoreAllPropertyNames = true;
break;
if (isAfterNew) {
//we are after a new keyword; so we can only be namespaces that have a class or classes at this point
currentSymbols = currentSymbols.filter(symbol => isClassType(symbol.type) || this.isNamespaceTypeWithMemberType(symbol.type, isClassType));
ignoreAllPropertyNames = true;
}

result.push(...this.getSymbolsCompletion(currentSymbols, shouldLookForMembers || shouldLookForCallFuncMembers));
result.push(...this.getSymbolsCompletion(currentSymbols, areMembers));
if (shouldLookForMembers && currentSymbols.length === 0 && !ignoreAllPropertyNames) {
// could not find members of actual known types.. just try everything
result.push(...this.getPropertyNameCompletions(scope),
Expand All @@ -325,7 +331,7 @@ export class CompletionsProcessor {
}
scope.unlinkSymbolTable();
}
return result;
return { global: globalCompletions, scoped: result };
}

private getScopeSymbolCompletions(file: BrsFile, scope: Scope, symbolTableLookupFlag: SymbolTypeFlag) {
Expand Down