refactor(compiler-cli): split out template diagnostics package (#38576)

The template type-checking engine includes utilities for creating
`ts.Diagnostic`s for component templates. Previously only the template type-
checker itself created such diagnostics. However, the template parser also
produces errors which should be represented as template diagnostics.

This commit prepares for that conversion by extracting the machinery for
producing template diagnostics into its own sub-package, so that other parts
of the compiler can depend on it without depending on the entire template
type-checker.

PR Close #38576

goldens/public-api/compiler-cli/error_code.d.ts

@@ -19,6 +19,7 @@ export declare enum ErrorCode {
     CONFIG_FLAT_MODULE_NO_INDEX = 4001,
     CONFIG_STRICT_TEMPLATES_IMPLIES_FULL_TEMPLATE_TYPECHECK = 4002,
     HOST_BINDING_PARSE_ERROR = 5001,
+    TEMPLATE_PARSE_ERROR = 5002,
     NGMODULE_INVALID_DECLARATION = 6001,
     NGMODULE_INVALID_IMPORT = 6002,
     NGMODULE_INVALID_EXPORT = 6003,

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

@@ -34,6 +34,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/transform",
         "//packages/compiler-cli/src/ngtsc/typecheck",
         "//packages/compiler-cli/src/ngtsc/typecheck/api",
+        "//packages/compiler-cli/src/ngtsc/typecheck/diagnostics",
         "//packages/compiler-cli/src/ngtsc/util",
         "@npm//typescript",
     ],

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

@@ -28,8 +28,9 @@ import {ComponentScopeReader, LocalModuleScopeRegistry, MetadataDtsModuleScopeRe
 import {generatedFactoryTransform} from '../../shims';
 import {ivySwitchTransform} from '../../switch';
 import {aliasTransformFactory, declarationTransformFactory, DecoratorHandler, DtsTransformRegistry, ivyTransformFactory, TraitCompiler} from '../../transform';
-import {isTemplateDiagnostic, TemplateTypeCheckerImpl} from '../../typecheck';
+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';
 

packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel

@@ -20,6 +20,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/shims:api",
         "//packages/compiler-cli/src/ngtsc/translator",
         "//packages/compiler-cli/src/ngtsc/typecheck/api",
+        "//packages/compiler-cli/src/ngtsc/typecheck/diagnostics",
         "//packages/compiler-cli/src/ngtsc/util",
         "@npm//@types/node",
         "@npm//typescript",

packages/compiler-cli/src/ngtsc/typecheck/diagnostics/BUILD.bazel

@@ -0,0 +1,15 @@
+load("//tools:defaults.bzl", "ts_library")
+
+package(default_visibility = ["//visibility:public"])
+
+ts_library(
+    name = "diagnostics",
+    srcs = glob(["**/*.ts"]),
+    module_name = "@angular/compiler-cli/src/ngtsc/typecheck/diagnostics",
+    deps = [
+        "//packages:types",
+        "//packages/compiler",
+        "//packages/compiler-cli/src/ngtsc/typecheck/api",
+        "@npm//typescript",
+    ],
+)

packages/compiler-cli/src/ngtsc/typecheck/diagnostics/index.ts

@@ -0,0 +1,10 @@
+/**
+ * @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
+ */
+
+export * from './src/diagnostic';
+export * from './src/id';

packages/compiler-cli/src/ngtsc/typecheck/diagnostics/src/diagnostic.ts

@@ -0,0 +1,128 @@
+/**
+ * @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 {ParseSourceSpan} from '@angular/compiler';
+import * as ts from 'typescript';
+
+import {ExternalTemplateSourceMapping, TemplateId, TemplateSourceMapping} from '../../api';
+
+/**
+ * A `ts.Diagnostic` with additional information about the diagnostic related to template
+ * type-checking.
+ */
+export interface TemplateDiagnostic extends ts.Diagnostic {
+  /**
+   * The component with the template that resulted in this diagnostic.
+   */
+  componentFile: ts.SourceFile;
+
+  /**
+   * The template id of the component that resulted in this diagnostic.
+   */
+  templateId: TemplateId;
+}
+
+/**
+ * Constructs a `ts.Diagnostic` for a given `ParseSourceSpan` within a template.
+ */
+export function makeTemplateDiagnostic(
+    templateId: TemplateId, mapping: TemplateSourceMapping, span: ParseSourceSpan,
+    category: ts.DiagnosticCategory, code: number, messageText: string|ts.DiagnosticMessageChain,
+    relatedMessage?: {
+      text: string,
+      span: ParseSourceSpan,
+    }): TemplateDiagnostic {
+  if (mapping.type === 'direct') {
+    let relatedInformation: ts.DiagnosticRelatedInformation[]|undefined = undefined;
+    if (relatedMessage !== undefined) {
+      relatedInformation = [{
+        category: ts.DiagnosticCategory.Message,
+        code: 0,
+        file: mapping.node.getSourceFile(),
+        start: relatedMessage.span.start.offset,
+        length: relatedMessage.span.end.offset - relatedMessage.span.start.offset,
+        messageText: relatedMessage.text,
+      }];
+    }
+    // For direct mappings, the error is shown inline as ngtsc was able to pinpoint a string
+    // constant within the `@Component` decorator for the template. This allows us to map the error
+    // directly into the bytes of the source file.
+    return {
+      source: 'ngtsc',
+      code,
+      category,
+      messageText,
+      file: mapping.node.getSourceFile(),
+      componentFile: mapping.node.getSourceFile(),
+      templateId,
+      start: span.start.offset,
+      length: span.end.offset - span.start.offset,
+      relatedInformation,
+    };
+  } else if (mapping.type === 'indirect' || mapping.type === 'external') {
+    // For indirect mappings (template was declared inline, but ngtsc couldn't map it directly
+    // to a string constant in the decorator), the component's file name is given with a suffix
+    // indicating it's not the TS file being displayed, but a template.
+    // For external temoplates, the HTML filename is used.
+    const componentSf = mapping.componentClass.getSourceFile();
+    const componentName = mapping.componentClass.name.text;
+    // TODO(alxhub): remove cast when TS in g3 supports this narrowing.
+    const fileName = mapping.type === 'indirect' ?
+        `${componentSf.fileName} (${componentName} template)` :
+        (mapping as ExternalTemplateSourceMapping).templateUrl;
+    // TODO(alxhub): investigate creating a fake `ts.SourceFile` here instead of invoking the TS
+    // parser against the template (HTML is just really syntactically invalid TypeScript code ;).
+    // Also investigate caching the file to avoid running the parser multiple times.
+    const sf = ts.createSourceFile(
+        fileName, mapping.template, ts.ScriptTarget.Latest, false, ts.ScriptKind.JSX);
+
+    let relatedInformation: ts.DiagnosticRelatedInformation[] = [];
+    if (relatedMessage !== undefined) {
+      relatedInformation.push({
+        category: ts.DiagnosticCategory.Message,
+        code: 0,
+        file: sf,
+        start: relatedMessage.span.start.offset,
+        length: relatedMessage.span.end.offset - relatedMessage.span.start.offset,
+        messageText: relatedMessage.text,
+      });
+    }
+
+    relatedInformation.push({
+      category: ts.DiagnosticCategory.Message,
+      code: 0,
+      file: componentSf,
+      // mapping.node represents either the 'template' or 'templateUrl' expression. getStart()
+      // and getEnd() are used because they don't include surrounding whitespace.
+      start: mapping.node.getStart(),
+      length: mapping.node.getEnd() - mapping.node.getStart(),
+      messageText: `Error occurs in the template of component ${componentName}.`,
+    });
+
+    return {
+      source: 'ngtsc',
+      category,
+      code,
+      messageText,
+      file: sf,
+      componentFile: componentSf,
+      templateId,
+      start: span.start.offset,
+      length: span.end.offset - span.start.offset,
+      // Show a secondary message indicating the component whose template contains the error.
+      relatedInformation,
+    };
+  } else {
+    throw new Error(`Unexpected source mapping type: ${(mapping as {type: string}).type}`);
+  }
+}
+
+export function isTemplateDiagnostic(diagnostic: ts.Diagnostic): diagnostic is TemplateDiagnostic {
+  return diagnostic.hasOwnProperty('componentFile') &&
+      ts.isSourceFile((diagnostic as any).componentFile);
+}

packages/compiler-cli/src/ngtsc/typecheck/diagnostics/src/id.ts

@@ -0,0 +1,38 @@
+/**
+ * @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 * as ts from 'typescript';
+
+import {TemplateId} from '../../api';
+
+
+const TEMPLATE_ID = Symbol('ngTemplateId');
+const NEXT_TEMPLATE_ID = Symbol('ngNextTemplateId');
+
+interface HasTemplateId {
+  [TEMPLATE_ID]: TemplateId;
+}
+
+interface HasNextTemplateId {
+  [NEXT_TEMPLATE_ID]: number;
+}
+
+export function getTemplateId(clazz: ts.Declaration): TemplateId {
+  const node = clazz as ts.Declaration & Partial<HasTemplateId>;
+  if (node[TEMPLATE_ID] === undefined) {
+    node[TEMPLATE_ID] = allocateTemplateId(node.getSourceFile());
+  }
+  return node[TEMPLATE_ID]!;
+}
+
+function allocateTemplateId(sf: ts.SourceFile&Partial<HasNextTemplateId>): TemplateId {
+  if (sf[NEXT_TEMPLATE_ID] === undefined) {
+    sf[NEXT_TEMPLATE_ID] = 1;
+  }
+  return (`tcb${sf[NEXT_TEMPLATE_ID]!++}`) as TemplateId;
+}

packages/compiler-cli/src/ngtsc/typecheck/index.ts

@@ -9,7 +9,6 @@
 export {ReusedProgramStrategy} from './src/augmented_program';
 export {FileTypeCheckingData, TemplateTypeCheckerImpl} from './src/checker';
 export {TypeCheckContextImpl} from './src/context';
-export {isTemplateDiagnostic, TemplateDiagnostic} from './src/diagnostics';
 export {TypeCheckProgramHost} from './src/host';
 export {TypeCheckShimGenerator} from './src/shim';
 export {typeCheckFilePath} from './src/type_check_file';

packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts

@@ -16,9 +16,10 @@ import {ReflectionHost} from '../../reflection';
 import {isShim} from '../../shims';
 import {getSourceFileOrNull} from '../../util/src/typescript';
 import {OptimizeFor, ProgramTypeCheckAdapter, TemplateId, TemplateTypeChecker, TypeCheckingConfig, TypeCheckingProgramStrategy, UpdateMode} from '../api';
+import {TemplateDiagnostic} from '../diagnostics';
 
 import {InliningMode, ShimTypeCheckingData, TypeCheckContextImpl, TypeCheckingHost} from './context';
-import {findTypeCheckBlock, shouldReportDiagnostic, TemplateDiagnostic, TemplateSourceResolver, translateDiagnostic} from './diagnostics';
+import {findTypeCheckBlock, shouldReportDiagnostic, TemplateSourceResolver, translateDiagnostic} from './diagnostics';
 import {TemplateSourceManager} from './source';
 
 /**

packages/compiler-cli/src/ngtsc/typecheck/src/context.ts

@@ -14,8 +14,8 @@ import {NoopImportRewriter, Reference, ReferenceEmitter} from '../../imports';
 import {ClassDeclaration, ReflectionHost} from '../../reflection';
 import {ImportManager} from '../../translator';
 import {ComponentToShimMappingStrategy, TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckContext, TypeCheckingConfig, TypeCtorMetadata} from '../api';
+import {TemplateDiagnostic} from '../diagnostics';
 
-import {TemplateDiagnostic} from './diagnostics';
 import {DomSchemaChecker, RegistryDomSchemaChecker} from './dom';
 import {Environment} from './environment';
 import {OutOfBandDiagnosticRecorder, OutOfBandDiagnosticRecorderImpl} from './oob';

packages/compiler-cli/src/ngtsc/typecheck/src/diagnostics.ts

@@ -10,23 +10,7 @@ import * as ts from 'typescript';
 
 import {getTokenAtPosition} from '../../util/src/typescript';
 import {ExternalTemplateSourceMapping, TemplateId, TemplateSourceMapping} from '../api';
-
+import {makeTemplateDiagnostic, TemplateDiagnostic} from '../diagnostics';
-
-/**
- * A `ts.Diagnostic` with additional information about the diagnostic related to template
- * type-checking.
- */
-export interface TemplateDiagnostic extends ts.Diagnostic {
-  /**
-   * The component with the template that resulted in this diagnostic.
-   */
-  componentFile: ts.SourceFile;
-
-  /**
-   * The template id of the component that resulted in this diagnostic.
-   */
-  templateId: TemplateId;
-}
 
 /**
  * Adapter interface which allows the template type-checking diagnostics code to interpret offsets
@@ -157,101 +141,6 @@ export function findTypeCheckBlock(file: ts.SourceFile, id: TemplateId): ts.Node
   return null;
 }
 
-/**
- * Constructs a `ts.Diagnostic` for a given `ParseSourceSpan` within a template.
- */
-export function makeTemplateDiagnostic(
-    templateId: TemplateId, mapping: TemplateSourceMapping, span: ParseSourceSpan,
-    category: ts.DiagnosticCategory, code: number, messageText: string|ts.DiagnosticMessageChain,
-    relatedMessage?: {
-      text: string,
-      span: ParseSourceSpan,
-    }): TemplateDiagnostic {
-  if (mapping.type === 'direct') {
-    let relatedInformation: ts.DiagnosticRelatedInformation[]|undefined = undefined;
-    if (relatedMessage !== undefined) {
-      relatedInformation = [{
-        category: ts.DiagnosticCategory.Message,
-        code: 0,
-        file: mapping.node.getSourceFile(),
-        start: relatedMessage.span.start.offset,
-        length: relatedMessage.span.end.offset - relatedMessage.span.start.offset,
-        messageText: relatedMessage.text,
-      }];
-    }
-    // For direct mappings, the error is shown inline as ngtsc was able to pinpoint a string
-    // constant within the `@Component` decorator for the template. This allows us to map the error
-    // directly into the bytes of the source file.
-    return {
-      source: 'ngtsc',
-      code,
-      category,
-      messageText,
-      file: mapping.node.getSourceFile(),
-      componentFile: mapping.node.getSourceFile(),
-      templateId,
-      start: span.start.offset,
-      length: span.end.offset - span.start.offset,
-      relatedInformation,
-    };
-  } else if (mapping.type === 'indirect' || mapping.type === 'external') {
-    // For indirect mappings (template was declared inline, but ngtsc couldn't map it directly
-    // to a string constant in the decorator), the component's file name is given with a suffix
-    // indicating it's not the TS file being displayed, but a template.
-    // For external temoplates, the HTML filename is used.
-    const componentSf = mapping.componentClass.getSourceFile();
-    const componentName = mapping.componentClass.name.text;
-    // TODO(alxhub): remove cast when TS in g3 supports this narrowing.
-    const fileName = mapping.type === 'indirect' ?
-        `${componentSf.fileName} (${componentName} template)` :
-        (mapping as ExternalTemplateSourceMapping).templateUrl;
-    // TODO(alxhub): investigate creating a fake `ts.SourceFile` here instead of invoking the TS
-    // parser against the template (HTML is just really syntactically invalid TypeScript code ;).
-    // Also investigate caching the file to avoid running the parser multiple times.
-    const sf = ts.createSourceFile(
-        fileName, mapping.template, ts.ScriptTarget.Latest, false, ts.ScriptKind.JSX);
-
-    let relatedInformation: ts.DiagnosticRelatedInformation[] = [];
-    if (relatedMessage !== undefined) {
-      relatedInformation.push({
-        category: ts.DiagnosticCategory.Message,
-        code: 0,
-        file: sf,
-        start: relatedMessage.span.start.offset,
-        length: relatedMessage.span.end.offset - relatedMessage.span.start.offset,
-        messageText: relatedMessage.text,
-      });
-    }
-
-    relatedInformation.push({
-      category: ts.DiagnosticCategory.Message,
-      code: 0,
-      file: componentSf,
-      // mapping.node represents either the 'template' or 'templateUrl' expression. getStart()
-      // and getEnd() are used because they don't include surrounding whitespace.
-      start: mapping.node.getStart(),
-      length: mapping.node.getEnd() - mapping.node.getStart(),
-      messageText: `Error occurs in the template of component ${componentName}.`,
-    });
-
-    return {
-      source: 'ngtsc',
-      category,
-      code,
-      messageText,
-      file: sf,
-      componentFile: componentSf,
-      templateId,
-      start: span.start.offset,
-      length: span.end.offset - span.start.offset,
-      // Show a secondary message indicating the component whose template contains the error.
-      relatedInformation,
-    };
-  } else {
-    throw new Error(`Unexpected source mapping type: ${(mapping as {type: string}).type}`);
-  }
-}
-
 interface SourceLocation {
   id: TemplateId;
   span: AbsoluteSourceSpan;
@@ -338,8 +227,3 @@ function hasIgnoreMarker(node: ts.Node, sourceFile: ts.SourceFile): boolean {
     return commentText === IGNORE_MARKER;
   }) === true;
 }
-
-export function isTemplateDiagnostic(diagnostic: ts.Diagnostic): diagnostic is TemplateDiagnostic {
-  return diagnostic.hasOwnProperty('componentFile') &&
-      ts.isSourceFile((diagnostic as any).componentFile);
-}

packages/compiler-cli/src/ngtsc/typecheck/src/dom.ts

@@ -11,8 +11,9 @@ import * as ts from 'typescript';
 
 import {ErrorCode, ngErrorCode} from '../../diagnostics';
 import {TemplateId} from '../api';
+import {makeTemplateDiagnostic, TemplateDiagnostic} from '../diagnostics';
 
-import {makeTemplateDiagnostic, TemplateDiagnostic, TemplateSourceResolver} from './diagnostics';
+import {TemplateSourceResolver} from './diagnostics';
 
 const REGISTRY = new DomElementSchemaRegistry();
 const REMOVE_XHTML_REGEX = /^:xhtml:/;

packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts

@@ -12,8 +12,9 @@ import * as ts from 'typescript';
 import {ErrorCode, makeDiagnostic, makeRelatedInformation, ngErrorCode} from '../../diagnostics';
 import {ClassDeclaration} from '../../reflection';
 import {TemplateId} from '../api';
+import {makeTemplateDiagnostic, TemplateDiagnostic} from '../diagnostics';
 
-import {makeTemplateDiagnostic, TemplateDiagnostic, TemplateSourceResolver} from './diagnostics';
+import {TemplateSourceResolver} from './diagnostics';
 
 
 

packages/compiler-cli/src/ngtsc/typecheck/src/source.ts

@@ -8,7 +8,9 @@
 
 import {AbsoluteSourceSpan, ParseLocation, ParseSourceFile, ParseSourceSpan} from '@angular/compiler';
 import * as ts from 'typescript';
+
 import {TemplateId, TemplateSourceMapping} from '../api';
+import {getTemplateId} from '../diagnostics';
 
 import {TemplateSourceResolver} from './diagnostics';
 import {computeLineStartsMap, getLineAndCharacterFromPosition} from './line_mappings';
@@ -81,28 +83,3 @@ export class TemplateSourceManager implements TemplateSourceResolver {
     return templateSource.toParseSourceSpan(span.start, span.end);
   }
 }
