fix(language-service): Invalidate Reflector caches when program changes (#32357)

This commit fixes a bug introduced in the recent refactoring whereby
caches become stale when the program changes.
This is because StaticReflector keeps its own caches that are not
clearable. The previous refactoring tried to reuse the same instance,
leading to out-of-sync program state.

Clearing out the *entire* cache is very inefficient. Instead, we could
just invalidate the symbols in the files that have changed. This
requires changes to the API of StaticReflector, but put this on hold
until the refactoring of language service for Ivy commences.

PR closes https://github.com/angular/angular/issues/32301

PR Close #32357

packages/language-service/src/typescript_host.ts

@@ -57,8 +57,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
   private readonly summaryResolver: AotSummaryResolver;
   private readonly reflectorHost: ReflectorHost;
   private readonly staticSymbolResolver: StaticSymbolResolver;
-  private readonly reflector: StaticReflector;
+  private resolver: CompileMetadataResolver;
-  private readonly resolver: CompileMetadataResolver;
 
   private readonly staticSymbolCache = new StaticSymbolCache();
   private readonly fileToComponent = new Map<string, StaticSymbol>();
@@ -82,28 +81,31 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
           fromSummaryFileName(filePath: string): string{return filePath;},
         },
         this.staticSymbolCache);
-    this.reflectorHost = new ReflectorHost(() => tsLS.getProgram() !, host);
+    this.reflectorHost = new ReflectorHost(() => this.program, host);
     this.staticSymbolResolver = new StaticSymbolResolver(
         this.reflectorHost, this.staticSymbolCache, this.summaryResolver,
         (e, filePath) => this.collectError(e, filePath));
-    this.reflector = new StaticReflector(
-        this.summaryResolver, this.staticSymbolResolver,
-        [],  // knownMetadataClasses
-        [],  // knownMetadataFunctions
-        (e, filePath) => this.collectError(e, filePath));
     this.resolver = this.createMetadataResolver();
   }
 
   /**
-   * Creates a new metadata resolver. This should only be called once.
+   * Creates a new metadata resolver. This is needed whenever the program
+   * changes.
    */
   private createMetadataResolver(): CompileMetadataResolver {
-    if (this.resolver) {
+    // StaticReflector keeps its own private caches that are not clearable.
-      return this.resolver;  // There should only be a single instance
+    // We have no choice but to create a new instance to invalidate the caches.
-    }
+    // TODO: Revisit this when language service gets rewritten for Ivy.
-    const moduleResolver = new NgModuleResolver(this.reflector);
+    const staticReflector = new StaticReflector(
-    const directiveResolver = new DirectiveResolver(this.reflector);
+        this.summaryResolver, this.staticSymbolResolver,
-    const pipeResolver = new PipeResolver(this.reflector);
+        [],  // knownMetadataClasses
+        [],  // knownMetadataFunctions
+        (e, filePath) => this.collectError(e, filePath));
+    // Because static reflector above is changed, we need to create a new
+    // resolver.
+    const moduleResolver = new NgModuleResolver(staticReflector);
+    const directiveResolver = new DirectiveResolver(staticReflector);
+    const pipeResolver = new PipeResolver(staticReflector);
     const elementSchemaRegistry = new DomElementSchemaRegistry();
     const resourceLoader = new DummyResourceLoader();
     const urlResolver = createOfflineCompileUrlResolver();
@@ -119,7 +121,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     return new CompileMetadataResolver(
         config, htmlParser, moduleResolver, directiveResolver, pipeResolver,
         new JitSummaryResolver(), elementSchemaRegistry, directiveNormalizer, new Console(),
-        this.staticSymbolCache, this.reflector,
+        this.staticSymbolCache, staticReflector,
         (error, type) => this.collectError(error, type && type.filePath));
   }
 
@@ -141,6 +143,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     this.templateReferences = [];
     this.fileToComponent.clear();
     this.collectedErrors.clear();
+    this.resolver = this.createMetadataResolver();
 
     const analyzeHost = {isSourceFile(filePath: string) { return true; }};
     const programFiles = this.program.getSourceFiles().map(sf => sf.fileName);
@@ -247,7 +250,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     return this.program.getSourceFile(fileName);
   }
 
-  get program() {
+  get program(): ts.Program {
     const program = this.tsLS.getProgram();
     if (!program) {
       // Program is very very unlikely to be undefined.
@@ -256,6 +259,8 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     return program;
   }
 
+  get reflector(): StaticReflector { return this.resolver.getReflector() as StaticReflector; }
+
   /**
    * Checks whether the program has changed, and invalidate caches if it has.
    * Returns true if modules are up-to-date, false otherwise.

packages/language-service/test/typescript_host_spec.ts

@@ -95,4 +95,57 @@ describe('TypeScriptServiceHost', () => {
     const template = templates[0];
     expect(template.source).toContain('<h2>{{hero.name}} details!</h2>');
   });
+
+  // https://github.com/angular/angular/issues/32301
+  it('should clear caches when program changes', () => {
+    const tsLSHost = new MockTypescriptHost(['/app/main.ts'], toh);
+    const tsLS = ts.createLanguageService(tsLSHost);
+    const ngLSHost = new TypeScriptServiceHost(tsLSHost, tsLS);
+    const fileName = '/app/app.component.ts';
+
+    // Get initial state
+    const oldModules = ngLSHost.getAnalyzedModules();
+    // First, make sure there is no missing modules
+    expect(oldModules.symbolsMissingModule).toEqual([]);
+    // Expect to find AppComponent in the old modules
+    const oldFile = oldModules.files.find(f => f.fileName === fileName);
+    expect(oldFile !.directives.length).toBe(1);
+    const appComp = oldFile !.directives[0];
+    expect(appComp.name).toBe('AppComponent');
+    expect(oldModules.ngModuleByPipeOrDirective.has(appComp)).toBe(true);
+
+    // Now, override app.component.ts with a different component
+    tsLSHost.override(fileName, `
+      import {Component} from '@angular/core';
+
+      @Component({
+        template: '<div>Hello</div>
+      })
+      export class HelloComponent {}
+    `);
+    // And override the containing NgModule to import the new component
+    tsLSHost.override('/app/main.ts', `
+      import {NgModule} from '@angular/core';
+      import {HelloComponent} from './app.component';
+
+      @NgModule({
+        declarations: [
+          HelloComponent,
+        ]
+      })
+      export class AppModule {}
+    `);
+    // Get the new state
+    const newModules = ngLSHost.getAnalyzedModules();
+    // Make sure there's no missing modules. If caches are not cleared properly,
+    // it will be a non-empty array
+    expect(newModules.symbolsMissingModule).toEqual([]);
+    // Expect to find HelloComponent in the new modules
+    const newFile = newModules.files.find(f => f.fileName === fileName);
+    expect(newFile !.directives.length).toBe(1);
+    const helloComp = newFile !.directives[0];
+    expect(helloComp.name).toBe('HelloComponent');
+    expect(newModules.ngModuleByPipeOrDirective.has(helloComp)).toBe(true);
+    expect(newModules.ngModuleByPipeOrDirective.has(appComp)).toBe(false);
+  });
 });