From 098b461b69862ad011c7b13dc539e187d3761f86 Mon Sep 17 00:00:00 2001
From: Pawel Kozlowski <pkozlowski.opensource@gmail.com>
Date: Wed, 22 Jun 2016 17:43:27 +0200
Subject: [PATCH] fix(core): report duplicate template bindings in templates

Fixes #7315

BREAKING CHANGES:

Previously multiple template bindings on one element
(ex. `<div *ngIf='..' *ngFor='...'>`) were allowed but most of the time
were leading to undesired result. It is possible that a small number
of applications will see template parse errors that shuld be fixed by
nesting elements or using `<template>` tags explicitly.

Closes #9462
---
 .../@angular/compiler/src/template_parser.ts  | 27 ++++++++++++++-----
 .../compiler/test/template_parser_spec.ts     | 22 ++++++++++++++-
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/modules/@angular/compiler/src/template_parser.ts b/modules/@angular/compiler/src/template_parser.ts
index 33d42f3b3d..1c383a71ae 100644
--- a/modules/@angular/compiler/src/template_parser.ts
+++ b/modules/@angular/compiler/src/template_parser.ts
@@ -10,7 +10,7 @@ import {Parser} from './expression_parser/parser';
 import {CompileDirectiveMetadata, CompilePipeMetadata, CompileMetadataWithType,} from './compile_metadata';
 import {HtmlParser} from './html_parser';
 import {splitNsName, mergeNsAndName} from './html_tags';
-import {ParseSourceSpan, ParseError, ParseLocation, ParseErrorLevel} from './parse_util';
+import {ParseSourceSpan, ParseError, ParseErrorLevel} from './parse_util';
 import {InterpolationConfig} from './interpolation_config';
 
 import {ElementAst, BoundElementPropertyAst, BoundEventAst, ReferenceAst, TemplateAst, TemplateAstVisitor, templateVisitAll, TextAst, BoundTextAst, EmbeddedTemplateAst, AttrAst, NgContentAst, PropertyBindingType, DirectiveAst, BoundDirectivePropertyAst, ProviderAst, ProviderAstType, VariableAst} from './template_ast';
@@ -312,6 +312,19 @@ class TemplateParseVisitor implements HtmlAstVisitor {
           elementOrDirectiveRefs, elementVars);
       var hasTemplateBinding = this._parseInlineTemplateBinding(
           attr, templateMatchableAttrs, templateElementOrDirectiveProps, templateElementVars);
