From c4f99b6e52ef4118b6fc593beb027b15c19f2168 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Thu, 8 Oct 2020 12:56:05 -0700 Subject: [PATCH] refactor(compiler-cli): move global completion into new CompletionEngine (#39278) This commit refactors the previously introduced `getGlobalCompletions()` API for the template type-checker in a couple ways: * The return type is adjusted to use a `Map` instead of an array, and separate out the component context completion position. This allows for a cleaner integration in the language service. * A new `CompletionEngine` class is introduced which powers autocompletion for a single component, and can cache completion results. * The `CompletionEngine` for each component is itself cached on the `TemplateTypeCheckerImpl` and is invalidated when the component template is overridden or reset. This refactoring simplifies the `TemplateTypeCheckerImpl` class by extracting the autocompletion logic, enables caching for better performance, and prepares for the introduction of other autocompletion APIs. PR Close #39278 --- .../src/ngtsc/typecheck/api/checker.ts | 2 +- .../src/ngtsc/typecheck/api/completion.ts | 46 ++-- .../src/ngtsc/typecheck/src/checker.ts | 77 ++++--- .../src/ngtsc/typecheck/src/completion.ts | 82 ++++++++ .../test/type_checker__completion_spec.ts | 199 +++++++++--------- 5 files changed, 248 insertions(+), 158 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts index 97f36e6b63..2199a91fc7 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts @@ -100,7 +100,7 @@ export interface TemplateTypeChecker { * template variables which are in scope for that expression. */ getGlobalCompletions(context: TmplAstTemplate|null, component: ts.ClassDeclaration): - GlobalCompletion[]; + 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 9d5c5e16ca..7fb996cfb4 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/completion.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/completion.ts @@ -13,34 +13,21 @@ import {ShimLocation} from './symbols'; /** * An autocompletion source of any kind. */ -export type Completion = CompletionContextComponent|CompletionReference|CompletionVariable; - -/** - * An autocompletion source that drives completion in a global context. - */ -export type GlobalCompletion = CompletionContextComponent|CompletionReference|CompletionVariable; +export type Completion = ReferenceCompletion|VariableCompletion; /** * Discriminant of an autocompletion source (a `Completion`). */ + export enum CompletionKind { - ContextComponent, Reference, Variable, } -/** - * An autocompletion source backed by a shim file position where TS APIs can be used to retrieve - * completions for the context component of a template. - */ -export interface CompletionContextComponent extends ShimLocation { - kind: CompletionKind.ContextComponent; -} - /** * An autocompletion result representing a local reference declared in the template. */ -export interface CompletionReference { +export interface ReferenceCompletion { kind: CompletionKind.Reference; /** @@ -52,7 +39,7 @@ export interface CompletionReference { /** * An autocompletion result representing a variable declared in the template. */ -export interface CompletionVariable { +export interface VariableCompletion { kind: CompletionKind.Variable; /** @@ -60,3 +47,28 @@ export interface CompletionVariable { */ node: TmplAstVariable; } + +/** + * Autocompletion data for an expression in the global scope. + * + * Global completion is accomplished by merging data from two sources: + * * TypeScript completion of the component's class members. + * * Local references and variables that are in scope at a given template level. + */ +export interface GlobalCompletion { + /** + * A location within the type-checking shim where TypeScript's completion APIs can be used to + * access completions for the template's component context (component class members). + */ + componentContext: ShimLocation; + + /** + * `Map` of local references and variables that are visible at the requested level of the + * template. + * + * Shadowing of references/variables from multiple levels of the template has already been + * accounted for in the preparation of `templateContext`. Entries here shadow component members of + * the same name (from the `componentContext` completions). + */ + templateContext: Map; +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts index ceab60d4c9..ad8895d4de 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts @@ -20,6 +20,7 @@ import {CompletionKind, GlobalCompletion, OptimizeFor, ProgramTypeCheckAdapter, import {TemplateDiagnostic} from '../diagnostics'; import {ExpressionIdentifier, findFirstMatchingNode} from './comments'; +import {CompletionEngine} from './completion'; import {InliningMode, ShimTypeCheckingData, TemplateData, TypeCheckContextImpl, TypeCheckingHost} from './context'; import {findTypeCheckBlock, shouldReportDiagnostic, TemplateSourceResolver, translateDiagnostic} from './diagnostics'; import {TemplateSourceManager} from './source'; @@ -32,6 +33,16 @@ import {SymbolBuilder} from './template_symbol_builder'; */ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { private state = new Map(); + + /** + * Stores the `CompletionEngine` which powers autocompletion for each component class. + * + * Must be invalidated whenever the component's template or the `ts.Program` changes. Invalidation + * on template changes is performed within this `TemplateTypeCheckerImpl` instance. When the + * `ts.Program` changes, the `TemplateTypeCheckerImpl` as a whole is destroyed and replaced. + */ + private completionCache = new Map(); + private isComplete = false; constructor( @@ -51,6 +62,11 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { fileRecord.isComplete = false; } } + + // Ideally only those components with overridden templates would have their caches invalidated, + // but the `TemplateTypeCheckerImpl` does not track the class for components with overrides. As + // a quick workaround, clear the entire cache instead. + this.completionCache.clear(); } getTemplate(component: ts.ClassDeclaration): TmplAstNode[]|null { @@ -130,6 +146,9 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { fileRecord.isComplete = false; this.isComplete = false; + // Overriding a component's template invalidates its autocompletion results. + this.completionCache.delete(component); + return {nodes}; } @@ -209,51 +228,27 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { } getGlobalCompletions(context: TmplAstTemplate|null, component: ts.ClassDeclaration): - GlobalCompletion[] { + GlobalCompletion|null { + const engine = this.getOrCreateCompletionEngine(component); + if (engine === null) { + return null; + } + return engine.getGlobalCompletions(context); + } + + private getOrCreateCompletionEngine(component: ts.ClassDeclaration): CompletionEngine|null { + if (this.completionCache.has(component)) { + return this.completionCache.get(component)!; + } + const {tcb, data, shimPath} = this.getLatestComponentState(component); if (tcb === null || data === null) { - return []; + return null; } - const {boundTarget} = data; - - // Global completions are the union of two separate pieces: a `ContextComponentCompletion` which - // is created from an expression within the TCB, and a list of named entities (variables and - // references) which are visible within the given `context` template. - const completions: GlobalCompletion[] = []; - - const globalRead = findFirstMatchingNode(tcb, { - filter: ts.isPropertyAccessExpression, - withExpressionIdentifier: ExpressionIdentifier.COMPONENT_COMPLETION - }); - - if (globalRead === null) { - return []; - } - - completions.push({ - kind: CompletionKind.ContextComponent, - shimPath, - positionInShimFile: globalRead.name.getStart(), - }); - - // Add completions for each entity in the template scope. Since each entity is uniquely named, - // there is no special ordering applied here. - for (const node of boundTarget.getEntitiesInTemplateScope(context)) { - if (node instanceof TmplAstReference) { - completions.push({ - kind: CompletionKind.Reference, - node: node, - }); - } else { - completions.push({ - kind: CompletionKind.Variable, - node: node, - }); - } - } - - return completions; + const engine = new CompletionEngine(tcb, data, shimPath); + this.completionCache.set(component, engine); + return engine; } private maybeAdoptPriorResultsForFile(sf: ts.SourceFile): void { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts new file mode 100644 index 0000000000..7421ef557a --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts @@ -0,0 +1,82 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {TmplAstReference, TmplAstTemplate} from '@angular/compiler'; +import * as ts from 'typescript'; + +import {AbsoluteFsPath} from '../../file_system'; +import {CompletionKind, GlobalCompletion, ReferenceCompletion, VariableCompletion} from '../api'; + +import {ExpressionIdentifier, findFirstMatchingNode} from './comments'; +import {TemplateData} from './context'; + +/** + * Powers autocompletion for a specific component. + * + * Internally caches autocompletion results, and must be discarded if the component template or + * surrounding TS program have changed. + */ +export class CompletionEngine { + /** + * Cache of `GlobalCompletion`s for various levels of the template, including the root template + * (`null`). + */ + private globalCompletionCache = 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)!; + } + + // 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: { + 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(), + }; + + // 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, + }); + } + } + + this.globalCompletionCache.set(context, completion); + return completion; + } +} 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 f9364d787b..7b8e6f6a44 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 @@ -12,127 +12,128 @@ import * as ts from 'typescript'; import {absoluteFrom, getSourceFileOrError} from '../../file_system'; import {runInEachFileSystem} from '../../file_system/testing'; import {getTokenAtPosition} from '../../util/src/typescript'; -import {CompletionKind, TypeCheckingConfig} from '../api'; +import {CompletionKind, GlobalCompletion, TemplateTypeChecker, TypeCheckingConfig} from '../api'; -import {getClass, setup as baseTestSetup, TypeCheckingTarget} from './test_utils'; +import {getClass, setup, TypeCheckingTarget} from './test_utils'; runInEachFileSystem(() => { describe('TemplateTypeChecker.getGlobalCompletions()', () => { it('should return a completion point in the TCB for the component context', () => { - const MAIN_TS = absoluteFrom('/main.ts'); - const {templateTypeChecker, programStrategy} = setup([ - { - fileName: MAIN_TS, - templates: {'SomeCmp': `No special template needed`}, - source: ` - export class SomeCmp {} - `, - }, - ]); - const sf = getSourceFileOrError(programStrategy.getProgram(), MAIN_TS); - const SomeCmp = getClass(sf, 'SomeCmp'); - - const [global, ...rest] = - templateTypeChecker.getGlobalCompletions(/* root template */ null, SomeCmp); - expect(rest.length).toBe(0); - if (global.kind !== CompletionKind.ContextComponent) { - return fail(`Expected a ContextComponent completion`); - } - const tcbSf = - getSourceFileOrError(programStrategy.getProgram(), absoluteFrom(global.shimPath)); - const node = getTokenAtPosition(tcbSf, global.positionInShimFile).parent; + const {completions, program} = setupCompletions(`No special template needed`); + expect(completions.templateContext.size).toBe(0); + const {shimPath, positionInShimFile} = completions.componentContext; + const tcbSf = getSourceFileOrError(program, shimPath); + const node = getTokenAtPosition(tcbSf, positionInShimFile).parent; if (!ts.isExpressionStatement(node)) { return fail(`Expected a ts.ExpressionStatement`); } expect(node.expression.getText()).toEqual('ctx.'); // The position should be between the '.' and a following space. - expect(tcbSf.text.substr(global.positionInShimFile - 1, 2)).toEqual('. '); + expect(tcbSf.text.substr(positionInShimFile - 1, 2)).toEqual('. '); }); it('should return additional completions for references and variables when available', () => { - const MAIN_TS = absoluteFrom('/main.ts'); - const {templateTypeChecker, programStrategy} = setup([ - { - fileName: MAIN_TS, - templates: { - 'SomeCmp': ` -
-
-
-
-
-
-
- ` - }, - source: ` - export class SomeCmp { - users: string[]; - } - `, - }, - ]); - const sf = getSourceFileOrError(programStrategy.getProgram(), MAIN_TS); - const SomeCmp = getClass(sf, 'SomeCmp'); - - const tmpl = templateTypeChecker.getTemplate(SomeCmp)!; - const ngForTemplate = tmpl[0] as TmplAstTemplate; - - const [contextCmp, ...rest] = - templateTypeChecker.getGlobalCompletions(ngForTemplate, SomeCmp); - if (contextCmp.kind !== CompletionKind.ContextComponent) { - return fail(`Expected first completion to be a ContextComponent`); - } - - const completionKeys: string[] = []; - for (const completion of rest) { - if (completion.kind !== CompletionKind.Reference && - completion.kind !== CompletionKind.Variable) { - return fail(`Unexpected CompletionKind, expected a Reference or Variable`); - } - completionKeys.push(completion.node.name); - } - - expect(new Set(completionKeys)).toEqual(new Set(['innerRef', 'user', 'topLevelRef'])); + const template = ` +
+
+
+
+
+
+
+ `; + const members = `users: string[];`; + // Embedded view in question is the first node in the template (index 0). + const {completions} = setupCompletions(template, members, 0); + expect(new Set(completions.templateContext.keys())).toEqual(new Set([ + 'innerRef', 'user', 'topLevelRef' + ])); }); it('should support shadowing between outer and inner templates ', () => { - const MAIN_TS = absoluteFrom('/main.ts'); - const {templateTypeChecker, programStrategy} = setup([ - { - fileName: MAIN_TS, - templates: { - 'SomeCmp': ` -
- Within this template, 'user' should be a variable, not a reference. -
-
Out here, 'user' is the reference.
- ` - }, - source: ` - export class SomeCmp { - users: string[]; - } - `, - }, - ]); - const sf = getSourceFileOrError(programStrategy.getProgram(), MAIN_TS); - const SomeCmp = getClass(sf, 'SomeCmp'); + const template = ` +
+ Within this template, 'user' should be a variable, not a reference. +
+
Out here, 'user' is the reference.
+ `; + const members = `users: string[];`; + // Completions for the top level. + const {completions: topLevel} = setupCompletions(template, members); + // Completions within the embedded view at index 0. + const {completions: inNgFor} = setupCompletions(template, members, 0); - const tmpl = templateTypeChecker.getTemplate(SomeCmp)!; - const ngForTemplate = tmpl[0] as TmplAstTemplate; - - const [_a, userAtTopLevel] = - templateTypeChecker.getGlobalCompletions(/* root template */ null, SomeCmp); - const [_b, userInNgFor] = templateTypeChecker.getGlobalCompletions(ngForTemplate, SomeCmp); + expect(topLevel.templateContext.has('user')).toBeTrue(); + const userAtTopLevel = topLevel.templateContext.get('user')!; + expect(inNgFor.templateContext.has('user')).toBeTrue(); + const userInNgFor = inNgFor.templateContext.get('user')!; expect(userAtTopLevel.kind).toBe(CompletionKind.Reference); expect(userInNgFor.kind).toBe(CompletionKind.Variable); }); + + it('should invalidate cached completions when overrides change', () => { + // The template starts with a #foo local reference. + const {completions: before, templateTypeChecker, component} = + setupCompletions('
'); + expect(Array.from(before.templateContext.keys())).toEqual(['foo']); + + // Override the template and change the name of the local reference to #bar. This should + // invalidate any cached completions. + templateTypeChecker.overrideComponentTemplate(component, '
'); + + // Fresh completions should include the #bar reference instead. + const afterOverride = + templateTypeChecker.getGlobalCompletions(/* root template */ null, component)!; + expect(afterOverride).toBeDefined(); + expect(Array.from(afterOverride.templateContext.keys())).toEqual(['bar']); + + // Reset the template to its original. This should also invalidate any cached completions. + templateTypeChecker.resetOverrides(); + + // Fresh completions should include the original #foo now. + const afterReset = + templateTypeChecker.getGlobalCompletions(/* root template */ null, component)!; + expect(afterReset).toBeDefined(); + expect(Array.from(afterReset.templateContext.keys())).toEqual(['foo']); + }); }); }); -function setup(targets: TypeCheckingTarget[], config?: Partial) { - return baseTestSetup( - targets, {inlining: false, config: {...config, enableTemplateTypeChecker: true}}); +function setupCompletions( + template: string, componentMembers: string = '', inChildTemplateAtIndex: number|null = null): { + completions: GlobalCompletion, + program: ts.Program, + templateTypeChecker: TemplateTypeChecker, + component: ts.ClassDeclaration, +} { + const MAIN_TS = absoluteFrom('/main.ts'); + const {templateTypeChecker, programStrategy} = setup( + [{ + fileName: MAIN_TS, + templates: {'SomeCmp': template}, + source: `export class SomeCmp { ${componentMembers} }`, + }], + ({inlining: false, config: {enableTemplateTypeChecker: true}})); + const sf = getSourceFileOrError(programStrategy.getProgram(), MAIN_TS); + const SomeCmp = getClass(sf, 'SomeCmp'); + + let context: TmplAstTemplate|null = null; + if (inChildTemplateAtIndex !== null) { + const tmpl = templateTypeChecker.getTemplate(SomeCmp)![inChildTemplateAtIndex]; + if (!(tmpl instanceof TmplAstTemplate)) { + throw new Error( + `AssertionError: expected TmplAstTemplate at index ${inChildTemplateAtIndex}`); + } + context = tmpl; + } + + const completions = templateTypeChecker.getGlobalCompletions(context, SomeCmp)!; + expect(completions).toBeDefined(); + return { + completions, + program: programStrategy.getProgram(), + templateTypeChecker, + component: SomeCmp, + }; }