diff --git a/modules/@angular/compiler-cli/src/static_reflector.ts b/modules/@angular/compiler-cli/src/static_reflector.ts index f5ec5cd615..aae9382c86 100644 --- a/modules/@angular/compiler-cli/src/static_reflector.ts +++ b/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}, diff --git a/modules/@angular/compiler-cli/test/static_reflector_spec.ts b/modules/@angular/compiler-cli/test/static_reflector_spec.ts index d1c75ba09f..42528866fe 100644 --- a/modules/@angular/compiler-cli/test/static_reflector_spec.ts +++ b/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]; } diff --git a/tools/@angular/tsc-wrapped/src/collector.ts b/tools/@angular/tsc-wrapped/src/collector.ts index 6ca076c084..83c1eda9b7 100644 --- a/tools/@angular/tsc-wrapped/src/collector.ts +++ b/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 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 [[, ; // 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 = nameNode; + const varValue = errorSym('Destructuring not supported', nameNode); locals.define(name.text, varValue); if (node.flags & ts.NodeFlags.Export) { if (!metadata) metadata = {}; diff --git a/tools/@angular/tsc-wrapped/src/evaluator.ts b/tools/@angular/tsc-wrapped/src/evaluator.ts index 28dad992a9..3c9a6b5039 100644 --- a/tools/@angular/tsc-wrapped/src/evaluator.ts +++ b/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 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 = 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 = 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 = 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); } } diff --git a/tools/@angular/tsc-wrapped/src/schema.ts b/tools/@angular/tsc-wrapped/src/schema.ts index 5aa008a9a2..e6a1fdcd8d 100644 --- a/tools/@angular/tsc-wrapped/src/schema.ts +++ b/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'; diff --git a/tools/@angular/tsc-wrapped/test/collector.spec.ts b/tools/@angular/tsc-wrapped/test/collector.spec.ts index dc7ab3d1fa..0e6655b079 100644 --- a/tools/@angular/tsc-wrapped/test/collector.spec.ts +++ b/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', - message: 'Destructuring declarations cannot be referenced statically' + __symbolic: 'error', + message: 'Destructuring declarations cannot be referenced statically', + line: 1, + character: 16 }, b: { - __symbolc: 'error', - message: 'Destructuring declarations cannot be referenced statically' + __symbolic: 'error', + message: 'Destructuring declarations cannot be referenced statically', + line: 1, + character: 18 }, c: { - __symbolc: 'error', - message: 'Destructuring declarations cannot be referenced statically' + __symbolic: 'error', + message: 'Destructuring declarations cannot be referenced statically', + line: 2, + character: 16 }, d: { - __symbolc: 'error', - message: 'Destructuring declarations cannot be referenced statically' + __symbolic: 'error', + 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 = metadata.metadata['Bar']; let ctor = barClass.members['__ctor__'][0]; let parameter = ctor.parameters[0]; - expect(parameter).toEqual( - {__symbolic: 'error', message: 'Reference to non-exported class Foo'}); + expect(parameter).toEqual({ + __symbolic: 'error', + message: 'Reference to non-exported class Foo', + line: 1, + character: 45 + }); }); }); diff --git a/tools/@angular/tsc-wrapped/test/evaluator.spec.ts b/tools/@angular/tsc-wrapped/test/evaluator.spec.ts index 76f58ac5ae..dd509f4b7c 100644 --- a/tools/@angular/tsc-wrapped/test/evaluator.spec.ts +++ b/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; + `, };