fix(core): make decorators closure safe (#16905)

This is required as e.g. `token` from `@Inject` is accessed in string form via makeParamDecorator but as a property in the `ReflectiveInjector`. Closes #16889 as this is a more general fix.
angular · May 23, 2017 · a80ac0a · a80ac0a
1 parent 5af143e
commit a80ac0a
Show file tree

Hide file tree

Showing 9 changed files with 77 additions and 123 deletions.
diff --git a/packages/compiler/test/directive_resolver_spec.ts b/packages/compiler/test/directive_resolver_spec.ts
@@ -114,9 +114,15 @@ export function main() {
 
     it('should read out the Directive metadata', () => {
       const directiveMetadata = resolver.resolve(SomeDirective);
-      expect(directiveMetadata)
-          .toEqual(new Directive(
-              {selector: 'someDirective', inputs: [], outputs: [], host: {}, queries: {}}));
+      expect(directiveMetadata).toEqual(new Directive({
+        selector: 'someDirective',
+        inputs: [],
+        outputs: [],
+        host: {},
+        queries: {},
+        exportAs: undefined,
+        providers: undefined
+      }));
     });
 
     it('should throw if not matching metadata is found', () => {
@@ -136,11 +142,25 @@ export function main() {
       class ChildWithDecorator extends Parent {
       }
 
-      expect(resolver.resolve(ChildNoDecorator))
-          .toEqual(new Directive({selector: 'p', inputs: [], outputs: [], host: {}, queries: {}}));
-
-      expect(resolver.resolve(ChildWithDecorator))
-          .toEqual(new Directive({selector: 'c', inputs: [], outputs: [], host: {}, queries: {}}));
+      expect(resolver.resolve(ChildNoDecorator)).toEqual(new Directive({
+        selector: 'p',
+        inputs: [],
+        outputs: [],
+        host: {},
+        queries: {},
+        exportAs: undefined,
+        providers: undefined
+      }));
+
+      expect(resolver.resolve(ChildWithDecorator)).toEqual(new Directive({
+        selector: 'c',
+        inputs: [],
+        outputs: [],
+        host: {},
+        queries: {},
+        exportAs: undefined,
+        providers: undefined
+      }));
     });
 
     describe('inputs', () => {

diff --git a/packages/core/src/di/metadata.ts b/packages/core/src/di/metadata.ts
@@ -58,7 +58,7 @@ export interface Inject { token: any; }
  * @stable
  * @Annotation
  */
-export const Inject: InjectDecorator = makeParamDecorator('Inject', [['token', undefined]]);
+export const Inject: InjectDecorator = makeParamDecorator('Inject', (token: any) => ({token}));
 
 
 /**
@@ -104,7 +104,7 @@ export interface Optional {}
  * @stable
  * @Annotation
  */
-export const Optional: OptionalDecorator = makeParamDecorator('Optional', []);
+export const Optional: OptionalDecorator = makeParamDecorator('Optional');
 
 /**
  * Type of the Injectable decorator / constructor function.
@@ -151,7 +151,7 @@ export interface Injectable {}
  * @stable
  * @Annotation
  */
-export const Injectable: InjectableDecorator = <InjectableDecorator>makeDecorator('Injectable', []);
+export const Injectable: InjectableDecorator = <InjectableDecorator>makeDecorator('Injectable');
 
 /**
  * Type of the Self decorator / constructor function.
@@ -195,7 +195,7 @@ export interface Self {}
  * @stable
  * @Annotation
  */
-export const Self: SelfDecorator = makeParamDecorator('Self', []);
+export const Self: SelfDecorator = makeParamDecorator('Self');
 
 
 /**
@@ -240,7 +240,7 @@ export interface SkipSelf {}
  * @stable
  * @Annotation
  */
-export const SkipSelf: SkipSelfDecorator = makeParamDecorator('SkipSelf', []);
+export const SkipSelf: SkipSelfDecorator = makeParamDecorator('SkipSelf');
 
 /**
  * Type of the Host decorator / constructor function.
@@ -285,4 +285,4 @@ export interface Host {}
  * @stable
  * @Annotation
  */
-export const Host: HostDecorator = makeParamDecorator('Host', []);
+export const Host: HostDecorator = makeParamDecorator('Host');
diff --git a/packages/core/src/di/reflective_provider.ts b/packages/core/src/di/reflective_provider.ts
@@ -225,7 +225,7 @@ function _extractToken(
 
   if (!Array.isArray(metadata)) {
     if (metadata instanceof Inject) {
-      return _createDependency(metadata['token'], optional, null);
+      return _createDependency(metadata.token, optional, null);
     } else {
       return _createDependency(metadata, optional, null);
     }
@@ -240,7 +240,7 @@ function _extractToken(
       token = paramMetadata;
 
     } else if (paramMetadata instanceof Inject) {
-      token = paramMetadata['token'];
+      token = paramMetadata.token;
 
     } else if (paramMetadata instanceof Optional) {
       optional = true;

diff --git a/packages/core/src/metadata/di.ts b/packages/core/src/metadata/di.ts
@@ -119,7 +119,7 @@ export interface Attribute { attributeName?: string; }
  * @Annotation
  */
 export const Attribute: AttributeDecorator =
-    makeParamDecorator('Attribute', [['attributeName', undefined]]);
+    makeParamDecorator('Attribute', (attributeName?: string) => ({attributeName}));
 
 /**
  * Type of the Query metadata.
@@ -207,14 +207,8 @@ export type ContentChildren = Query;
 export const ContentChildren: ContentChildrenDecorator =
     <ContentChildrenDecorator>makePropDecorator(
         'ContentChildren',
-        [
-          ['selector', undefined], {
-            first: false,
-            isViewQuery: false,
-            descendants: false,
-            read: undefined,
-          }
-        ],
+        (selector?: any, data: any = {}) =>
+            ({selector, first: false, isViewQuery: false, descendants: false, ...data}),
         Query);
 
 /**
@@ -273,15 +267,8 @@ export type ContentChild = Query;
  * @Annotation
  */
 export const ContentChild: ContentChildDecorator = makePropDecorator(
-    'ContentChild',
-    [
-      ['selector', undefined], {
-        first: true,
-        isViewQuery: false,
-        descendants: true,
-        read: undefined,
-      }
-    ],
+    'ContentChild', (selector?: any, data: any = {}) =>
+                        ({selector, first: true, isViewQuery: false, descendants: true, ...data}),
     Query);
 
 /**
@@ -339,15 +326,8 @@ export type ViewChildren = Query;
  * @Annotation
  */
 export const ViewChildren: ViewChildrenDecorator = makePropDecorator(
-    'ViewChildren',
-    [
-      ['selector', undefined], {
-        first: false,
-        isViewQuery: true,
-        descendants: true,
-        read: undefined,
-      }
-    ],
+    'ViewChildren', (selector?: any, data: any = {}) =>
+                        ({selector, first: false, isViewQuery: true, descendants: true, ...data}),
     Query);
 
 /**
@@ -403,13 +383,6 @@ export type ViewChild = Query;
  * @Annotation
  */
 export const ViewChild: ViewChildDecorator = makePropDecorator(
-    'ViewChild',
-    [
-      ['selector', undefined], {
-        first: true,
-        isViewQuery: true,
-        descendants: true,
-        read: undefined,
-      }
-    ],
+    'ViewChild', (selector: any, data: any) =>
+                     ({selector, first: true, isViewQuery: true, descendants: true, ...data}),
     Query);
diff --git a/packages/core/src/metadata/directives.ts b/packages/core/src/metadata/directives.ts
@@ -399,15 +399,8 @@ export interface Directive {
  * @stable
  * @Annotation
  */
-export const Directive: DirectiveDecorator = <DirectiveDecorator>makeDecorator('Directive', {
-  selector: undefined,
-  inputs: undefined,
-  outputs: undefined,
-  host: undefined,
-  providers: undefined,
-  exportAs: undefined,
-  queries: undefined
-});
+export const Directive: DirectiveDecorator =
+    <DirectiveDecorator>makeDecorator('Directive', (dir: Directive = {}) => dir);
 
 /**
  * Type of the Component decorator / constructor function.
@@ -691,26 +684,7 @@ export interface Component extends Directive {
  * @Annotation
  */
 export const Component: ComponentDecorator = <ComponentDecorator>makeDecorator(
-    'Component', {
-      selector: undefined,
-      inputs: undefined,
-      outputs: undefined,
-      host: undefined,
-      exportAs: undefined,
-      moduleId: undefined,
-      providers: undefined,
-      viewProviders: undefined,
-      changeDetection: ChangeDetectionStrategy.Default,
-      queries: undefined,
-      templateUrl: undefined,
-      template: undefined,
-      styleUrls: undefined,
-      styles: undefined,
-      animations: undefined,
-      encapsulation: undefined,
-      interpolation: undefined,
-      entryComponents: undefined
-    },
+    'Component', (c: Component = {}) => ({changeDetection: ChangeDetectionStrategy.Default, ...c}),
     Directive);
 
 /**
@@ -750,10 +724,8 @@ export interface Pipe {
  * @stable
  * @Annotation
  */
-export const Pipe: PipeDecorator = <PipeDecorator>makeDecorator('Pipe', {
-  name: undefined,
-  pure: true,
-});
+export const Pipe: PipeDecorator =
+    <PipeDecorator>makeDecorator('Pipe', (p: Pipe) => ({pure: true, ...p}));
 
 
 /**
@@ -825,7 +797,7 @@ export interface Input {
  * @Annotation
  */
 export const Input: InputDecorator =
-    makePropDecorator('Input', [['bindingPropertyName', undefined]]);
+    makePropDecorator('Input', (bindingPropertyName?: string) => ({bindingPropertyName}));
 
 /**
  * Type of the Output decorator / constructor function.
@@ -891,7 +863,7 @@ export interface Output { bindingPropertyName?: string; }
  * @Annotation
  */
 export const Output: OutputDecorator =
-    makePropDecorator('Output', [['bindingPropertyName', undefined]]);
+    makePropDecorator('Output', (bindingPropertyName?: string) => ({bindingPropertyName}));
 
 
 /**
@@ -951,7 +923,7 @@ export interface HostBinding { hostPropertyName?: string; }
  * @Annotation
  */
 export const HostBinding: HostBindingDecorator =
-    makePropDecorator('HostBinding', [['hostPropertyName', undefined]]);
+    makePropDecorator('HostBinding', (hostPropertyName?: string) => ({hostPropertyName}));
 
 
 /**
@@ -1013,4 +985,4 @@ export interface HostListener {
  * @Annotation
  */
 export const HostListener: HostListenerDecorator =
-    makePropDecorator('HostListener', [['eventName', undefined], ['args', []]]);
+    makePropDecorator('HostListener', (eventName?: string, args?: string[]) => ({eventName, args}));
diff --git a/packages/core/src/metadata/ng_module.ts b/packages/core/src/metadata/ng_module.ts
@@ -190,13 +190,5 @@ export interface NgModule {
  * @stable
  * @Annotation
  */
-export const NgModule: NgModuleDecorator = <NgModuleDecorator>makeDecorator('NgModule', {
-  providers: undefined,
-  declarations: undefined,
-  imports: undefined,
-  exports: undefined,
-  entryComponents: undefined,
-  bootstrap: undefined,
-  schemas: undefined,
-  id: undefined,
-});
+export const NgModule: NgModuleDecorator =
+    <NgModuleDecorator>makeDecorator('NgModule', (ngModule: NgModule) => ngModule);
diff --git a/packages/core/src/util/decorators.ts b/packages/core/src/util/decorators.ts
@@ -262,9 +262,9 @@ export function Class(clsDef: ClassDefinition): Type<any> {
  * @suppress {globalThis}
  */
 export function makeDecorator(
-    name: string, props: {[name: string]: any}, parentClass?: any,
+    name: string, props?: (...args: any[]) => any, parentClass?: any,
     chainFn?: (fn: Function) => void): (...args: any[]) => (cls: any) => any {
-  const metaCtor = makeMetadataCtor([props]);
+  const metaCtor = makeMetadataCtor(props);
 
   function DecoratorFactory(objOrType: any): (cls: any) => any {
     if (!(Reflect && Reflect.getOwnMetadata)) {
@@ -301,25 +301,19 @@ export function makeDecorator(
   return DecoratorFactory;
 }
 
-function makeMetadataCtor(props: ([string, any] | {[key: string]: any})[]): any {
+function makeMetadataCtor(props?: (...args: any[]) => any): any {
   return function ctor(...args: any[]) {
-    props.forEach((prop, i) => {
-      const argVal = args[i];
-      if (Array.isArray(prop)) {
-        // plain parameter
-        this[prop[0]] = argVal === undefined ? prop[1] : argVal;
-      } else {
-        for (const propName in prop) {
-          this[propName] =
-              argVal && argVal.hasOwnProperty(propName) ? argVal[propName] : prop[propName];
-        }
+    if (props) {
+      const values = props(...args);
+      for (const propName in values) {
+        this[propName] = values[propName];
       }
-    });
+    }
   };
 }
 
 export function makeParamDecorator(
-    name: string, props: ([string, any] | {[name: string]: any})[], parentClass?: any): any {
+    name: string, props?: (...args: any[]) => any, parentClass?: any): any {
   const metaCtor = makeMetadataCtor(props);
   function ParamDecoratorFactory(...args: any[]): any {
     if (this instanceof ParamDecoratorFactory) {
@@ -356,7 +350,7 @@ export function makeParamDecorator(
 }
 
 export function makePropDecorator(
-    name: string, props: ([string, any] | {[key: string]: any})[], parentClass?: any): any {
+    name: string, props?: (...args: any[]) => any, parentClass?: any): any {
   const metaCtor = makeMetadataCtor(props);
 
   function PropDecoratorFactory(...args: any[]): any {

diff --git a/packages/core/test/reflection/reflector_spec.ts b/packages/core/test/reflection/reflector_spec.ts
@@ -29,10 +29,11 @@ interface PropDecorator {
 }
 
 /** @Annotation */ const ClassDecorator =
-    <ClassDecoratorFactory>makeDecorator('ClassDecorator', {value: undefined});
+    <ClassDecoratorFactory>makeDecorator('ClassDecorator', (data: any) => data);
 /** @Annotation */ const ParamDecorator =
-    makeParamDecorator('ParamDecorator', [['value', undefined]]);
-/** @Annotation */ const PropDecorator = makePropDecorator('PropDecorator', [['value', undefined]]);
+    makeParamDecorator('ParamDecorator', (value: any) => ({value}));
+/** @Annotation */ const PropDecorator =
+    makePropDecorator('PropDecorator', (value: any) => ({value}));
 
 class AType {
   constructor(public value: any) {}