From 46caf88b2c837285f4bc040c80454f7a3314f8d3 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Tue, 20 Aug 2019 13:13:46 -0500 Subject: [PATCH] feat(language-service): add definitions for templateUrl (#32238) Adds support for `getDefinitionAt` when called on a templateUrl property assignment. The currrent architecture for getting definitions is designed to be called on templates, so we have to introduce a new `getTsDefinitionAndBoundSpan` method to get Angular-specific definitions in TypeScript files and pass a `readTemplate` closure that will read the contents of a template using `TypeScriptServiceHost#getTemplates`. We can probably go in and make this nicer in a future PR, though I'm not sure what the best architecture should be yet. Part of angular/vscode-ng-language-service#111 PR Close #32238 --- .../goldens/templateUrlDefinition.json | 32 +++++++ integration/language_service_plugin/test.ts | 90 +++++++++++-------- packages/language-service/src/definitions.ts | 69 +++++++++++++- .../language-service/src/language_service.ts | 11 ++- .../language-service/src/typescript_host.ts | 3 +- .../language-service/test/definitions_spec.ts | 22 +++++ 6 files changed, 187 insertions(+), 40 deletions(-) create mode 100644 integration/language_service_plugin/goldens/templateUrlDefinition.json diff --git a/integration/language_service_plugin/goldens/templateUrlDefinition.json b/integration/language_service_plugin/goldens/templateUrlDefinition.json new file mode 100644 index 0000000000..f89d4a72ec --- /dev/null +++ b/integration/language_service_plugin/goldens/templateUrlDefinition.json @@ -0,0 +1,32 @@ +{ + "seq": 0, + "type": "response", + "command": "definitionAndBoundSpan", + "request_seq": 2, + "success": true, + "body": { + "definitions": [ + { + "file": "${PWD}/project/app/widget.component.html", + "start": { + "line": 1, + "offset": 1 + }, + "end": { + "line": 1, + "offset": 1 + } + } + ], + "textSpan": { + "start": { + "line": 5, + "offset": 17 + }, + "end": { + "line": 5, + "offset": 40 + } + } + } +} diff --git a/integration/language_service_plugin/test.ts b/integration/language_service_plugin/test.ts index 73a920b476..160ae8d3c1 100644 --- a/integration/language_service_plugin/test.ts +++ b/integration/language_service_plugin/test.ts @@ -1,47 +1,52 @@ -import { fork, ChildProcess } from 'child_process'; -import { join } from 'path'; -import { Client } from './tsclient'; -import { goldenMatcher } from './matcher'; +import {ChildProcess, fork} from 'child_process'; +import {join} from 'path'; +import {goldenMatcher} from './matcher'; +import {Client} from './tsclient'; describe('Angular Language Service', () => { jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000; /* 10 seconds */ - const PWD = process.env.PWD!; - const SERVER_PATH = "./node_modules/typescript/lib/tsserver.js"; + const PWD = process.env.PWD !; + const SERVER_PATH = './node_modules/typescript/lib/tsserver.js'; let server: ChildProcess; let client: Client; beforeEach(() => { jasmine.addMatchers(goldenMatcher); - server = fork(SERVER_PATH, [ - '--logVerbosity', 'verbose', - '--logFile', join(PWD, 'tsserver.log'), - ], { - stdio: ['pipe', 'pipe', 'inherit', 'ipc'], - }); + server = fork( + SERVER_PATH, + [ + '--logVerbosity', + 'verbose', + '--logFile', + join(PWD, 'tsserver.log'), + ], + { + stdio: ['pipe', 'pipe', 'inherit', 'ipc'], + }); client = new Client(server); client.listen(); }); - afterEach(async () => { + afterEach(async() => { client.sendRequest('exit', {}); // Give server process some time to flush all messages await new Promise((resolve) => setTimeout(resolve, 1000)); }); - it('should be launched as tsserver plugin', async () => { + it('should be launched as tsserver plugin', async() => { let response = await client.sendRequest('configure', { hostInfo: 'vscode', }); expect(response).toMatchGolden('configure.json'); response = await client.sendRequest('compilerOptionsForInferredProjects', { - "options": { - module: "CommonJS", - target: "ES6", + 'options': { + module: 'CommonJS', + target: 'ES6', allowSyntheticDefaultImports: true, allowNonTsExtensions: true, allowJs: true, - jsx: "Preserve" + jsx: 'Preserve' } }); expect(response).toMatchGolden('compilerOptionsForInferredProjects.json'); @@ -52,24 +57,21 @@ describe('Angular Language Service', () => { }); // Server does not send response to geterr request // https://github.com/Microsoft/TypeScript/blob/master/lib/protocol.d.ts#L1770 - client.sendRequest('geterr', { - delay: 0, - files: [`${PWD}/project/app/app.module.ts`] - }); + client.sendRequest('geterr', {delay: 0, files: [`${PWD}/project/app/app.module.ts`]}); }); - it('should perform completions', async () => { + it('should perform completions', async() => { await client.sendRequest('configure', { hostInfo: 'vscode', }); await client.sendRequest('compilerOptionsForInferredProjects', { - "options": { - module: "CommonJS", - target: "ES6", + 'options': { + module: 'CommonJS', + target: 'ES6', allowSyntheticDefaultImports: true, allowNonTsExtensions: true, allowJs: true, - jsx: "Preserve" + jsx: 'Preserve' } }); @@ -77,10 +79,7 @@ describe('Angular Language Service', () => { file: `${PWD}/project/app/app.component.ts`, }); - client.sendRequest('geterr', { - delay: 0, - files: [`${PWD}/project/app/app.component.ts`] - }); + client.sendRequest('geterr', {delay: 0, files: [`${PWD}/project/app/app.component.ts`]}); client.sendRequest('change', { file: `${PWD}/project/app/app.component.ts`, @@ -99,7 +98,7 @@ describe('Angular Language Service', () => { expect(response).toMatchGolden('completionInfo.json'); }); - it('should perform quickinfo', async () => { + it('should perform quickinfo', async() => { client.sendRequest('open', { file: `${PWD}/project/app/app.component.ts`, }); @@ -119,7 +118,7 @@ describe('Angular Language Service', () => { expect(resp2).toMatchGolden('quickinfo.json'); }); - it('should perform definition', async () => { + it('should perform definition', async() => { client.sendRequest('open', { file: `${PWD}/project/app/app.component.ts`, }); @@ -139,19 +138,19 @@ describe('Angular Language Service', () => { expect(resp2).toMatchGolden('definition.json'); }); - it('should perform definitionAndBoundSpan', async () => { + it('should perform definitionAndBoundSpan', async() => { client.sendRequest('open', { file: `${PWD}/project/app/app.component.ts`, }); - const resp1 = await client.sendRequest('reload', { + 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', { + const resp2 = await client.sendRequest('definitionAndBoundSpan', { file: `${PWD}/project/app/app.component.ts`, line: 5, offset: 28, @@ -159,4 +158,23 @@ describe('Angular Language Service', () => { expect(resp2).toMatchGolden('definitionAndBoundSpan.json'); }); + it('should perform definitionAndBoundSpan for template URLs', async() => { + client.sendRequest('open', { + file: `${PWD}/project/app/widget.component.ts`, + }); + + const resp1 = await client.sendRequest('reload', { + file: `${PWD}/project/app/widget.component.ts`, + tmpFile: `${PWD}/project/app/widget.component.ts`, + }) as any; + expect(resp1.command).toBe('reload'); + expect(resp1.success).toBe(true); + + const resp2 = await client.sendRequest('definitionAndBoundSpan', { + file: `${PWD}/project/app/widget.component.ts`, + line: 5, + offset: 19, + }); + expect(resp2).toMatchGolden('templateUrlDefinition.json'); + }); }); diff --git a/packages/language-service/src/definitions.ts b/packages/language-service/src/definitions.ts index fe8c5dffa2..6a0eb7029b 100644 --- a/packages/language-service/src/definitions.ts +++ b/packages/language-service/src/definitions.ts @@ -6,10 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ +import * as path from 'path'; import * as ts from 'typescript'; // used as value and is provided at runtime import {AstResult} from './common'; import {locateSymbol} from './locate_symbol'; -import {Span} from './types'; +import {getPropertyAssignmentFromValue, isClassDecoratorProperty} from './template'; +import {Span, TemplateSource} from './types'; +import {findTightestNode} from './utils'; /** * Convert Angular Span to TypeScript TextSpan. Angular Span has 'start' and @@ -59,3 +62,67 @@ export function getDefinitionAndBoundSpan( definitions, textSpan, }; } + +/** + * Gets an Angular-specific definition in a TypeScript source file. + */ +export function getTsDefinitionAndBoundSpan( + sf: ts.SourceFile, position: number, + tsLsHost: Readonly): ts.DefinitionInfoAndBoundSpan|undefined { + const node = findTightestNode(sf, position); + if (!node) return; + switch (node.kind) { + case ts.SyntaxKind.StringLiteral: + case ts.SyntaxKind.NoSubstitutionTemplateLiteral: + // Attempt to extract definition of a URL in a property assignment. + return getUrlFromProperty(node as ts.StringLiteralLike, tsLsHost); + default: + return undefined; + } +} + +/** + * Attempts to get the definition of a file whose URL is specified in a property assignment in a + * directive decorator. + * Currently applies to `templateUrl` properties. + */ +function getUrlFromProperty( + urlNode: ts.StringLiteralLike, + tsLsHost: Readonly): ts.DefinitionInfoAndBoundSpan|undefined { + const asgn = getPropertyAssignmentFromValue(urlNode); + if (!asgn) return; + // If the URL is not a property of a class decorator, don't generate definitions for it. + if (!isClassDecoratorProperty(asgn)) return; + + const sf = urlNode.getSourceFile(); + switch (asgn.name.getText()) { + case 'templateUrl': + // Extract definition of the template file specified by this `templateUrl` property. + const url = path.join(path.dirname(sf.fileName), urlNode.text); + + // If the file does not exist, bail. It is possible that the TypeScript language service host + // does not have a `fileExists` method, in which case optimistically assume the file exists. + if (tsLsHost.fileExists && !tsLsHost.fileExists(url)) return; + + const templateDefinitions: ts.DefinitionInfo[] = [{ + kind: ts.ScriptElementKind.externalModuleName, + name: url, + containerKind: ts.ScriptElementKind.unknown, + containerName: '', + // Reading the template is expensive, so don't provide a preview. + textSpan: {start: 0, length: 0}, + fileName: url, + }]; + + return { + definitions: templateDefinitions, + textSpan: { + // Exclude opening and closing quotes in the url span. + start: urlNode.getStart() + 1, + length: urlNode.getWidth() - 2, + }, + }; + default: + return undefined; + } +} diff --git a/packages/language-service/src/language_service.ts b/packages/language-service/src/language_service.ts index 7fa9b4942c..bb887367f0 100644 --- a/packages/language-service/src/language_service.ts +++ b/packages/language-service/src/language_service.ts @@ -10,7 +10,7 @@ import * as tss from 'typescript/lib/tsserverlibrary'; import {isAstResult} from './common'; import {getTemplateCompletions, ngCompletionToTsCompletionEntry} from './completions'; -import {getDefinitionAndBoundSpan} from './definitions'; +import {getDefinitionAndBoundSpan, getTsDefinitionAndBoundSpan} from './definitions'; import {getDeclarationDiagnostics, getTemplateDiagnostics, ngDiagnosticToTsDiagnostic, uniqueBySpan} from './diagnostics'; import {getHover} from './hover'; import {Diagnostic, LanguageService} from './types'; @@ -80,6 +80,15 @@ class LanguageServiceImpl implements LanguageService { if (templateInfo) { return getDefinitionAndBoundSpan(templateInfo, position); } + + // Attempt to get Angular-specific definitions in a TypeScript file, like templates defined + // in a `templateUrl` property. + if (fileName.endsWith('.ts')) { + const sf = this.host.getSourceFile(fileName); + if (sf) { + return getTsDefinitionAndBoundSpan(sf, position, this.host.host); + } + } } getHoverAt(fileName: string, position: number): tss.QuickInfo|undefined { diff --git a/packages/language-service/src/typescript_host.ts b/packages/language-service/src/typescript_host.ts index 517ddd73a2..d1d05d0996 100644 --- a/packages/language-service/src/typescript_host.ts +++ b/packages/language-service/src/typescript_host.ts @@ -73,8 +73,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { ngModules: [], }; - constructor( - private readonly host: ts.LanguageServiceHost, private readonly tsLS: ts.LanguageService) { + constructor(readonly host: ts.LanguageServiceHost, private readonly tsLS: ts.LanguageService) { this.summaryResolver = new AotSummaryResolver( { loadSummary(filePath: string) { return null; }, diff --git a/packages/language-service/test/definitions_spec.ts b/packages/language-service/test/definitions_spec.ts index 48f608e01b..a0e299f082 100644 --- a/packages/language-service/test/definitions_spec.ts +++ b/packages/language-service/test/definitions_spec.ts @@ -254,6 +254,28 @@ describe('definitions', () => { } }); + it('should be able to find a template from a url', () => { + const fileName = addCode(` + @Component({ + templateUrl: './«test».ng', + }) + export class MyComponent {}`); + + const marker = getReferenceMarkerFor(fileName, 'test'); + const result = ngService.getDefinitionAt(fileName, marker.start); + + expect(result).toBeDefined(); + const {textSpan, definitions} = result !; + + expect(textSpan).toEqual({start: marker.start - 2, length: 9}); + + expect(definitions).toBeDefined(); + expect(definitions !.length).toBe(1); + const [def] = definitions !; + expect(def.fileName).toBe('/app/test.ng'); + expect(def.textSpan).toEqual({start: 0, length: 0}); + }); + /** * Append a snippet of code to `app.component.ts` and return the file name. * There must not be any name collision with existing code.