Skip to content
This repository was archived by the owner on Oct 16, 2020. It is now read-only.

Switch to resolving expensive completionItem details #344

Merged
merged 9 commits into from
Sep 8, 2017
Merged

Switch to resolving expensive completionItem details #344

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a74c79e
fix: Move lookup of completion details to completionItem/resolve
tomv564 Sep 4, 2017
47b8bb0
fix: lint errors
tomv564 Sep 4, 2017
afa33f9
fix: Restrict CompletionItem.data, restore full test cases for resolv…
tomv564 Sep 4, 2017
ba9de9f
fix: Restore unresolved completion test with snippets
tomv564 Sep 5, 2017
90a6515
fix: Clean commented out code + unneeded documentation
tomv564 Sep 6, 2017
7fc166e
Merge branch 'master' into resolve-completions
felixfbecker Sep 6, 2017
5bfe836
refactor: use rx
felixfbecker Sep 8, 2017
9b2f9ea
style: don't compare with null
felixfbecker Sep 8, 2017
f5913ac
Merge branch 'master' into resolve-completions
felixfbecker Sep 8, 2017
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
27 changes: 27 additions & 0 deletions src/request-type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,30 @@ export interface PartialResultParams {
*/
patch: Operation[];
}

/**
* Restriction on vscode's CompletionItem interface
*/
export interface CompletionItem extends vscode.CompletionItem {
data?: CompletionItemData;
}

