fix(compiler-cli): report non-template diagnostics (angular#40331)

Report non-template diagnotics when calling `getDiagnotics` function of
the language service we only returned template diagnotics. This change
causes it to return all diagnotics, not just diagnostics from the
template type checker.

PR Close angular#40331

Author: zarend

packages/compiler-cli/BUILD.bazel

@@ -33,6 +33,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/shims",
         "//packages/compiler-cli/src/ngtsc/translator",
         "//packages/compiler-cli/src/ngtsc/typecheck",
+        "//packages/compiler-cli/src/ngtsc/typecheck/api",
         "@npm//@bazel/typescript",
         "@npm//@types/node",
         "@npm//chokidar",

packages/compiler-cli/src/ngtsc/core/src/compiler.ts

@@ -30,7 +30,6 @@ import {ivySwitchTransform} from '../../switch';
 import {aliasTransformFactory, CompilationMode, declarationTransformFactory, DecoratorHandler, DtsTransformRegistry, ivyTransformFactory, TraitCompiler} from '../../transform';
 import {TemplateTypeCheckerImpl} from '../../typecheck';
 import {OptimizeFor, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy} from '../../typecheck/api';
-import {isTemplateDiagnostic} from '../../typecheck/diagnostics';
 import {getSourceFileOrNull, isDtsPath, resolveModuleName} from '../../util/src/typescript';
 import {LazyRoute, NgCompilerAdapter, NgCompilerOptions} from '../api';
 
@@ -84,11 +83,12 @@ export class NgCompiler {
   private constructionDiagnostics: ts.Diagnostic[] = [];
 
   /**
-   * Semantic diagnostics related to the program itself.
+   * Non-template diagnostics related to the program itself. Does not include template
+   * diagnostics because the template type checker memoizes them itself.
    *
-   * This is set by (and memoizes) `getDiagnostics`.
+   * This is set by (and memoizes) `getNonTemplateDiagnostics`.
    */
-  private diagnostics: ts.Diagnostic[]|null = null;
+  private nonTemplateDiagnostics: ts.Diagnostic[]|null = null;
 
   private closureCompilerEnabled: boolean;
   private nextProgram: ts.Program;
@@ -175,38 +175,23 @@ export class NgCompiler {
     return this.incrementalDriver.depGraph.getResourceDependencies(file);
   }
 
+  /**
+   * Get all Angular-related diagnostics for this compilation.
+   */
+  getDiagnostics(): ts.Diagnostic[] {
+    return [...this.getNonTemplateDiagnostics(), ...this.getTemplateDiagnostics()];
+  }
+
   /**
    * Get all Angular-related diagnostics for this compilation.
    *
    * If a `ts.SourceFile` is passed, only diagnostics related to that file are returned.
    */
-  getDiagnostics(file?: ts.SourceFile): ts.Diagnostic[] {
-    if (this.diagnostics === null) {
-      const compilation = this.ensureAnalyzed();
-      this.diagnostics =
-          [...compilation.traitCompiler.diagnostics, ...this.getTemplateDiagnostics()];
-      if (this.entryPoint !== null && compilation.exportReferenceGraph !== null) {
-        this.diagnostics.push(...checkForPrivateExports(
-            this.entryPoint, this.tsProgram.getTypeChecker(), compilation.exportReferenceGraph));
-      }
-    }
-
-    if (file === undefined) {
-      return this.diagnostics;
-    } else {
-      return this.diagnostics.filter(diag => {
-        if (diag.file === file) {
-          return true;
-        } else if (isTemplateDiagnostic(diag) && diag.componentFile === file) {
-          // Template diagnostics are reported when diagnostics for the component file are
-          // requested (since no consumer of `getDiagnostics` would ever ask for diagnostics from
-          // the fake ts.SourceFile for templates).
-          return true;
-        } else {
-          return false;
-        }
-      });
-    }
+  getDiagnosticsForFile(file: ts.SourceFile, optimizeFor: OptimizeFor): ts.Diagnostic[] {
+    return [
+      ...this.getNonTemplateDiagnostics().filter(diag => diag.file === file),
+      ...this.getTemplateDiagnosticsForFile(file, optimizeFor)
+    ];
   }
 
   /**
@@ -582,6 +567,37 @@ export class NgCompiler {
     return diagnostics;
   }
 
+  private getTemplateDiagnosticsForFile(sf: ts.SourceFile, optimizeFor: OptimizeFor):
+      ReadonlyArray<ts.Diagnostic> {
+    const compilation = this.ensureAnalyzed();
+
+    // Get the diagnostics.
+    const typeCheckSpan = this.perfRecorder.start('typeCheckDiagnostics');
+    const diagnostics: ts.Diagnostic[] = [];
+    if (!sf.isDeclarationFile && !this.adapter.isShim(sf)) {
+      diagnostics.push(...compilation.templateTypeChecker.getDiagnosticsForFile(sf, optimizeFor));
+    }
+
+    const program = this.typeCheckingProgramStrategy.getProgram();
+    this.perfRecorder.stop(typeCheckSpan);
+    this.incrementalStrategy.setIncrementalDriver(this.incrementalDriver, program);
+    this.nextProgram = program;
+
+    return diagnostics;
+  }
+
+  private getNonTemplateDiagnostics(): ts.Diagnostic[] {
+    if (this.nonTemplateDiagnostics === null) {
+      const compilation = this.ensureAnalyzed();
+      this.nonTemplateDiagnostics = [...compilation.traitCompiler.diagnostics];
+      if (this.entryPoint !== null && compilation.exportReferenceGraph !== null) {
+        this.nonTemplateDiagnostics.push(...checkForPrivateExports(
+            this.entryPoint, this.tsProgram.getTypeChecker(), compilation.exportReferenceGraph));
+      }
+    }
+    return this.nonTemplateDiagnostics;
+  }
+
   /**
    * Reifies the inter-dependencies of NgModules and the components within their compilation scopes
    * into the `IncrementalDriver`'s dependency graph.

packages/compiler-cli/src/ngtsc/core/test/BUILD.bazel

@@ -17,6 +17,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/incremental",
         "//packages/compiler-cli/src/ngtsc/reflection",
         "//packages/compiler-cli/src/ngtsc/typecheck",
+        "//packages/compiler-cli/src/ngtsc/typecheck/api",
         "@npm//typescript",
     ],
 )

packages/compiler-cli/src/ngtsc/core/test/compiler_test.ts

@@ -13,6 +13,7 @@ import {runInEachFileSystem} from '../../file_system/testing';
 import {NoopIncrementalBuildStrategy} from '../../incremental';
 import {ClassDeclaration, isNamedClassDeclaration} from '../../reflection';
 import {ReusedProgramStrategy} from '../../typecheck';
+import {OptimizeFor} from '../../typecheck/api';
 
 import {NgCompilerOptions} from '../api';
 
@@ -54,7 +55,8 @@ runInEachFileSystem(() => {
           new NoopIncrementalBuildStrategy(), /** enableTemplateTypeChecker */ false,
           /* usePoisonedData */ false);
 
-      const diags = compiler.getDiagnostics(getSourceFileOrError(program, COMPONENT));
+      const diags = compiler.getDiagnosticsForFile(
+          getSourceFileOrError(program, COMPONENT), OptimizeFor.SingleFile);
       expect(diags.length).toBe(1);
       expect(diags[0].messageText).toContain('does_not_exist');
     });

