From d7b9345b6d5f105109991bf3a3a8ee8d310a7a3d Mon Sep 17 00:00:00 2001
From: Pawel Kozlowski <pkozlowski.opensource@gmail.com>
Date: Wed, 17 Jun 2015 16:05:35 +0200
Subject: [PATCH] feat(compiler): detect dangling property bindings

BREAKING CHANGE: compiler will throw on binding to non-existing properties.

Till now it was possible to have a binding to a non-existing property,
ex.: `<div [foo]="exp">`. From now on this is compilation error - any
property binding needs to have at least one associated property:
eaither on an HTML element or on any directive associated with a
given element (directives' properites need to be declared using the
`properties` field in the `@Directive` / `@Component` annotation).

Closes #2598
---
 .../render/dom/compiler/directive_parser.ts   |  1 +
 .../dom/view/property_setter_factory.ts       |  4 +-
 .../src/render/dom/view/proto_view_builder.ts | 20 ++++++-
 .../test/core/compiler/integration_spec.ts    | 59 ++++++++++---------
 .../directive_lifecycle_integration_spec.ts   |  2 +-
 .../angular2/test/directives/ng_for_spec.ts   |  2 +-
 .../test/directives/ng_switch_spec.ts         |  6 +-
 .../shadow_dom_emulation_integration_spec.ts  |  2 +-
 modules/benchmarks/src/costs/index.ts         |  2 +-
 .../src/largetable/largetable_benchmark.ts    |  4 +-
 10 files changed, 61 insertions(+), 41 deletions(-)

diff --git a/modules/angular2/src/render/dom/compiler/directive_parser.ts b/modules/angular2/src/render/dom/compiler/directive_parser.ts
index 7bcb499ebf..08acb733b4 100644
--- a/modules/angular2/src/render/dom/compiler/directive_parser.ts
+++ b/modules/angular2/src/render/dom/compiler/directive_parser.ts
@@ -149,6 +149,7 @@ export class DirectiveParser implements CompileStep {
     if (isPresent(bindingAst)) {
       directiveBinderBuilder.bindProperty(dirProperty, bindingAst);
     }
+    compileElement.bindElement().bindPropertyToDirective(dashCaseToCamelCase(elProp));
   }
 
   _bindDirectiveEvent(eventName, action, compileElement, directiveBinderBuilder) {
diff --git a/modules/angular2/src/render/dom/view/property_setter_factory.ts b/modules/angular2/src/render/dom/view/property_setter_factory.ts
index 5fe4d8df13..403d897904 100644
--- a/modules/angular2/src/render/dom/view/property_setter_factory.ts
+++ b/modules/angular2/src/render/dom/view/property_setter_factory.ts
@@ -18,7 +18,7 @@ const CLASS_PREFIX = 'class.';
 const STYLE_PREFIX = 'style.';
 
 export class PropertySetterFactory {
-  private static _noopSetter(el, value) {}
+  static noopSetter(el, value) {}
 
   private _lazyPropertySettersCache: StringMap<string, Function> = StringMapWrapper.create();
   private _eagerPropertySettersCache: StringMap<string, Function> = StringMapWrapper.create();
@@ -69,7 +69,7 @@ export class PropertySetterFactory {
         if (DOM.hasProperty(protoElement, property)) {
           setterFn = reflector.setter(property);
         } else {
-          setterFn = PropertySetterFactory._noopSetter;
+          setterFn = PropertySetterFactory.noopSetter;
         }
         StringMapWrapper.set(this._eagerPropertySettersCache, property, setterFn);
       }
diff --git a/modules/angular2/src/render/dom/view/proto_view_builder.ts b/modules/angular2/src/render/dom/view/proto_view_builder.ts
index 61b7e10da0..06a8ff97ae 100644
--- a/modules/angular2/src/render/dom/view/proto_view_builder.ts
+++ b/modules/angular2/src/render/dom/view/proto_view_builder.ts
@@ -75,10 +75,17 @@ export class ProtoViewBuilder {
       });
 
       MapWrapper.forEach(ebb.propertyBindings, (_, propertyName) => {
+        var propSetter =
+            setterFactory.createSetter(ebb.element, isPresent(ebb.componentId), propertyName);
 
-        propertySetters.set(
-            propertyName,
-            setterFactory.createSetter(ebb.element, isPresent(ebb.componentId), propertyName));
+        if (propSetter === PropertySetterFactory.noopSetter) {
+          if (!SetWrapper.has(ebb.propertyBindingsToDirectives, propertyName)) {
+            throw new BaseException(
+                `Can't bind to '${propertyName}' since it isn't a know property of the '${DOM.tagName(ebb.element).toLowerCase()}' element and there are no matching directives with a corresponding property`);
+          }
+        }
+
+        propertySetters.set(propertyName, propSetter);
       });
 
       var nestedProtoView =
@@ -170,6 +177,7 @@ export class ElementBinderBuilder {
   nestedProtoView: ProtoViewBuilder = null;
   propertyBindings: Map<string, ASTWithSource> = new Map();
   variableBindings: Map<string, string> = new Map();
+  propertyBindingsToDirectives: Set<string> = new Set();
   eventBindings: List<api.EventBinding> = [];
   eventBuilder: EventBuilder = new EventBuilder();
   textBindingNodes: List</*node*/ any> = [];
@@ -210,6 +218,12 @@ export class ElementBinderBuilder {
 
   bindProperty(name, expression) { this.propertyBindings.set(name, expression); }
 
+  bindPropertyToDirective(name: string) {
+    // we are filling in a set of property names that are bound to a property
+    // of at least one directive. This allows us to report "dangling" bindings.
+    this.propertyBindingsToDirectives.add(name);
+  }
+
   bindVariable(name, value) {
     // When current is a view root, the variable bindings are set to the *nested* proto view.
     // The root view conceptually signifies a new "block scope" (the nested view), to which
diff --git a/modules/angular2/test/core/compiler/integration_spec.ts b/modules/angular2/test/core/compiler/integration_spec.ts
index f898bdfd1e..2183ae5e31 100644
--- a/modules/angular2/test/core/compiler/integration_spec.ts
+++ b/modules/angular2/test/core/compiler/integration_spec.ts
@@ -192,22 +192,6 @@ export function main() {
                });
          }));
 
