From 2ccc65d7fdb4d34037422a855c668b483b0ca3fb Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Mon, 18 May 2015 17:19:54 -0700 Subject: [PATCH] fix: Improve error message on missing dependency --- .../angular2/src/core/annotations_impl/di.ts | 4 +++- .../src/core/annotations_impl/visibility.ts | 7 ++++++ modules/angular2/src/di/annotations_impl.ts | 6 ++++- modules/angular2/src/di/binding.ts | 23 +++++++++++-------- modules/angular2/src/di/exceptions.ts | 16 ++++++++++--- modules/angular2/src/di/forward_ref.ts | 3 ++- modules/angular2/src/di/type_info.dart | 4 ++++ modules/angular2/src/di/type_info.ts | 0 .../platform_reflection_capabilities.ts | 13 +++++++++++ .../angular2/src/reflection/reflection.dart | 7 +++++- .../reflection/reflection_capabilities.dart | 3 ++- .../src/reflection/reflection_capabilities.ts | 5 ++-- modules/angular2/src/reflection/reflector.ts | 10 ++++---- modules/angular2/src/reflection/types.dart | 10 -------- modules/angular2/src/reflection/types.ts | 20 +++++----------- modules/angular2/test/di/injector_spec.ts | 2 +- 16 files changed, 85 insertions(+), 48 deletions(-) create mode 100644 modules/angular2/src/di/type_info.dart create mode 100644 modules/angular2/src/di/type_info.ts create mode 100644 modules/angular2/src/reflection/platform_reflection_capabilities.ts diff --git a/modules/angular2/src/core/annotations_impl/di.ts b/modules/angular2/src/core/annotations_impl/di.ts index 9ed4933d4d..8d297ac1d3 100644 --- a/modules/angular2/src/core/annotations_impl/di.ts +++ b/modules/angular2/src/core/annotations_impl/di.ts @@ -1,4 +1,4 @@ -import {CONST} from 'angular2/src/facade/lang'; +import {CONST, stringify} from 'angular2/src/facade/lang'; import {DependencyAnnotation} from 'angular2/src/di/annotations_impl'; /** @@ -41,6 +41,7 @@ export class Attribute extends DependencyAnnotation { // account. return this; } + toString() { return `@Attribute(${stringify(this.attributeName)})`; } } /** @@ -53,4 +54,5 @@ export class Attribute extends DependencyAnnotation { @CONST() export class Query extends DependencyAnnotation { constructor(public directive: any) { super(); } + toString() { return `@Query(${stringify(this.directive)})`; } } diff --git a/modules/angular2/src/core/annotations_impl/visibility.ts b/modules/angular2/src/core/annotations_impl/visibility.ts index 324f27edf1..a8daedaaaf 100644 --- a/modules/angular2/src/core/annotations_impl/visibility.ts +++ b/modules/angular2/src/core/annotations_impl/visibility.ts @@ -9,6 +9,9 @@ export class Visibility extends DependencyAnnotation { } get includeSelf(): boolean { return isBlank(this._includeSelf) ? false : this._includeSelf; } + toString() { + return `@Visibility(depth: ${this.depth}, crossComponentBoundaries: ${this.crossComponentBoundaries}, includeSelf: ${this.includeSelf}})`; + } } /** @@ -51,6 +54,7 @@ export class Visibility extends DependencyAnnotation { @CONST() export class Self extends Visibility { constructor() { super(0, false, true); } + toString() { return `@Self()`; } } // make constants after switching to ts2dart @@ -102,6 +106,7 @@ export var self = new Self(); @CONST() export class Parent extends Visibility { constructor({self}: {self?: boolean} = {}) { super(1, false, self); } + toString() { return `@Parent(self: ${this.includeSelf}})`; } } /** @@ -164,6 +169,7 @@ export class Parent extends Visibility { @CONST() export class Ancestor extends Visibility { constructor({self}: {self?: boolean} = {}) { super(999999, false, self); } + toString() { return `@Ancestor(self: ${this.includeSelf}})`; } } /** @@ -203,4 +209,5 @@ export class Ancestor extends Visibility { @CONST() export class Unbounded extends Visibility { constructor({self}: {self?: boolean} = {}) { super(999999, true, self); } + toString() { return `@Unbounded(self: ${this.includeSelf}})`; } } diff --git a/modules/angular2/src/di/annotations_impl.ts b/modules/angular2/src/di/annotations_impl.ts index 44aaf6ff96..566e94a27f 100644 --- a/modules/angular2/src/di/annotations_impl.ts +++ b/modules/angular2/src/di/annotations_impl.ts @@ -1,4 +1,4 @@ -import {CONST} from "angular2/src/facade/lang"; +import {CONST, stringify} from "angular2/src/facade/lang"; /** * A parameter annotation that specifies a dependency. @@ -15,6 +15,7 @@ import {CONST} from "angular2/src/facade/lang"; @CONST() export class Inject { constructor(public token) {} + toString() { return `@Inject(${stringify(this.token)})`; } } /** @@ -33,6 +34,7 @@ export class Inject { @CONST() export class InjectPromise { constructor(public token) {} + toString() { return `@InjectPromise(${stringify(this.token)})`; } } /** @@ -51,6 +53,7 @@ export class InjectPromise { @CONST() export class InjectLazy { constructor(public token) {} + toString() { return `@InjectLazy(${stringify(this.token)})`; } } /** @@ -69,6 +72,7 @@ export class InjectLazy { */ @CONST() export class Optional { + toString() { return `@Optional()`; } } /** diff --git a/modules/angular2/src/di/binding.ts b/modules/angular2/src/di/binding.ts index 32f5d5376e..2d5e7db41e 100644 --- a/modules/angular2/src/di/binding.ts +++ b/modules/angular2/src/di/binding.ts @@ -437,22 +437,27 @@ export class BindingBuilder { } } -function _constructDependencies(factoryFunction: Function, dependencies: List) { - return isBlank(dependencies) ? - _dependenciesFor(factoryFunction) : - ListWrapper.map(dependencies, (t) => _extractToken(factoryFunction, t)); +function _constructDependencies(factoryFunction: Function, + dependencies: List): List { + if (isBlank(dependencies)) { + return _dependenciesFor(factoryFunction); + } else { + var params: List> = ListWrapper.map(dependencies, (t) => [t]); + return ListWrapper.map(dependencies, (t) => _extractToken(factoryFunction, t, params)); + } } -function _dependenciesFor(typeOrFunc): List { +function _dependenciesFor(typeOrFunc): List { var params = reflector.parameters(typeOrFunc); if (isBlank(params)) return []; if (ListWrapper.any(params, (p) => isBlank(p))) { - throw new NoAnnotationError(typeOrFunc); + throw new NoAnnotationError(typeOrFunc, params); } - return ListWrapper.map(params, (p) => _extractToken(typeOrFunc, p)); + return ListWrapper.map(params, (p: List) => _extractToken(typeOrFunc, p, params)); } -function _extractToken(typeOrFunc, annotations) { +function _extractToken(typeOrFunc, annotations /*List | any*/, + params: List>): Dependency { var depProps = []; var token = null; var optional = false; @@ -496,7 +501,7 @@ function _extractToken(typeOrFunc, annotations) { if (isPresent(token)) { return _createDependency(token, asPromise, lazy, optional, depProps); } else { - throw new NoAnnotationError(typeOrFunc); + throw new NoAnnotationError(typeOrFunc, params); } } diff --git a/modules/angular2/src/di/exceptions.ts b/modules/angular2/src/di/exceptions.ts index d6ed807651..5b2098eb3e 100644 --- a/modules/angular2/src/di/exceptions.ts +++ b/modules/angular2/src/di/exceptions.ts @@ -1,5 +1,5 @@ import {ListWrapper, List} from 'angular2/src/facade/collection'; -import {stringify, BaseException} from 'angular2/src/facade/lang'; +import {stringify, BaseException, isBlank} from 'angular2/src/facade/lang'; function findFirstClosedCycle(keys: List): List { var res = []; @@ -179,9 +179,19 @@ export class InvalidBindingError extends BaseException { export class NoAnnotationError extends BaseException { name: string; message: string; - constructor(typeOrFunc) { + constructor(typeOrFunc, params: List>) { super(); - this.message = "Cannot resolve all parameters for " + stringify(typeOrFunc) + ". " + + var signature = ListWrapper.create(); + for (var i = 0, ii = params.length; i < ii; i++) { + var parameter = params[i]; + if (isBlank(parameter) || parameter.length == 0) { + ListWrapper.push(signature, '?'); + } else { + ListWrapper.push(signature, ListWrapper.map(parameter, stringify).join(' ')); + } + } + this.message = "Cannot resolve all parameters for " + stringify(typeOrFunc) + "(" + + signature.join(', ') + "). " + 'Make sure they all have valid type or annotations.'; } diff --git a/modules/angular2/src/di/forward_ref.ts b/modules/angular2/src/di/forward_ref.ts index 9e103d4899..b94faa1c91 100644 --- a/modules/angular2/src/di/forward_ref.ts +++ b/modules/angular2/src/di/forward_ref.ts @@ -1,4 +1,4 @@ -import {Type} from 'angular2/src/facade/lang'; +import {Type, stringify} from 'angular2/src/facade/lang'; export interface ForwardRefFn { (): any; } @@ -30,6 +30,7 @@ export interface ForwardRefFn { (): any; } */ export function forwardRef(forwardRefFn: ForwardRefFn): Type { (forwardRefFn).__forward_ref__ = forwardRef; + (forwardRefFn).toString = function() { return stringify(this()); }; return (forwardRefFn); } diff --git a/modules/angular2/src/di/type_info.dart b/modules/angular2/src/di/type_info.dart new file mode 100644 index 0000000000..10d02c4a23 --- /dev/null +++ b/modules/angular2/src/di/type_info.dart @@ -0,0 +1,4 @@ +library angular2.di.type_info; + +// In dart always return empty, as we can get the co +argsLength(Type type) => 0; diff --git a/modules/angular2/src/di/type_info.ts b/modules/angular2/src/di/type_info.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/modules/angular2/src/reflection/platform_reflection_capabilities.ts b/modules/angular2/src/reflection/platform_reflection_capabilities.ts new file mode 100644 index 0000000000..58830d5d8b --- /dev/null +++ b/modules/angular2/src/reflection/platform_reflection_capabilities.ts @@ -0,0 +1,13 @@ +import {Type} from 'angular2/src/facade/lang'; +import {GetterFn, SetterFn, MethodFn} from './types'; +import {List} from 'angular2/src/facade/collection'; + +export interface PlatformReflectionCapabilities { + factory(type: Type): Function; + interfaces(type: Type): List; + parameters(type: Type): List>; + annotations(type: Type): List; + getter(name: string): GetterFn; + setter(name: string): SetterFn; + method(name: string): MethodFn; +} diff --git a/modules/angular2/src/reflection/reflection.dart b/modules/angular2/src/reflection/reflection.dart index 8c81ee3671..8c8849b34e 100644 --- a/modules/angular2/src/reflection/reflection.dart +++ b/modules/angular2/src/reflection/reflection.dart @@ -3,13 +3,18 @@ library reflection.reflection; import 'reflector.dart'; import 'types.dart'; export 'reflector.dart'; +import 'platform_reflection_capabilities.dart'; import 'package:angular2/src/facade/lang.dart'; -class NoReflectionCapabilities implements IReflectionCapabilities { +class NoReflectionCapabilities implements PlatformReflectionCapabilities { Function factory(Type type) { throw "Cannot find reflection information on ${stringify(type)}"; } + List interfaces(Type type) { + throw "Cannot find reflection information on ${stringify(type)}"; + } + List parameters(Type type) { throw "Cannot find reflection information on ${stringify(type)}"; } diff --git a/modules/angular2/src/reflection/reflection_capabilities.dart b/modules/angular2/src/reflection/reflection_capabilities.dart index 7e28114042..14a51da509 100644 --- a/modules/angular2/src/reflection/reflection_capabilities.dart +++ b/modules/angular2/src/reflection/reflection_capabilities.dart @@ -3,8 +3,9 @@ library reflection.reflection_capabilities; import 'package:angular2/src/facade/lang.dart'; import 'types.dart'; import 'dart:mirrors'; +import 'platform_reflection_capabilities.dart'; -class ReflectionCapabilities implements IReflectionCapabilities { +class ReflectionCapabilities implements PlatformReflectionCapabilities { ReflectionCapabilities([metadataReader]) {} Function factory(Type type) { diff --git a/modules/angular2/src/reflection/reflection_capabilities.ts b/modules/angular2/src/reflection/reflection_capabilities.ts index ecbe2cf69d..25f7abf84a 100644 --- a/modules/angular2/src/reflection/reflection_capabilities.ts +++ b/modules/angular2/src/reflection/reflection_capabilities.ts @@ -1,8 +1,9 @@ import {Type, isPresent, global, stringify, BaseException} from 'angular2/src/facade/lang'; import {List, ListWrapper} from 'angular2/src/facade/collection'; -import {GetterFn, SetterFn, MethodFn, IReflectionCapabilities} from './types'; +import {GetterFn, SetterFn, MethodFn} from './types'; +import {PlatformReflectionCapabilities} from 'platform_reflection_capabilities'; -export class ReflectionCapabilities implements IReflectionCapabilities { +export class ReflectionCapabilities implements PlatformReflectionCapabilities { private _reflect: any; constructor(reflect?: any) { this._reflect = isPresent(reflect) ? reflect : global.Reflect; } diff --git a/modules/angular2/src/reflection/reflector.ts b/modules/angular2/src/reflection/reflector.ts index 05c2fe6c9f..5582ece148 100644 --- a/modules/angular2/src/reflection/reflector.ts +++ b/modules/angular2/src/reflection/reflector.ts @@ -7,17 +7,19 @@ import { StringMap, StringMapWrapper } from 'angular2/src/facade/collection'; -import {SetterFn, GetterFn, MethodFn, IReflectionCapabilities} from './types'; -export {SetterFn, GetterFn, MethodFn, IReflectionCapabilities} from './types'; +import {SetterFn, GetterFn, MethodFn} from './types'; +import {PlatformReflectionCapabilities} from './platform_reflection_capabilities'; +export {SetterFn, GetterFn, MethodFn} from './types'; +export {PlatformReflectionCapabilities} from './platform_reflection_capabilities'; export class Reflector { _typeInfo: Map; _getters: Map; _setters: Map; _methods: Map; - reflectionCapabilities: IReflectionCapabilities; + reflectionCapabilities: PlatformReflectionCapabilities; - constructor(reflectionCapabilities: IReflectionCapabilities) { + constructor(reflectionCapabilities: PlatformReflectionCapabilities) { this._typeInfo = MapWrapper.create(); this._getters = MapWrapper.create(); this._setters = MapWrapper.create(); diff --git a/modules/angular2/src/reflection/types.dart b/modules/angular2/src/reflection/types.dart index 1e0b46b765..d1bc7fc9a6 100644 --- a/modules/angular2/src/reflection/types.dart +++ b/modules/angular2/src/reflection/types.dart @@ -3,13 +3,3 @@ library reflection.types; typedef SetterFn(Object obj, value); typedef GetterFn(Object obj); typedef MethodFn(Object obj, List args); - -abstract class IReflectionCapabilities { - Function factory(Type type); - List> parameters(Type type); - List interfaces(Type type); - List annotations(Type type); - GetterFn getter(String name); - SetterFn setter(String name); - MethodFn method(String name); -} diff --git a/modules/angular2/src/reflection/types.ts b/modules/angular2/src/reflection/types.ts index 88c0fde461..f416eda1b3 100644 --- a/modules/angular2/src/reflection/types.ts +++ b/modules/angular2/src/reflection/types.ts @@ -1,21 +1,13 @@ import {Type} from 'angular2/src/facade/lang'; import {List} from 'angular2/src/facade/collection'; -// HACK: workaround for Traceur behavior. -// It expects all transpiled modules to contain this marker. -// TODO: remove this when we no longer use traceur -export var __esModule = true; +export {Function as GetterFn}; +export {Function as SetterFn}; +export {Function as MethodFn}; +// TODO replace once dgeni is fixed +/** export type SetterFn = (obj: any, value: any) => void; export type GetterFn = (obj: any) => any; export type MethodFn = (obj: any, args: List) => any; - -export interface IReflectionCapabilities { - factory(type: Type): Function; - parameters(type: Type): List>; - interfaces(type: Type): List; - annotations(type: Type): List; - getter(name: string): GetterFn; - setter(name: string): SetterFn; - method(name: string): MethodFn; -} +**/ diff --git a/modules/angular2/test/di/injector_spec.ts b/modules/angular2/test/di/injector_spec.ts index 2b6574a866..b1de28cf3f 100644 --- a/modules/angular2/test/di/injector_spec.ts +++ b/modules/angular2/test/di/injector_spec.ts @@ -106,7 +106,7 @@ export function main() { it('should throw when no type and not @Inject', () => { expect(() => Injector.resolveAndCreate([NoAnnotations])) - .toThrowError('Cannot resolve all parameters for NoAnnotations. ' + + .toThrowError('Cannot resolve all parameters for NoAnnotations(?). ' + 'Make sure they all have valid type or annotations.'); });