/**
* The necessary fields for a completion item details to be resolved by typescript
*/
export interface CompletionItemData {
/**
* The document from which the completion was requested
*/
uri: string;

/**
* The offset into the document at which the completion was requested
*/
offset: number;

/**
* The name field from typescript's returned completion entry
*/
entryName: string;
}
176 changes: 156 additions & 20 deletions src/test/typescript-service-helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { CompletionItemKind, CompletionList, DiagnosticSeverity, InsertTextForma
import { Command, Diagnostic, Hover, Location, SignatureHelp, SymbolInformation, SymbolKind } from 'vscode-languageserver-types';
import { LanguageClient, RemoteLanguageClient } from '../lang-handler';
import { DependencyReference, PackageInformation, ReferenceInformation, TextDocumentContentParams, WorkspaceFilesParams } from '../request-type';
import { ClientCapabilities, SymbolLocationInformation } from '../request-type';
import { ClientCapabilities, CompletionItem, SymbolLocationInformation } from '../request-type';
import { TypeScriptService, TypeScriptServiceFactory } from '../typescript-service';
import { observableFromIterable, toUnixPath, uri2path } from '../util';

Expand Down Expand Up @@ -2154,7 +2154,62 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor

afterEach(shutdownService);

it('should produce completions with snippets if supported', async function (this: TestContext & ITestCallbackContext) {
it('should produce completions', async function (this: TestContext & ITestCallbackContext) {
const result: CompletionList = await this.service.textDocumentCompletion({
textDocument: {
uri: rootUri + 'a.ts'
},
position: {
line: 11,
character: 2
}
}).reduce<Operation, CompletionList>(applyReducer, null as any).toPromise();
assert.equal(result.isIncomplete, false);
assert.sameDeepMembers(result.items, [
{
label: 'bar',
kind: CompletionItemKind.Method,
sortText: '0',
data: {
entryName: 'bar',
offset: 210,
uri: rootUri + 'a.ts'
}
},
{
label: 'baz',
kind: CompletionItemKind.Method,
sortText: '0',
data: {
entryName: 'baz',
offset: 210,
uri: rootUri + 'a.ts'
}
},
{
label: 'foo',
kind: CompletionItemKind.Method,
sortText: '0',
data: {
entryName: 'foo',
offset: 210,
uri: rootUri + 'a.ts'
}
},
{
label: 'qux',
kind: CompletionItemKind.Property,
sortText: '0',
data: {
entryName: 'qux',
offset: 210,
uri: rootUri + 'a.ts'
}
}
]);
});

it('should resolve completions with snippets', async function (this: TestContext & ITestCallbackContext) {
const result: CompletionList = await this.service.textDocumentCompletion({
textDocument: {
uri: rootUri + 'a.ts'
Expand All @@ -2169,15 +2224,23 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
// * the end of the snippet. Placeholders with equal identifiers are linked,
// * that is typing in one will update others too.
assert.equal(result.isIncomplete, false);
assert.sameDeepMembers(result.items, [

const resolveItem = (item: CompletionItem) => this.service
.completionItemResolve(item)
.reduce<Operation, CompletionItem>(applyReducer, null as any).toPromise();

const resolvedItems = await Promise.all(result.items.map(resolveItem));

assert.sameDeepMembers(resolvedItems, [
{
label: 'bar',
kind: CompletionItemKind.Method,
documentation: 'bar doc',
sortText: '0',
insertTextFormat: InsertTextFormat.Snippet,
insertText: 'bar(${1:num})',
detail: '(method) A.bar(num: number): number'
detail: '(method) A.bar(num: number): number',
data: undefined
},
{
label: 'baz',
Expand All @@ -2186,7 +2249,8 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
sortText: '0',
insertTextFormat: InsertTextFormat.Snippet,
insertText: 'baz(${1:num})',
detail: '(method) A.baz(num: number): string'
detail: '(method) A.baz(num: number): string',
data: undefined
},
{
label: 'foo',
Expand All @@ -2195,7 +2259,8 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
sortText: '0',
insertTextFormat: InsertTextFormat.Snippet,
insertText: 'foo()',
detail: '(method) A.foo(): void'
detail: '(method) A.foo(): void',
data: undefined
},
{
label: 'qux',
Expand All @@ -2204,9 +2269,11 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
sortText: '0',
insertTextFormat: InsertTextFormat.Snippet,
insertText: 'qux',
detail: '(property) A.qux: number'
detail: '(property) A.qux: number',
data: undefined
}
]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this assertion removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The snippets logic is only used when resolving completion items, and I thought it would be a lot of logic to resolve all the items from the initial response.
Now that I think of it, it's probably not that bad - I'll give it a try!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave this test as is (just change the objects) and add a separate test for the resolving


});
});

Expand Down Expand Up @@ -2258,14 +2325,77 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
}).reduce<Operation, CompletionList>(applyReducer, null as any).toPromise();
assert.equal(result.isIncomplete, false);
assert.sameDeepMembers(result.items, [
{
data: {
entryName: 'bar',
offset: 188,
uri: rootUri + 'a.ts'
},
label: 'bar',
kind: CompletionItemKind.Method,
sortText: '0'
},
{
data: {
entryName: 'baz',
offset: 188,
uri: rootUri + 'a.ts'
},
label: 'baz',
kind: CompletionItemKind.Method,
sortText: '0'
},
{
data: {
entryName: 'foo',
offset: 188,
uri: rootUri + 'a.ts'
},
label: 'foo',
kind: CompletionItemKind.Method,
sortText: '0'
},
{
data: {
entryName: 'qux',
offset: 188,
uri: rootUri + 'a.ts'
},
label: 'qux',
kind: CompletionItemKind.Property,
sortText: '0'
}
]);
});

it('resolves completions in the same file', async function (this: TestContext & ITestCallbackContext) {
const result: CompletionList = await this.service.textDocumentCompletion({
textDocument: {
uri: rootUri + 'a.ts'
},
position: {
line: 11,
character: 2
}
}).reduce<Operation, CompletionList>(applyReducer, null as any).toPromise();
assert.equal(result.isIncomplete, false);

const resolveItem = (item: CompletionItem) => this.service
.completionItemResolve(item)
.reduce<Operation, CompletionItem>(applyReducer, null as any).toPromise();

const resolvedItems = await Promise.all(result.items.map(resolveItem));

assert.sameDeepMembers(resolvedItems, [
{
label: 'bar',
kind: CompletionItemKind.Method,
documentation: 'bar doc',
insertText: 'bar',
insertTextFormat: InsertTextFormat.PlainText,
sortText: '0',
detail: '(method) A.bar(): number'
detail: '(method) A.bar(): number',
data: undefined
},
{
label: 'baz',
Expand All @@ -2274,7 +2404,8 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
insertText: 'baz',
insertTextFormat: InsertTextFormat.PlainText,
sortText: '0',
detail: '(method) A.baz(): string'
detail: '(method) A.baz(): string',
data: undefined
},
{
label: 'foo',
Expand All @@ -2283,7 +2414,8 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
insertText: 'foo',
insertTextFormat: InsertTextFormat.PlainText,
sortText: '0',
detail: '(method) A.foo(): void'
detail: '(method) A.foo(): void',
data: undefined
},
{
label: 'qux',
Expand All @@ -2292,9 +2424,11 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
insertText: 'qux',
insertTextFormat: InsertTextFormat.PlainText,
sortText: '0',
detail: '(property) A.qux: number'
detail: '(property) A.qux: number',
data: undefined
}
]);

});

it('produces completions for imported symbols', async function (this: TestContext & ITestCallbackContext) {
Expand All @@ -2310,12 +2444,13 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
assert.deepEqual(result, {
isIncomplete: false,
items: [{
data: {
entryName: 'd',
offset: 32,
uri: rootUri + 'uses-import.ts'
},
label: 'd',
kind: CompletionItemKind.Function,
documentation: 'd doc',
insertText: 'd',
insertTextFormat: InsertTextFormat.PlainText,
detail: 'function d(): void',
sortText: '0'
}]
});
Expand All @@ -2333,13 +2468,14 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor
assert.deepEqual(result, {
isIncomplete: false,
items: [{
data: {
entryName: 'bar',
offset: 51,
uri: rootUri + 'uses-reference.ts'
},
label: 'bar',
kind: CompletionItemKind.Interface,
documentation: 'bar doc',
insertText: 'bar',
insertTextFormat: InsertTextFormat.PlainText,
sortText: '0',
detail: 'interface foo.bar'
sortText: '0'
}]
});
});
Expand Down
Loading