From d1a3e3aff189e2635a25061b6caf8c9d5d283661 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Mon, 11 Jul 2016 16:01:49 -0700 Subject: [PATCH] fix(DirectiveResolver): throw on duplicate Input & Output names --- .../compiler/src/directive_resolver.ts | 46 ++++++++++++++----- .../compiler/test/directive_resolver_spec.ts | 29 +++++++++++- 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/modules/@angular/compiler/src/directive_resolver.ts b/modules/@angular/compiler/src/directive_resolver.ts index 4b63043546..283d6ac5b4 100644 --- a/modules/@angular/compiler/src/directive_resolver.ts +++ b/modules/@angular/compiler/src/directive_resolver.ts @@ -7,11 +7,13 @@ */ import {ComponentMetadata, DirectiveMetadata, HostBindingMetadata, HostListenerMetadata, Injectable, InputMetadata, OutputMetadata, QueryMetadata, resolveForwardRef} from '@angular/core'; -import {ReflectorReader, reflector} from '../core_private'; -import {ListWrapper, StringMapWrapper} from '../src/facade/collection'; -import {BaseException} from '../src/facade/exceptions'; -import {Type, isPresent, stringify} from '../src/facade/lang'; +import {ReflectorReader, reflector} from '../core_private'; + +import {StringMapWrapper} from './facade/collection'; +import {BaseException} from './facade/exceptions'; +import {Type, isPresent, stringify} from './facade/lang'; +import {splitAtColon} from './util'; function _isDirectiveMetadata(type: any): type is DirectiveMetadata { return type instanceof DirectiveMetadata; @@ -91,20 +93,42 @@ export class DirectiveResolver { return this._merge(dm, inputs, outputs, host, queries, directiveType); } + private _extractPublicName(def: string) { return splitAtColon(def, [null, def])[1].trim(); } + private _merge( dm: DirectiveMetadata, inputs: string[], outputs: string[], host: {[key: string]: string}, queries: {[key: string]: any}, directiveType: Type): DirectiveMetadata { - var mergedInputs = isPresent(dm.inputs) ? ListWrapper.concat(dm.inputs, inputs) : inputs; + let mergedInputs: string[]; - var mergedOutputs: string[]; - if (isPresent(dm.outputs)) { - dm.outputs.forEach((propName: string) => { - if (ListWrapper.contains(outputs, propName)) { + if (isPresent(dm.inputs)) { + const inputNames: string[] = + dm.inputs.map((def: string): string => this._extractPublicName(def)); + inputs.forEach((inputDef: string) => { + const publicName = this._extractPublicName(inputDef); + if (inputNames.indexOf(publicName) > -1) { throw new BaseException( - `Output event '${propName}' defined multiple times in '${stringify(directiveType)}'`); + `Input '${publicName}' defined multiple times in '${stringify(directiveType)}'`); } }); - mergedOutputs = ListWrapper.concat(dm.outputs, outputs); + mergedInputs = dm.inputs.concat(inputs); + } else { + mergedInputs = inputs; + } + + let mergedOutputs: string[]; + + if (isPresent(dm.outputs)) { + const outputNames: string[] = + dm.outputs.map((def: string): string => this._extractPublicName(def)); + + outputs.forEach((outputDef: string) => { + const publicName = this._extractPublicName(outputDef); + if (outputNames.indexOf(publicName) > -1) { + throw new BaseException( + `Output event '${publicName}' defined multiple times in '${stringify(directiveType)}'`); + } + }); + mergedOutputs = dm.outputs.concat(outputs); } else { mergedOutputs = outputs; } diff --git a/modules/@angular/compiler/test/directive_resolver_spec.ts b/modules/@angular/compiler/test/directive_resolver_spec.ts index 6179b6d582..f885756a2c 100644 --- a/modules/@angular/compiler/test/directive_resolver_spec.ts +++ b/modules/@angular/compiler/test/directive_resolver_spec.ts @@ -31,7 +31,6 @@ class SomeDirectiveWithOutputs { c: any; } - @Directive({selector: 'someDirective', outputs: ['a']}) class SomeDirectiveWithDuplicateOutputs { @Output() a: any; @@ -43,6 +42,17 @@ class SomeDirectiveWithDuplicateRenamedOutputs { localA: any; } +@Directive({selector: 'someDirective', inputs: ['a']}) +class SomeDirectiveWithDuplicateInputs { + @Input() a: any; +} + +@Directive({selector: 'someDirective', inputs: ['localA: a']}) +class SomeDirectiveWithDuplicateRenamedInputs { + @Input() a: any; + localA: any; +} + @Directive({selector: 'someDirective'}) class SomeDirectiveWithSetterProps { @Input('renamed') @@ -133,6 +143,17 @@ export function main() { expect(directiveMetadata.inputs).toEqual(['a: renamed']); }); + it('should throw if duplicate inputs', () => { + expect(() => { + resolver.resolve(SomeDirectiveWithDuplicateInputs); + }).toThrowError(`Input 'a' defined multiple times in 'SomeDirectiveWithDuplicateInputs'`); + }); + + it('should throw if duplicate inputs (with rename)', () => { + expect(() => { resolver.resolve(SomeDirectiveWithDuplicateRenamedInputs); }) + .toThrowError( + `Input 'a' defined multiple times in 'SomeDirectiveWithDuplicateRenamedInputs'`); + }); }); describe('outputs', () => { @@ -151,6 +172,12 @@ export function main() { .toThrowError( `Output event 'a' defined multiple times in 'SomeDirectiveWithDuplicateOutputs'`); }); + + it('should throw if duplicate outputs (with rename)', () => { + expect(() => { resolver.resolve(SomeDirectiveWithDuplicateRenamedOutputs); }) + .toThrowError( + `Output event 'a' defined multiple times in 'SomeDirectiveWithDuplicateRenamedOutputs'`); + }); }); describe('host', () => {