From 19b88cef71b4c8e046ea751d5470dce5f9e1438b Mon Sep 17 00:00:00 2001
From: ayazhafiz <ayaz.hafiz.1@gmail.com>
Date: Sat, 24 Oct 2020 12:50:00 -0500
Subject: [PATCH] fix(compiler): do not throw away render3 AST on errors
 (#39413)

Currently render3's `parseTemplate` throws away the parsed AST and
returns an empty list of HTML nodes if HTML->R3 translation failed. This
is not preferrable in some contexts like that of a language service,
where we would like a well-formed AST even if it is has errors.

PR Close #39413
---
 .../compiler/src/render3/view/template.ts     | 18 +++++-----------
 .../ivy/test/hybrid_visitor_spec.ts           | 21 ++++++++++---------
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts
index f7b0ebebaf..cc313ce1c0 100644
--- a/packages/compiler/src/render3/view/template.ts
+++ b/packages/compiler/src/render3/view/template.ts
@@ -2046,6 +2046,8 @@ export function parseTemplate(
       {leadingTriviaChars: LEADING_TRIVIA_CHARS, ...options, tokenizeExpansionForms: true});
 
   if (parseResult.errors && parseResult.errors.length > 0) {
+    // TODO(ayazhafiz): we may not always want to bail out at this point (e.g. in
+    // the context of a language service).
     return {
       interpolationConfig,
       preserveWhitespaces,
@@ -2084,23 +2086,11 @@ export function parseTemplate(
 
   const {nodes, errors, styleUrls, styles, ngContentSelectors} =
       htmlAstToRender3Ast(rootNodes, bindingParser);
-  if (errors && errors.length > 0) {
-    return {
-      interpolationConfig,
-      preserveWhitespaces,
-      template,
-      errors,
-      nodes: [],
-      styleUrls: [],
-      styles: [],
-      ngContentSelectors: []
-    };
-  }
 
   return {
     interpolationConfig,
     preserveWhitespaces,
-    errors: null,
+    errors: errors.length > 0 ? errors : null,
     template,
     nodes,
     styleUrls,
@@ -2265,6 +2255,8 @@ export interface ParsedTemplate {
 
   /**
    * Any errors from parsing the template the first time.
+   *
+   * `null` if there are no errors. Otherwise, the array of errors is guaranteed to be non-empty.
    */
   errors: ParseError[]|null;
 
diff --git a/packages/language-service/ivy/test/hybrid_visitor_spec.ts b/packages/language-service/ivy/test/hybrid_visitor_spec.ts
index 04d3578238..c238cb97f5 100644
--- a/packages/language-service/ivy/test/hybrid_visitor_spec.ts
+++ b/packages/language-service/ivy/test/hybrid_visitor_spec.ts
@@ -433,16 +433,17 @@ describe('findNodeAtPosition for expression AST', () => {
     expect(node).toBeInstanceOf(e.BindingPipe);
   });
 
-  it('should locate binding pipe without identifier',
-     () => {
-         // TODO: We are not able to locate pipe if identifier is missing because the
-         // parser throws an error. This case is important for autocomplete.
-         // const {errors, nodes, position} = parse(`{{ title | ¦ }}`);
-         // expect(errors).toBe(null);
-         // const node = findNodeAtPosition(nodes, position);
-         // expect(isExpressionNode(node!)).toBe(true);
-         // expect(node).toBeInstanceOf(e.BindingPipe);
-     });
+  it('should locate binding pipe without identifier', () => {
+    const {errors, nodes, position} = parse(`{{ title | ¦ }}`);
+    expect(errors?.length).toBe(1);
+    expect(errors![0].toString())
+        .toContain(
+            'Unexpected end of input, expected identifier or keyword at the end of the expression');
+    const node = findNodeAtPosition(nodes, position);
+    expect(isExpressionNode(node!)).toBe(true);
+    // TODO: We want this to be a BindingPipe.
+    expect(node).toBeInstanceOf(e.Interpolation);
+  });
 
   it('should locate method call', () => {
     const {errors, nodes, position} = parse(`{{ title.toString(¦) }}`);