fix(compiler): Improved error reporting of the static reflector.

StaticReflector provides more context on errors reported by the
collector.

The metadata collector now records the line and character of the node that
caused it to report the error.

Includes other minor fixes to error reporting and a wording change.

Fixes #8978
Closes #9011

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

@@ -30,7 +30,7 @@ import {
   keyframes
 } from "@angular/core";
 import {ReflectorReader} from "./core_private";
- 
+
 const SUPPORTED_SCHEMA_VERSION = 1;
 
 /**
@@ -390,7 +390,11 @@ export class StaticReflector implements ReflectorReader {
                 return context;
               }
              case "error":
-              throw new Error(expression['message']);
+              let message = produceErrorMessage(expression);
+              if (expression['line']) {
+                message = `${message} (position ${expression['line']}:${expression['character']} in the original .ts file)`;
+              }
+              throw new Error(message);
           }
           return null;
         }
@@ -399,7 +403,11 @@ export class StaticReflector implements ReflectorReader {
       return null;
     }
 
-    return simplify(value);
+    try {
+      return simplify(value);
+    } catch(e) {
+      throw new Error(`${e.message}, resolving symbol ${context.name} in ${context.filePath}`);
+    }
   }
 
   /**
@@ -431,6 +439,33 @@ export class StaticReflector implements ReflectorReader {
     }
     return result;
   }
+
+}
+
+function expandedMessage(error: any): string {
+  switch (error.message) {
+    case 'Reference to non-exported class':
+      if (error.context && error.context.className) {
+        return `Reference to a non-exported class ${error.context.className}`;
+      }
+      break;
+    case 'Variable not initialized':
+      return 'Only initialized variables and constants can be referenced';
+    case 'Destructuring not supported':
+      return 'Referencing an exported destructured variable or constant is not supported';
+    case 'Could not resolve type':
+      if (error.context && error.context.typeName) {
+        return `Could not resolve type ${error.context.typeName}`;
+      }
+      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';
+  }
+  return error.message;
+}
+
+function produceErrorMessage(error: any): string {
+  return `Error encountered resolving symbol values statically. ${expandedMessage(error)}`;
 }
 
 function mapStringMap(input: {[key: string]: any},

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

@@ -109,6 +109,11 @@ describe('StaticReflector', () => {
     expect(parameters).toEqual([]);
   });
 
+  it('should provide context for errors reported by the collector', () => {
+    let SomeClass = host.findDeclaration('src/error-reporting', 'SomeClass');
+    expect(() => reflector.annotations(SomeClass)).toThrow(new Error('Error encountered resolving symbol values statically. A reasonable error message (position 12:33 in the original .ts file), resolving symbol ErrorSym in /tmp/src/error-references.d.ts, resolving symbol Link2 in /tmp/src/error-references.d.ts, resolving symbol Link1 in /tmp/src/error-references.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts, resolving symbol SomeClass in /tmp/src/error-reporting.d.ts'));
+  });
+
   it('should simplify primitive into itself', () => {
     expect(simplify(noContext, 1)).toBe(1);
     expect(simplify(noContext, true)).toBe(true);
@@ -537,6 +542,58 @@ class MockReflectorHost implements StaticReflectorHost {
       },
       '/src/extern.d.ts': {"__symbolic": "module", "version": 1, metadata: {s: "s"}},
       '/tmp/src/version-error.d.ts': {"__symbolic": "module", "version": 100, metadata: {e: "s"}},
+      '/tmp/src/error-reporting.d.ts': {
+        __symbolic: "module",
+        version: 1,
+        metadata: {
+          SomeClass: {
+            __symbolic: "class",
+            decorators: [
+              {
+                __symbolic: "call",
+                expression: {
+                  __symbolic: "reference",
+                  name: "Component",
+                  module: "angular2/src/core/metadata"
+                },
+                arguments: [
+                  {
+                    directives: [
+                      {
+                        __symbolic: "reference",
+                        module: "src/error-references",
+                        name: "Link1",
+                      }
+                    ]
+                  }
+                ]
+              }
+            ],
+          }
+        }
+      },
+      '/tmp/src/error-references.d.ts': {
+        __symbolic: "module",
+        version: 1,
+        metadata: {
+          Link1: {
+            __symbolic: "reference",
+            module: "src/error-references",
+            name: "Link2"
+          },
+          Link2: {
+            __symbolic: "reference",
+            module: "src/error-references",
+            name: "ErrorSym"
+          },
+          ErrorSym: {
+            __symbolic: "error",
+            message: "A reasonable error message",
+            line: 12,
+            character: 33
+          }
+        }
+      }
     };
     return data[moduleId];
   }

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

@@ -1,6 +1,6 @@
 import * as ts from 'typescript';
 
-import {Evaluator, ImportMetadata, ImportSpecifierMetadata, isPrimitive} from './evaluator';
+import {Evaluator, ImportMetadata, ImportSpecifierMetadata, errorSymbol, isPrimitive} from './evaluator';
 import {ClassMetadata, ConstructorMetadata, ModuleMetadata, MemberMetadata, MetadataError, MetadataMap, MetadataSymbolicExpression, MetadataSymbolicReferenceExpression, MetadataValue, MethodMetadata, isMetadataError, isMetadataSymbolicReferenceExpression, VERSION} from './schema';
 import {Symbols} from './symbols';
 
@@ -23,6 +23,11 @@ export class MetadataCollector {
       return <MetadataSymbolicExpression>evaluator.evaluateNode(decoratorNode.expression);
     }
 
+    function errorSym(
+        message: string, node?: ts.Node, context?: {[name: string]: string}): MetadataError {
+      return errorSymbol(message, node, context, sourceFile);
+    }
+
     function classMetadataOf(classDeclaration: ts.ClassDeclaration): ClassMetadata {
       let result: ClassMetadata = {__symbolic: 'class'};
 
@@ -37,7 +42,7 @@ export class MetadataCollector {
         if (isMetadataError(result) || isMetadataSymbolicReferenceExpression(result)) {
           return result;
         } else {
-          return {__symbolic: 'error', message: 'Symbol reference expected'};
+          return errorSym('Symbol reference expected', node);
         }
       }
 
@@ -128,8 +133,7 @@ export class MetadataCollector {
             locals.define(className, {__symbolic: 'reference', name: className});
           } else {
             locals.define(
-                className,
+                className, errorSym('Reference to non-exported class', node, {className}));
-                {__symbolic: 'error', message: `Reference to non-exported class ${className}`});
           }
           break;
       }
@@ -156,10 +160,7 @@ export class MetadataCollector {
               if (variableDeclaration.initializer) {
                 varValue = evaluator.evaluateNode(variableDeclaration.initializer);
               } else {
-                varValue = {
+                varValue = errorSym('Variable not initialized', nameNode);
-                  __symbolic: 'error',
-                  message: 'Only intialized variables and constants can be referenced statically'
-                };
               }
               if (variableStatement.flags & ts.NodeFlags.Export ||
                   variableDeclaration.flags & ts.NodeFlags.Export) {
@@ -175,14 +176,11 @@ export class MetadataCollector {
               //   or
               // var [<identifier>[, <identifier}+] = <expression>;
               // are not supported.
-              let varValue = {
-                __symbolc: 'error',
-                message: 'Destructuring declarations cannot be referenced statically'
-              };
               const report = (nameNode: ts.Node) => {
                 switch (nameNode.kind) {
                   case ts.SyntaxKind.Identifier:
                     const name = <ts.Identifier>nameNode;
+                    const varValue = errorSym('Destructuring not supported', nameNode);
                     locals.define(name.text, varValue);
                     if (node.flags & ts.NodeFlags.Export) {
                       if (!metadata) metadata = {};

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

@@ -54,6 +54,28 @@ export interface ImportMetadata {
   from: string;                              // from 'place'
 }
 
+
+function getSourceFileOfNode(node: ts.Node): ts.SourceFile {
+  while (node && node.kind != ts.SyntaxKind.SourceFile) {
+    node = node.parent
+  }
+  return <ts.SourceFile>node;
+}
+
+/* @internal */
+export function errorSymbol(
+    message: string, node?: ts.Node, context?: {[name: string]: string},
+    sourceFile?: ts.SourceFile): MetadataError {
+  if (node) {
+    sourceFile = sourceFile || getSourceFileOfNode(node);
+    if (sourceFile) {
+      let {line, character} = ts.getLineAndCharacterOfPosition(sourceFile, node.pos);
+      return {__symbolic: 'error', message, line, character, context};
+    };
+  }
+  return {__symbolic: 'error', message, context};
+}
+
 /**
  * Produce a symbolic representation of an expression folding values into their final value when
  * possible.
@@ -69,10 +91,7 @@ export class Evaluator {
     if (isMetadataError(result) || typeof result === 'string') {
       return result;
     } else {
-      return {
+      return errorSymbol('Name expected', node, {received: node.getText()});
-        __symbolic: 'error',
-        message: `Name expected a string or an identifier but received "${node.getText()}""`
-      };
     }
   }
 
@@ -185,7 +204,8 @@ export class Evaluator {
               const assignment = <ts.PropertyAssignment>child;
               const propertyName = this.nameOf(assignment.name);
               if (isMetadataError(propertyName)) {
-                return propertyName;
+                error = propertyName;
+                return true;
               }
               const propertyValue = this.evaluateNode(assignment.initializer);
               if (isMetadataError(propertyValue)) {
@@ -306,13 +326,13 @@ export class Evaluator {
         const typeReferenceNode = <ts.TypeReferenceNode>node;
         const typeNameNode = typeReferenceNode.typeName;
         if (typeNameNode.kind != ts.SyntaxKind.Identifier) {
-          return { __symbolic: 'error', message: 'Qualified type names not supported' }
+          return errorSymbol('Qualified type names not supported', node);
         }
         const typeNameIdentifier = <ts.Identifier>typeReferenceNode.typeName;
         const typeName = typeNameIdentifier.text;
         const typeReference = this.symbols.resolve(typeName);
         if (!typeReference) {
-          return {__symbolic: 'error', message: `Could not resolve type ${typeName}`};
+          return errorSymbol('Could not resolve type', node, {typeName});
         }
         if (typeReferenceNode.typeArguments && typeReferenceNode.typeArguments.length) {
           const args = typeReferenceNode.typeArguments.map(element => this.evaluateNode(element));
@@ -452,7 +472,10 @@ export class Evaluator {
           };
         }
         break;
+      case ts.SyntaxKind.FunctionExpression:
+      case ts.SyntaxKind.ArrowFunction:
+        return errorSymbol('Function call not supported', node);
     }
-    return {__symbolic: 'error', message: 'Expression is too complex to resolve statically'};
+    return errorSymbol('Expression form not supported', node);
   }
 }

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

@@ -191,7 +191,30 @@ export function isMetadataSymbolicSelectExpression(value: any):
 
 export interface MetadataError {
   __symbolic: 'error';
+
+  /**
+   * This message should be short and relatively discriptive and should be fixed once it is created.
+   * If the reader doesn't recognize the message, it will display the message unmodified. If the
+   * reader recognizes the error message is it free to use substitute message the is more
+   * descriptive and/or localized.
+   */
   message: string;
