diff --git a/modules/@angular/compiler/src/expression_parser/parser.ts b/modules/@angular/compiler/src/expression_parser/parser.ts index 4f747082a1..ee5bb77f32 100644 --- a/modules/@angular/compiler/src/expression_parser/parser.ts +++ b/modules/@angular/compiler/src/expression_parser/parser.ts @@ -60,10 +60,11 @@ export class Parser { parseSimpleBinding( input: string, location: string, interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource { - var ast = this._parseBindingAst(input, location, interpolationConfig); - if (!SimpleExpressionChecker.check(ast)) { + const ast = this._parseBindingAst(input, location, interpolationConfig); + const errors = SimpleExpressionChecker.check(ast); + if (errors.length > 0) { this._reportError( - 'Host binding expression can only contain field access and constants', input, location); + `Host binding expression cannot contain ${errors.join(' ')}`, input, location); } return new ASTWithSource(ast, input, location, this.errors); } @@ -751,51 +752,51 @@ export class _ParseAST { } class SimpleExpressionChecker implements AstVisitor { - static check(ast: AST): boolean { + static check(ast: AST): string[] { var s = new SimpleExpressionChecker(); ast.visit(s); - return s.simple; + return s.errors; } - simple = true; + errors: string[] = []; visitImplicitReceiver(ast: ImplicitReceiver, context: any) {} - visitInterpolation(ast: Interpolation, context: any) { this.simple = false; } + visitInterpolation(ast: Interpolation, context: any) {} visitLiteralPrimitive(ast: LiteralPrimitive, context: any) {} visitPropertyRead(ast: PropertyRead, context: any) {} - visitPropertyWrite(ast: PropertyWrite, context: any) { this.simple = false; } + visitPropertyWrite(ast: PropertyWrite, context: any) {} - visitSafePropertyRead(ast: SafePropertyRead, context: any) { this.simple = false; } + visitSafePropertyRead(ast: SafePropertyRead, context: any) {} - visitMethodCall(ast: MethodCall, context: any) { this.simple = false; } + visitMethodCall(ast: MethodCall, context: any) {} - visitSafeMethodCall(ast: SafeMethodCall, context: any) { this.simple = false; } + visitSafeMethodCall(ast: SafeMethodCall, context: any) {} - visitFunctionCall(ast: FunctionCall, context: any) { this.simple = false; } + visitFunctionCall(ast: FunctionCall, context: any) {} visitLiteralArray(ast: LiteralArray, context: any) { this.visitAll(ast.expressions); } visitLiteralMap(ast: LiteralMap, context: any) { this.visitAll(ast.values); } - visitBinary(ast: Binary, context: any) { this.simple = false; } + visitBinary(ast: Binary, context: any) {} - visitPrefixNot(ast: PrefixNot, context: any) { this.simple = false; } + visitPrefixNot(ast: PrefixNot, context: any) {} - visitConditional(ast: Conditional, context: any) { this.simple = false; } + visitConditional(ast: Conditional, context: any) {} - visitPipe(ast: BindingPipe, context: any) { this.simple = false; } + visitPipe(ast: BindingPipe, context: any) { this.errors.push('pipes'); } - visitKeyedRead(ast: KeyedRead, context: any) { this.simple = false; } + visitKeyedRead(ast: KeyedRead, context: any) {} - visitKeyedWrite(ast: KeyedWrite, context: any) { this.simple = false; } + visitKeyedWrite(ast: KeyedWrite, context: any) {} visitAll(asts: any[]): any[] { return asts.map(node => node.visit(this)); } - visitChain(ast: Chain, context: any) { this.simple = false; } + visitChain(ast: Chain, context: any) {} - visitQuote(ast: Quote, context: any) { this.simple = false; } + visitQuote(ast: Quote, context: any) {} } diff --git a/modules/@angular/compiler/src/template_parser/template_parser.ts b/modules/@angular/compiler/src/template_parser/template_parser.ts index ff5690dbbf..a7091bf65b 100644 --- a/modules/@angular/compiler/src/template_parser/template_parser.ts +++ b/modules/@angular/compiler/src/template_parser/template_parser.ts @@ -277,11 +277,14 @@ class TemplateParseVisitor implements html.Visitor { } } - private _parseBinding(value: string, sourceSpan: ParseSourceSpan): ASTWithSource { + private _parseBinding(value: string, isHostBinding: boolean, sourceSpan: ParseSourceSpan): + ASTWithSource { const sourceInfo = sourceSpan.start.toString(); try { - const ast = this._exprParser.parseBinding(value, sourceInfo, this._interpolationConfig); + const ast = isHostBinding ? + this._exprParser.parseSimpleBinding(value, sourceInfo, this._interpolationConfig) : + this._exprParser.parseBinding(value, sourceInfo, this._interpolationConfig); if (ast) this._reportParserErrors(ast.errors, sourceSpan); this._checkPipes(ast, sourceSpan); return ast; @@ -668,7 +671,7 @@ class TemplateParseVisitor implements html.Visitor { targetAnimationProps); } else { this._parsePropertyAst( - name, this._parseBinding(expression, sourceSpan), sourceSpan, targetMatchableAttrs, + name, this._parseBinding(expression, false, sourceSpan), sourceSpan, targetMatchableAttrs, targetProps); } } @@ -683,7 +686,7 @@ class TemplateParseVisitor implements html.Visitor { expression = 'null'; } - const ast = this._parseBinding(expression, sourceSpan); + const ast = this._parseBinding(expression, false, sourceSpan); targetMatchableAttrs.push([name, ast.source]); targetAnimationProps.push(new BoundElementPropertyAst( name, PropertyBindingType.Animation, SecurityContext.NONE, ast, null, sourceSpan)); @@ -846,7 +849,7 @@ class TemplateParseVisitor implements html.Visitor { Object.keys(hostProps).forEach(propName => { const expression = hostProps[propName]; if (typeof expression === 'string') { - const exprAst = this._parseBinding(expression, sourceSpan); + const exprAst = this._parseBinding(expression, true, sourceSpan); targetPropertyAsts.push( this._createElementPropertyAst(elementName, propName, exprAst, sourceSpan)); } else { diff --git a/modules/@angular/compiler/src/view_compiler/event_binder.ts b/modules/@angular/compiler/src/view_compiler/event_binder.ts index c063057b55..70910ad579 100644 --- a/modules/@angular/compiler/src/view_compiler/event_binder.ts +++ b/modules/@angular/compiler/src/view_compiler/event_binder.ts @@ -16,7 +16,7 @@ import {CompileBinding} from './compile_binding'; import {CompileElement} from './compile_element'; import {CompileMethod} from './compile_method'; import {EventHandlerVars, ViewProperties} from './constants'; -import {convertCdStatementToIr} from './expression_converter'; +import {NoLocalsNameResolver, convertCdStatementToIr} from './expression_converter'; export class CompileEventListener { private _method: CompileMethod; @@ -62,7 +62,8 @@ export class CompileEventListener { this._method.resetDebugInfo(this.compileElement.nodeIndex, hostEvent); var context = directiveInstance || this.compileElement.view.componentContext; var actionStmts = convertCdStatementToIr( - this.compileElement.view, context, hostEvent.handler, this.compileElement.nodeIndex); + directive ? new NoLocalsNameResolver(this.compileElement.view) : this.compileElement.view, + context, hostEvent.handler, this.compileElement.nodeIndex); var lastIndex = actionStmts.length - 1; if (lastIndex >= 0) { var lastStatement = actionStmts[lastIndex]; diff --git a/modules/@angular/compiler/src/view_compiler/expression_converter.ts b/modules/@angular/compiler/src/view_compiler/expression_converter.ts index 504fca8270..9e3fbc106a 100644 --- a/modules/@angular/compiler/src/view_compiler/expression_converter.ts +++ b/modules/@angular/compiler/src/view_compiler/expression_converter.ts @@ -11,6 +11,7 @@ import * as cdAst from '../expression_parser/ast'; import {isBlank, isPresent} from '../facade/lang'; import {Identifiers, resolveIdentifier} from '../identifiers'; import * as o from '../output/output_ast'; +import {EventHandlerVars} from './constants'; export interface NameResolver { callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression; @@ -19,6 +20,26 @@ export interface NameResolver { createLiteralMap(values: Array>): o.Expression; } +/** + * A wrapper around another NameResolver that removes all locals and pipes. + */ +export class NoLocalsNameResolver implements NameResolver { + constructor(private _delegate: NameResolver) {} + callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression { return null; } + getLocal(name: string): o.Expression { + if (name == EventHandlerVars.event.name) { + return EventHandlerVars.event; + } + return null; + } + createLiteralArray(values: o.Expression[]): o.Expression { + return this._delegate.createLiteralArray(values); + } + createLiteralMap(values: Array>): o.Expression { + return this._delegate.createLiteralMap(values); + } +} + export class ExpressionWithWrappedValueInfo { constructor( public expression: o.Expression, public needsValueUnwrapper: boolean, @@ -170,6 +191,9 @@ class _AstToIrVisitor implements cdAst.AstVisitor { const input = this.visit(ast.exp, _Mode.Expression); const args = this.visitAll(ast.args, _Mode.Expression); const value = this._nameResolver.callPipe(ast.name, input, args); + if (!value) { + throw new Error(`Illegal state: Pipe ${ast.name} is not allowed here!`); + } this.needsValueUnwrapper = true; return convertToStatementIfNeeded(mode, this._valueUnwrapper.callMethod('unwrap', [value])); } diff --git a/modules/@angular/compiler/src/view_compiler/property_binder.ts b/modules/@angular/compiler/src/view_compiler/property_binder.ts index 7863a71ef3..5104428b2b 100644 --- a/modules/@angular/compiler/src/view_compiler/property_binder.ts +++ b/modules/@angular/compiler/src/view_compiler/property_binder.ts @@ -22,7 +22,7 @@ import {CompileMethod} from './compile_method'; import {CompileView} from './compile_view'; import {DetectChangesVars, ViewProperties} from './constants'; import {CompileEventListener} from './event_binder'; -import {convertCdExpressionToIr, temporaryDeclaration} from './expression_converter'; +import {NameResolver, NoLocalsNameResolver, convertCdExpressionToIr, temporaryDeclaration} from './expression_converter'; function createBindFieldExpr(exprIndex: number): o.ReadPropExpr { return o.THIS_EXPR.prop(`_expr_${exprIndex}`); @@ -38,9 +38,10 @@ class EvalResult { function evalCdAst( view: CompileView, currValExpr: o.ReadVarExpr, parsedExpression: cdAst.AST, - context: o.Expression, method: CompileMethod, bindingIndex: number): EvalResult { + context: o.Expression, nameResolver: NameResolver, method: CompileMethod, + bindingIndex: number): EvalResult { var checkExpression = convertCdExpressionToIr( - view, context, parsedExpression, DetectChangesVars.valUnwrapper, bindingIndex); + nameResolver, context, parsedExpression, DetectChangesVars.valUnwrapper, bindingIndex); if (!checkExpression.expression) { // e.g. an empty expression was given return null; @@ -67,9 +68,10 @@ function evalCdAst( function bind( view: CompileView, currValExpr: o.ReadVarExpr, fieldExpr: o.ReadPropExpr, - parsedExpression: cdAst.AST, context: o.Expression, actions: o.Statement[], - method: CompileMethod, bindingIndex: number) { - const evalResult = evalCdAst(view, currValExpr, parsedExpression, context, method, bindingIndex); + parsedExpression: cdAst.AST, context: o.Expression, nameResolver: NameResolver, + actions: o.Statement[], method: CompileMethod, bindingIndex: number) { + const evalResult = + evalCdAst(view, currValExpr, parsedExpression, context, nameResolver, method, bindingIndex); if (!evalResult) { return; } @@ -100,7 +102,7 @@ export function bindRenderText( view.detectChangesRenderPropertiesMethod.resetDebugInfo(compileNode.nodeIndex, boundText); bind( - view, currValExpr, valueField, boundText.value, view.componentContext, + view, currValExpr, valueField, boundText.value, view.componentContext, view, [o.THIS_EXPR.prop('renderer') .callMethod('setText', [compileNode.renderNode, currValExpr]) .toStmt()], @@ -205,7 +207,8 @@ function bindAndWriteToRenderer( } bind( - view, currValExpr, fieldExpr, boundProp.value, context, updateStmts, compileMethod, + view, currValExpr, fieldExpr, boundProp.value, context, + isHostProp ? new NoLocalsNameResolver(view) : view, updateStmts, compileMethod, view.bindings.length); }); } @@ -267,7 +270,7 @@ export function bindDirectiveInputs( detectChangesInInputsMethod.resetDebugInfo(compileElement.nodeIndex, input); var currValExpr = createCurrValueExpr(bindingIndex); const evalResult = evalCdAst( - view, currValExpr, input.value, view.componentContext, detectChangesInInputsMethod, + view, currValExpr, input.value, view.componentContext, view, detectChangesInInputsMethod, bindingIndex); if (!evalResult) { return; diff --git a/modules/@angular/compiler/test/expression_parser/parser_spec.ts b/modules/@angular/compiler/test/expression_parser/parser_spec.ts index 10886b1cb7..e5eb96bfc8 100644 --- a/modules/@angular/compiler/test/expression_parser/parser_spec.ts +++ b/modules/@angular/compiler/test/expression_parser/parser_spec.ts @@ -74,7 +74,10 @@ export function main() { return; } } - throw Error(`Expected an error containing "${message}" to be reported`); + const errMsgs = ast.errors.map(err => err.message).join('\n'); + throw Error( + `Expected an error containing "${message}" to be reported, but got the errors:\n` + + errMsgs); } function expectActionError(text: string, message: string) { @@ -504,16 +507,10 @@ export function main() { validate(p); }); - it('should parse a constant', () => { - var p = parseSimpleBinding('[1, 2]'); - expect(unparse(p)).toEqual('[1, 2]'); - validate(p); - }); - - it('should report when the given expression is not just a field name', () => { + it('should report when encountering pipes', () => { expectError( - validate(parseSimpleBinding('name + 1')), - 'Host binding expression can only contain field access and constants'); + validate(parseSimpleBinding('a | somePipe')), + 'Host binding expression cannot contain pipes'); }); it('should report when encountering interpolation', () => { @@ -521,6 +518,10 @@ export function main() { validate(parseSimpleBinding('{{exp}}')), 'Got interpolation ({{}}) where expression was expected'); }); + + it('should report when encountering field write', () => { + expectError(validate(parseSimpleBinding('a = b')), 'Bindings cannot contain assignments'); + }); }); describe('wrapLiteralPrimitive', () => { diff --git a/modules/@angular/core/test/linker/integration_spec.ts b/modules/@angular/core/test/linker/integration_spec.ts index 47be830973..9458187ac6 100644 --- a/modules/@angular/core/test/linker/integration_spec.ts +++ b/modules/@angular/core/test/linker/integration_spec.ts @@ -826,6 +826,73 @@ function declareTests({useJit}: {useJit: boolean}) { expect(tc.nativeElement.id).toEqual('newId'); }); + it('should not use template variables for expressions in hostProperties', () => { + @Directive({selector: '[host-properties]', host: {'[id]': 'id', '[title]': 'unknownProp'}}) + class DirectiveWithHostProps { + id = 'one'; + } + + const fixture = + TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostProps]}) + .overrideComponent( + MyComp, + {set: {template: `
`}}) + .createComponent(MyComp); + fixture.detectChanges(); + + var tc = fixture.debugElement.children[0]; + expect(tc.properties['id']).toBe('one'); + expect(tc.properties['title']).toBe(undefined); + }); + + it('should not allow pipes in hostProperties', () => { + @Directive({selector: '[host-properties]', host: {'[id]': 'id | uppercase'}}) + class DirectiveWithHostProps { + } + + TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostProps]}); + const template = '
'; + TestBed.overrideComponent(MyComp, {set: {template}}); + expect(() => TestBed.createComponent(MyComp)) + .toThrowError(/Host binding expression cannot contain pipes/); + }); + + it('should not use template variables for expressions in hostListeners', () => { + @Directive({selector: '[host-listener]', host: {'(click)': 'doIt(id, unknownProp)'}}) + class DirectiveWithHostListener { + id = 'one'; + receivedArgs: any[]; + + doIt(...args: any[]) { this.receivedArgs = args; } + } + + const fixture = + TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostListener]}) + .overrideComponent( + MyComp, + {set: {template: `
`}}) + .createComponent(MyComp); + fixture.detectChanges(); + var tc = fixture.debugElement.children[0]; + tc.triggerEventHandler('click', {}); + var dir: DirectiveWithHostListener = tc.injector.get(DirectiveWithHostListener); + expect(dir.receivedArgs).toEqual(['one', undefined]); + }); + + it('should not allow pipes in hostListeners', () => { + @Directive({selector: '[host-listener]', host: {'(click)': 'doIt() | somePipe'}}) + class DirectiveWithHostListener { + } + + TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostListener]}); + const template = '
'; + TestBed.overrideComponent(MyComp, {set: {template}}); + expect(() => TestBed.createComponent(MyComp)) + .toThrowError(/Cannot have a pipe in an action expression/); + }); + + + if (getDOM().supportsDOMEvents()) { it('should support preventing default on render events', () => { TestBed.configureTestingModule({