From fa55f77784139408f7846d2005284cae01e27934 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Thu, 8 Jun 2017 07:54:38 +0200 Subject: [PATCH 1/9] Minimal snippet support --- src/test/typescript-service-helpers.ts | 102 +++++++++++++++++++++++-- src/typescript-service.ts | 30 ++++++++ 2 files changed, 125 insertions(+), 7 deletions(-) diff --git a/src/test/typescript-service-helpers.ts b/src/test/typescript-service-helpers.ts index 81b185a17..5f3b310a2 100644 --- a/src/test/typescript-service-helpers.ts +++ b/src/test/typescript-service-helpers.ts @@ -1,11 +1,11 @@ import * as chai from 'chai'; import * as sinon from 'sinon'; import * as ts from 'typescript'; -import { CompletionItemKind, CompletionList, DiagnosticSeverity, TextDocumentIdentifier, TextDocumentItem, WorkspaceEdit } from 'vscode-languageserver'; +import { CompletionItemKind, CompletionList, DiagnosticSeverity, InsertTextFormat, TextDocumentIdentifier, TextDocumentItem, WorkspaceEdit } from 'vscode-languageserver'; 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 { SymbolLocationInformation } from '../request-type'; +import { ClientCapabilities, SymbolLocationInformation } from '../request-type'; import { TypeScriptService, TypeScriptServiceFactory } from '../typescript-service'; import { observableFromIterable, toUnixPath, uri2path } from '../util'; import chaiAsPromised = require('chai-as-promised'); @@ -16,6 +16,11 @@ import { IBeforeAndAfterContext, ISuiteCallbackContext, ITestCallbackContext } f chai.use(chaiAsPromised); const assert = chai.assert; +const defaultCapabilities: ClientCapabilities = { + xcontentProvider: true, + xfilesProvider: true +}; + export interface TestContext { /** TypeScript service under test */ @@ -31,7 +36,7 @@ export interface TestContext { * @param createService A factory that creates the TypeScript service. Allows to test subclasses of TypeScriptService * @param files A Map from URI to file content of files that should be available in the workspace */ -export const initializeTypeScriptService = (createService: TypeScriptServiceFactory, rootUri: string, files: Map) => async function (this: TestContext & IBeforeAndAfterContext): Promise { +export const initializeTypeScriptService = (createService: TypeScriptServiceFactory, rootUri: string, files: Map, clientCapabilities?: ClientCapabilities) => async function (this: TestContext & IBeforeAndAfterContext): Promise { // Stub client this.client = sinon.createStubInstance(RemoteLanguageClient); @@ -56,10 +61,7 @@ export const initializeTypeScriptService = (createService: TypeScriptServiceFact await this.service.initialize({ processId: process.pid, rootUri, - capabilities: { - xcontentProvider: true, - xfilesProvider: true - } + capabilities: clientCapabilities || defaultCapabilities }).toPromise(); }; @@ -2123,6 +2125,91 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor }); + describe.only('textDocumentCompletion() with snippets', function (this: TestContext & ISuiteCallbackContext){ + beforeEach(initializeTypeScriptService(createService, rootUri, new Map([ + [rootUri + 'a.ts', [ + 'class A {', + ' /** foo doc*/', + ' foo() {}', + ' /** bar doc*/', + ' bar(num: number): number { return 1; }', + ' /** baz doc*/', + ' baz(num: number): string { return ""; }', + ' /** qux doc*/', + ' qux: number;', + '}', + 'const a = new A();', + 'a.' + ].join('\n')] + ]), { + textDocument: { + completion: { + completionItem: { + snippetSupport: true + } + } + }, + ...defaultCapabilities + })); + + afterEach(shutdownService); + + it('shoudl produce completions with snippets if supported', async function (this: TestContext & ITestCallbackContext) { + const result: CompletionList = await this.service.textDocumentCompletion({ + textDocument: { + uri: rootUri + 'a.ts' + }, + position: { + line: 11, + character: 2 + } + }).reduce(jsonpatch.applyReducer, null as any).toPromise(); + // * A snippet can define tab stops and placeholders with `$1`, `$2` + // * and `${3:foo}`. `$0` defines the final tab stop, it defaults to + // * 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, [ + { + label: 'bar', + kind: CompletionItemKind.Method, + documentation: 'bar doc', + sortText: '0', + insertTextFormat: InsertTextFormat.Snippet, + insertText: 'bar(${1:num})', + detail: '(method) A.bar(num: number): number' + }, + { + label: 'baz', + kind: CompletionItemKind.Method, + documentation: 'baz doc', + sortText: '0', + insertTextFormat: InsertTextFormat.Snippet, + insertText: 'baz(${1:num})', + detail: '(method) A.baz(num: number): string' + }, + { + label: 'foo', + kind: CompletionItemKind.Method, + documentation: 'foo doc', + sortText: '0', + insertTextFormat: InsertTextFormat.Snippet, + insertText: 'foo()', + detail: '(method) A.foo(): void' + }, + { + label: 'qux', + kind: CompletionItemKind.Property, + documentation: 'qux doc', + sortText: '0', + insertTextFormat: InsertTextFormat.Snippet, + insertText: 'qux', + detail: '(property) A.qux: number' + } + ]); + }); + }); + describe('textDocumentCompletion()', function (this: TestContext & ISuiteCallbackContext) { beforeEach(initializeTypeScriptService(createService, rootUri, new Map([ [rootUri + 'a.ts', [ @@ -2201,6 +2288,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor } ]); }); + it('produces completions for imported symbols', async function (this: TestContext & ITestCallbackContext) { const result: CompletionList = await this.service.textDocumentCompletion({ textDocument: { diff --git a/src/typescript-service.ts b/src/typescript-service.ts index 9e6cfede8..057090f2a 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -18,6 +18,7 @@ import { DocumentSymbolParams, ExecuteCommandParams, Hover, + InsertTextFormat, Location, MarkedString, ParameterInformation, @@ -173,6 +174,8 @@ export class TypeScriptService { } }; + private completionWithSnippets: boolean = false; + constructor(protected client: LanguageClient, protected options: TypeScriptServiceOptions = {}) { this.logger = new LSPLogger(client); } @@ -200,6 +203,10 @@ export class TypeScriptService { if (params.rootUri || params.rootPath) { this.root = params.rootPath || uri2path(params.rootUri!); this.rootUri = params.rootUri || path2uri(params.rootPath!); + + if (params.capabilities.textDocument && params.capabilities.textDocument.completion && params.capabilities.textDocument.completion.completionItem) { + this.completionWithSnippets = params.capabilities.textDocument.completion.completionItem.snippetSupport || false; + } // The root URI always refers to a directory if (!this.rootUri.endsWith('/')) { this.rootUri += '/'; @@ -998,10 +1005,26 @@ export class TypeScriptService { if (completions == null) { return []; } + function createSnippet(entry: ts.CompletionEntryDetails): string { + let index = 0; + const parameters = entry.displayParts + .filter(p => p.kind === 'parameterName') + .map(p => { + index++; + return '${' + `${index}:${p.text}` + '}'; + }); + if (entry.kind === 'property') { + return entry.name; + } else { + const paramString = parameters.join(', '); + return entry.name + `(${paramString})`; + } + } return Observable.from(completions.entries) .map(entry => { const item: CompletionItem = { label: entry.name }; + const kind = completionKinds[entry.kind]; if (kind) { item.kind = kind; @@ -1013,6 +1036,13 @@ export class TypeScriptService { if (details) { item.documentation = ts.displayPartsToString(details.documentation); item.detail = ts.displayPartsToString(details.displayParts); + if (this.completionWithSnippets) { + item.insertTextFormat = InsertTextFormat.Snippet; + item.insertText = createSnippet(details); + } else { + item.insertTextFormat = InsertTextFormat.PlainText; + item.insertText = details.name; + } } return { op: 'add', path: '/items/-', value: item } as Operation; }) From d3c49f2cc08cb9534a37ea1bce16d5b0a9718b7c Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Wed, 26 Jul 2017 23:11:01 +0200 Subject: [PATCH 2/9] Update tests for completions without snippets --- src/test/typescript-service-helpers.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/test/typescript-service-helpers.ts b/src/test/typescript-service-helpers.ts index 5f3b310a2..c56741654 100644 --- a/src/test/typescript-service-helpers.ts +++ b/src/test/typescript-service-helpers.ts @@ -2125,7 +2125,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor }); - describe.only('textDocumentCompletion() with snippets', function (this: TestContext & ISuiteCallbackContext){ + describe('textDocumentCompletion() with snippets', function (this: TestContext & ISuiteCallbackContext){ beforeEach(initializeTypeScriptService(createService, rootUri, new Map([ [rootUri + 'a.ts', [ 'class A {', @@ -2154,7 +2154,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor afterEach(shutdownService); - it('shoudl produce completions with snippets if supported', async function (this: TestContext & ITestCallbackContext) { + it('should produce completions with snippets if supported', async function (this: TestContext & ITestCallbackContext) { const result: CompletionList = await this.service.textDocumentCompletion({ textDocument: { uri: rootUri + 'a.ts' @@ -2262,6 +2262,8 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor label: 'bar', kind: CompletionItemKind.Method, documentation: 'bar doc', + insertText: 'bar', + insertTextFormat: InsertTextFormat.PlainText, sortText: '0', detail: '(method) A.bar(): number' }, @@ -2269,6 +2271,8 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor label: 'baz', kind: CompletionItemKind.Method, documentation: 'baz doc', + insertText: 'baz', + insertTextFormat: InsertTextFormat.PlainText, sortText: '0', detail: '(method) A.baz(): string' }, @@ -2276,6 +2280,8 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor label: 'foo', kind: CompletionItemKind.Method, documentation: 'foo doc', + insertText: 'foo', + insertTextFormat: InsertTextFormat.PlainText, sortText: '0', detail: '(method) A.foo(): void' }, @@ -2283,6 +2289,8 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor label: 'qux', kind: CompletionItemKind.Property, documentation: 'qux doc', + insertText: 'qux', + insertTextFormat: InsertTextFormat.PlainText, sortText: '0', detail: '(property) A.qux: number' } @@ -2305,6 +2313,8 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor label: 'd', kind: CompletionItemKind.Function, documentation: 'd doc', + insertText: 'd', + insertTextFormat: InsertTextFormat.PlainText, detail: 'function d(): void', sortText: '0' }] @@ -2326,6 +2336,8 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor label: 'bar', kind: CompletionItemKind.Interface, documentation: 'bar doc', + insertText: 'bar', + insertTextFormat: InsertTextFormat.PlainText, sortText: '0', detail: 'interface foo.bar' }] From be2cb4e75d20d2d46b7f345fd7c8c66f352c9001 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Wed, 26 Jul 2017 23:38:01 +0200 Subject: [PATCH 3/9] fix: reduce operation after rebase --- src/test/typescript-service-helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/typescript-service-helpers.ts b/src/test/typescript-service-helpers.ts index c56741654..fbd5b5d58 100644 --- a/src/test/typescript-service-helpers.ts +++ b/src/test/typescript-service-helpers.ts @@ -2163,7 +2163,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor line: 11, character: 2 } - }).reduce(jsonpatch.applyReducer, null as any).toPromise(); + }).reduce(applyReducer, null as any).toPromise(); // * A snippet can define tab stops and placeholders with `$1`, `$2` // * and `${3:foo}`. `$0` defines the final tab stop, it defaults to // * the end of the snippet. Placeholders with equal identifiers are linked, From f62570f3f862f496ef2367d8156ea7d9ef5aaa39 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Wed, 26 Jul 2017 23:48:10 +0200 Subject: [PATCH 4/9] fix: Move param snippet creation where it is needed --- src/typescript-service.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/typescript-service.ts b/src/typescript-service.ts index 057090f2a..1a993fbcf 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -1006,13 +1006,6 @@ export class TypeScriptService { return []; } function createSnippet(entry: ts.CompletionEntryDetails): string { - let index = 0; - const parameters = entry.displayParts - .filter(p => p.kind === 'parameterName') - .map(p => { - index++; - return '${' + `${index}:${p.text}` + '}'; - }); if (entry.kind === 'property') { return entry.name; } else { From cf5d05a13e377a995c0c22fc744ab10d67f829de Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Thu, 3 Aug 2017 16:10:46 +0200 Subject: [PATCH 5/9] fix: Forgot to include this in last commit --- src/typescript-service.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/typescript-service.ts b/src/typescript-service.ts index 1a993fbcf..c12345a69 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -1009,6 +1009,13 @@ export class TypeScriptService { if (entry.kind === 'property') { return entry.name; } else { + let index = 0; + const parameters = entry.displayParts + .filter(p => p.kind === 'parameterName') + .map(p => { + index++; + return '${' + `${index}:${p.text}` + '}'; + }); const paramString = parameters.join(', '); return entry.name + `(${paramString})`; } From 59e574f43786f7e1a8e3dc126e1e922040c3a7c9 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Thu, 3 Aug 2017 16:53:05 +0200 Subject: [PATCH 6/9] fix: Changes requested in review Rename to supportsCompletionWithSnippets + doctype Fix up assignment Pull createSnippet into function Use index param from map --- src/typescript-service.ts | 43 ++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/typescript-service.ts b/src/typescript-service.ts index c12345a69..243f7e382 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -174,7 +174,11 @@ export class TypeScriptService { } }; - private completionWithSnippets: boolean = false; + /** + * Indicates if the client prefers completion results formatted as snippets. + * @type {boolean} + */ + private supportsCompletionWithSnippets: boolean = false; constructor(protected client: LanguageClient, protected options: TypeScriptServiceOptions = {}) { this.logger = new LSPLogger(client); @@ -204,9 +208,11 @@ export class TypeScriptService { this.root = params.rootPath || uri2path(params.rootUri!); this.rootUri = params.rootUri || path2uri(params.rootPath!); - if (params.capabilities.textDocument && params.capabilities.textDocument.completion && params.capabilities.textDocument.completion.completionItem) { - this.completionWithSnippets = params.capabilities.textDocument.completion.completionItem.snippetSupport || false; - } + this.supportsCompletionWithSnippets = params.capabilities.textDocument && + params.capabilities.textDocument.completion && + params.capabilities.textDocument.completion.completionItem && + params.capabilities.textDocument.completion.completionItem.snippetSupport || false; + // The root URI always refers to a directory if (!this.rootUri.endsWith('/')) { this.rootUri += '/'; @@ -1005,21 +1011,6 @@ export class TypeScriptService { if (completions == null) { return []; } - function createSnippet(entry: ts.CompletionEntryDetails): string { - if (entry.kind === 'property') { - return entry.name; - } else { - let index = 0; - const parameters = entry.displayParts - .filter(p => p.kind === 'parameterName') - .map(p => { - index++; - return '${' + `${index}:${p.text}` + '}'; - }); - const paramString = parameters.join(', '); - return entry.name + `(${paramString})`; - } - } return Observable.from(completions.entries) .map(entry => { @@ -1036,9 +1027,19 @@ export class TypeScriptService { if (details) { item.documentation = ts.displayPartsToString(details.documentation); item.detail = ts.displayPartsToString(details.displayParts); - if (this.completionWithSnippets) { + if (this.supportsCompletionWithSnippets) { item.insertTextFormat = InsertTextFormat.Snippet; - item.insertText = createSnippet(details); + if (entry.kind === 'property') { + item.insertText = details.name; + } else { + const parameters = details.displayParts + .filter(p => p.kind === 'parameterName') + .map((p, index) => { + return '${' + `${index + 1}:${p.text}` + '}'; + }); + const paramString = parameters.join(', '); + item.insertText = details.name + `(${paramString})`; + } } else { item.insertTextFormat = InsertTextFormat.PlainText; item.insertText = details.name; From a7c59d9bf120767b83062b032fcf2ddd0a21c2b6 Mon Sep 17 00:00:00 2001 From: Tom van Ommeren Date: Thu, 3 Aug 2017 16:53:54 +0200 Subject: [PATCH 7/9] fix: test changes requested in review rename constant, use as default value --- src/test/typescript-service-helpers.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/typescript-service-helpers.ts b/src/test/typescript-service-helpers.ts index fbd5b5d58..8305d686f 100644 --- a/src/test/typescript-service-helpers.ts +++ b/src/test/typescript-service-helpers.ts @@ -16,7 +16,7 @@ import { IBeforeAndAfterContext, ISuiteCallbackContext, ITestCallbackContext } f chai.use(chaiAsPromised); const assert = chai.assert; -const defaultCapabilities: ClientCapabilities = { +const DEFAULT_CAPABILITIES: ClientCapabilities = { xcontentProvider: true, xfilesProvider: true }; @@ -36,7 +36,7 @@ export interface TestContext { * @param createService A factory that creates the TypeScript service. Allows to test subclasses of TypeScriptService * @param files A Map from URI to file content of files that should be available in the workspace */ -export const initializeTypeScriptService = (createService: TypeScriptServiceFactory, rootUri: string, files: Map, clientCapabilities?: ClientCapabilities) => async function (this: TestContext & IBeforeAndAfterContext): Promise { +export const initializeTypeScriptService = (createService: TypeScriptServiceFactory, rootUri: string, files: Map, clientCapabilities: ClientCapabilities = DEFAULT_CAPABILITIES) => async function (this: TestContext & IBeforeAndAfterContext): Promise { // Stub client this.client = sinon.createStubInstance(RemoteLanguageClient); @@ -61,7 +61,7 @@ export const initializeTypeScriptService = (createService: TypeScriptServiceFact await this.service.initialize({ processId: process.pid, rootUri, - capabilities: clientCapabilities || defaultCapabilities + capabilities: clientCapabilities || DEFAULT_CAPABILITIES }).toPromise(); }; @@ -2149,7 +2149,7 @@ export function describeTypeScriptService(createService: TypeScriptServiceFactor } } }, - ...defaultCapabilities + ...DEFAULT_CAPABILITIES })); afterEach(shutdownService); From 29d9ccf65c7a1fbf926220c44e94ded878423259 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Thu, 3 Aug 2017 08:25:47 -0700 Subject: [PATCH 8/9] Update typescript-service.ts --- src/typescript-service.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/typescript-service.ts b/src/typescript-service.ts index 243f7e382..77bd036e9 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -176,7 +176,6 @@ export class TypeScriptService { /** * Indicates if the client prefers completion results formatted as snippets. - * @type {boolean} */ private supportsCompletionWithSnippets: boolean = false; @@ -1034,9 +1033,7 @@ export class TypeScriptService { } else { const parameters = details.displayParts .filter(p => p.kind === 'parameterName') - .map((p, index) => { - return '${' + `${index + 1}:${p.text}` + '}'; - }); + .map((p, index) => '${' + `${index + 1}:${p.text}` + '}'}); const paramString = parameters.join(', '); item.insertText = details.name + `(${paramString})`; } From a9cb785f438a98ea49706caed8c80fdf5f654255 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Thu, 3 Aug 2017 08:49:44 -0700 Subject: [PATCH 9/9] fix: syntax error --- src/typescript-service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/typescript-service.ts b/src/typescript-service.ts index 77bd036e9..1288bde0e 100644 --- a/src/typescript-service.ts +++ b/src/typescript-service.ts @@ -1033,7 +1033,7 @@ export class TypeScriptService { } else { const parameters = details.displayParts .filter(p => p.kind === 'parameterName') - .map((p, index) => '${' + `${index + 1}:${p.text}` + '}'}); + .map((p, i) => '${' + `${i + 1}:${p.text}` + '}'); const paramString = parameters.join(', '); item.insertText = details.name + `(${paramString})`; }