packages/compiler-cli/src/ngtsc/program.ts

@@ -20,6 +20,7 @@ import {NOOP_PERF_RECORDER, PerfRecorder, PerfTracker} from './perf';
 import {DeclarationNode} from './reflection';
 import {retagAllTsFiles, untagAllTsFiles} from './shims';
 import {ReusedProgramStrategy} from './typecheck';
+import {OptimizeFor} from './typecheck/api';
 
 
 
@@ -182,7 +183,9 @@ export class NgtscProgram implements api.Program {
       }
     }
 
-    const diagnostics = this.compiler.getDiagnostics(sf);
+    const diagnostics = sf === undefined ?
+        this.compiler.getDiagnostics() :
+        this.compiler.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram);
     this.reuseTsProgram = this.compiler.getNextProgram();
     return diagnostics;
   }

packages/compiler-cli/src/ngtsc/tsc_plugin.ts

@@ -14,6 +14,7 @@ import {NodeJSFileSystem, setFileSystem} from './file_system';
 import {PatchedProgramIncrementalBuildStrategy} from './incremental';
 import {NOOP_PERF_RECORDER} from './perf';
 import {untagAllTsFiles} from './shims';
+import {OptimizeFor} from './typecheck/api';
 import {ReusedProgramStrategy} from './typecheck/src/augmented_program';
 
 // The following is needed to fix a the chicken-and-egg issue where the sync (into g3) script will