-
-const TEMPLATE_ID = Symbol('ngTemplateId');
-const NEXT_TEMPLATE_ID = Symbol('ngNextTemplateId');
-
-interface HasTemplateId {
-  [TEMPLATE_ID]: TemplateId;
-}
-
-interface HasNextTemplateId {
-  [NEXT_TEMPLATE_ID]: number;
-}
-
-function getTemplateId(node: ts.ClassDeclaration&Partial<HasTemplateId>): TemplateId {
-  if (node[TEMPLATE_ID] === undefined) {
-    node[TEMPLATE_ID] = allocateTemplateId(node.getSourceFile());
-  }
-  return node[TEMPLATE_ID]!;
-}
-
-function allocateTemplateId(sf: ts.SourceFile&Partial<HasNextTemplateId>): TemplateId {
-  if (sf[NEXT_TEMPLATE_ID] === undefined) {
-    sf[NEXT_TEMPLATE_ID] = 1;
-  }
-  return (`tcb${sf[NEXT_TEMPLATE_ID]!++}`) as TemplateId;
-}

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

@@ -21,6 +21,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/testing",
         "//packages/compiler-cli/src/ngtsc/typecheck",
         "//packages/compiler-cli/src/ngtsc/typecheck/api",
+        "//packages/compiler-cli/src/ngtsc/typecheck/diagnostics",
         "//packages/compiler-cli/src/ngtsc/util",
         "@npm//typescript",
     ],

packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts

@@ -18,9 +18,9 @@ import {makeProgram} from '../../testing';
 import {getRootDirs} from '../../util/src/typescript';
 import {ProgramTypeCheckAdapter, TemplateTypeChecker, TypeCheckContext} from '../api';
 import {TemplateId, TemplateSourceMapping, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata, TypeCheckingConfig, UpdateMode} from '../api/api';
+import {TemplateDiagnostic} from '../diagnostics';
 import {ReusedProgramStrategy} from '../src/augmented_program';
 import {TemplateTypeCheckerImpl} from '../src/checker';
-import {TemplateDiagnostic} from '../src/diagnostics';
 import {DomSchemaChecker} from '../src/dom';
 import {Environment} from '../src/environment';
 import {OutOfBandDiagnosticRecorder} from '../src/oob';