From f4916730b5a45cec711bab2f265f918da13288bb Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Thu, 25 Apr 2019 11:11:33 -0700 Subject: [PATCH] feat(language-service): Implement `definitionAndBoundSpan` (#30125) This PR adds the implementation for `definitionAndBoundSpan` because it's now preferred over `definition`. vscode would send this new request for `Go to definition`. As part of this PR the implementation for `definition` is refactored and simplified. Goldens for both methods are checked in. PR Close #30125 --- .../goldens/definitionAndBoundSpan.json | 32 +++++++ integration/language_service_plugin/test.ts | 21 +++++ integration/language_service_plugin/yarn.lock | 18 ++-- packages/language-service/src/ts_plugin.ts | 85 ++++++++++++------- 4 files changed, 117 insertions(+), 39 deletions(-) create mode 100644 integration/language_service_plugin/goldens/definitionAndBoundSpan.json diff --git a/integration/language_service_plugin/goldens/definitionAndBoundSpan.json b/integration/language_service_plugin/goldens/definitionAndBoundSpan.json new file mode 100644 index 0000000000..b5c8560563 --- /dev/null +++ b/integration/language_service_plugin/goldens/definitionAndBoundSpan.json @@ -0,0 +1,32 @@ +{ + "seq": 0, + "type": "response", + "command": "definitionAndBoundSpan", + "request_seq": 2, + "success": true, + "body": { + "definitions": [ + { + "file": "${PWD}/project/app/app.component.ts", + "start": { + "line": 7, + "offset": 30 + }, + "end": { + "line": 7, + "offset": 47 + } + } + ], + "textSpan": { + "start": { + "line": 7, + "offset": 30 + }, + "end": { + "line": 7, + "offset": 47 + } + } + } +} diff --git a/integration/language_service_plugin/test.ts b/integration/language_service_plugin/test.ts index 0875a7f1d3..b1b7cc498e 100644 --- a/integration/language_service_plugin/test.ts +++ b/integration/language_service_plugin/test.ts @@ -139,4 +139,25 @@ describe('Angular Language Service', () => { }); expect(resp2).toMatchGolden('definition.json'); }); + + it('should perform definitionAndBoundSpan', async () => { + client.sendRequest('open', { + file: `${PWD}/project/app/app.component.ts`, + }); + + const resp1 = await client.sendRequest('reload', { + file: `${PWD}/project/app/app.component.ts`, + tmpFile: `${PWD}/project/app/app.component.ts`, + }) as any; + expect(resp1.command).toBe('reload'); + expect(resp1.success).toBe(true); + + const resp2 = await client.sendRequest('definitionAndBoundSpan', { + file: `${PWD}/project/app/app.component.ts`, + line: 5, + offset: 28, + }); + expect(resp2).toMatchGolden('definitionAndBoundSpan.json'); + }); + }); diff --git a/integration/language_service_plugin/yarn.lock b/integration/language_service_plugin/yarn.lock index 73dcc87276..e5eef1f6d5 100644 --- a/integration/language_service_plugin/yarn.lock +++ b/integration/language_service_plugin/yarn.lock @@ -3,12 +3,12 @@ "@angular/core@file:../../dist/packages-dist/core": - version "7.2.0" + version "8.0.0-beta.13" dependencies: tslib "^1.9.0" "@angular/language-service@file:../../dist/packages-dist/language-service": - version "7.2.0" + version "8.0.0-beta.13" "@types/node@file:../../node_modules/@types/node": version "10.9.4" @@ -61,16 +61,16 @@ inherits@2: resolved "https://registry.yarnpkg.com/inherits/-/inherits-2.0.3.tgz#633c2c83e3da42a502f52466022480f4208261de" integrity sha1-Yzwsg+PaQqUC9SRmAiSA9CCCYd4= -jasmine-core@~3.1.0: - version "3.1.0" - resolved "https://registry.yarnpkg.com/jasmine-core/-/jasmine-core-3.1.0.tgz#a4785e135d5df65024dfc9224953df585bd2766c" - integrity sha1-pHheE11d9lAk38kiSVPfWFvSdmw= +jasmine-core@~3.3.0: + version "3.3.0" + resolved "https://registry.yarnpkg.com/jasmine-core/-/jasmine-core-3.3.0.tgz#dea1cdc634bc93c7e0d4ad27185df30fa971b10e" + integrity sha512-3/xSmG/d35hf80BEN66Y6g9Ca5l/Isdeg/j6zvbTYlTzeKinzmaTM4p9am5kYqOmE05D7s1t8FGjzdSnbUbceA== "jasmine@file:../../node_modules/jasmine": - version "3.1.0" + version "3.3.1" dependencies: glob "^7.0.6" - jasmine-core "~3.1.0" + jasmine-core "~3.3.0" minimatch@^3.0.4: version "3.0.4" @@ -97,7 +97,7 @@ tslib@^1.9.0: integrity sha512-4krF8scpejhaOgqzBEcGM7yDIEfi0/8+8zDRZhNZZ2kjmHJ4hv3zCbQWxoJGz1iw5U0Jl0nma13xzHXcncMavQ== "typescript@file:../../node_modules/typescript": - version "3.2.2" + version "3.4.2" wrappy@1: version "1.0.2" diff --git a/packages/language-service/src/ts_plugin.ts b/packages/language-service/src/ts_plugin.ts index cc4f4c4121..368dba70e8 100644 --- a/packages/language-service/src/ts_plugin.ts +++ b/packages/language-service/src/ts_plugin.ts @@ -10,7 +10,7 @@ import * as ts from 'typescript'; // used as value, passed in by tsserver at run import * as tss from 'typescript/lib/tsserverlibrary'; // used as type only import {createLanguageService} from './language_service'; -import {Completion, Diagnostic, DiagnosticMessageChain} from './types'; +import {Completion, Diagnostic, DiagnosticMessageChain, Location} from './types'; import {TypeScriptServiceHost} from './typescript_host'; const projectHostMap = new WeakMap(); @@ -156,36 +156,61 @@ export function create(info: tss.server.PluginCreateInfo): ts.LanguageService { return base; }; - proxy.getDefinitionAtPosition = function( - fileName: string, position: number): ReadonlyArray { - let base = oldLS.getDefinitionAtPosition(fileName, position); - if (base && base.length) { - return base; - } + proxy.getDefinitionAtPosition = function(fileName: string, position: number): + ReadonlyArray| + undefined { + const base = oldLS.getDefinitionAtPosition(fileName, position); + if (base && base.length) { + return base; + } + const ours = ls.getDefinitionAt(fileName, position); + if (ours && ours.length) { + return ours.map((loc: Location) => { + return { + fileName: loc.fileName, + textSpan: { + start: loc.span.start, + length: loc.span.end - loc.span.start, + }, + name: '', + kind: ts.ScriptElementKind.unknown, + containerName: loc.fileName, + containerKind: ts.ScriptElementKind.unknown, + }; + }); + } + }; - return tryOperation('get definition', () => { - const ours = ls.getDefinitionAt(fileName, position); - let combined; - - if (ours && ours.length) { - combined = base && base.concat([]) || []; - for (const loc of ours) { - combined.push({ - fileName: loc.fileName, - textSpan: {start: loc.span.start, length: loc.span.end - loc.span.start}, - name: '', - // TODO: remove any and fix type error. - kind: 'definition' as any, - containerName: loc.fileName, - containerKind: 'file' as any, - }); - } - } else { - combined = base; - } - return combined; - }) || []; - }; + proxy.getDefinitionAndBoundSpan = function(fileName: string, position: number): + ts.DefinitionInfoAndBoundSpan | + undefined { + const base = oldLS.getDefinitionAndBoundSpan(fileName, position); + if (base && base.definitions && base.definitions.length) { + return base; + } + const ours = ls.getDefinitionAt(fileName, position); + if (ours && ours.length) { + return { + definitions: ours.map((loc: Location) => { + return { + fileName: loc.fileName, + textSpan: { + start: loc.span.start, + length: loc.span.end - loc.span.start, + }, + name: '', + kind: ts.ScriptElementKind.unknown, + containerName: loc.fileName, + containerKind: ts.ScriptElementKind.unknown, + }; + }), + textSpan: { + start: ours[0].span.start, + length: ours[0].span.end - ours[0].span.start, + }, + }; + } + }; return proxy; }