+
+  /**
+   * The line number of the error in the .ts file the metadata was created for.
+   */
+  line?: number;
+
+  /**
+   * The number of utf8 code-units from the beginning of the file of the error.
+   */
+  character?: number;
+
+  /**
+   * Context information that can be used to generate a more descriptive error message. The content
+   * of the context is dependent on the error message.
+   */
+  context?: {[name: string]: string};
 }
 export function isMetadataError(value: any): value is MetadataError {
   return value && value.__symbolic === 'error';

tools/@angular/tsc-wrapped/test/collector.spec.ts

@@ -33,6 +33,7 @@ describe('Collector', () => {
     const metadata = collector.getMetadata(sourceFile);
     expect(metadata).toEqual({
       __symbolic: 'module',
+      version: 1,
       metadata: {
         HeroDetailComponent: {
           __symbolic: 'class',
@@ -73,6 +74,7 @@ describe('Collector', () => {
     const metadata = collector.getMetadata(sourceFile);
     expect(metadata).toEqual({
       __symbolic: 'module',
+      version: 1,
       metadata: {
         AppComponent: {
           __symbolic: 'class',
@@ -126,6 +128,7 @@ describe('Collector', () => {
     const metadata = collector.getMetadata(sourceFile);
     expect(metadata).toEqual({
       __symbolic: 'module',
+      version: 1,
       metadata: {
         HEROES: [
           {'id': 11, 'name': 'Mr. Nice'}, {'id': 12, 'name': 'Narco'},
@@ -206,26 +209,37 @@ describe('Collector', () => {
     let metadata = collector.getMetadata(unsupported1);
     expect(metadata).toEqual({
       __symbolic: 'module',
+      version: 1,
       metadata: {
         a: {
-          __symbolc: 'error',
+          __symbolic: 'error',
-          message: 'Destructuring declarations cannot be referenced statically'
+          message: 'Destructuring declarations cannot be referenced statically',
+          line: 1,
+          character: 16
         },
         b: {
-          __symbolc: 'error',
+          __symbolic: 'error',
-          message: 'Destructuring declarations cannot be referenced statically'
+          message: 'Destructuring declarations cannot be referenced statically',
+          line: 1,
+          character: 18
         },
         c: {
-          __symbolc: 'error',
+          __symbolic: 'error',
-          message: 'Destructuring declarations cannot be referenced statically'
+          message: 'Destructuring declarations cannot be referenced statically',
+          line: 2,
+          character: 16
         },
         d: {
-          __symbolc: 'error',
+          __symbolic: 'error',
-          message: 'Destructuring declarations cannot be referenced statically'
+          message: 'Destructuring declarations cannot be referenced statically',
+          line: 2,
+          character: 18
         },
         e: {
           __symbolic: 'error',
-          message: 'Only intialized variables and constants can be referenced statically'
+          message: 'Only intialized variables and constants can be referenced statically',
+          line: 3,
+          character: 14
         }
       }
     });
@@ -237,8 +251,12 @@ describe('Collector', () => {
     let barClass = <ClassMetadata>metadata.metadata['Bar'];
     let ctor = <ConstructorMetadata>barClass.members['__ctor__'][0];
     let parameter = ctor.parameters[0];
-    expect(parameter).toEqual(
+    expect(parameter).toEqual({
-        {__symbolic: 'error', message: 'Reference to non-exported class Foo'});
+      __symbolic: 'error',
+      message: 'Reference to non-exported class Foo',
+      line: 1,
+      character: 45
+    });
   });
 });
 

tools/@angular/tsc-wrapped/test/evaluator.spec.ts

@@ -7,6 +7,7 @@ import {Symbols} from '../src/symbols';
 import {Directory, Host, expectNoDiagnostics, findVar} from './typescript.mocks';
 
 describe('Evaluator', () => {
+  let documentRegistry = ts.createDocumentRegistry();
   let host: ts.LanguageServiceHost;
   let service: ts.LanguageService;
   let program: ts.Program;
@@ -17,9 +18,9 @@ describe('Evaluator', () => {
   beforeEach(() => {
     host = new Host(FILES, [
       'expressions.ts', 'consts.ts', 'const_expr.ts', 'forwardRef.ts', 'classes.ts',
-      'newExpression.ts'
+      'newExpression.ts', 'errors.ts'
     ]);
-    service = ts.createLanguageService(host);
+    service = ts.createLanguageService(host, documentRegistry);
     program = service.getProgram();
     typeChecker = program.getTypeChecker();
     symbols = new Symbols(null);
@@ -30,7 +31,10 @@ describe('Evaluator', () => {
     expectNoDiagnostics(service.getCompilerOptionsDiagnostics());
     for (const sourceFile of program.getSourceFiles()) {
       expectNoDiagnostics(service.getSyntacticDiagnostics(sourceFile.fileName));
-      expectNoDiagnostics(service.getSemanticDiagnostics(sourceFile.fileName));
+      if (sourceFile.fileName != 'errors.ts') {
+        // Skip errors.ts because we it has intentional semantic errors that we are testing for.
+        expectNoDiagnostics(service.getSemanticDiagnostics(sourceFile.fileName));
+      }
     }
   });
 
@@ -141,6 +145,46 @@ describe('Evaluator', () => {
       arguments: ['name', 12]
     });
   });
+
+  it('should return errors for unsupported expressions', () => {
+    let errors = program.getSourceFile('errors.ts');
+    let aDecl = findVar(errors, 'a');
+    expect(evaluator.evaluateNode(aDecl.type)).toEqual({
+      __symbolic: 'error',
+      message: 'Qualified type names not supported',
+      line: 5,
+      character: 10
+    });
+    let fDecl = findVar(errors, 'f');
+    expect(evaluator.evaluateNode(fDecl.initializer)).toEqual({
+      __symbolic: 'error',
+      message:
+          'Functions cannot be evaluated statically; consider replacing with a reference to an exported function',
+      line: 6,
+      character: 11
+    });
+    let eDecl = findVar(errors, 'e');
+    expect(evaluator.evaluateNode(eDecl.type)).toEqual({
+      __symbolic: 'error',
+      message: 'Could not resolve type NotFound',
+      line: 7,
+      character: 10
+    });
+    let sDecl = findVar(errors, 's');
+    expect(evaluator.evaluateNode(sDecl.initializer)).toEqual({
+      __symbolic: 'error',
+      message: 'Name expected a string or an identifier but received "1"',
+      line: 8,
+      character: 13
+    });
+    let tDecl = findVar(errors, 't');
+    expect(evaluator.evaluateNode(tDecl.initializer)).toEqual({
+      __symbolic: 'error',
+      message: 'Expression form not supported statically',
+      line: 9,
+      character: 11
+    });
+  });
 });
 
 const FILES: Directory = {
@@ -193,7 +237,7 @@ const FILES: Directory = {
     export var bShiftLeft = 1 << 2;              // 0x04
     export var bShiftRight = -1 >> 2;            // -1
     export var bShiftRightU = -1 >>> 2;          // 0x3fffffff
-    
+
     export var recursiveA = recursiveB;
     export var recursiveB = recursiveA;
   `,
@@ -224,5 +268,16 @@ const FILES: Directory = {
     function forwardRef(value: any) { return value; }
     export const someValue = new Value("name", 12);
     export const complex = CONST_EXPR(new Value("name", forwardRef(() => 12)));
-  `
+  `,
+  'errors.ts': `
+    declare namespace Foo {
+      type A = string;
+    }
+
+    let a: Foo.A = 'some value';
+    let f = () => 1;
+    let e: NotFound;
+    let s = { 1: 1, 2: 2 };
+    let t = typeof 12;
+  `,
 };