refactor(DirectiveParser): remove checks for missing directives

Based on the discussion in angular#776 we can't reliably check if a given
element has a particular property at the compilation time. As such
the existing algorithm detecting "missing" directives can't be used.

We need to see if there is a different / better algorithm or maybe
those checks need to be moved later in the process (runtime). Leaving
integration tests in place (disabled) so we can come back to the
topic after unblocking the situation.

This commit effectivelly reverts 94e203b

Author: pkozlowski-opensource

modules/angular2/src/core/compiler/pipeline/directive_parser.js

@@ -1,5 +1,5 @@
 import {isPresent, isBlank, BaseException, assertionsEnabled, RegExpWrapper} from 'angular2/src/facade/lang';
-import {List, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
+import {List, MapWrapper} from 'angular2/src/facade/collection';
 import {DOM} from 'angular2/src/dom/dom_adapter';
 import {SelectorMatcher} from '../selector';
 import {CssSelector} from '../selector';
@@ -10,9 +10,6 @@ import {CompileStep} from './compile_step';
 import {CompileElement} from './compile_element';
 import {CompileControl} from './compile_control';
 
-import {isSpecialProperty} from './element_binder_builder';
-import {dashCaseToCamelCase, camelCaseToDashCase} from './util';
-
 var PROPERTY_BINDING_REGEXP = RegExpWrapper.create('^ *([^\\s\\|]+)');
 
 /**
@@ -60,53 +57,15 @@ export class DirectiveParser extends CompileStep {
     });
 
     // Note: We assume that the ViewSplitter already did its work, i.e. template directive should
-    // only be present on <template> elements any more!
+    // only be present on <template> elements!
     var isTemplateElement = DOM.isTemplateElement(current.element);
-    var matchedProperties; // StringMap - used in dev mode to store all properties that have been matched
 
     this._selectorMatcher.match(cssSelector, (selector, directive) => {
-      matchedProperties = updateMatchedProperties(matchedProperties, selector, directive);
-      checkDirectiveValidity(directive, current, isTemplateElement);
-      current.addDirective(directive);
+      current.addDirective(checkDirectiveValidity(directive, current, isTemplateElement));
     });
-
-    // raise error if some directives are missing
-    checkMissingDirectives(current, matchedProperties, isTemplateElement);
   }
 }
 
-// calculate all the properties that are used or interpreted by all directives
-// those properties correspond to the directive selectors and the directive bindings
-function updateMatchedProperties(matchedProperties, selector, directive) {
-  if (assertionsEnabled()) {
-    var attrs = selector.attrs;
-    if (!isPresent(matchedProperties)) {
-      matchedProperties = StringMapWrapper.create();
-    }
-    if (isPresent(attrs)) {
-      for (var idx = 0; idx<attrs.length; idx+=2) {
-        // attribute name is stored on even indexes
-        StringMapWrapper.set(matchedProperties, dashCaseToCamelCase(attrs[idx]), true);
-      }
-    }
-    // some properties can be used by the directive, so we need to register them
-    if (isPresent(directive.annotation) && isPresent(directive.annotation.bind)) {
-      var bindMap = directive.annotation.bind;
-      StringMapWrapper.forEach(bindMap, (value, key) => {
-        // value is the name of the property that is interpreted
-        // e.g. 'myprop' or 'myprop | double' when a pipe is used to transform the property
-
-        // keep the property name and remove the pipe
-        var bindProp = RegExpWrapper.firstMatch(PROPERTY_BINDING_REGEXP, value);
-        if (isPresent(bindProp) && isPresent(bindProp[1])) {
-          StringMapWrapper.set(matchedProperties, dashCaseToCamelCase(bindProp[1]), true);
-        }
-      });
-    }
-  }
-  return matchedProperties;
-}
-
 // check if the directive is compatible with the current element
 function checkDirectiveValidity(directive, current, isTemplateElement) {
   var isComponent = directive.annotation instanceof Component || directive.annotation instanceof DynamicComponent;
@@ -125,26 +84,6 @@ function checkDirectiveValidity(directive, current, isTemplateElement) {
   } else if (isComponent && alreadyHasComponent) {
     throw new BaseException(`Multiple component directives not allowed on the same element - check ${current.elementDescription}`);
   }
-}
 
-// validates that there is no missing directive - dev mode only
-function checkMissingDirectives(current, matchedProperties, isTemplateElement) {
-  if (assertionsEnabled()) {
-    var ppBindings=current.propertyBindings;
-    if (isPresent(ppBindings)) {
-      // check that each property corresponds to a real property or has been matched by a directive
-      MapWrapper.forEach(ppBindings, (expression, prop) => {
-        if (!DOM.hasProperty(current.element, prop) && !isSpecialProperty(prop)) {
-          if (!isPresent(matchedProperties) || !isPresent(StringMapWrapper.get(matchedProperties, prop))) {
-            throw new BaseException(`Missing directive to handle '${camelCaseToDashCase(prop)}' in ${current.elementDescription}`);
-          }
-        }
-      });
-    }
-    // template only store directives as attribute when they are not bound to expressions
-    // so we have to validate the expression case too (e.g. !if="condition")
-    if (isTemplateElement && !current.isViewRoot && !isPresent(current.viewportDirective)) {
-      throw new BaseException(`Missing directive to handle: ${current.elementDescription}`);
-    }
-  }
+  return directive;
 }

modules/angular2/src/core/compiler/pipeline/element_binder_builder.js

@@ -93,12 +93,16 @@ function styleSetterFactory(styleName:string, stylesuffix:string) {
   return setterFn;
 }
 
-// tells if an attribute is handled by the ElementBinderBuilder step
-export function isSpecialProperty(propName:string) {
-  return StringWrapper.startsWith(propName, ATTRIBUTE_PREFIX)
-        || StringWrapper.startsWith(propName, CLASS_PREFIX)
-        || StringWrapper.startsWith(propName, STYLE_PREFIX)
-        || StringMapWrapper.contains(DOM.attrToPropMap, propName);
+const ROLE_ATTR = 'role';
+function roleSetter(element, value) {
+  if (isString(value)) {
+    DOM.setAttribute(element, ROLE_ATTR, value);
+  } else {
+    DOM.removeAttribute(element, ROLE_ATTR);
+    if (isPresent(value)) {
+      throw new BaseException("Invalid role attribute, only string values are allowed, got '" + stringify(value) + "'");
+    }
+  }
 }
 
 /**

modules/angular2/test/core/compiler/integration_spec.js

@@ -2,6 +2,7 @@ import {
   AsyncTestCompleter,
   beforeEach,
   ddescribe,
+  xdescribe,
   describe,
   el,
   expect,
@@ -566,61 +567,57 @@ export function main() {
       }));
     });
 
-    if (assertionsEnabled()) {
-
-      function expectCompileError(inlineTpl, errMessage, done) {
-        tplResolver.setTemplate(MyComp, new Template({inline: inlineTpl}));
-        PromiseWrapper.then(compiler.compile(MyComp),
-          (value) => {
-            throw new BaseException("Test failure: should not have come here as an exception was expected");
-          },
-          (err) => {
-            expect(err.message).toEqual(errMessage);
-            done();
-          }
-        );
-      }
-
-      it('should raise an error if no directive is registered for an unsupported DOM property', inject([AsyncTestCompleter], (async) => {
-        expectCompileError(
-          '<div [some-prop]="foo"></div>',
-          'Missing directive to handle \'some-prop\' in MyComp: <div [some-prop]="foo">',
-          () => async.done()
-        );
-      }));
+    xdescribe('Missing directive checks', () => {
+
+      if (assertionsEnabled()) {
+
+        function expectCompileError(inlineTpl, errMessage, done) {
+          tplResolver.setTemplate(MyComp, new Template({inline: inlineTpl}));
+          PromiseWrapper.then(compiler.compile(MyComp),
+            (value) => {
+              throw new BaseException("Test failure: should not have come here as an exception was expected");
+            },
+            (err) => {
+              expect(err.message).toEqual(errMessage);
+              done();
+            }
+          );
+        }
+
+        it('should raise an error if no directive is registered for a template with template bindings', inject([AsyncTestCompleter], (async) => {
+          expectCompileError(
+            '<div><div template="if: foo"></div></div>',
+            'Missing directive to handle \'if\' in <div template="if: foo">',
+            () => async.done()
+          );
+        }));
 
-      it('should raise an error if no directive is registered for a template with template bindings', inject([AsyncTestCompleter], (async) => {
-        expectCompileError(
-          '<div><div template="if: foo"></div></div>',
-          'Missing directive to handle \'if\' in <div template="if: foo">',
-          () => async.done()
-        );
-      }));
+        it('should raise an error for missing template directive (1)', inject([AsyncTestCompleter], (async) => {
+          expectCompileError(
+            '<div><template foo></template></div>',
+            'Missing directive to handle: <template foo>',
+            () => async.done()
+          );
+        }));
 
-      it('should raise an error for missing template directive (1)', inject([AsyncTestCompleter], (async) => {
-        expectCompileError(
-          '<div><template foo></template></div>',
-          'Missing directive to handle: <template foo>',
-          () => async.done()
-        );
-      }));
+        it('should raise an error for missing template directive (2)', inject([AsyncTestCompleter], (async) => {
+          expectCompileError(
+            '<div><template *if="condition"></template></div>',
+            'Missing directive to handle: <template *if="condition">',
+            () => async.done()
+          );
+        }));
 
-      it('should raise an error for missing template directive (2)', inject([AsyncTestCompleter], (async) => {
-        expectCompileError(
-          '<div><template *if="condition"></template></div>',
-          'Missing directive to handle: <template *if="condition">',
-          () => async.done()
-        );
-      }));
+        it('should raise an error for missing template directive (3)', inject([AsyncTestCompleter], (async) => {
+          expectCompileError(
+            '<div *if="condition"></div>',
+            'Missing directive to handle \'if\' in MyComp: <div *if="condition">',
+            () => async.done()
+          );
+        }));
+      }
+    });
 
-      it('should raise an error for missing template directive (3)', inject([AsyncTestCompleter], (async) => {
-        expectCompileError(
-          '<div *if="condition"></div>',
-          'Missing directive to handle \'if\' in MyComp: <div *if="condition">',
-          () => async.done()
-        );
-      }));
-    }
   });
 }