From 5a562d8a0ae2a07b7e9c7acc688703aa63e7c058 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Mon, 12 Aug 2019 16:54:36 -0700 Subject: [PATCH] refactor(language-service): Return ts.CompletionInfo for getCompletionsAt() (#32116) Part 3/3 of language-service refactoring: Change all language service APIs to return TS value since Angular LS will be a proper tsserver plugin. This reduces the need to transform results among Angular <--> TS <--> LSP. PR Close #32116 --- packages/language-service/src/completions.ts | 9 ++ packages/language-service/src/diagnostics.ts | 18 +++ .../language-service/src/language_service.ts | 50 ++----- packages/language-service/src/ts_plugin.ts | 23 +-- packages/language-service/src/types.ts | 9 +- .../language-service/src/typescript_host.ts | 135 +++++++++--------- .../language-service/test/completions_spec.ts | 23 ++- 7 files changed, 124 insertions(+), 143 deletions(-) diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index 4e9a89b263..ff983438ca 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -498,3 +498,12 @@ function expandedAttr(attr: AttrInfo): AttrInfo[] { function lowerName(name: string): string { return name && (name[0].toLowerCase() + name.substr(1)); } + +export function ngCompletionToTsCompletionEntry(completion: Completion): ts.CompletionEntry { + return { + name: completion.name, + kind: completion.kind as ts.ScriptElementKind, + kindModifiers: '', + sortText: completion.sort, + }; +} diff --git a/packages/language-service/src/diagnostics.ts b/packages/language-service/src/diagnostics.ts index ecc60989a6..d33cf5e853 100644 --- a/packages/language-service/src/diagnostics.ts +++ b/packages/language-service/src/diagnostics.ts @@ -130,3 +130,21 @@ export function ngDiagnosticToTsDiagnostic( source: 'ng', }; } + +export function uniqueBySpan(elements: T[]): T[] { + const result: T[] = []; + const map = new Map>(); + for (const element of elements) { + const {span} = element; + let set = map.get(span.start); + if (!set) { + set = new Set(); + map.set(span.start, set); + } + if (!set.has(span.end)) { + set.add(span.end); + result.push(element); + } + } + return result; +} diff --git a/packages/language-service/src/language_service.ts b/packages/language-service/src/language_service.ts index 0857d23ab6..d451209f13 100644 --- a/packages/language-service/src/language_service.ts +++ b/packages/language-service/src/language_service.ts @@ -6,17 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ -import {CompilePipeSummary} from '@angular/compiler'; import * as tss from 'typescript/lib/tsserverlibrary'; - -import {getTemplateCompletions} from './completions'; +import {getTemplateCompletions, ngCompletionToTsCompletionEntry} from './completions'; import {getDefinitionAndBoundSpan} from './definitions'; -import {getDeclarationDiagnostics, getTemplateDiagnostics, ngDiagnosticToTsDiagnostic} from './diagnostics'; +import {getDeclarationDiagnostics, getTemplateDiagnostics, ngDiagnosticToTsDiagnostic, uniqueBySpan} from './diagnostics'; import {getHover} from './hover'; -import {Completion, Diagnostic, LanguageService, Span} from './types'; +import {Diagnostic, LanguageService} from './types'; import {TypeScriptServiceHost} from './typescript_host'; - /** * Create an instance of an Angular `LanguageService`. * @@ -53,21 +50,22 @@ class LanguageServiceImpl implements LanguageService { return uniqueBySpan(results).map(d => ngDiagnosticToTsDiagnostic(d, sourceFile)); } - getPipesAt(fileName: string, position: number): CompilePipeSummary[] { + getCompletionsAt(fileName: string, position: number): tss.CompletionInfo|undefined { this.host.getAnalyzedModules(); // same role as 'synchronizeHostData' const templateInfo = this.host.getTemplateAstAtPosition(fileName, position); - if (templateInfo) { - return templateInfo.pipes; + if (!templateInfo) { + return; } - return []; - } - - getCompletionsAt(fileName: string, position: number): Completion[]|undefined { - this.host.getAnalyzedModules(); // same role as 'synchronizeHostData' - const templateInfo = this.host.getTemplateAstAtPosition(fileName, position); - if (templateInfo) { - return getTemplateCompletions(templateInfo); + const results = getTemplateCompletions(templateInfo); + if (!results || !results.length) { + return; } + return { + isGlobalCompletion: false, + isMemberCompletion: false, + isNewIdentifierLocation: false, + entries: results.map(ngCompletionToTsCompletionEntry), + }; } getDefinitionAt(fileName: string, position: number): tss.DefinitionInfoAndBoundSpan|undefined { @@ -86,21 +84,3 @@ class LanguageServiceImpl implements LanguageService { } } } - -function uniqueBySpan(elements: T[]): T[] { - const result: T[] = []; - const map = new Map>(); - for (const element of elements) { - const {span} = element; - let set = map.get(span.start); - if (!set) { - set = new Set(); - map.set(span.start, set); - } - if (!set.has(span.end)) { - set.add(span.end); - result.push(element); - } - } - return result; -} diff --git a/packages/language-service/src/ts_plugin.ts b/packages/language-service/src/ts_plugin.ts index 26d562a673..c6c6c149dc 100644 --- a/packages/language-service/src/ts_plugin.ts +++ b/packages/language-service/src/ts_plugin.ts @@ -6,11 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ -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 {createLanguageService} from './language_service'; -import {Completion} from './types'; import {TypeScriptServiceHost} from './typescript_host'; const projectHostMap = new WeakMap(); @@ -24,16 +22,6 @@ export function getExternalFiles(project: tss.server.Project): string[]|undefine } } -function completionToEntry(c: Completion): tss.CompletionEntry { - return { - // TODO: remove any and fix type error. - kind: c.kind as any, - name: c.name, - sortText: c.sort, - kindModifiers: '' - }; -} - export function create(info: tss.server.PluginCreateInfo): tss.LanguageService { const {project, languageService: tsLS, languageServiceHost: tsLSHost, config} = info; // This plugin could operate under two different modes: @@ -60,16 +48,7 @@ export function create(info: tss.server.PluginCreateInfo): tss.LanguageService { return results; } } - const results = ngLS.getCompletionsAt(fileName, position); - if (!results || !results.length) { - return; - } - return { - isGlobalCompletion: false, - isMemberCompletion: false, - isNewIdentifierLocation: false, - entries: results.map(completionToEntry), - }; + return ngLS.getCompletionsAt(fileName, position); } function getQuickInfoAtPosition(fileName: string, position: number): tss.QuickInfo|undefined { diff --git a/packages/language-service/src/types.ts b/packages/language-service/src/types.ts index 1fc3cde624..56ef99a3f2 100644 --- a/packages/language-service/src/types.ts +++ b/packages/language-service/src/types.ts @@ -376,7 +376,7 @@ export interface LanguageService { /** * Returns a list of all the external templates referenced by the project. */ - getTemplateReferences(): string[]|undefined; + getTemplateReferences(): string[]; /** * Returns a list of all error for all templates in the given file. @@ -386,7 +386,7 @@ export interface LanguageService { /** * Return the completions at the given position. */ - getCompletionsAt(fileName: string, position: number): Completion[]|undefined; + getCompletionsAt(fileName: string, position: number): tss.CompletionInfo|undefined; /** * Return the definition location for the symbol at position. @@ -397,9 +397,4 @@ export interface LanguageService { * Return the hover information for the symbol at position. */ getHoverAt(fileName: string, position: number): tss.QuickInfo|undefined; - - /** - * Return the pipes that are available at the given position. - */ - getPipesAt(fileName: string, position: number): CompilePipeSummary[]; } diff --git a/packages/language-service/src/typescript_host.ts b/packages/language-service/src/typescript_host.ts index 869cf9a017..3235964c21 100644 --- a/packages/language-service/src/typescript_host.ts +++ b/packages/language-service/src/typescript_host.ts @@ -219,8 +219,8 @@ export class TypeScriptServiceHost implements LanguageServiceHost { const result: Declarations = []; const sourceFile = this.getSourceFile(fileName); if (sourceFile) { - let visit = (child: ts.Node) => { - let declaration = this.getDeclarationFromNode(sourceFile, child); + const visit = (child: ts.Node) => { + const declaration = this.getDeclarationFromNode(sourceFile, child); if (declaration) { result.push(declaration); } else { @@ -383,13 +383,14 @@ export class TypeScriptServiceHost implements LanguageServiceHost { } private get reflector(): StaticReflector { - let result = this._reflector; - if (!result) { - const ssr = this.staticSymbolResolver; - result = this._reflector = new StaticReflector( - this.summaryResolver, ssr, [], [], (e, filePath) => this.collectError(e, filePath !)); + if (!this._reflector) { + this._reflector = new StaticReflector( + this.summaryResolver, this.staticSymbolResolver, + [], // knownMetadataClasses + [], // knownMetadataFunctions + (e, filePath) => this.collectError(e, filePath !)); } - return result; + return this._reflector; } private getTemplateClassFromStaticSymbol(type: StaticSymbol): ts.ClassDeclaration|undefined { @@ -442,12 +443,12 @@ export class TypeScriptServiceHost implements LanguageServiceHost { } const callTarget = (parentNode).expression; - let decorator = parentNode.parent; // Decorator + const decorator = parentNode.parent; // Decorator if (!decorator || decorator.kind !== ts.SyntaxKind.Decorator) { return TypeScriptServiceHost.missingTemplate; } - let declaration = decorator.parent; // ClassDeclaration + const declaration = decorator.parent; // ClassDeclaration if (!declaration || declaration.kind !== ts.SyntaxKind.ClassDeclaration) { return TypeScriptServiceHost.missingTemplate; } @@ -531,73 +532,73 @@ export class TypeScriptServiceHost implements LanguageServiceHost { } getTemplateAstAtPosition(fileName: string, position: number): TemplateInfo|undefined { - let template = this.getTemplateAt(fileName, position); - if (template) { - let astResult = this.getTemplateAst(template, fileName); - if (astResult && astResult.htmlAst && astResult.templateAst && astResult.directive && - astResult.directives && astResult.pipes && astResult.expressionParser) - return { - position, - fileName, - template, - htmlAst: astResult.htmlAst, - directive: astResult.directive, - directives: astResult.directives, - pipes: astResult.pipes, - templateAst: astResult.templateAst, - expressionParser: astResult.expressionParser - }; + const template = this.getTemplateAt(fileName, position); + if (!template) { + return; + } + const astResult = this.getTemplateAst(template, fileName); + if (astResult && astResult.htmlAst && astResult.templateAst && astResult.directive && + astResult.directives && astResult.pipes && astResult.expressionParser) { + return { + position, + fileName, + template, + htmlAst: astResult.htmlAst, + directive: astResult.directive, + directives: astResult.directives, + pipes: astResult.pipes, + templateAst: astResult.templateAst, + expressionParser: astResult.expressionParser + }; } - return undefined; } getTemplateAst(template: TemplateSource, contextFile: string): AstResult { - let result: AstResult|undefined = undefined; try { - const resolvedMetadata = - this.resolver.getNonNormalizedDirectiveMetadata(template.type as any); + const resolvedMetadata = this.resolver.getNonNormalizedDirectiveMetadata(template.type); const metadata = resolvedMetadata && resolvedMetadata.metadata; - if (metadata) { - const rawHtmlParser = new HtmlParser(); - const htmlParser = new I18NHtmlParser(rawHtmlParser); - const expressionParser = new Parser(new Lexer()); - const config = new CompilerConfig(); - const parser = new TemplateParser( - config, this.resolver.getReflector(), expressionParser, new DomElementSchemaRegistry(), - htmlParser, null !, []); - const htmlResult = htmlParser.parse(template.source, '', {tokenizeExpansionForms: true}); - let errors: Diagnostic[]|undefined = undefined; - let ngModule = this.analyzedModules.ngModuleByPipeOrDirective.get(template.type); - if (!ngModule) { + if (!metadata) { + return {}; + } + const rawHtmlParser = new HtmlParser(); + const htmlParser = new I18NHtmlParser(rawHtmlParser); + const expressionParser = new Parser(new Lexer()); + const config = new CompilerConfig(); + const parser = new TemplateParser( + config, this.resolver.getReflector(), expressionParser, new DomElementSchemaRegistry(), + htmlParser, null !, []); + const htmlResult = htmlParser.parse(template.source, '', {tokenizeExpansionForms: true}); + const errors: Diagnostic[]|undefined = undefined; + const ngModule = this.analyzedModules.ngModuleByPipeOrDirective.get(template.type) || // Reported by the the declaration diagnostics. - ngModule = findSuitableDefaultModule(this.analyzedModules); - } - if (ngModule) { - const directives = - ngModule.transitiveModule.directives - .map(d => this.resolver.getNonNormalizedDirectiveMetadata(d.reference)) - .filter(d => d) - .map(d => d !.metadata.toSummary()); - const pipes = ngModule.transitiveModule.pipes.map( - p => this.resolver.getOrLoadPipeMetadata(p.reference).toSummary()); - const schemas = ngModule.schemas; - const parseResult = parser.tryParseHtml(htmlResult, metadata, directives, pipes, schemas); - result = { - htmlAst: htmlResult.rootNodes, - templateAst: parseResult.templateAst, - directive: metadata, directives, pipes, - parseErrors: parseResult.errors, expressionParser, errors - }; - } + findSuitableDefaultModule(this.analyzedModules); + if (!ngModule) { + return {}; } + const directives = ngModule.transitiveModule.directives + .map(d => this.resolver.getNonNormalizedDirectiveMetadata(d.reference)) + .filter(d => d) + .map(d => d !.metadata.toSummary()); + const pipes = ngModule.transitiveModule.pipes.map( + p => this.resolver.getOrLoadPipeMetadata(p.reference).toSummary()); + const schemas = ngModule.schemas; + const parseResult = parser.tryParseHtml(htmlResult, metadata, directives, pipes, schemas); + return { + htmlAst: htmlResult.rootNodes, + templateAst: parseResult.templateAst, + directive: metadata, directives, pipes, + parseErrors: parseResult.errors, expressionParser, errors + }; } catch (e) { - let span = template.span; - if (e.fileName == contextFile) { - span = template.query.getSpanAt(e.line, e.column) || span; - } - result = {errors: [{kind: DiagnosticKind.Error, message: e.message, span}]}; + const span = + e.fileName === contextFile && template.query.getSpanAt(e.line, e.column) || template.span; + return { + errors: [{ + kind: DiagnosticKind.Error, + message: e.message, span, + }], + }; } - return result || {}; } } diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index 0f5813d26a..cac0f71f25 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -229,24 +229,23 @@ export class MyComponent { function expectEntries( - locationMarker: string, completions: Completion[] | undefined, ...names: string[]) { + locationMarker: string, completion: ts.CompletionInfo | undefined, ...names: string[]) { let entries: {[name: string]: boolean} = {}; - if (!completions) { + if (!completion) { throw new Error( `Expected result from ${locationMarker} to include ${names.join(', ')} but no result provided`); } - if (!completions.length) { + if (!completion.entries.length) { throw new Error( `Expected result from ${locationMarker} to include ${names.join(', ')} an empty result provided`); - } else { - for (let entry of completions) { - entries[entry.name] = true; - } - let missing = names.filter(name => !entries[name]); - if (missing.length) { - throw new Error( - `Expected result from ${locationMarker} to include at least one of the following, ${missing.join(', ')}, in the list of entries ${completions.map(entry => entry.name).join(', ')}`); - } + } + for (const entry of completion.entries) { + entries[entry.name] = true; + } + let missing = names.filter(name => !entries[name]); + if (missing.length) { + throw new Error( + `Expected result from ${locationMarker} to include at least one of the following, ${missing.join(', ')}, in the list of entries ${completion.entries.map(entry => entry.name).join(', ')}`); } }