fix(compiler): StaticReflector ignores unregistered decorators. (#9266)

Also modified static reflector to allow writing tests in using
the .ts and using the MetadataCollector.

Also made MetadataCollector be able to use SourceFiles that have
not been bound (that is, don't have the parent property set).

Fixes #9182
Fixes #9265

modules/@angular/compiler-cli/src/static_reflector.ts

@@ -151,31 +151,15 @@ export class StaticReflector implements ReflectorReader {
 
   private registerDecoratorOrConstructor(type: StaticSymbol, ctor: any): void {
     this.conversionMap.set(type, (context: StaticSymbol, args: any[]) => {
-      let argValues: any[] = [];
-      args.forEach((arg, index) => {
-        let argValue: any;
-        if (typeof arg === 'object' && !arg['__symbolic']) {
-          argValue = mapStringMap(arg, (value, key) => this.simplify(context, value));
-        } else {
-          argValue = this.simplify(context, arg);
-        }
-        argValues.push(argValue);
-      });
       var metadata = Object.create(ctor.prototype);
-      ctor.apply(metadata, argValues);
+      ctor.apply(metadata, args);
       return metadata;
     });
   }
 
   private registerFunction(type: StaticSymbol, fn: any): void {
-    this.conversionMap.set(type, (context: StaticSymbol, args: any[]) => {
+    this.conversionMap.set(
-      let argValues: any[] = [];
+        type, (context: StaticSymbol, args: any[]) => { return fn.apply(undefined, args); });
-      args.forEach((arg, index) => {
-        let argValue = this.simplify(context, arg);
-        argValues.push(argValue);
-      });
-      return fn.apply(null, argValues);
-    });
   }
 
   private initializeConversionMap(): void {
@@ -251,7 +235,7 @@ export class StaticReflector implements ReflectorReader {
     let scope = BindingScope.empty;
     let calling = new Map<StaticSymbol, boolean>();
 
-    function simplifyInContext(context: StaticSymbol, value: any): any {
+    function simplifyInContext(context: StaticSymbol, value: any, depth: number): any {
       function resolveReference(expression: any): StaticSymbol {
         let staticSymbol: StaticSymbol;
         if (expression['module']) {
@@ -274,8 +258,12 @@ export class StaticReflector implements ReflectorReader {
       }
 
       function simplifyCall(expression: any) {
+        let context: {[name: string]: string}|undefined = undefined;
         if (expression['__symbolic'] == 'call') {
           let target = expression['expression'];
+          if (target && target.__symbolic === 'reference') {
+            context = {name: target.name};
+          }
           let targetFunction = simplify(target);
           if (targetFunction['__symbolic'] == 'function') {
             if (calling.get(targetFunction)) {
@@ -305,7 +293,13 @@ export class StaticReflector implements ReflectorReader {
           }
         }
 
-        return simplify({__symbolic: 'error', message: 'Function call not supported'});
+        if (depth === 0) {
+          // If depth is 0 we are evaluating the top level expression that is describing element
+          // decorator. In this case, it is a decorator we don't understand, such as a custom
+          // non-angular decorator, and we should just ignore it.
+          return {__symbolic: 'ignore'};
+        }
+        return simplify({__symbolic: 'error', message: 'Function call not supported', context});
       }
 
       function simplify(expression: any): any {
@@ -325,7 +319,11 @@ export class StaticReflector implements ReflectorReader {
                 continue;
               }
             }
-            result.push(simplify(item));
+            let value = simplify(item);
+            if (shouldIgnore(value)) {
+              continue;
+            }
+            result.push(value);
           }
           return result;
         }
@@ -335,7 +333,9 @@ export class StaticReflector implements ReflectorReader {
             switch (expression['__symbolic']) {
               case 'binop':
                 let left = simplify(expression['left']);
+                if (shouldIgnore(left)) return left;
                 let right = simplify(expression['right']);
+                if (shouldIgnore(right)) return right;
                 switch (expression['operator']) {
                   case '&&':
                     return left && right;
@@ -381,6 +381,7 @@ export class StaticReflector implements ReflectorReader {
                 return null;
               case 'pre':
                 let operand = simplify(expression['operand']);
+                if (shouldIgnore(operand)) return operand;
                 switch (expression['operator']) {
                   case '+':
                     return operand;
@@ -421,7 +422,7 @@ export class StaticReflector implements ReflectorReader {
                     // reference to the symbol.
                     return staticSymbol;
                   }
-                  result = simplifyInContext(staticSymbol, declarationValue);
+                  result = simplifyInContext(staticSymbol, declarationValue, depth + 1);
                 }
                 return result;
               case 'class':
@@ -440,11 +441,12 @@ export class StaticReflector implements ReflectorReader {
                 }
                 let converter = _this.conversionMap.get(staticSymbol);
                 if (converter) {
-                  let args = expression['arguments'];
+                  let args: any[] = expression['arguments'];
                   if (!args) {
                     args = [];
                   }
-                  return converter(context, args);
+                  return converter(
+                      context, args.map(arg => simplifyInContext(context, arg, depth + 1)));
                 }
 
                 // Determine if the function is one we can simplify.
@@ -472,7 +474,11 @@ export class StaticReflector implements ReflectorReader {
       }
     }
 
-    return simplifyInContext(context, value);
+    let result = simplifyInContext(context, value, 0);
+    if (shouldIgnore(result)) {
+      return undefined;
+    }
+    return result;
   }
 
   /**
@@ -527,7 +533,10 @@ function expandedMessage(error: any): string {
       }
       break;
     case 'Function call not supported':
-      return 'Function calls are not supported. Consider replacing the function or lambda with a reference to an exported function';
+      let prefix =
+          error.context && error.context.name ? `Calling function '${error.context.name}', f` : 'F';
+      return prefix +
+          'unction calls are not supported. Consider replacing the function or lambda with a reference to an exported function';
   }
   return error.message;
 }
@@ -540,7 +549,12 @@ function mapStringMap(input: {[key: string]: any}, transform: (value: any, key:
     {[key: string]: any} {
   if (!input) return {};
   var result: {[key: string]: any} = {};
-  Object.keys(input).forEach((key) => { result[key] = transform(input[key], key); });
+  Object.keys(input).forEach((key) => {
+    let value = transform(input[key], key);
+    if (!shouldIgnore(value)) {
+      result[key] = value;
+    }
+  });
   return result;
 }
 
@@ -584,3 +598,7 @@ class PopulatedScope extends BindingScope {
 function sameSymbol(a: StaticSymbol, b: StaticSymbol): boolean {
   return a === b || (a.name == b.name && a.filePath == b.filePath);
 }
+
+function shouldIgnore(value: any): boolean {
+  return value && value.__symbolic == 'ignore';
+}

modules/@angular/compiler-cli/test/static_reflector_spec.ts

@@ -3,6 +3,12 @@ import {animate, group, keyframes, sequence, state, style, transition, trigger}
 import {beforeEach, ddescribe, describe, expect, iit, it} from '@angular/core/testing/testing_internal';
 import {ListWrapper} from '@angular/facade/src/collection';
 import {isBlank} from '@angular/facade/src/lang';
+import {MetadataCollector} from '@angular/tsc-wrapped';
+import * as ts from 'typescript';
+
+
+// This matches .ts files but not .d.ts files.
+const TS_EXT = /(^.|(?!\.d)..)\.ts$/;
 
 describe('StaticReflector', () => {
   let noContext = new StaticSymbol('', '');
@@ -322,16 +328,65 @@ describe('StaticReflector', () => {
         .toThrow(new Error(
             'Recursion not supported, resolving symbol indirectRecursion in /tmp/src/function-reference.ts, resolving symbol  in /tmp/src/function-reference.ts'));
   });
+
   it('should simplify a spread expression', () => {
     expect(simplify(new StaticSymbol('/tmp/src/spread.ts', ''), {
       __symbolic: 'reference',
       name: 'spread'
     })).toEqual([0, 1, 2, 3, 4, 5]);
   });
+
+  it('should be able to get metadata from a ts file', () => {
+    let metadata = reflector.getModuleMetadata('/tmp/src/custom-decorator-reference.ts');
+    expect(metadata).toEqual({
+      __symbolic: 'module',
+      version: 1,
+      metadata: {
+        Foo: {
+          __symbolic: 'class',
+          decorators: [{
+            __symbolic: 'call',
+            expression:
+                {__symbolic: 'reference', module: './custom-decorator', name: 'CustomDecorator'}
+          }],
+          members: {
+            foo: [{
+              __symbolic: 'property',
+              decorators: [{
+                __symbolic: 'call',
+                expression: {
+                  __symbolic: 'reference',
+                  module: './custom-decorator',
+                  name: 'CustomDecorator'
+                }
+              }]
+            }]
+          }
+        }
+      }
+    });
+  });
+
+  it('should be able to get metadata for a class containing a custom decorator', () => {
+    let props = reflector.propMetadata(
+        host.getStaticSymbol('/tmp/src/custom-decorator-reference.ts', 'Foo'));
+    expect(props).toEqual({foo: []});
+  });
+
+  it('should report an error for invalid function calls', () => {
+    expect(
+        () =>
+            reflector.annotations(host.getStaticSymbol('/tmp/src/invalid-calls.ts', 'MyComponent')))
+        .toThrow(new Error(
+            `Error encountered resolving symbol values statically. Calling function 'someFunction', function calls are not supported. Consider replacing the function or lambda with a reference to an exported function, resolving symbol MyComponent in /tmp/src/invalid-calls.ts, resolving symbol MyComponent in /tmp/src/invalid-calls.ts`));
+  });
 });
 
 class MockReflectorHost implements StaticReflectorHost {
   private staticTypeCache = new Map<string, StaticSymbol>();
+  private collector = new MetadataCollector();
+
+  constructor() {}
 
   angularImportLocations() {
     return {
@@ -792,8 +847,60 @@ class MockReflectorHost implements StaticReflectorHost {
         metadata: {
           spread: [0, {__symbolic: 'spread', expression: [1, 2, 3, 4]}, 5]
         }
-      }
+      },
+      '/tmp/src/custom-decorator.ts': `
+        export function CustomDecorator(): any {
+          return () => {};
+        }
+      `,
+      '/tmp/src/custom-decorator-reference.ts': `
+        import {CustomDecorator} from './custom-decorator';
+
+        @CustomDecorator()
+        export class Foo {
+          @CustomDecorator() get foo(): string { return ''; }
+        }
+      `,
+      '/tmp/src/invalid-calll-definitions.ts': `
+        export function someFunction(a: any) {
+          if (Array.isArray(a)) {
+            return a;
+          }
+          return undefined;
+        }
+      `,
+      '/tmp/src/invalid-calls.ts': `
+        import {someFunction} from './nvalid-calll-definitions.ts';
+        import {Component} from 'angular2/src/core/metadata';
+        import {NgIf} from 'angular2/common';
+
+        @Component({
+          selector: 'my-component',
+          directives: [someFunction([NgIf])]
+        })
+        export class MyComponent {}
+
+        @someFunction()
+        @Component({
+          selector: 'my-component',
+          directives: [NgIf]
+        })
+        export class MyOtherComponent { }
+      `
     };
+
+
+    if (data[moduleId] && moduleId.match(TS_EXT)) {
+      let text = data[moduleId];
+      if (typeof text === 'string') {
+        let sf = ts.createSourceFile(moduleId, data[moduleId], ts.ScriptTarget.ES5);
+        let diagnostics: ts.Diagnostic[] = (<any>sf).parseDiagnostics;
+        if (diagnostics && diagnostics.length) {
+          throw Error(`Error encountered during parse of file ${moduleId}`);
+        }
+        return this.collector.getMetadata(sf);
+      }
+    }
     return data[moduleId];
   }
 }

tools/@angular/tsc-wrapped/src/symbols.ts

@@ -36,6 +36,15 @@ export class Symbols {
             const externalReference =
                 <ts.ExternalModuleReference>importEqualsDeclaration.moduleReference;
             // An `import <identifier> = require(<module-specifier>);
+            if (!externalReference.expression.parent) {
+              // The `parent` field of a node is set by the TypeScript binder (run as
+              // part of the type checker). Setting it here allows us to call `getText()`
+              // even if the `SourceFile` was not type checked (which looks for `SourceFile`
+              // in the parent chain). This doesn't damage the node as the binder unconditionally
+              // sets the parent.
+              externalReference.expression.parent = externalReference;
+              externalReference.parent = this.sourceFile;
+            }
             const from = stripQuotes(externalReference.expression.getText());
             symbols.set(importEqualsDeclaration.name.text, {__symbolic: 'reference', module: from});
           } else {
@@ -50,6 +59,11 @@ export class Symbols {
             // An `import <module-specifier>` clause which does not bring symbols into scope.
             break;
           }
+          if (!importDecl.moduleSpecifier.parent) {
+            // See note above in the `ImportEqualDeclaration` case.
+            importDecl.moduleSpecifier.parent = importDecl;
+            importDecl.parent = this.sourceFile;
+          }
           const from = stripQuotes(importDecl.moduleSpecifier.getText());
           if (importDecl.importClause.name) {
             // An `import <identifier> form <module-specifier>` clause. Record the defualt symbol.