Skip to content

Commit

Permalink
Finally fixed the completions bug
Browse files Browse the repository at this point in the history
  • Loading branch information
TwitchBronBron committed Jun 13, 2024
1 parent 15c7d15 commit 963bcf0
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 10 deletions.
12 changes: 8 additions & 4 deletions src/LanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ export class LanguageServer {

@AddStackToErrorMessage
private async onTextDocumentDidChangeContent(event: TextDocumentChangeEvent<TextDocument>) {
this.logger.log('onTextDocumentDidChangeContent', event.document.uri);
this.logger.debug('onTextDocumentDidChangeContent', event.document.uri);

await this.projectManager.handleFileChanges([{
srcPath: URI.parse(event.document.uri).fsPath,
Expand Down Expand Up @@ -400,10 +400,14 @@ export class LanguageServer {
* Provide a list of completion items based on the current cursor position
*/
@AddStackToErrorMessage
public async onCompletion(params: CompletionParams, token: CancellationToken, workDoneProgress: WorkDoneProgressReporter, resultProgress: ResultProgressReporter<CompletionItem[]>): Promise<CompletionList> {
this.logger.log('onCompletion', params, token);
public async onCompletion(params: CompletionParams, cancellationToken: CancellationToken, workDoneProgress: WorkDoneProgressReporter, resultProgress: ResultProgressReporter<CompletionItem[]>): Promise<CompletionList> {
this.logger.info('onCompletion', params, cancellationToken);
const srcPath = util.uriToPath(params.textDocument.uri);
const completions = await this.projectManager.getCompletions({ srcPath: srcPath, position: params.position });
const completions = await this.projectManager.getCompletions({
srcPath: srcPath,
position: params.position,
cancellationToken: cancellationToken
});
return completions;
}

Expand Down
18 changes: 18 additions & 0 deletions src/Program.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,24 @@ describe('Program', () => {
}]);
});

it('finds enum member after dot in if statement', () => {
program.setFile('source/main.bs', `
sub test()
if alpha.beta. then
end if
end sub
namespace alpha.beta
const isEnabled = true
end namespace
`);
program.validate();
const completions = program.getCompletions(`${rootDir}/source/main.bs`, Position.create(2, 34));
expect(completions.map(x => ({ kind: x.kind, label: x.label }))).to.eql([{
label: 'isEnabled',
kind: CompletionItemKind.Constant
}]);
});

it('includes `for` variable', () => {
program.setFile('source/main.brs', `
sub main()
Expand Down
35 changes: 33 additions & 2 deletions src/lsp/DocumentManager.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { expect } from 'chai';
import util from '../util';
import type { DocumentAction } from './DocumentManager';
import { util, standardizePath as s } from '../util';
import type { DocumentAction, SetDocumentAction } from './DocumentManager';
import { DocumentManager } from './DocumentManager';
import { rootDir } from '../testHelpers.spec';

describe('DocumentManager', () => {
let manager: DocumentManager;
Expand Down Expand Up @@ -36,6 +37,36 @@ describe('DocumentManager', () => {
}]);
});

it('does not lose newly added that arrives during a flush operation', async () => {
const srcPath = s`${rootDir}/source/main.bs`;

let contentsQueue = [
'two',
'three',
'four'
];
manager = new DocumentManager({
delay: 5,
flushHandler: (event) => {
//once the flush happens, add NEW data to the queue. this is the data we need to ensure we don't lose
if (contentsQueue.length > 0) {
manager.set({ srcPath: srcPath, fileContents: contentsQueue.shift() });
}

//store the actions
results.push(...event.actions);
}
});
manager.set({ srcPath: srcPath, fileContents: 'one' });
await manager.onIdle();
expect(results.map(x => (x as SetDocumentAction).fileContents)).to.eql([
'one',
'two',
'three',
'four'
]);
});

it('any file change delays the first one', async () => {
manager.set({ srcPath: 'alpha', fileContents: 'one' });
await util.sleep(1);
Expand Down
2 changes: 1 addition & 1 deletion src/lsp/DocumentManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ export class DocumentManager {
const event: FlushEvent = {
actions: [...this.queue.values()]
};
await this.options.flushHandler?.(event);
this.queue.clear();
await this.options.flushHandler?.(event);
} catch (e) {
console.error(e);
}
Expand Down
10 changes: 10 additions & 0 deletions src/lsp/Project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,10 @@ export class Project implements LspProject {
* This will cancel any pending validation cycles and queue a future validation cycle instead.
*/
public async applyFileChanges(documentActions: DocumentAction[]): Promise<DocumentActionWithStatus[]> {
this.logger.debug('project.applyFileChanges', documentActions.map(x => x.srcPath));

await this.onIdle();

let didChangeFiles = false;
const result = [...documentActions] as DocumentActionWithStatus[];
// eslint-disable-next-line @typescript-eslint/prefer-for-of
Expand Down Expand Up @@ -251,6 +254,9 @@ export class Project implements LspProject {
//trigger a validation (but don't wait for it. That way we can cancel it sooner if we get new incoming data or requests)
void this.validate();
}

this.logger.debug('project.applyFileChanges done', documentActions.map(x => x.srcPath));

return result;
}

Expand Down Expand Up @@ -383,6 +389,7 @@ export class Project implements LspProject {

public async getCodeActions(options: { srcPath: string; range: Range }) {
await this.onIdle();

if (this.builder.program.hasFile(options.srcPath)) {
const codeActions = this.builder.program.getCodeActions(options.srcPath, options.range);
//clone each diagnostic since certain diagnostics can have circular reference properties that kill the language server if serialized
Expand All @@ -397,6 +404,9 @@ export class Project implements LspProject {

public async getCompletions(options: { srcPath: string; position: Position }): Promise<CompletionList> {
await this.onIdle();

this.logger.debug('project.getCompletions', options.srcPath, options.position);

if (this.builder.program.hasFile(options.srcPath)) {
const completions = this.builder.program.getCompletions(options.srcPath, options.position);
const result = CompletionList.create(completions);
Expand Down
19 changes: 16 additions & 3 deletions src/lsp/ProjectManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { LspDiagnostic, LspProject, ProjectConfig } from './LspProject';
import { Project } from './Project';
import { WorkerThreadProject } from './worker/WorkerThreadProject';
import { FileChangeType } from 'vscode-languageserver-protocol';
import type { Hover, Position, Range, Location, SignatureHelp, DocumentSymbol, SymbolInformation, WorkspaceSymbol, CompletionList } from 'vscode-languageserver-protocol';
import type { Hover, Position, Range, Location, SignatureHelp, DocumentSymbol, SymbolInformation, WorkspaceSymbol, CompletionList, CancellationToken } from 'vscode-languageserver-protocol';
import { Deferred } from '../deferred';
import type { DocumentActionWithStatus, FlushEvent } from './DocumentManager';
import { DocumentManager } from './DocumentManager';
Expand Down Expand Up @@ -73,6 +73,7 @@ export class ProjectManager {
@TrackBusyStatus
private async flushDocumentChanges(event: FlushEvent) {
this.logger.log('flushDocumentChanges', event.actions.map(x => x.srcPath));

//ensure that we're fully initialized before proceeding
await this.onInitialized();

Expand Down Expand Up @@ -102,7 +103,9 @@ export class ProjectManager {
const projectActions = actions.filter(action => {
return action.type === 'delete' || filterer.isMatch(action.srcPath);
});
return project.applyFileChanges(projectActions);
if (projectActions.length > 0) {
return project.applyFileChanges(projectActions);
}
}));

//create standalone projects for any files not handled by any project
Expand Down Expand Up @@ -189,13 +192,17 @@ export class ProjectManager {
//There are race conditions where the fileChangesQueue will become idle, but that causes the documentManager
//to start a new flush. So we must keep waiting until everything is idle
while (!this.documentManager.isIdle || !this.fileChangesQueue.isIdle) {
this.logger.debug('onIdle', { documentManagerIdle: this.documentManager.isIdle, fileChangesQueueIdle: this.fileChangesQueue.isIdle });

await Promise.allSettled([
//make sure all pending file changes have been flushed
this.documentManager.onIdle(),
//wait for the file changes queue to be idle
this.fileChangesQueue.onIdle()
]);
}

this.logger.info('onIdle debug', { documentManagerIdle: this.documentManager.isIdle, fileChangesQueueIdle: this.fileChangesQueue.isIdle });
}

/**
Expand Down Expand Up @@ -398,9 +405,15 @@ export class ProjectManager {
* Get the completions for the given position in the file
*/
@TrackBusyStatus
public async getCompletions(options: { srcPath: string; position: Position }): Promise<CompletionList> {
public async getCompletions(options: { srcPath: string; position: Position; cancellationToken?: CancellationToken }): Promise<CompletionList> {
await this.onIdle();

//if the request has been cancelled since originall requested due to idle time being slow, skip the rest of the wor
if (options?.cancellationToken?.isCancellationRequested) {
this.logger.log('ProjectManager getCompletions cancelled', options);
return;
}

this.logger.log('ProjectManager getCompletions', options);
//Ask every project for results, keep whichever one responds first that has a valid response
let result = await util.promiseRaceMatch(
Expand Down

0 comments on commit 963bcf0

Please sign in to comment.