From 1d12c50f63f90c91636185b2287e31e9c0291121 Mon Sep 17 00:00:00 2001 From: Zach Arend Date: Thu, 1 Apr 2021 15:22:44 -0700 Subject: [PATCH] fix(compiler-cli): autocomplete literal types in templates. (#41456) This adds string literals, number literals, `true`, `false`, `null` and `undefined` to autocomplete results in templates. For example, when completing an input of union type. Component: `@Input('input') input!: 'a'|'b'|null;` Template: `[input]="|"` Provide `'a'`, `'b'`, and `null` as autocompletion entries. Previously we did not include literal types because we only included results from the component context (`ctx.`) and the template scope. PR Close #41456 --- .../src/ngtsc/typecheck/api/checker.ts | 5 +- .../src/ngtsc/typecheck/api/completion.ts | 6 + .../src/ngtsc/typecheck/src/checker.ts | 7 +- .../src/ngtsc/typecheck/src/completion.ts | 120 ++++++++++++------ .../test/type_checker__completion_spec.ts | 2 +- packages/language-service/ivy/completions.ts | 51 +++++++- .../ivy/test/completions_spec.ts | 28 +++- 7 files changed, 166 insertions(+), 53 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts index da1d0bb294..d2b022f603 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts @@ -105,8 +105,9 @@ export interface TemplateTypeChecker { * include completions from the template's context component, as well as any local references or * template variables which are in scope for that expression. */ - getGlobalCompletions(context: TmplAstTemplate|null, component: ts.ClassDeclaration): - GlobalCompletion|null; + getGlobalCompletions( + context: TmplAstTemplate|null, component: ts.ClassDeclaration, + node: AST|TmplAstNode): GlobalCompletion|null; /** diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/completion.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/completion.ts index 7fb996cfb4..47d09ddb26 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/completion.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/completion.ts @@ -71,4 +71,10 @@ export interface GlobalCompletion { * the same name (from the `componentContext` completions). */ templateContext: Map; + + /** + * A location within the type-checking shim where TypeScript's completion APIs can be used to + * access completions for the AST node of the cursor position (primitive constants). + */ + nodeContext: ShimLocation|null; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts index 2490ad92bb..bcb5960ad3 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts @@ -257,14 +257,15 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { return this.getLatestComponentState(component).tcb; } - getGlobalCompletions(context: TmplAstTemplate|null, component: ts.ClassDeclaration): - GlobalCompletion|null { + getGlobalCompletions( + context: TmplAstTemplate|null, component: ts.ClassDeclaration, + node: AST|TmplAstNode): GlobalCompletion|null { const engine = this.getOrCreateCompletionEngine(component); if (engine === null) { return null; } return this.perf.inPhase( - PerfPhase.TtcAutocompletion, () => engine.getGlobalCompletions(context)); + PerfPhase.TtcAutocompletion, () => engine.getGlobalCompletions(context, node)); } getExpressionCompletionLocation( diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts index 6157a663d7..473e5b8a6d 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts @@ -7,7 +7,7 @@ */ import {TmplAstReference, TmplAstTemplate} from '@angular/compiler'; -import {MethodCall, PropertyRead, PropertyWrite, SafeMethodCall, SafePropertyRead} from '@angular/compiler/src/compiler'; +import {AST, EmptyExpr, MethodCall, PropertyRead, PropertyWrite, SafeMethodCall, SafePropertyRead, TmplAstNode} from '@angular/compiler/src/compiler'; import * as ts from 'typescript'; import {AbsoluteFsPath} from '../../file_system'; @@ -23,65 +23,77 @@ import {TemplateData} from './context'; * surrounding TS program have changed. */ export class CompletionEngine { + private componentContext: ShimLocation|null; + /** - * Cache of `GlobalCompletion`s for various levels of the template, including the root template - * (`null`). + * Cache of completions for various levels of the template, including the root template (`null`). + * Memoizes `getTemplateContextCompletions`. */ - private globalCompletionCache = new Map(); + private templateContextCache = + new Map>(); private expressionCompletionCache = new Map(); - constructor(private tcb: ts.Node, private data: TemplateData, private shimPath: AbsoluteFsPath) {} - - /** - * Get global completions within the given template context - either a `TmplAstTemplate` embedded - * view, or `null` for the root template context. - */ - getGlobalCompletions(context: TmplAstTemplate|null): GlobalCompletion|null { - if (this.globalCompletionCache.has(context)) { - return this.globalCompletionCache.get(context)!; - } + constructor(private tcb: ts.Node, private data: TemplateData, private shimPath: AbsoluteFsPath) { // Find the component completion expression within the TCB. This looks like: `ctx. /* ... */;` const globalRead = findFirstMatchingNode(this.tcb, { filter: ts.isPropertyAccessExpression, withExpressionIdentifier: ExpressionIdentifier.COMPONENT_COMPLETION }); - if (globalRead === null) { - return null; - } - - const completion: GlobalCompletion = { - componentContext: { + if (globalRead !== null) { + this.componentContext = { shimPath: this.shimPath, // `globalRead.name` is an empty `ts.Identifier`, so its start position immediately follows // the `.` in `ctx.`. TS autocompletion APIs can then be used to access completion results // for the component context. positionInShimFile: globalRead.name.getStart(), - }, - templateContext: new Map(), - }; + }; + } else { + this.componentContext = null; + } + } - // The bound template already has details about the references and variables in scope in the - // `context` template - they just need to be converted to `Completion`s. - for (const node of this.data.boundTarget.getEntitiesInTemplateScope(context)) { - if (node instanceof TmplAstReference) { - completion.templateContext.set(node.name, { - kind: CompletionKind.Reference, - node, - }); - } else { - completion.templateContext.set(node.name, { - kind: CompletionKind.Variable, - node, - }); + /** + * Get global completions within the given template context and AST node. + * + * @param context the given template context - either a `TmplAstTemplate` embedded view, or `null` + * for the root + * template context. + * @param node the given AST node + */ + getGlobalCompletions(context: TmplAstTemplate|null, node: AST|TmplAstNode): GlobalCompletion + |null { + if (this.componentContext === null) { + return null; + } + + const templateContext = this.getTemplateContextCompletions(context); + if (templateContext === null) { + return null; + } + + let nodeContext: ShimLocation|null = null; + if (node instanceof EmptyExpr) { + const nodeLocation = findFirstMatchingNode(this.tcb, { + filter: ts.isIdentifier, + withSpan: node.sourceSpan, + }); + if (nodeLocation !== null) { + nodeContext = { + shimPath: this.shimPath, + positionInShimFile: nodeLocation.getStart(), + }; } } - this.globalCompletionCache.set(context, completion); - return completion; + return { + componentContext: this.componentContext, + templateContext, + nodeContext, + }; } getExpressionCompletionLocation(expr: PropertyRead|PropertyWrite|MethodCall| @@ -131,4 +143,36 @@ export class CompletionEngine { this.expressionCompletionCache.set(expr, res); return res; } + + /** + * Get global completions within the given template context - either a `TmplAstTemplate` embedded + * view, or `null` for the root context. + */ + private getTemplateContextCompletions(context: TmplAstTemplate|null): + Map|null { + if (this.templateContextCache.has(context)) { + return this.templateContextCache.get(context)!; + } + + const templateContext = new Map(); + + // The bound template already has details about the references and variables in scope in the + // `context` template - they just need to be converted to `Completion`s. + for (const node of this.data.boundTarget.getEntitiesInTemplateScope(context)) { + if (node instanceof TmplAstReference) { + templateContext.set(node.name, { + kind: CompletionKind.Reference, + node, + }); + } else { + templateContext.set(node.name, { + kind: CompletionKind.Variable, + node, + }); + } + } + + this.templateContextCache.set(context, templateContext); + return templateContext; + } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts index 104b3ed279..d86e0bc8ac 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts @@ -141,7 +141,7 @@ function setupCompletions( context = tmpl; } - const completions = templateTypeChecker.getGlobalCompletions(context, SomeCmp)!; + const completions = templateTypeChecker.getGlobalCompletions(context, SomeCmp, null!)!; expect(completions).toBeDefined(); return { completions, diff --git a/packages/language-service/ivy/completions.ts b/packages/language-service/ivy/completions.ts index 69a0a8bd75..84f89938f2 100644 --- a/packages/language-service/ivy/completions.ts +++ b/packages/language-service/ivy/completions.ts @@ -216,21 +216,21 @@ export class CompletionBuilder { options: ts.GetCompletionsAtPositionOptions| undefined): ts.WithMetadata|undefined { const completions = - this.templateTypeChecker.getGlobalCompletions(this.template, this.component); + this.templateTypeChecker.getGlobalCompletions(this.template, this.component, this.node); if (completions === null) { return undefined; } - const {componentContext, templateContext} = completions; + const {componentContext, templateContext, nodeContext: astContext} = completions; const replacementSpan = makeReplacementSpanFromAst(this.node); // Merge TS completion results with results from the template scope. let entries: ts.CompletionEntry[] = []; - const tsLsCompletions = this.tsLS.getCompletionsAtPosition( + const componentCompletions = this.tsLS.getCompletionsAtPosition( componentContext.shimPath, componentContext.positionInShimFile, options); - if (tsLsCompletions !== undefined) { - for (const tsCompletion of tsLsCompletions.entries) { + if (componentCompletions !== undefined) { + for (const tsCompletion of componentCompletions.entries) { // Skip completions that are shadowed by a template entity definition. if (templateContext.has(tsCompletion.name)) { continue; @@ -244,6 +244,24 @@ export class CompletionBuilder { } } + // Merge TS completion results with results from the ast context. + if (astContext !== null) { + const nodeCompletions = this.tsLS.getCompletionsAtPosition( + astContext.shimPath, astContext.positionInShimFile, options); + if (nodeCompletions !== undefined) { + for (const tsCompletion of nodeCompletions.entries) { + if (this.isValidNodeContextCompletion(tsCompletion)) { + entries.push({ + ...tsCompletion, + // Substitute the TS completion's `replacementSpan` (which uses offsets within the + // TCB) with the `replacementSpan` within the template source. + replacementSpan, + }); + } + } + } + } + for (const [name, entity] of templateContext) { entries.push({ name, @@ -276,7 +294,7 @@ export class CompletionBuilder { formatOptions: ts.FormatCodeOptions|ts.FormatCodeSettings|undefined, preferences: ts.UserPreferences|undefined): ts.CompletionEntryDetails|undefined { const completions = - this.templateTypeChecker.getGlobalCompletions(this.template, this.component); + this.templateTypeChecker.getGlobalCompletions(this.template, this.component, this.node); if (completions === null) { return undefined; } @@ -317,7 +335,7 @@ export class CompletionBuilder { private getGlobalPropertyExpressionCompletionSymbol( this: PropertyExpressionCompletionBuilder, entryName: string): ts.Symbol|undefined { const completions = - this.templateTypeChecker.getGlobalCompletions(this.template, this.component); + this.templateTypeChecker.getGlobalCompletions(this.template, this.component, this.node); if (completions === null) { return undefined; } @@ -638,6 +656,25 @@ export class CompletionBuilder { isNewIdentifierLocation: false, }; } + + /** + * From the AST node of the cursor position, include completion of string literals, number + * literals, `true`, `false`, `null`, and `undefined`. + */ + private isValidNodeContextCompletion(completion: ts.CompletionEntry): boolean { + if (completion.kind === ts.ScriptElementKind.string) { + // 'string' kind includes both string literals and number literals + return true; + } + if (completion.kind === ts.ScriptElementKind.keyword) { + return completion.name === 'true' || completion.name === 'false' || + completion.name === 'null'; + } + if (completion.kind === ts.ScriptElementKind.variableElement) { + return completion.name === 'undefined'; + } + return false; + } } function makeReplacementSpanFromParseSourceSpan(span: ParseSourceSpan): ts.TextSpan { diff --git a/packages/language-service/ivy/test/completions_spec.ts b/packages/language-service/ivy/test/completions_spec.ts index b881a614e7..5fa39aa5c8 100644 --- a/packages/language-service/ivy/test/completions_spec.ts +++ b/packages/language-service/ivy/test/completions_spec.ts @@ -24,6 +24,18 @@ const DIR_WITH_INPUT = { ` }; +const DIR_WITH_UNION_TYPE_INPUT = { + 'Dir': ` + @Directive({ + selector: '[dir]', + inputs: ['myInput'] + }) + export class Dir { + myInput!: 'foo'|42|null|undefined + } + ` +}; + const DIR_WITH_OUTPUT = { 'Dir': ` @Directive({ @@ -203,6 +215,18 @@ describe('completions', () => { const completions = templateFile.getCompletionsAtPosition(); expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['title']); }); + + it('should return completions of string literals, number literals, `true`, `false`, `null` and `undefined`', + () => { + const {templateFile} = setup(``, '', DIR_WITH_UNION_TYPE_INPUT); + templateFile.moveCursorToText('dir [myInput]="¦">'); + + const completions = templateFile.getCompletionsAtPosition(); + expectContain(completions, ts.ScriptElementKind.string, [`'foo'`, '42']); + expectContain(completions, ts.ScriptElementKind.keyword, ['null']); + expectContain(completions, ts.ScriptElementKind.variableElement, ['undefined']); + expectDoesNotContain(completions, ts.ScriptElementKind.parameterElement, ['ctx']); + }); }); describe('in an expression scope', () => { @@ -789,7 +813,7 @@ function setup( export class AppCmp { ${classContents} } - + ${otherDirectiveClassDecls} @NgModule({ @@ -822,7 +846,7 @@ function setupInlineTemplate( export class AppCmp { ${classContents} } - + ${otherDirectiveClassDecls} @NgModule({