From a8e2ee134303e2c2f9650e4ce4e61af4629aeea8 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Thu, 1 Aug 2019 13:07:32 -0700 Subject: [PATCH] fix(language-service): Make Definition and QuickInfo compatible with TS LS (#31972) Now that the Angular LS is a proper tsserver plugin, it does not make sense for it to maintain its own language service API. This is part one of the effort to remove our custom LanguageService interface. This interface is cumbersome because we have to do two transformations: ng def -> ts def -> lsp definition The TS LS interface is more comprehensive, so this allows the Angular LS to return more information. PR Close #31972 --- .../goldens/definitionAndBoundSpan.json | 8 +- .../goldens/quickinfo.json | 4 +- packages/language-service/src/definitions.ts | 60 ++- packages/language-service/src/hover.ts | 49 ++- .../language-service/src/language_service.ts | 12 +- packages/language-service/src/ts_plugin.ts | 127 +++--- packages/language-service/src/types.ts | 6 +- .../language-service/test/definitions_spec.ts | 402 ++++++++++++------ packages/language-service/test/hover_spec.ts | 30 +- packages/language-service/test/test_data.ts | 4 +- 10 files changed, 442 insertions(+), 260 deletions(-) diff --git a/integration/language_service_plugin/goldens/definitionAndBoundSpan.json b/integration/language_service_plugin/goldens/definitionAndBoundSpan.json index b5c8560563..c7cc7cc9f0 100644 --- a/integration/language_service_plugin/goldens/definitionAndBoundSpan.json +++ b/integration/language_service_plugin/goldens/definitionAndBoundSpan.json @@ -20,12 +20,12 @@ ], "textSpan": { "start": { - "line": 7, - "offset": 30 + "line": 5, + "offset": 26 }, "end": { - "line": 7, - "offset": 47 + "line": 5, + "offset": 30 } } } diff --git a/integration/language_service_plugin/goldens/quickinfo.json b/integration/language_service_plugin/goldens/quickinfo.json index 9bd1f4a75f..5171068d64 100644 --- a/integration/language_service_plugin/goldens/quickinfo.json +++ b/integration/language_service_plugin/goldens/quickinfo.json @@ -5,7 +5,7 @@ "request_seq": 2, "success": true, "body": { - "kind": "", + "kind": "property", "kindModifiers": "", "start": { "line": 5, @@ -15,7 +15,7 @@ "line": 5, "offset": 30 }, - "displayString": "property name of AppComponent", + "displayString": "(property) AppComponent.name", "documentation": "", "tags": [] } diff --git a/packages/language-service/src/definitions.ts b/packages/language-service/src/definitions.ts index ae3e0cc241..297861d6f8 100644 --- a/packages/language-service/src/definitions.ts +++ b/packages/language-service/src/definitions.ts @@ -6,28 +6,50 @@ * found in the LICENSE file at https://angular.io/license */ -import * as tss from 'typescript/lib/tsserverlibrary'; - +import * as ts from 'typescript'; // used as value and is provided at runtime import {TemplateInfo} from './common'; import {locateSymbol} from './locate_symbol'; -import {Location} from './types'; +import {Span} from './types'; -export function getDefinition(info: TemplateInfo): Location[]|undefined { - const result = locateSymbol(info); - return result && result.symbol.definition; -} - -export function ngLocationToTsDefinitionInfo(loc: Location): tss.DefinitionInfo { +/** + * Convert Angular Span to TypeScript TextSpan. Angular Span has 'start' and + * 'end' whereas TS TextSpan has 'start' and 'length'. + * @param span Angular Span + */ +function ngSpanToTsTextSpan(span: Span): ts.TextSpan { return { - fileName: loc.fileName, - textSpan: { - start: loc.span.start, - length: loc.span.end - loc.span.start, - }, - // TODO(kyliau): Provide more useful info for name, kind and containerKind - name: '', // should be name of symbol but we don't have enough information here. - kind: tss.ScriptElementKind.unknown, - containerName: loc.fileName, - containerKind: tss.ScriptElementKind.unknown, + start: span.start, + length: span.end - span.start, + }; +} + +export function getDefinitionAndBoundSpan(info: TemplateInfo): ts.DefinitionInfoAndBoundSpan| + undefined { + const symbolInfo = locateSymbol(info); + if (!symbolInfo) { + return; + } + const textSpan = ngSpanToTsTextSpan(symbolInfo.span); + const {symbol} = symbolInfo; + const {container, definition: locations} = symbol; + if (!locations || !locations.length) { + // symbol.definition is really the locations of the symbol. There could be + // more than one. No meaningful info could be provided without any location. + return {textSpan}; + } + const containerKind = container ? container.kind : ts.ScriptElementKind.unknown; + const containerName = container ? container.name : ''; + const definitions = locations.map((location) => { + return { + kind: symbol.kind as ts.ScriptElementKind, + name: symbol.name, + containerKind: containerKind as ts.ScriptElementKind, + containerName: containerName, + textSpan: ngSpanToTsTextSpan(location.span), + fileName: location.fileName, + }; + }); + return { + definitions, textSpan, }; } diff --git a/packages/language-service/src/hover.ts b/packages/language-service/src/hover.ts index bc28e2f349..d1f6f50990 100644 --- a/packages/language-service/src/hover.ts +++ b/packages/language-service/src/hover.ts @@ -6,23 +6,42 @@ * found in the LICENSE file at https://angular.io/license */ +import * as ts from 'typescript'; import {TemplateInfo} from './common'; import {locateSymbol} from './locate_symbol'; -import {Hover, HoverTextSection, Symbol} from './types'; -export function getHover(info: TemplateInfo): Hover|undefined { - const result = locateSymbol(info); - if (result) { - return {text: hoverTextOf(result.symbol), span: result.span}; +// Reverse mappings of enum would generate strings +const SYMBOL_SPACE = ts.SymbolDisplayPartKind[ts.SymbolDisplayPartKind.space]; +const SYMBOL_PUNC = ts.SymbolDisplayPartKind[ts.SymbolDisplayPartKind.punctuation]; + +export function getHover(info: TemplateInfo): ts.QuickInfo|undefined { + const symbolInfo = locateSymbol(info); + if (!symbolInfo) { + return; } + const {symbol, span} = symbolInfo; + const containerDisplayParts: ts.SymbolDisplayPart[] = symbol.container ? + [ + {text: symbol.container.name, kind: symbol.container.kind}, + {text: '.', kind: SYMBOL_PUNC}, + ] : + []; + return { + kind: symbol.kind as ts.ScriptElementKind, + kindModifiers: '', // kindModifier info not available on 'ng.Symbol' + textSpan: { + start: span.start, + length: span.end - span.start, + }, + // this would generate a string like '(property) ClassX.propY' + // 'kind' in displayParts does not really matter because it's dropped when + // displayParts get converted to string. + displayParts: [ + {text: '(', kind: SYMBOL_PUNC}, {text: symbol.kind, kind: symbol.kind}, + {text: ')', kind: SYMBOL_PUNC}, {text: ' ', kind: SYMBOL_SPACE}, ...containerDisplayParts, + {text: symbol.name, kind: symbol.kind}, + // TODO: Append type info as well, but Symbol doesn't expose that! + // Ideally hover text should be like '(property) ClassX.propY: string' + ], + }; } - -function hoverTextOf(symbol: Symbol): HoverTextSection[] { - const result: HoverTextSection[] = - [{text: symbol.kind}, {text: ' '}, {text: symbol.name, language: symbol.language}]; - const container = symbol.container; - if (container) { - result.push({text: ' of '}, {text: container.name, language: container.language}); - } - return result; -} \ No newline at end of file diff --git a/packages/language-service/src/language_service.ts b/packages/language-service/src/language_service.ts index b0a341cb5f..13866d7df4 100644 --- a/packages/language-service/src/language_service.ts +++ b/packages/language-service/src/language_service.ts @@ -8,9 +8,9 @@ import {CompileMetadataResolver, CompilePipeSummary} from '@angular/compiler'; import {DiagnosticTemplateInfo, getTemplateExpressionDiagnostics} from '@angular/compiler-cli/src/language_services'; - +import * as tss from 'typescript/lib/tsserverlibrary'; import {getTemplateCompletions} from './completions'; -import {getDefinition} from './definitions'; +import {getDefinitionAndBoundSpan} from './definitions'; import {getDeclarationDiagnostics} from './diagnostics'; import {getHover} from './hover'; import {Completion, Diagnostic, DiagnosticKind, Diagnostics, Hover, LanguageService, LanguageServiceHost, Location, Span, TemplateSource} from './types'; @@ -30,8 +30,6 @@ export function createLanguageService(host: LanguageServiceHost): LanguageServic class LanguageServiceImpl implements LanguageService { constructor(private host: LanguageServiceHost) {} - private get metadataResolver(): CompileMetadataResolver { return this.host.resolver; } - getTemplateReferences(): string[] { return this.host.getTemplateReferences(); } getDiagnostics(fileName: string): Diagnostic[] { @@ -65,14 +63,14 @@ class LanguageServiceImpl implements LanguageService { } } - getDefinitionAt(fileName: string, position: number): Location[]|undefined { + getDefinitionAt(fileName: string, position: number): tss.DefinitionInfoAndBoundSpan|undefined { let templateInfo = this.host.getTemplateAstAtPosition(fileName, position); if (templateInfo) { - return getDefinition(templateInfo); + return getDefinitionAndBoundSpan(templateInfo); } } - getHoverAt(fileName: string, position: number): Hover|undefined { + getHoverAt(fileName: string, position: number): tss.QuickInfo|undefined { let templateInfo = this.host.getTemplateAstAtPosition(fileName, position); if (templateInfo) { return getHover(templateInfo); diff --git a/packages/language-service/src/ts_plugin.ts b/packages/language-service/src/ts_plugin.ts index eb41009748..3622dd9599 100644 --- a/packages/language-service/src/ts_plugin.ts +++ b/packages/language-service/src/ts_plugin.ts @@ -9,9 +9,8 @@ import * as ts from 'typescript'; // used as value, passed in by tsserver at runtime import * as tss from 'typescript/lib/tsserverlibrary'; // used as type only -import {ngLocationToTsDefinitionInfo} from './definitions'; import {createLanguageService} from './language_service'; -import {Completion, Diagnostic, DiagnosticMessageChain, Location} from './types'; +import {Completion, Diagnostic, DiagnosticMessageChain} from './types'; import {TypeScriptServiceHost} from './typescript_host'; const projectHostMap = new WeakMap(); @@ -76,13 +75,13 @@ export function create(info: tss.server.PluginCreateInfo): tss.LanguageService { // This effectively disables native TS features and is meant for internal // use only. const angularOnly = config ? config.angularOnly === true : false; - const proxy: tss.LanguageService = Object.assign({}, tsLS); const ngLSHost = new TypeScriptServiceHost(tsLSHost, tsLS); const ngLS = createLanguageService(ngLSHost); projectHostMap.set(project, ngLSHost); - proxy.getCompletionsAtPosition = function( - fileName: string, position: number, options: tss.GetCompletionsAtPositionOptions|undefined) { + function getCompletionsAtPosition( + fileName: string, position: number, + options: tss.GetCompletionsAtPositionOptions | undefined) { if (!angularOnly) { const results = tsLS.getCompletionsAtPosition(fileName, position, options); if (results && results.entries.length) { @@ -100,39 +99,20 @@ export function create(info: tss.server.PluginCreateInfo): tss.LanguageService { isNewIdentifierLocation: false, entries: results.map(completionToEntry), }; - }; + } - proxy.getQuickInfoAtPosition = function(fileName: string, position: number): tss.QuickInfo | - undefined { - if (!angularOnly) { - const result = tsLS.getQuickInfoAtPosition(fileName, position); - if (result) { - // If TS could answer the query, then return results immediately. - return result; - } - } - const result = ngLS.getHoverAt(fileName, position); - if (!result) { - return; - } - return { - // TODO(kyliau): Provide more useful info for kind and kindModifiers - kind: ts.ScriptElementKind.unknown, - kindModifiers: ts.ScriptElementKindModifier.none, - textSpan: { - start: result.span.start, - length: result.span.end - result.span.start, - }, - displayParts: result.text.map((part) => { - return { - text: part.text, - kind: part.language || 'angular', - }; - }), - }; - }; + function getQuickInfoAtPosition(fileName: string, position: number): tss.QuickInfo|undefined { + if (!angularOnly) { + const result = tsLS.getQuickInfoAtPosition(fileName, position); + if (result) { + // If TS could answer the query, then return results immediately. + return result; + } + } + return ngLS.getHoverAt(fileName, position); + } - proxy.getSemanticDiagnostics = function(fileName: string): tss.Diagnostic[] { + function getSemanticDiagnostics(fileName: string): tss.Diagnostic[] { const results: tss.Diagnostic[] = []; if (!angularOnly) { const tsResults = tsLS.getSemanticDiagnostics(fileName); @@ -146,48 +126,43 @@ export function create(info: tss.server.PluginCreateInfo): tss.LanguageService { const sourceFile = fileName.endsWith('.ts') ? ngLSHost.getSourceFile(fileName) : undefined; results.push(...ngResults.map(d => diagnosticToDiagnostic(d, sourceFile))); return results; - }; + } - proxy.getDefinitionAtPosition = function(fileName: string, position: number): - ReadonlyArray| - undefined { - if (!angularOnly) { - const results = tsLS.getDefinitionAtPosition(fileName, position); - if (results) { - // If TS could answer the query, then return results immediately. - return results; - } - } - const results = ngLS.getDefinitionAt(fileName, position); - if (!results) { - return; - } - return results.map(ngLocationToTsDefinitionInfo); - }; + function getDefinitionAtPosition( + fileName: string, position: number): ReadonlyArray|undefined { + if (!angularOnly) { + const results = tsLS.getDefinitionAtPosition(fileName, position); + if (results) { + // If TS could answer the query, then return results immediately. + return results; + } + } + const result = ngLS.getDefinitionAt(fileName, position); + if (!result || !result.definitions || !result.definitions.length) { + return; + } + return result.definitions; + } - proxy.getDefinitionAndBoundSpan = function(fileName: string, position: number): - tss.DefinitionInfoAndBoundSpan | - undefined { - if (!angularOnly) { - const result = tsLS.getDefinitionAndBoundSpan(fileName, position); - if (result) { - // If TS could answer the query, then return results immediately. - return result; - } - } - const results = ngLS.getDefinitionAt(fileName, position); - if (!results || !results.length) { - return; - } - const {span} = results[0]; - return { - definitions: results.map(ngLocationToTsDefinitionInfo), - textSpan: { - start: span.start, - length: span.end - span.start, - }, - }; - }; + function getDefinitionAndBoundSpan( + fileName: string, position: number): tss.DefinitionInfoAndBoundSpan|undefined { + if (!angularOnly) { + const result = tsLS.getDefinitionAndBoundSpan(fileName, position); + if (result) { + // If TS could answer the query, then return results immediately. + return result; + } + } + return ngLS.getDefinitionAt(fileName, position); + } + const proxy: tss.LanguageService = Object.assign( + // First clone the original TS language service + {}, tsLS, + // Then override the methods supported by Angular language service + { + getCompletionsAtPosition, getQuickInfoAtPosition, getSemanticDiagnostics, + getDefinitionAtPosition, getDefinitionAndBoundSpan, + }); return proxy; } diff --git a/packages/language-service/src/types.ts b/packages/language-service/src/types.ts index b6c25e245b..0132f65f10 100644 --- a/packages/language-service/src/types.ts +++ b/packages/language-service/src/types.ts @@ -8,6 +8,8 @@ import {CompileDirectiveMetadata, CompileMetadataResolver, CompilePipeSummary, NgAnalyzedModules, StaticSymbol} from '@angular/compiler'; import {BuiltinType, DeclarationKind, Definition, PipeInfo, Pipes, Signature, Span, Symbol, SymbolDeclaration, SymbolQuery, SymbolTable} from '@angular/compiler-cli/src/language_services'; +import * as tss from 'typescript/lib/tsserverlibrary'; + import {AstResult, TemplateInfo} from './common'; export { @@ -394,12 +396,12 @@ export interface LanguageService { /** * Return the definition location for the symbol at position. */ - getDefinitionAt(fileName: string, position: number): Location[]|undefined; + getDefinitionAt(fileName: string, position: number): tss.DefinitionInfoAndBoundSpan|undefined; /** * Return the hover information for the symbol at position. */ - getHoverAt(fileName: string, position: number): Hover|undefined; + getHoverAt(fileName: string, position: number): tss.QuickInfo|undefined; /** * Return the pipes that are available at the given position. diff --git a/packages/language-service/test/definitions_spec.ts b/packages/language-service/test/definitions_spec.ts index 752d0a0ba6..48f608e01b 100644 --- a/packages/language-service/test/definitions_spec.ts +++ b/packages/language-service/test/definitions_spec.ts @@ -9,159 +9,319 @@ import * as ts from 'typescript'; import {createLanguageService} from '../src/language_service'; -import {Span} from '../src/types'; +import {LanguageService} from '../src/types'; import {TypeScriptServiceHost} from '../src/typescript_host'; import {toh} from './test_data'; - -import {MockTypescriptHost,} from './test_utils'; +import {MockTypescriptHost} from './test_utils'; describe('definitions', () => { - let documentRegistry = ts.createDocumentRegistry(); - let mockHost = new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts'], toh); - let service = ts.createLanguageService(mockHost, documentRegistry); - let ngHost = new TypeScriptServiceHost(mockHost, service); - let ngService = createLanguageService(ngHost); + let mockHost: MockTypescriptHost; + let service: ts.LanguageService; + let ngHost: TypeScriptServiceHost; + let ngService: LanguageService; + + beforeEach(() => { + // Create a new mockHost every time to reset any files that are overridden. + mockHost = new MockTypescriptHost(['/app/main.ts', '/app/parsing-cases.ts'], toh); + service = ts.createLanguageService(mockHost); + ngHost = new TypeScriptServiceHost(mockHost, service); + ngService = createLanguageService(ngHost); + }); it('should be able to find field in an interpolation', () => { - localReference( - ` @Component({template: '{{«name»}}'}) export class MyComponent { «ᐱnameᐱ: string;» }`); + const fileName = addCode(` + @Component({ + template: '{{«name»}}' + }) + export class MyComponent { + «ᐱnameᐱ: string;» + }`); + + const marker = getReferenceMarkerFor(fileName, 'name'); + const result = ngService.getDefinitionAt(fileName, marker.start); + expect(result).toBeDefined(); + const {textSpan, definitions} = result !; + + expect(textSpan).toEqual(marker); + expect(definitions).toBeDefined(); + expect(definitions !.length).toBe(1); + const def = definitions ![0]; + + expect(def.fileName).toBe(fileName); + expect(def.name).toBe('name'); + expect(def.kind).toBe('property'); + expect(def.textSpan).toEqual(getDefinitionMarkerFor(fileName, 'name')); }); it('should be able to find a field in a attribute reference', () => { - localReference( - ` @Component({template: ''}) export class MyComponent { «ᐱnameᐱ: string;» }`); + const fileName = addCode(` + @Component({ + template: '' + }) + export class MyComponent { + «ᐱnameᐱ: string;» + }`); + + const marker = getReferenceMarkerFor(fileName, 'name'); + const result = ngService.getDefinitionAt(fileName, marker.start); + expect(result).toBeDefined(); + const {textSpan, definitions} = result !; + + expect(textSpan).toEqual(marker); + expect(definitions).toBeDefined(); + expect(definitions !.length).toBe(1); + const def = definitions ![0]; + + expect(def.fileName).toBe(fileName); + expect(def.name).toBe('name'); + expect(def.kind).toBe('property'); + expect(def.textSpan).toEqual(getDefinitionMarkerFor(fileName, 'name')); }); it('should be able to find a method from a call', () => { - localReference( - ` @Component({template: '
'}) export class MyComponent { «ᐱmyClickᐱ() { }»}`); + const fileName = addCode(` + @Component({ + template: '
' + }) + export class MyComponent { + «ᐱmyClickᐱ() { }» + }`); + + const marker = getReferenceMarkerFor(fileName, 'myClick'); + const result = ngService.getDefinitionAt(fileName, marker.start); + expect(result).toBeDefined(); + const {textSpan, definitions} = result !; + + expect(textSpan).toEqual(getLocationMarkerFor(fileName, 'my')); + expect(definitions).toBeDefined(); + expect(definitions !.length).toBe(1); + const def = definitions ![0]; + + expect(def.fileName).toBe(fileName); + expect(def.name).toBe('myClick'); + expect(def.kind).toBe('method'); + expect(def.textSpan).toEqual(getDefinitionMarkerFor(fileName, 'myClick')); }); it('should be able to find a field reference in an *ngIf', () => { - localReference( - ` @Component({template: '
'}) export class MyComponent { «ᐱincludeᐱ = true;»}`); + const fileName = addCode(` + @Component({ + template: '
' + }) + export class MyComponent { + «ᐱincludeᐱ = true;» + }`); + + const marker = getReferenceMarkerFor(fileName, 'include'); + const result = ngService.getDefinitionAt(fileName, marker.start); + expect(result).toBeDefined(); + const {textSpan, definitions} = result !; + + expect(textSpan).toEqual(marker); + expect(definitions).toBeDefined(); + expect(definitions !.length).toBe(1); + const def = definitions ![0]; + + expect(def.fileName).toBe(fileName); + expect(def.name).toBe('include'); + expect(def.kind).toBe('property'); + expect(def.textSpan).toEqual(getDefinitionMarkerFor(fileName, 'include')); }); it('should be able to find a reference to a component', () => { - reference( - 'parsing-cases.ts', - ` @Component({template: '<«test-comp»>'}) export class MyComponent { }`); + const fileName = addCode(` + @Component({ + template: '~{start-my}<«test-comp»>~{end-my}' + }) + export class MyComponent { }`); + + // Get the marker for «test-comp» in the code added above. + const marker = getReferenceMarkerFor(fileName, 'test-comp'); + + const result = ngService.getDefinitionAt(fileName, marker.start); + expect(result).toBeDefined(); + const {textSpan, definitions} = result !; + + // Get the marker for bounded text in the code added above. + const boundedText = getLocationMarkerFor(fileName, 'my'); + expect(textSpan).toEqual(boundedText); + + // There should be exactly 1 definition + expect(definitions).toBeDefined(); + expect(definitions !.length).toBe(1); + const def = definitions ![0]; + + const refFileName = '/app/parsing-cases.ts'; + expect(def.fileName).toBe(refFileName); + expect(def.name).toBe('TestComponent'); + expect(def.kind).toBe('component'); + expect(def.textSpan).toEqual(getLocationMarkerFor(refFileName, 'test-comp')); }); it('should be able to find an event provider', () => { - reference( - '/app/parsing-cases.ts', 'test', - ` @Component({template: ''}) export class MyComponent { myHandler() {} }`); + const fileName = addCode(` + @Component({ + template: '' + }) + export class MyComponent { myHandler() {} }`); + + // Get the marker for «test» in the code added above. + const marker = getReferenceMarkerFor(fileName, 'test'); + + const result = ngService.getDefinitionAt(fileName, marker.start); + expect(result).toBeDefined(); + const {textSpan, definitions} = result !; + + // Get the marker for bounded text in the code added above + const boundedText = getLocationMarkerFor(fileName, 'my'); + expect(textSpan).toEqual(boundedText); + + // There should be exactly 1 definition + expect(definitions).toBeDefined(); + expect(definitions !.length).toBe(1); + const def = definitions ![0]; + + const refFileName = '/app/parsing-cases.ts'; + expect(def.fileName).toBe(refFileName); + expect(def.name).toBe('testEvent'); + expect(def.kind).toBe('event'); + expect(def.textSpan).toEqual(getDefinitionMarkerFor(refFileName, 'test')); }); it('should be able to find an input provider', () => { - reference( - '/app/parsing-cases.ts', 'tcName', - ` @Component({template: ''}) export class MyComponent { name = 'my name'; }`); + // '/app/parsing-cases.ts', 'tcName', + const fileName = addCode(` + @Component({ + template: '' + }) + export class MyComponent { + name = 'my name'; + }`); + + // Get the marker for «test» in the code added above. + const marker = getReferenceMarkerFor(fileName, 'tcName'); + + const result = ngService.getDefinitionAt(fileName, marker.start); + expect(result).toBeDefined(); + const {textSpan, definitions} = result !; + + // Get the marker for bounded text in the code added above + const boundedText = getLocationMarkerFor(fileName, 'my'); + expect(textSpan).toEqual(boundedText); + + // There should be exactly 1 definition + expect(definitions).toBeDefined(); + expect(definitions !.length).toBe(1); + const def = definitions ![0]; + + const refFileName = '/app/parsing-cases.ts'; + expect(def.fileName).toBe(refFileName); + expect(def.name).toBe('name'); + expect(def.kind).toBe('property'); + expect(def.textSpan).toEqual(getDefinitionMarkerFor(refFileName, 'tcName')); }); it('should be able to find a pipe', () => { - reference( - 'common.d.ts', - ` @Component({template: '
'}) export class MyComponent { input: EventEmitter; }`); + const fileName = addCode(` + @Component({ + template: '
' + }) + export class MyComponent { + input: EventEmitter; + }`); + + // Get the marker for «test» in the code added above. + const marker = getReferenceMarkerFor(fileName, 'async'); + + const result = ngService.getDefinitionAt(fileName, marker.start); + expect(result).toBeDefined(); + const {textSpan, definitions} = result !; + + // Get the marker for bounded text in the code added above + const boundedText = getLocationMarkerFor(fileName, 'my'); + expect(textSpan).toEqual(boundedText); + + expect(definitions).toBeDefined(); + expect(definitions !.length).toBe(4); + + const refFileName = '/node_modules/@angular/common/common.d.ts'; + for (const def of definitions !) { + expect(def.fileName).toBe(refFileName); + expect(def.name).toBe('async'); + expect(def.kind).toBe('pipe'); + // Not asserting the textSpan of definition because it's external file + } }); - function localReference(code: string) { - addCode(code, fileName => { - const refResult = mockHost.getReferenceMarkers(fileName) !; - for (const name in refResult.references) { - const references = refResult.references[name]; - const definitions = refResult.definitions[name]; - expect(definitions).toBeDefined(); // If this fails the test data is wrong. - for (const reference of references) { - const definition = ngService.getDefinitionAt(fileName, reference.start); - if (definition) { - definition.forEach(d => expect(d.fileName).toEqual(fileName)); - const match = matchingSpan(definition.map(d => d.span), definitions); - if (!match) { - throw new Error( - `Expected one of ${stringifySpans(definition.map(d => d.span))} to match one of ${stringifySpans(definitions)}`); - } - } else { - throw new Error('Expected a definition'); - } - } - } - }); - } - - function reference(referencedFile: string, code: string): void; - function reference(referencedFile: string, span: Span, code: string): void; - function reference(referencedFile: string, definition: string, code: string): void; - function reference(referencedFile: string, p1?: any, p2?: any): void { - const code: string = p2 ? p2 : p1; - const definition: string = p2 ? p1 : undefined; - let span: Span = p2 && p1.start != null ? p1 : undefined; - if (definition && !span) { - const referencedFileMarkers = mockHost.getReferenceMarkers(referencedFile) !; - expect(referencedFileMarkers).toBeDefined(); // If this fails the test data is wrong. - const spans = referencedFileMarkers.definitions[definition]; - expect(spans).toBeDefined(); // If this fails the test data is wrong. - span = spans[0]; - } - addCode(code, fileName => { - const refResult = mockHost.getReferenceMarkers(fileName) !; - let tests = 0; - for (const name in refResult.references) { - const references = refResult.references[name]; - expect(reference).toBeDefined(); // If this fails the test data is wrong. - for (const reference of references) { - tests++; - const definition = ngService.getDefinitionAt(fileName, reference.start); - if (definition) { - definition.forEach(d => { - if (d.fileName.indexOf(referencedFile) < 0) { - throw new Error( - `Expected reference to file ${referencedFile}, received ${d.fileName}`); - } - if (span) { - expect(d.span).toEqual(span); - } - }); - } else { - throw new Error('Expected a definition'); - } - } - } - if (!tests) { - throw new Error('Expected at least one reference (test data error)'); - } - }); - } - - function addCode(code: string, cb: (fileName: string, content?: string) => void) { + /** + * Append a snippet of code to `app.component.ts` and return the file name. + * There must not be any name collision with existing code. + * @param code Snippet of code + */ + function addCode(code: string) { const fileName = '/app/app.component.ts'; const originalContent = mockHost.getFileContent(fileName); const newContent = originalContent + code; - mockHost.override(fileName, originalContent + code); - try { - cb(fileName, newContent); - } finally { - mockHost.override(fileName, undefined !); - } + mockHost.override(fileName, newContent); + return fileName; + } + + /** + * Returns the definition marker ᐱselectorᐱ for the specified 'selector'. + * Asserts that marker exists. + * @param fileName name of the file + * @param selector name of the marker + */ + function getDefinitionMarkerFor(fileName: string, selector: string): ts.TextSpan { + const markers = mockHost.getReferenceMarkers(fileName); + expect(markers).toBeDefined(); + expect(Object.keys(markers !.definitions)).toContain(selector); + expect(markers !.definitions[selector].length).toBe(1); + const marker = markers !.definitions[selector][0]; + expect(marker.start).toBeLessThanOrEqual(marker.end); + return { + start: marker.start, + length: marker.end - marker.start, + }; + } + + /** + * Returns the reference marker «selector» for the specified 'selector'. + * Asserts that marker exists. + * @param fileName name of the file + * @param selector name of the marker + */ + function getReferenceMarkerFor(fileName: string, selector: string): ts.TextSpan { + const markers = mockHost.getReferenceMarkers(fileName); + expect(markers).toBeDefined(); + expect(Object.keys(markers !.references)).toContain(selector); + expect(markers !.references[selector].length).toBe(1); + const marker = markers !.references[selector][0]; + expect(marker.start).toBeLessThanOrEqual(marker.end); + return { + start: marker.start, + length: marker.end - marker.start, + }; + } + + /** + * Returns the location marker ~{selector} for the specified 'selector'. + * Asserts that marker exists. + * @param fileName name of the file + * @param selector name of the marker + */ + function getLocationMarkerFor(fileName: string, selector: string): ts.TextSpan { + const markers = mockHost.getMarkerLocations(fileName); + expect(markers).toBeDefined(); + const start = markers ![`start-${selector}`]; + expect(start).toBeDefined(); + const end = markers ![`end-${selector}`]; + expect(end).toBeDefined(); + expect(start).toBeLessThanOrEqual(end); + return { + start: start, + length: end - start, + }; } }); - -function matchingSpan(aSpans: Span[], bSpans: Span[]): Span|undefined { - for (const a of aSpans) { - for (const b of bSpans) { - if (a.start == b.start && a.end == b.end) { - return a; - } - } - } -} - -function stringifySpan(span: Span) { - return span ? `(${span.start}-${span.end})` : ''; -} - -function stringifySpans(spans: Span[]) { - return spans ? `[${spans.map(stringifySpan).join(', ')}]` : ''; -} diff --git a/packages/language-service/test/hover_spec.ts b/packages/language-service/test/hover_spec.ts index 7e0d3ffdab..3cbc062dde 100644 --- a/packages/language-service/test/hover_spec.ts +++ b/packages/language-service/test/hover_spec.ts @@ -28,43 +28,43 @@ describe('hover', () => { it('should be able to find field in an interpolation', () => { hover( ` @Component({template: '{{«name»}}'}) export class MyComponent { name: string; }`, - 'property name of MyComponent'); + '(property) MyComponent.name'); }); it('should be able to find a field in a attribute reference', () => { hover( ` @Component({template: ''}) export class MyComponent { name: string; }`, - 'property name of MyComponent'); + '(property) MyComponent.name'); }); it('should be able to find a method from a call', () => { hover( ` @Component({template: '
'}) export class MyComponent { myClick() { }}`, - 'method myClick of MyComponent'); + '(method) MyComponent.myClick'); }); it('should be able to find a field reference in an *ngIf', () => { hover( ` @Component({template: '
'}) export class MyComponent { include = true;}`, - 'property include of MyComponent'); + '(property) MyComponent.include'); }); it('should be able to find a reference to a component', () => { hover( ` @Component({template: '«<ᐱtestᐱ-comp>»'}) export class MyComponent { }`, - 'component TestComponent'); + '(component) TestComponent'); }); it('should be able to find an event provider', () => { hover( ` @Component({template: ''}) export class MyComponent { myHandler() {} }`, - 'event testEvent of TestComponent'); + '(event) TestComponent.testEvent'); }); it('should be able to find an input provider', () => { hover( ` @Component({template: ''}) export class MyComponent { name = 'my name'; }`, - 'property name of TestComponent'); + '(property) TestComponent.name'); }); it('should be able to ignore a reference declaration', () => { @@ -87,10 +87,13 @@ describe('hover', () => { []).concat(markers.definitions[referenceName] || []); for (const reference of references) { tests++; - const hover = ngService.getHoverAt(fileName, reference.start); - if (!hover) throw new Error(`Expected a hover at location ${reference.start}`); - expect(hover.span).toEqual(reference); - expect(toText(hover)).toEqual(hoverText); + const quickInfo = ngService.getHoverAt(fileName, reference.start); + if (!quickInfo) throw new Error(`Expected a hover at location ${reference.start}`); + expect(quickInfo.textSpan).toEqual({ + start: reference.start, + length: reference.end - reference.start, + }); + expect(toText(quickInfo)).toEqual(hoverText); } } expect(tests).toBeGreaterThan(0); // If this fails the test is wrong. @@ -109,5 +112,8 @@ describe('hover', () => { } } - function toText(hover: Hover): string { return hover.text.map(h => h.text).join(''); } + function toText(quickInfo: ts.QuickInfo): string { + const displayParts = quickInfo.displayParts || []; + return displayParts.map(p => p.text).join(''); + } }); diff --git a/packages/language-service/test/test_data.ts b/packages/language-service/test/test_data.ts index ebc639f9e2..9db91e508d 100644 --- a/packages/language-service/test/test_data.ts +++ b/packages/language-service/test/test_data.ts @@ -139,11 +139,11 @@ export class ForUsingComponent { @Component({template: '
{{~{test-comp-content}}} {{test1.~{test-comp-after-test}name}} {{div.~{test-comp-after-div}.innerText}}
'}) export class References {} -@Component({selector: 'test-comp', template: '
Testing: {{name}}
'}) +~{start-test-comp}@Component({selector: 'test-comp', template: '
Testing: {{name}}
'}) export class TestComponent { «@Input('ᐱtcNameᐱ') name = 'test';» «@Output('ᐱtestᐱ') testEvent = new EventEmitter();» -} +}~{end-test-comp} @Component({templateUrl: 'test.ng'}) export class TemplateReference {