feat(ivy): add error reporting to the html to ivy transformer (#23546)

PR Close #23546

packages/compiler/src/aot/compiler.ts

@@ -24,7 +24,7 @@ import * as o from '../output/output_ast';
 import {ParseError} from '../parse_util';
 import {compileNgModule as compileIvyModule} from '../render3/r3_module_compiler';
 import {compilePipe as compileIvyPipe} from '../render3/r3_pipe_compiler';
-import {HtmlToTemplateTransform} from '../render3/r3_template_transform';
+import {htmlAstToRender3Ast} from '../render3/r3_template_transform';
 import {compileComponentFromRender2 as compileIvyComponent, compileDirectiveFromRender2 as compileIvyDirective} from '../render3/view/compiler';
 import {DomElementSchemaRegistry} from '../schema/dom_element_schema_registry';
 import {CompiledStylesheet, StyleCompiler} from '../style_compiler';
@@ -391,10 +391,7 @@ export class AotCompiler {
         if (!preserveWhitespaces) {
           htmlAst = removeWhitespaces(htmlAst);
         }
-        const transform = new HtmlToTemplateTransform(hostBindingParser);
+        const render3Ast = htmlAstToRender3Ast(htmlAst.rootNodes, hostBindingParser);
-        const nodes = html.visitAll(transform, htmlAst.rootNodes, null);
-        const hasNgContent = transform.hasNgContent;
-        const ngContentSelectors = transform.ngContentSelectors;
 
         // Map of StaticType by directive selectors
         const directiveTypeBySel = new Map<string, any>();
@@ -417,8 +414,8 @@ export class AotCompiler {
         pipes.forEach(pipe => { pipeTypeByName.set(pipe.name, pipe.type.reference); });
 
         compileIvyComponent(
-            context, directiveMetadata, nodes, hasNgContent, ngContentSelectors, this.reflector,
+            context, directiveMetadata, render3Ast, this.reflector, hostBindingParser,
-            hostBindingParser, directiveTypeBySel, pipeTypeByName);
+            directiveTypeBySel, pipeTypeByName);
       } else {
         compileIvyDirective(context, directiveMetadata, this.reflector, hostBindingParser);
       }

packages/compiler/src/render3/r3_template_transform.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ParsedEvent, ParsedProperty, ParsedVariable} from '../expression_parser/ast';
+import {ParsedEvent, ParsedProperty, ParsedVariable, ParserError} from '../expression_parser/ast';
 import * as html from '../ml_parser/ast';
 import {replaceNgsp} from '../ml_parser/html_whitespaces';
 import {isNgTemplate} from '../ml_parser/tags';
@@ -14,9 +14,10 @@ import {ParseError, ParseErrorLevel, ParseSourceSpan} from '../parse_util';
 import {isStyleUrlResolvable} from '../style_url_resolver';
 import {BindingParser} from '../template_parser/binding_parser';
 import {PreparsedElementType, preparseElement} from '../template_parser/template_preparser';
-
+import {syntaxError} from '../util';
 import * as t from './r3_ast';
 