@@ -111,7 +112,10 @@ export class NgTscPlugin implements TscPlugin {
   }
 
   getDiagnostics(file?: ts.SourceFile): ts.Diagnostic[] {
-    return this.compiler.getDiagnostics(file);
+    if (file === undefined) {
+      return this.compiler.getDiagnostics();
+    }
+    return this.compiler.getDiagnosticsForFile(file, OptimizeFor.WholeProgram);
   }
 
   getOptionDiagnostics(): ts.Diagnostic[] {

packages/language-service/ivy/language_service.ts

@@ -51,7 +51,7 @@ export class LanguageService {
       const program = compiler.getNextProgram();
       const sourceFile = program.getSourceFile(fileName);
       if (sourceFile) {
-        diagnostics.push(...ttc.getDiagnosticsForFile(sourceFile, OptimizeFor.SingleFile));
+        diagnostics.push(...compiler.getDiagnosticsForFile(sourceFile, OptimizeFor.SingleFile));
       }
     } else {
       const components = compiler.getComponentsWithTemplateFile(fileName);

packages/language-service/ivy/test/compiler_spec.ts

@@ -8,6 +8,7 @@
 
 import {absoluteFrom, getSourceFileOrError} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
+import {OptimizeFor} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
 
 import {LanguageServiceTestEnvironment} from './env';
 
@@ -68,9 +69,15 @@ describe('language-service/compiler integration', () => {
     // Expect that this program is clean diagnostically.
     const ngCompiler = env.ngLS.compilerFactory.getOrCreate();
     const program = ngCompiler.getNextProgram();
-    expect(ngCompiler.getDiagnostics(getSourceFileOrError(program, appCmpFile))).toEqual([]);
-    expect(ngCompiler.getDiagnostics(getSourceFileOrError(program, appModuleFile))).toEqual([]);
-    expect(ngCompiler.getDiagnostics(getSourceFileOrError(program, testFile))).toEqual([]);
+    expect(ngCompiler.getDiagnosticsForFile(
+               getSourceFileOrError(program, appCmpFile), OptimizeFor.WholeProgram))
+        .toEqual([]);
+    expect(ngCompiler.getDiagnosticsForFile(
+               getSourceFileOrError(program, appModuleFile), OptimizeFor.WholeProgram))
+        .toEqual([]);
+    expect(ngCompiler.getDiagnosticsForFile(
+               getSourceFileOrError(program, testFile), OptimizeFor.WholeProgram))
+        .toEqual([]);
   });
 
   it('should show type-checking errors from components with poisoned scopes', () => {
@@ -153,9 +160,9 @@ describe('language-service/compiler integration', () => {
         name: moduleFile,
         contents: `
           import {NgModule} from '@angular/core';
-    
+
           import {Cmp} from './cmp';
-    
+
           @NgModule({
             declarations: [Cmp],
           })
@@ -173,7 +180,8 @@ describe('language-service/compiler integration', () => {
     // Angular should be complaining about the module not being understandable.
     const programBefore = env.tsLS.getProgram()!;
     const moduleSfBefore = programBefore.getSourceFile(moduleFile)!;
-    const ngDiagsBefore = env.ngLS.compilerFactory.getOrCreate().getDiagnostics(moduleSfBefore);
+    const ngDiagsBefore = env.ngLS.compilerFactory.getOrCreate().getDiagnosticsForFile(
+        moduleSfBefore, OptimizeFor.SingleFile);
     expect(ngDiagsBefore.length).toBe(1);
 
     // Fix the import.
@@ -182,7 +190,8 @@ describe('language-service/compiler integration', () => {
     // Angular should stop complaining about the NgModule.
     const programAfter = env.tsLS.getProgram()!;
     const moduleSfAfter = programAfter.getSourceFile(moduleFile)!;
-    const ngDiagsAfter = env.ngLS.compilerFactory.getOrCreate().getDiagnostics(moduleSfAfter);
+    const ngDiagsAfter = env.ngLS.compilerFactory.getOrCreate().getDiagnosticsForFile(
+        moduleSfAfter, OptimizeFor.SingleFile);
     expect(ngDiagsAfter.length).toBe(0);
   });
 });

packages/language-service/ivy/test/diagnostic_spec.ts

@@ -174,4 +174,22 @@ describe('getSemanticDiagnostics', () => {
       'Parser Error: Bindings cannot contain assignments at column 8 in [{{nope = true}}] in /app2.html@0:0'
     ]);
   });
+
+  it('reports a diagnostic for a component without a template', () => {
+    const appFile = {
+      name: absoluteFrom('/app.ts'),
+      contents: `
+      import {Component} from '@angular/core';
+      @Component({})
+      export class MyComponent {}
+    `
+    };
+
+    const env = createModuleWithDeclarations([appFile]);
+    const diags = env.ngLS.getSemanticDiagnostics(absoluteFrom('/app.ts'));
+
+    expect(diags.map(x => x.messageText)).toEqual([
+      'component is missing a template',
+    ]);
+  });
 });

packages/language-service/ivy/test/env.ts

@@ -11,7 +11,7 @@ import {StrictTemplateOptions} from '@angular/compiler-cli/src/ngtsc/core/api';
 import {absoluteFrom, AbsoluteFsPath, FileSystem, getFileSystem, getSourceFileOrError} from '@angular/compiler-cli/src/ngtsc/file_system';
 import {MockFileSystem, TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
 import {loadStandardTestFiles} from '@angular/compiler-cli/src/ngtsc/testing';
-import {TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
+import {OptimizeFor, TemplateTypeChecker} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
 import * as ts from 'typescript/lib/tsserverlibrary';
 
 import {LanguageService} from '../language_service';
@@ -172,7 +172,9 @@ export class LanguageServiceTestEnvironment {
         continue;
       }
 
-      const ngDiagnostics = ngCompiler.getDiagnostics(sf);
+      // It's more efficient to optimize for WholeProgram since we call this with every file in the
+      // program.
+      const ngDiagnostics = ngCompiler.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram);
       expect(ngDiagnostics.map(diag => diag.messageText)).toEqual([]);
     }