refactor(compiler-cli): make template parsing errors into diagnostics (…

…#38576)

Previously, the compiler was not able to display template parsing errors as
true `ts.Diagnostic`s that point inside the template. Instead, it would
throw an actual `Error`, and "crash" with a stack trace containing the
template errors.

Not only is this a poor user experience, but it causes the Language Service
to also crash as the user is editing a template (in actuality the LS has to
work around this bug).

With this commit, such parsing errors are converted to true template
diagnostics with appropriate span information to be displayed contextually
along with all other diagnostics. This majorly improves the user experience
and unblocks the Language Service from having to deal with the compiler
"crashing" to report errors.

PR Close #38576

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

@@ -23,6 +23,7 @@ ts_library(
         "//packages/compiler-cli/src/ngtsc/shims:api",
         "//packages/compiler-cli/src/ngtsc/transform",
         "//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/annotations/src/component.ts

@@ -10,7 +10,7 @@ import {compileComponentFromMetadata, ConstantPool, CssSelector, DEFAULT_INTERPO
 import * as ts from 'typescript';
 
 import {CycleAnalyzer} from '../../cycles';
-import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
+import {ErrorCode, FatalDiagnosticError, ngErrorCode} from '../../diagnostics';
 import {absoluteFrom, relative} from '../../file_system';
 import {DefaultImportRecorder, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
 import {DependencyTracker} from '../../incremental/api';
@@ -22,6 +22,7 @@ import {ClassDeclaration, Decorator, ReflectionHost, reflectObjectLiteral} from
 import {ComponentScopeReader, LocalModuleScopeRegistry} from '../../scope';
 import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFlags, HandlerPrecedence, ResolveResult} from '../../transform';
 import {TemplateSourceMapping, TypeCheckContext} from '../../typecheck/api';
+import {getTemplateId, makeTemplateDiagnostic} from '../../typecheck/diagnostics';
 import {tsSourceMapBug29300Fixed} from '../../util/src/ts_source_map_bug_29300';
 import {SubsetOfKeys} from '../../util/src/typescript';
 
@@ -254,9 +255,26 @@ export class ComponentDecoratorHandler implements
       }
     }
 
+    let diagnostics: ts.Diagnostic[]|undefined = undefined;
+
     if (template.errors !== undefined) {
-      throw new Error(
-          `Errors parsing template: ${template.errors.map(e => e.toString()).join(', ')}`);
+      // If there are any template parsing errors, convert them to `ts.Diagnostic`s for display.
+      const id = getTemplateId(node);
+      diagnostics = template.errors.map(error => {
+        const span = error.span;
+
+        if (span.start.offset === span.end.offset) {
+          // Template errors can contain zero-length spans, if the error occurs at a single point.
+          // However, TypeScript does not handle displaying a zero-length diagnostic very well, so
+          // increase the ending offset by 1 for such errors, to ensure the position is shown in the
+          // diagnostic.
+          span.end.offset++;
+        }
+
+        return makeTemplateDiagnostic(
+            id, template.sourceMapping, span, ts.DiagnosticCategory.Error,
+            ngErrorCode(ErrorCode.TEMPLATE_PARSE_ERROR), error.msg);
+      });
     }
 
     // Figure out the set of styles. The ordering here is important: external resources (styleUrls)
@@ -335,6 +353,7 @@ export class ComponentDecoratorHandler implements
         providersRequiringFactory,
         viewProvidersRequiringFactory,
       },
+      diagnostics,
     };
     if (changeDetection !== null) {
       output.analysis!.meta.changeDetection = changeDetection;

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

@@ -56,6 +56,11 @@ export enum ErrorCode {
    */
   HOST_BINDING_PARSE_ERROR = 5001,
 
+  /**
+   * Raised when the compiler cannot parse a component's template.
+   */
+  TEMPLATE_PARSE_ERROR = 5002,
+
   /**
    * Raised when an NgModule contains an invalid reference in `declarations`.
    */

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -7378,6 +7378,54 @@ export const Foo = Foo__PRE_R3__;
            const diags = env.driveDiagnostics();
            expect(diags.length).toBe(0);
          });
+
+      describe('template parsing diagnostics', () => {
+        // These tests validate that errors which occur during template parsing are expressed as
+        // diagnostics instead of a compiler crash (which used to be the case). They only assert
+        // that the error is produced with an accurate span - the exact semantics of the errors are
+        // tested separately, via the parser tests.
+        it('should emit a diagnostic for a template parsing error', () => {
+          env.write('test.ts', `
+            import {Component} from '@angular/core';
+            @Component({
+              template: '<div></span>',
+              selector: 'test-cmp',
+            })
+            export class TestCmp {}
+          `);
+          const diags = env.driveDiagnostics();
+          expect(diags.length).toBe(1);
+          expect(getDiagnosticSourceCode(diags[0])).toBe('</span>');
+        });
+
+        it('should emit a diagnostic for an expression parsing error', () => {
+          env.write('test.ts', `
+            import {Component} from '@angular/core';
+            @Component({
+              template: '<cmp [input]="x ? y">',
+              selector: 'test-cmp',
+            })
+            export class TestCmp {}
+          `);
+          const diags = env.driveDiagnostics();
+          expect(diags.length).toBe(1);
+          expect(getDiagnosticSourceCode(diags[0])).toBe('x ? y');
+        });
+
+        it('should use a single character span for an unexpected EOF parsing error', () => {
+          env.write('test.ts', `
+              import {Component} from '@angular/core';
+              @Component({
+                template: '<cmp [input]="x>',
+                selector: 'test-cmp',
+              })
+              export class TestCmp {}
+            `);
+          const diags = env.driveDiagnostics();
+          expect(diags.length).toBe(1);
+          expect(getDiagnosticSourceCode(diags[0])).toBe('\'');
+        });
+      });
     });
   });
 

packages/compiler/src/render3/r3_template_transform.ts

@@ -62,12 +62,6 @@ export function htmlAstToRender3Ast(
 
   // Errors might originate in either the binding parser or the html to ivy transformer
   const allErrors = bindingParser.errors.concat(transformer.errors);
-  const errors: ParseError[] = allErrors.filter(e => e.level === ParseErrorLevel.ERROR);
-
-  if (errors.length > 0) {
-    const errorString = errors.join('\n');
-    throw syntaxError(`Template parse errors:\n${errorString}`, errors);
-  }
 
   return {
     nodes: ivyNodes,

packages/compiler/test/render3/view/util.ts

@@ -101,7 +101,14 @@ export function parseR3(
       ['onEvent'], ['onEvent']);
   const bindingParser =
       new BindingParser(expressionParser, DEFAULT_INTERPOLATION_CONFIG, schemaRegistry, null, []);
-  return htmlAstToRender3Ast(htmlNodes, bindingParser);
+  const r3Result = htmlAstToRender3Ast(htmlNodes, bindingParser);
+
+  if (r3Result.errors.length > 0) {
+    const msg = r3Result.errors.map(e => e.toString()).join('\n');
+    throw new Error(msg);
+  }
+
+  return r3Result;
 }
 
 export function processI18nMeta(