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 angular#8978
Closes angular#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,
-                {__symbolic: 'error', message: `Reference to non-exported class ${className}`});
+                className, errorSym('Reference to non-exported class', node, {className}));
           }
           break;
       }
@@ -156,10 +160,7 @@ export class MetadataCollector {
               if (variableDeclaration.initializer) {
                 varValue = evaluator.evaluateNode(variableDeclaration.initializer);
               } else {
-                varValue = {
-                  __symbolic: 'error',
-                  message: 'Only intialized variables and constants can be referenced statically'
-                };
+                varValue = errorSym('Variable not initialized', nameNode);
               }
               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 {
-        __symbolic: 'error',
-        message: `Name expected a string or an identifier but received "${node.getText()}""`
-      };
+      return errorSymbol('Name expected', node, {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';