+
+      if (hasTemplateBinding && isTemplateElement) {
+        this._reportError(
+            `Can't have template bindings on a <template> element but the '${attr.name}' attribute was used`,
+            attr.sourceSpan);
+      }
+
+      if (hasTemplateBinding && hasInlineTemplates) {
+        this._reportError(
+            `Can't have multiple template bindings on one element. Use only one attribute named 'template' or prefixed with *`,
+            attr.sourceSpan);
+      }
+
       if (!hasBinding && !hasTemplateBinding) {
         // don't include the bindings as attributes as well in the AST
         attrs.push(this.visitAttr(attr, null));
@@ -403,7 +416,7 @@ class TemplateParseVisitor implements HtmlAstVisitor {
   private _parseInlineTemplateBinding(
       attr: HtmlAttrAst, targetMatchableAttrs: string[][],
       targetProps: BoundElementOrDirectiveProperty[], targetVars: VariableAst[]): boolean {
-    var templateBindingsSource: any /** TODO #???? */ = null;
+    var templateBindingsSource: string = null;
     if (attr.name == TEMPLATE_ATTR) {
       templateBindingsSource = attr.value;
     } else if (attr.name.startsWith(TEMPLATE_ATTR_PREFIX)) {
@@ -812,9 +825,9 @@ class TemplateParseVisitor implements HtmlAstVisitor {
       directives: DirectiveAst[], events: BoundEventAst[]) {
     var allDirectiveEvents = new Set<string>();
     directives.forEach(directive => {
-      StringMapWrapper.forEach(
-          directive.directive.outputs,
-          (eventName: string, _: any /** TODO #???? */) => { allDirectiveEvents.add(eventName); });
+      StringMapWrapper.forEach(directive.directive.outputs, (eventName: string) => {
+        allDirectiveEvents.add(eventName);
+      });
     });
     events.forEach(event => {
       if (isPresent(event.target) || !SetWrapper.has(allDirectiveEvents, event.name)) {
@@ -877,7 +890,7 @@ class ElementContext {
       isTemplateElement: boolean, directives: DirectiveAst[],
       providerContext: ProviderElementContext): ElementContext {
     var matcher = new SelectorMatcher();
-    var wildcardNgContentIndex: any /** TODO #???? */ = null;
+    var wildcardNgContentIndex: number = null;
     var component = directives.find(directive => directive.directive.isComponent);
     if (isPresent(component)) {
       var ngContentSelectors = component.directive.template.ngContentSelectors;
@@ -943,7 +956,7 @@ export class PipeCollector extends RecursiveAstVisitor {
 }
 
 function removeDuplicates(items: CompileMetadataWithType[]): CompileMetadataWithType[] {
-  let res: any[] /** TODO #???? */ = [];
+  let res: CompileMetadataWithType[] = [];
   items.forEach(item => {
     let hasMatch =
         res.filter(
diff --git a/modules/@angular/compiler/test/template_parser_spec.ts b/modules/@angular/compiler/test/template_parser_spec.ts
index 0741f24e62..f71af34df1 100644
--- a/modules/@angular/compiler/test/template_parser_spec.ts
+++ b/modules/@angular/compiler/test/template_parser_spec.ts
@@ -26,7 +26,7 @@ var MOCK_SCHEMA_REGISTRY = [{
 let zeConsole = console;
 
 export function main() {
-  var ngIf: any /** TODO #9100 */;
+  var ngIf: CompileDirectiveMetadata;
   var parse:
       (template: string, directives: CompileDirectiveMetadata[], pipes?: CompilePipeMetadata[]) =>
           TemplateAst[];
@@ -1145,6 +1145,26 @@ Reference "#a" is defined several times ("<div #a></div><div [ERROR ->]#a></div>
 <ng-content> element cannot have content. <ng-content> must be immediately followed by </ng-content> ("[ERROR ->]<ng-content>content</ng-content>"): TestComp@0:0`);
       });
 
+      it('should report when *attr is used on a template element', () => {
+        expect(() => parse('<template *ngIf>', [])).toThrowError(`Template parse errors:
+Can't have template bindings on a <template> element but the '*ngIf' attribute was used ("<template [ERROR ->]*ngIf>"): TestComp@0:10`);
+      });
+
+      it('should report when a template attribute is used on a template element', () => {
+        expect(() => parse('<template template="ngIf">', [])).toThrowError(`Template parse errors:
+Can't have template bindings on a <template> element but the 'template' attribute was used ("<template [ERROR ->]template="ngIf">"): TestComp@0:10`);
+      });
+
+      it('should report when mutliple *attrs are used on the same element', () => {
+        expect(() => parse('<div *ngIf *ngFor>', [])).toThrowError(`Template parse errors:
+Can't have multiple template bindings on one element. Use only one attribute named 'template' or prefixed with * ("<div *ngIf [ERROR ->]*ngFor>"): TestComp@0:11`);
+      });
+
+      it('should report when mix of template and *attrs are used on the same element', () => {
+        expect(() => parse('<div template="ngIf" *ngFor>', [])).toThrowError(`Template parse errors:
+Can't have multiple template bindings on one element. Use only one attribute named 'template' or prefixed with * ("<div template="ngIf" [ERROR ->]*ngFor>"): TestComp@0:21`);
+      });
+
       it('should report invalid property names', () => {
         expect(() => parse('<div [invalidProp]></div>', [])).toThrowError(`Template parse errors:
 Can't bind to 'invalidProp' since it isn't a known native property ("<div [ERROR ->][invalidProp]></div>"): TestComp@0:5`);