-      it('should ignore bindings to unknown properties',
-         inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
-           tb.overrideView(MyComp,
-                           new viewAnn.View({template: '<div unknown="{{ctxProp}}"></div>'}));
-
-           tb.createView(MyComp, {context: ctx})
-               .then((view) => {
-
-                 ctx.ctxProp = 'Some value';
-                 view.detectChanges();
-                 expect(DOM.hasProperty(view.rootNodes[0], 'unknown')).toBeFalsy();
-
-                 async.done();
-               });
-         }));
-
       it('should consume directive watch expression change.',
          inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
            var tpl = '<div>' +
@@ -247,7 +231,7 @@ export function main() {
         it("should support pipes in bindings",
            inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
              tb.overrideView(MyComp, new viewAnn.View({
-               template: '<div [my-dir] #dir="mydir" [elprop]="ctxProp | double"></div>',
+               template: '<div my-dir #dir="mydir" [elprop]="ctxProp | double"></div>',
                directives: [MyDir]
              }));
 
@@ -442,7 +426,7 @@ export function main() {
         it('should assign a directive to a var-',
            inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
              tb.overrideView(MyComp, new viewAnn.View({
-               template: '<p><div [export-dir] #localdir="dir"></div></p>',
+               template: '<p><div export-dir #localdir="dir"></div></p>',
                directives: [ExportDir]
              }));
 
@@ -1144,7 +1128,7 @@ export function main() {
       it('should specify a location of an error that happened during change detection (element property)',
          inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
 
-           tb.overrideView(MyComp, new viewAnn.View({template: '<div [prop]="a.b"></div>'}));
+           tb.overrideView(MyComp, new viewAnn.View({template: '<div [title]="a.b"></div>'}));
 
            tb.createView(MyComp, {context: ctx})
                .then((view) => {
@@ -1157,10 +1141,10 @@ export function main() {
       it('should specify a location of an error that happened during change detection (directive property)',
          inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
 
-           tb.overrideView(
-               MyComp,
-               new viewAnn.View(
-                   {template: '<child-cmp [prop]="a.b"></child-cmp>', directives: [ChildComp]}));
+           tb.overrideView(MyComp, new viewAnn.View({
+             template: '<child-cmp [dir-prop]="a.b"></child-cmp>',
+             directives: [ChildComp]
+           }));
 
            tb.createView(MyComp, {context: ctx})
                .then((view) => {
@@ -1205,6 +1189,30 @@ export function main() {
          });
        }));
 
+    describe('Missing property bindings', () => {
+      it('should throw on bindings to unknown properties',
+         inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
+           tb.overrideView(MyComp,
+                           new viewAnn.View({template: '<div unknown="{{ctxProp}}"></div>'}));
+
+           PromiseWrapper.catchError(tb.createView(MyComp, {context: ctx}), (e) => {
+             expect(e.message).toEqual(
+                 `Can't bind to 'unknown' since it isn't a know property of the 'div' element and there are no matching directives with a corresponding property`);
+             async.done();
+             return null;
+           });
+         }));
+
+      it('should not throw for property binding to a non-existing property when there is a matching directive property',
+         inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
+           tb.overrideView(
+               MyComp,
+               new viewAnn.View(
+                   {template: '<div my-dir [elprop]="ctxProp"></div>', directives: [MyDir]}));
+           tb.createView(MyComp, {context: ctx}).then((val) => { async.done(); });
+         }));
+    });
+
     // Disabled until a solution is found, refs:
     // - https://github.com/angular/angular/issues/776
     // - https://github.com/angular/angular/commit/81f3f32
@@ -1353,10 +1361,7 @@ class ComponentWithPipes {
   prop: string;
 }
 
-@Component({
-  selector: 'child-cmp',
-  appInjector: [MyService],
-})
+@Component({selector: 'child-cmp', properties: ['dirProp'], appInjector: [MyService]})
 @View({directives: [MyDir], template: '{{ctxProp}}'})
 @Injectable()
 class ChildComp {
diff --git a/modules/angular2/test/core/directive_lifecycle_integration_spec.ts b/modules/angular2/test/core/directive_lifecycle_integration_spec.ts
index 2ea9d8f9ac..a4d72553c3 100644
--- a/modules/angular2/test/core/directive_lifecycle_integration_spec.ts
+++ b/modules/angular2/test/core/directive_lifecycle_integration_spec.ts
@@ -36,7 +36,7 @@ export function main() {
          tb.overrideView(
              MyComp,
              new viewAnn.View(
-                 {template: '<div [field]="123" [lifecycle]></div>', directives: [LifecycleDir]}));
+                 {template: '<div [field]="123" lifecycle></div>', directives: [LifecycleDir]}));
 
          tb.createView(MyComp, {context: ctx})
              .then((view) => {
diff --git a/modules/angular2/test/directives/ng_for_spec.ts b/modules/angular2/test/directives/ng_for_spec.ts
index 66564597e6..108e03106d 100644
--- a/modules/angular2/test/directives/ng_for_spec.ts
+++ b/modules/angular2/test/directives/ng_for_spec.ts
@@ -205,7 +205,7 @@ export function main() {
 
     it('should repeat over nested arrays with no intermediate element',
        inject([TestBed, AsyncTestCompleter], (tb: TestBed, async) => {
-         var template = '<div><template [ng-for] #item [ng-for-of]="items">' +
+         var template = '<div><template ng-for #item [ng-for-of]="items">' +
                         '<div template="ng-for #subitem of item">' +
                         '{{subitem}}-{{item.length}};' +
                         '</div></template></div>';
diff --git a/modules/angular2/test/directives/ng_switch_spec.ts b/modules/angular2/test/directives/ng_switch_spec.ts
index 8982281aa2..895ee9b139 100644
--- a/modules/angular2/test/directives/ng_switch_spec.ts
+++ b/modules/angular2/test/directives/ng_switch_spec.ts
@@ -79,8 +79,8 @@ export function main() {
                           '<template [ng-switch-when]="\'b\'"><li>when b1;</li></template>' +
                           '<template [ng-switch-when]="\'a\'"><li>when a2;</li></template>' +
                           '<template [ng-switch-when]="\'b\'"><li>when b2;</li></template>' +
-                          '<template [ng-switch-default]><li>when default1;</li></template>' +
-                          '<template [ng-switch-default]><li>when default2;</li></template>' +
+                          '<template ng-switch-default><li>when default1;</li></template>' +
+                          '<template ng-switch-default><li>when default2;</li></template>' +
                           '</ul></div>';
 
            tb.createView(TestComponent, {html: template})
@@ -108,7 +108,7 @@ export function main() {
                           '<ul [ng-switch]="switchValue">' +
                           '<template [ng-switch-when]="when1"><li>when 1;</li></template>' +
                           '<template [ng-switch-when]="when2"><li>when 2;</li></template>' +
-                          '<template [ng-switch-default]><li>when default;</li></template>' +
+                          '<template ng-switch-default><li>when default;</li></template>' +
                           '</ul></div>';
 
            tb.createView(TestComponent, {html: template})
diff --git a/modules/angular2/test/render/dom/shadow_dom_emulation_integration_spec.ts b/modules/angular2/test/render/dom/shadow_dom_emulation_integration_spec.ts
index b541cd9304..6b79f245b9 100644
--- a/modules/angular2/test/render/dom/shadow_dom_emulation_integration_spec.ts
+++ b/modules/angular2/test/render/dom/shadow_dom_emulation_integration_spec.ts
@@ -511,7 +511,7 @@ var conditionalContentComponent = DirectiveMetadata.create({
 });
 
 var autoViewportDirective = DirectiveMetadata.create(
-    {selector: '[auto]', id: '[auto]', type: DirectiveMetadata.DIRECTIVE_TYPE});
+    {selector: '[auto]', id: 'auto', properties: ['auto'], type: DirectiveMetadata.DIRECTIVE_TYPE});
 
 var tabComponent =
     DirectiveMetadata.create({selector: 'tab', id: 'tab', type: DirectiveMetadata.COMPONENT_TYPE});
diff --git a/modules/benchmarks/src/costs/index.ts b/modules/benchmarks/src/costs/index.ts
index a39f98e306..9edfcc74d4 100644
--- a/modules/benchmarks/src/costs/index.ts
+++ b/modules/benchmarks/src/costs/index.ts
@@ -77,7 +77,7 @@ class DynamicDummy {
     </div>
 
     <div *ng-if="testingWithDirectives">
-      <dummy [dummy-decorator] *ng-for="#i of list"></dummy>
+      <dummy dummy-decorator *ng-for="#i of list"></dummy>
     </div>
 
     <div *ng-if="testingDynamicComponents">
diff --git a/modules/benchmarks/src/largetable/largetable_benchmark.ts b/modules/benchmarks/src/largetable/largetable_benchmark.ts
index e8a8d22173..f33c9e4485 100644
--- a/modules/benchmarks/src/largetable/largetable_benchmark.ts
+++ b/modules/benchmarks/src/largetable/largetable_benchmark.ts
@@ -232,7 +232,7 @@ class CellData {
         </tbody>
         <tbody template="ng-switch-when 'interpolationAttr'">
           <tr template="ng-for #row of data">
-            <td template="ng-for #column of row" i="{{column.i}}" j="{{column.j}}">
+            <td template="ng-for #column of row" attr.i="{{column.i}}" attr.j="{{column.j}}">
               i,j attrs
             </td>
           </tr>
@@ -269,7 +269,7 @@ class LargetableComponent {
 @Component({selector: 'app'})
 @View({
   directives: [LargetableComponent],
-  template: `<largetable [data]='data' [benchmarkType]='benchmarkType'></largetable>`
+  template: `<largetable [data]='data' [benchmark-type]='benchmarkType'></largetable>`
 })
 class AppComponent {
   data;