+
 const BIND_NAME_REGEXP =
     /^(?:(?:(?:(bind-)|(let-)|(ref-|#)|(on-)|(bindon-)|(@))(.+))|\[\(([^\)]+)\)\]|\[([^\]]+)\]|\(([^\)]+)\))$/;
 
@@ -46,9 +47,39 @@ const CLASS_ATTR = 'class';
 // Default selector used by `<ng-content>` if none specified
 const DEFAULT_CONTENT_SELECTOR = '*';
 
-export class HtmlToTemplateTransform implements html.Visitor {
+// Result of the html AST to Ivy AST transformation
-  errors: ParseError[];
+export type Render3ParseResult = {
+  nodes: t.Node[]; errors: ParseError[];
+  // Any non default (empty or '*') selector found in the template
+  ngContentSelectors: string[];
+  // Wether the template contains any `<ng-content>`
+  hasNgContent: boolean;
+};
 
+export function htmlAstToRender3Ast(
+    htmlNodes: html.Node[], bindingParser: BindingParser): Render3ParseResult {
+  const transformer = new HtmlAstToIvyAst(bindingParser);
+  const ivyNodes = html.visitAll(transformer, htmlNodes);
+
+  // 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,
+    errors: allErrors,
+    ngContentSelectors: transformer.ngContentSelectors,
+    hasNgContent: transformer.hasNgContent,
+  };
+}
+
+class HtmlAstToIvyAst implements html.Visitor {
+  errors: ParseError[] = [];
   // Selectors for the `ng-content` tags. Only non `*` selectors are recorded here
   ngContentSelectors: string[] = [];
   // Any `<ng-content>` in the template ?

packages/compiler/src/render3/view/compiler.ts

@@ -12,7 +12,7 @@ import {CompileReflector} from '../../compile_reflector';
 import {BindingForm, BuiltinFunctionCall, LocalResolver, convertActionBinding, convertPropertyBinding} from '../../compiler_util/expression_converter';
 import {ConstantPool, DefinitionKind} from '../../constant_pool';
 import * as core from '../../core';
-import {AST, AstMemoryEfficientTransformer, BindingPipe, BoundElementBindingType, FunctionCall, ImplicitReceiver, LiteralArray, LiteralMap, LiteralPrimitive, PropertyRead} from '../../expression_parser/ast';
+import {AST, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, LiteralArray, LiteralMap, LiteralPrimitive, PropertyRead} from '../../expression_parser/ast';
 import {Identifiers} from '../../identifiers';
 import {LifecycleHooks} from '../../lifecycle_reflector';
 import * as o from '../../output/output_ast';
@@ -21,9 +21,10 @@ import {CssSelector, SelectorMatcher} from '../../selector';
 import {BindingParser} from '../../template_parser/binding_parser';
 import {OutputContext, error} from '../../util';
 
-import * as t from './../r3_ast';
+import * as t from '../r3_ast';
-import {R3DependencyMetadata, R3ResolvedDependencyType, compileFactoryFunction, dependenciesFromGlobalMetadata} from './../r3_factory';
+import {R3DependencyMetadata, R3ResolvedDependencyType, compileFactoryFunction, dependenciesFromGlobalMetadata} from '../r3_factory';
-import {Identifiers as R3} from './../r3_identifiers';
+import {Identifiers as R3} from '../r3_identifiers';
+import {Render3ParseResult} from '../r3_template_transform';
 import {R3ComponentDef, R3ComponentMetadata, R3DirectiveDef, R3DirectiveMetadata, R3QueryMetadata} from './api';
 import {BindingScope, TemplateDefinitionBuilder} from './template';
 import {CONTEXT_NAME, DefinitionMap, ID_SEPARATOR, MEANING_SEPARATOR, TEMPORARY_NAME, asLiteral, conditionallyCreateMapObjectLiteral, getQueryPredicate, temporaryAllocator, unsupported} from './util';
@@ -191,9 +192,8 @@ export function compileDirectiveFromRender2(
  * information.
  */
 export function compileComponentFromRender2(
-    outputCtx: OutputContext, component: CompileDirectiveMetadata, nodes: t.Node[],
+    outputCtx: OutputContext, component: CompileDirectiveMetadata, render3Ast: Render3ParseResult,
-    hasNgContent: boolean, ngContentSelectors: string[], reflector: CompileReflector,
+    reflector: CompileReflector, bindingParser: BindingParser, directiveTypeBySel: Map<string, any>,
-    bindingParser: BindingParser, directiveTypeBySel: Map<string, any>,
     pipeTypeByName: Map<string, any>) {
   const name = identifierName(component.type) !;
   name || error(`Cannot resolver the name of ${component.type}`);
@@ -207,7 +207,9 @@ export function compileComponentFromRender2(
     ...directiveMetadataFromGlobalMetadata(component, outputCtx, reflector),
     selector: component.selector,
     template: {
-        nodes, hasNgContent, ngContentSelectors,
+      nodes: render3Ast.nodes,
+      hasNgContent: render3Ast.hasNgContent,
+      ngContentSelectors: render3Ast.ngContentSelectors,
     },
     lifecycle: {
       usesOnChanges:

packages/compiler/src/template_parser/binding_parser.ts

@@ -29,12 +29,13 @@ const ANIMATE_PROP_PREFIX = 'animate-';
  */
 export class BindingParser {
   pipesByName: Map<string, CompilePipeSummary>|null = null;
+
   private _usedPipes: Map<string, CompilePipeSummary> = new Map();
 
   constructor(
       private _exprParser: Parser, private _interpolationConfig: InterpolationConfig,
       private _schemaRegistry: ElementSchemaRegistry, pipes: CompilePipeSummary[]|null,
-      private _targetErrors: ParseError[]) {
+      public errors: ParseError[]) {
     // When the `pipes` parameter is `null`, do not check for used pipes
     // This is used in IVY when we might not know the available pipes at compile time
     if (pipes) {
@@ -364,7 +365,7 @@ export class BindingParser {
   private _reportError(
       message: string, sourceSpan: ParseSourceSpan,
       level: ParseErrorLevel = ParseErrorLevel.ERROR) {
-    this._targetErrors.push(new ParseError(sourceSpan, message, level));
+    this.errors.push(new ParseError(sourceSpan, message, level));
   }
 
   private _reportExpressionParserErrors(errors: ParserError[], sourceSpan: ParseSourceSpan) {

packages/compiler/src/template_parser/template_parser.ts

@@ -59,18 +59,6 @@ const CLASS_ATTR = 'class';
 
 const TEXT_CSS_SELECTOR = CssSelector.parse('*')[0];
 
-let warningCounts: {[warning: string]: number} = {};
-
-function warnOnlyOnce(warnings: string[]): (warning: ParseError) => boolean {
-  return (error: ParseError) => {
-    if (warnings.indexOf(error.msg) !== -1) {
-      warningCounts[error.msg] = (warningCounts[error.msg] || 0) + 1;
-      return warningCounts[error.msg] <= 1;
-    }
-    return true;
-  };
-}
-
 export class TemplateParseError extends ParseError {
   constructor(message: string, span: ParseSourceSpan, level: ParseErrorLevel) {
     super(span, message, level);

packages/compiler/test/render3/mock_compile.ts

@@ -16,7 +16,7 @@ import * as html from '../../src/ml_parser/ast';
 import {removeWhitespaces} from '../../src/ml_parser/html_whitespaces';
 import * as o from '../../src/output/output_ast';
 import {compilePipe} from '../../src/render3/r3_pipe_compiler';
-import {HtmlToTemplateTransform} from '../../src/render3/r3_template_transform';
+import {htmlAstToRender3Ast} from '../../src/render3/r3_template_transform';
 import {compileComponentFromRender2, compileDirectiveFromRender2} from '../../src/render3/view/compiler';
 import {BindingParser} from '../../src/template_parser/binding_parser';
 import {OutputContext, escapeRegExp} from '../../src/util';
@@ -308,14 +308,12 @@ export function compile(
               if (!preserveWhitespaces) {
                 htmlAst = removeWhitespaces(htmlAst);
               }
-              const transform = new HtmlToTemplateTransform(hostBindingParser);
+
-              const nodes = html.visitAll(transform, htmlAst.rootNodes, null);
+              const render3Ast = htmlAstToRender3Ast(htmlAst.rootNodes, hostBindingParser);
-              const hasNgContent = transform.hasNgContent;
-              const ngContentSelectors = transform.ngContentSelectors;
 
               compileComponentFromRender2(
-                  outputCtx, directive, nodes, hasNgContent, ngContentSelectors, reflector,
+                  outputCtx, directive, render3Ast, reflector, hostBindingParser,
-                  hostBindingParser, directiveTypeBySel, pipeTypeByName);
+                  directiveTypeBySel, pipeTypeByName);
             } else {
               compileDirectiveFromRender2(outputCtx, directive, reflector, hostBindingParser);
             }

packages/compiler/test/render3/r3_template_transform_spec.ts

@@ -14,13 +14,14 @@ import {HtmlParser} from '../../src/ml_parser/html_parser';
 import {DEFAULT_INTERPOLATION_CONFIG} from '../../src/ml_parser/interpolation_config';
 import {ParseError} from '../../src/parse_util';
 import * as t from '../../src/render3/r3_ast';
-import {HtmlToTemplateTransform} from '../../src/render3/r3_template_transform';
+import {Render3ParseResult, htmlAstToRender3Ast} from '../../src/render3/r3_template_transform';
 import {BindingParser} from '../../src/template_parser/binding_parser';
 import {MockSchemaRegistry} from '../../testing';
 import {unparse} from '../expression_parser/utils/unparser';
 
+
 // Parse an html string to IVY specific info
-function parse(html: string) {
+function parse(html: string): Render3ParseResult {
   const htmlParser = new HtmlParser();
 
   const parseResult = htmlParser.parse(html, 'path:://to/template', true);
@@ -31,27 +32,13 @@ function parse(html: string) {
   }
 
   const htmlNodes = parseResult.rootNodes;
-  const expressionErrors: ParseError[] = [];
   const expressionParser = new Parser(new Lexer());
   const schemaRegistry = new MockSchemaRegistry(
       {'invalidProp': false}, {'mappedAttr': 'mappedProp'}, {'unknown': false, 'un-known': false},
       ['onEvent'], ['onEvent']);
-  const bindingParser = new BindingParser(
+  const bindingParser =
-      expressionParser, DEFAULT_INTERPOLATION_CONFIG, schemaRegistry, null, expressionErrors);
+      new BindingParser(expressionParser, DEFAULT_INTERPOLATION_CONFIG, schemaRegistry, null, []);
-  const r3Transform = new HtmlToTemplateTransform(bindingParser);
+  return htmlAstToRender3Ast(htmlNodes, bindingParser);
-
-  const r3Nodes = visitAll(r3Transform, htmlNodes);
-
-  if (r3Transform.errors) {
-    const msg = r3Transform.errors.map(e => e.toString()).join('\n');
-    throw new Error(msg);
-  }
-
-  return {
-    nodes: r3Nodes,
-    hasNgContent: r3Transform.hasNgContent,
-    ngContentSelectors: r3Transform.ngContentSelectors,
-  };
 }
 
 // Transform an IVY AST to a flat list of nodes to ease testing
@@ -361,10 +348,8 @@ describe('R3 template transform', () => {
       ]);
     });
 
-    // TODO(vicb): Should Error
+    it('should report an error on empty expression', () => {
-    xit('should report an error on empty expression', () => {
       expect(() => parse('<div (event)="">')).toThrowError(/Empty expressions are not allowed/);
-
       expect(() => parse('<div (event)="   ">')).toThrowError(/Empty expressions are not allowed/);
     });
   });
@@ -392,8 +377,63 @@ describe('R3 template transform', () => {
     });
   });
 
-  // TODO(vicb): Add ng-content test
+  describe('ng-content', () => {
-  describe('ng-content', () => {});
+    it('should parse ngContent without selector', () => {
+      const res = parse('<ng-content></ng-content>');
+      expect(res.hasNgContent).toEqual(true);
+      expect(res.ngContentSelectors).toEqual([]);
+      expectFromR3Nodes(res.nodes).toEqual([
+        ['Content', 0],
+      ]);
+    });
+
+    it('should parse ngContent with a * selector', () => {
+      const res = parse('<ng-content></ng-content>');
+      const selectors = [''];
+      expect(res.hasNgContent).toEqual(true);
+      expect(res.ngContentSelectors).toEqual([]);
+      expectFromR3Nodes(res.nodes).toEqual([
+        ['Content', 0],
+      ]);
+    });
+
+    it('should parse ngContent with a specific selector', () => {
+      const res = parse('<ng-content select="tag[attribute]"></ng-content>');
+      const selectors = ['', 'tag[attribute]'];
+      expect(res.hasNgContent).toEqual(true);
+      expect(res.ngContentSelectors).toEqual(['tag[attribute]']);
+      expectFromR3Nodes(res.nodes).toEqual([
+        ['Content', 1],
+        ['TextAttribute', 'select', selectors[1]],
+      ]);
+    });
+
+    it('should parse ngContent with a selector', () => {
+      const res = parse(
+          '<ng-content select="a"></ng-content><ng-content></ng-content><ng-content select="b"></ng-content>');
+      const selectors = ['', 'a', 'b'];
+      expect(res.hasNgContent).toEqual(true);
+      expect(res.ngContentSelectors).toEqual(['a', 'b']);
+      expectFromR3Nodes(res.nodes).toEqual([
+        ['Content', 1],
+        ['TextAttribute', 'select', selectors[1]],
+        ['Content', 0],
+        ['Content', 2],
+        ['TextAttribute', 'select', selectors[2]],
+      ]);
+    });
+
+    it('should parse ngProjectAs as an attribute', () => {
+      const res = parse('<ng-content ngProjectAs="a"></ng-content>');
+      const selectors = [''];
+      expect(res.hasNgContent).toEqual(true);
+      expect(res.ngContentSelectors).toEqual([]);
+      expectFromR3Nodes(res.nodes).toEqual([
+        ['Content', 0],
+        ['TextAttribute', 'ngProjectAs', 'a'],
+      ]);
+    });
+  });
 
   describe('Ignored elements', () => {
     it('should ignore <script> elements', () => {