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

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;
 }
 
 /**

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;
+export type Completion = ReferenceCompletion|VariableCompletion;
-
-/**
- * An autocompletion source that drives completion in a global context.
- */
-export type GlobalCompletion = CompletionContextComponent|CompletionReference|CompletionVariable;
 
 /**
  * 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<string, ReferenceCompletion|VariableCompletion>;
+}

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<AbsoluteFsPath, FileTypeCheckingData>();
+
+  /**
+   * 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<ts.ClassDeclaration, CompletionEngine>();
+
   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;
+    const engine = new CompletionEngine(tcb, data, shimPath);
-
+    this.completionCache.set(component, engine);
-    // Global completions are the union of two separate pieces: a `ContextComponentCompletion` which
+    return engine;
-    // 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;
   }
 
   private maybeAdoptPriorResultsForFile(sf: ts.SourceFile): void {

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<TmplAstTemplate|null, GlobalCompletion>();
+
+  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<string, ReferenceCompletion|VariableCompletion>(),
+    };
+
+    // 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;
+  }
+}

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 {completions, program} = setupCompletions(`No special template needed`);
-      const {templateTypeChecker, programStrategy} = setup([
+      expect(completions.templateContext.size).toBe(0);
-        {
+      const {shimPath, positionInShimFile} = completions.componentContext;
-          fileName: MAIN_TS,
+      const tcbSf = getSourceFileOrError(program, shimPath);
-          templates: {'SomeCmp': `No special template needed`},
+      const node = getTokenAtPosition(tcbSf, positionInShimFile).parent;
-          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;
       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 template = `
-      const {templateTypeChecker, programStrategy} = setup([
+        <div *ngFor="let user of users">
-        {
+          <div #innerRef></div>
-          fileName: MAIN_TS,
+          <div *ngIf="user">
-          templates: {
+            <div #notInScope></div>
-            'SomeCmp': `
+          </div>
-              <div *ngFor="let user of users">
+        </div>
-                <div #innerRef></div>
+        <div #topLevelRef></div>
-                <div *ngIf="user">
+      `;
-                  <div #notInScope></div>
+      const members = `users: string[];`;
-                </div>
+      // Embedded view in question is the first node in the template (index 0).
-              </div>
+      const {completions} = setupCompletions(template, members, 0);
-              <div #topLevelRef></div>
+      expect(new Set(completions.templateContext.keys())).toEqual(new Set([
-          `
+        'innerRef', 'user', 'topLevelRef'
-          },
+      ]));
-          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']));
     });
 
     it('should support shadowing between outer and inner templates  ', () => {
-      const MAIN_TS = absoluteFrom('/main.ts');
+      const template = `
-      const {templateTypeChecker, programStrategy} = setup([
+        <div *ngFor="let user of users">
-        {
+          Within this template, 'user' should be a variable, not a reference.
-          fileName: MAIN_TS,
+        </div>
-          templates: {
+        <div #user>Out here, 'user' is the reference.</div>
-            'SomeCmp': `
+      `;
-              <div *ngFor="let user of users">
+      const members = `users: string[];`;
-                Within this template, 'user' should be a variable, not a reference.
+      // Completions for the top level.
-              </div>
+      const {completions: topLevel} = setupCompletions(template, members);
-              <div #user>Out here, 'user' is the reference.</div>
+      // Completions within the embedded view at index 0.
-          `
+      const {completions: inNgFor} = setupCompletions(template, members, 0);
-          },
-          source: `
-            export class SomeCmp {
-              users: string[];
-            }
-          `,
-        },
-      ]);
-      const sf = getSourceFileOrError(programStrategy.getProgram(), MAIN_TS);
-      const SomeCmp = getClass(sf, 'SomeCmp');
 
-      const tmpl = templateTypeChecker.getTemplate(SomeCmp)!;
+      expect(topLevel.templateContext.has('user')).toBeTrue();
-      const ngForTemplate = tmpl[0] as TmplAstTemplate;
+      const userAtTopLevel = topLevel.templateContext.get('user')!;
-
+      expect(inNgFor.templateContext.has('user')).toBeTrue();
-      const [_a, userAtTopLevel] =
+      const userInNgFor = inNgFor.templateContext.get('user')!;
-          templateTypeChecker.getGlobalCompletions(/* root template */ null, SomeCmp);
-      const [_b, userInNgFor] = templateTypeChecker.getGlobalCompletions(ngForTemplate, SomeCmp);
 
       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('<div #foo></div>');
+      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, '<div #bar></div>');
+
+      // 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<TypeCheckingConfig>) {
+function setupCompletions(
-  return baseTestSetup(
+    template: string, componentMembers: string = '', inChildTemplateAtIndex: number|null = null): {
-      targets, {inlining: false, config: {...config, enableTemplateTypeChecker: true}});
+  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,
+  };
 }