From 39ec188003458e4e2f666e61e66f74e6d57447c3 Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Mon, 6 Jan 2020 17:27:29 -0800 Subject: [PATCH] fix(ivy): more accurate detection of pipes in host bindings (#34655) Pipes in host binding expressions are not supported in View Engine and Ivy, but in some more complex cases (like `(value | pipe) === true`) compiler was not reporting errors. This commit extends Ivy logic to detect pipes in host binding expressions and throw in cases bindings are present. View Engine behavior remains the same. PR Close #34655 --- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 62 +++++++++++++++---- .../compiler/src/expression_parser/parser.ts | 36 ++++++++--- .../compiler/src/render3/view/template.ts | 5 +- 3 files changed, 82 insertions(+), 21 deletions(-) diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index 5f8611f8ae..8abe51302a 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -2250,30 +2250,68 @@ runInEachFileSystem(os => { } }) class FooCmp {} - `); + `); const errors = env.driveDiagnostics(); expect(trim(errors[0].messageText as string)) .toContain('Cannot have a pipe in an action expression'); }); - it('should throw in case pipes are used in host bindings', () => { + it('should throw in case pipes are used in host bindings (defined as `value | pipe`)', () => { env.write(`test.ts`, ` - import {Component} from '@angular/core'; + import {Component} from '@angular/core'; - @Component({ - selector: 'test', - template: '...', - host: { - '[id]': 'id | myPipe' - } - }) - class FooCmp {} - `); + @Component({ + selector: 'test', + template: '...', + host: { + '[id]': 'id | myPipe' + } + }) + class FooCmp {} + `); const errors = env.driveDiagnostics(); expect(trim(errors[0].messageText as string)) .toContain('Host binding expression cannot contain pipes'); }); + it('should throw in case pipes are used in host bindings (defined as `!(value | pipe)`)', + () => { + env.write(`test.ts`, ` + import {Component} from '@angular/core'; + + @Component({ + selector: 'test', + template: '...', + host: { + '[id]': '!(id | myPipe)' + } + }) + class FooCmp {} + `); + const errors = env.driveDiagnostics(); + expect(trim(errors[0].messageText as string)) + .toContain('Host binding expression cannot contain pipes'); + }); + + it('should throw in case pipes are used in host bindings (defined as `(value | pipe) === X`)', + () => { + env.write(`test.ts`, ` + import {Component} from '@angular/core'; + + @Component({ + selector: 'test', + template: '...', + host: { + '[id]': '(id | myPipe) === true' + } + }) + class FooCmp {} + `); + const errors = env.driveDiagnostics(); + expect(trim(errors[0].messageText as string)) + .toContain('Host binding expression cannot contain pipes'); + }); + it('should generate host bindings for directives', () => { env.write(`test.ts`, ` import {Component, HostBinding, HostListener, TemplateRef} from '@angular/core'; diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index 23d804d19b..b34855b825 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -42,6 +42,8 @@ export class Parser { constructor(private _lexer: Lexer) {} + simpleExpressionChecker = SimpleExpressionChecker; + parseAction( input: string, location: any, absoluteOffset: number, interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource { @@ -62,11 +64,17 @@ export class Parser { return new ASTWithSource(ast, input, location, absoluteOffset, this.errors); } + private checkSimpleExpression(ast: AST): string[] { + const checker = new this.simpleExpressionChecker(); + ast.visit(checker); + return checker.errors; + } + parseSimpleBinding( input: string, location: string, absoluteOffset: number, interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource { const ast = this._parseBindingAst(input, location, absoluteOffset, interpolationConfig); - const errors = SimpleExpressionChecker.check(ast); + const errors = this.checkSimpleExpression(ast); if (errors.length > 0) { this._reportError( `Host binding expression cannot contain ${errors.join(' ')}`, input, location); @@ -234,6 +242,10 @@ export class Parser { } } +export class IvyParser extends Parser { + simpleExpressionChecker = IvySimpleExpressionChecker; // +} + export class _ParseAST { private rparensExpected = 0; private rbracketsExpected = 0; @@ -825,12 +837,6 @@ export class _ParseAST { } class SimpleExpressionChecker implements AstVisitor { - static check(ast: AST): string[] { - const s = new SimpleExpressionChecker(); - ast.visit(s); - return s.errors; - } - errors: string[] = []; visitImplicitReceiver(ast: ImplicitReceiver, context: any) {} @@ -875,3 +881,19 @@ class SimpleExpressionChecker implements AstVisitor { visitQuote(ast: Quote, context: any) {} } + +/** + * This class extends SimpleExpressionChecker used in View Engine and performs more strict checks to + * make sure host bindings do not contain pipes. In View Engine, having pipes in host bindings is + * not supported as well, but in some cases (like `!(value | async)`) the error is not triggered at + * compile time. In order to preserve View Engine behavior, more strict checks are introduced for + * Ivy mode only. + */ +class IvySimpleExpressionChecker extends SimpleExpressionChecker { + visitBinary(ast: Binary, context: any) { + ast.left.visit(this); + ast.right.visit(this); + } + + visitPrefixNot(ast: PrefixNot, context: any) { ast.expression.visit(this); } +} \ No newline at end of file diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index d94c65d6dd..23a8d60fb1 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -12,7 +12,7 @@ import {ConstantPool} from '../../constant_pool'; import * as core from '../../core'; import {AST, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, Interpolation, LiteralArray, LiteralMap, LiteralPrimitive, ParsedEventType, PropertyRead} from '../../expression_parser/ast'; import {Lexer} from '../../expression_parser/lexer'; -import {Parser} from '../../expression_parser/parser'; +import {IvyParser} from '../../expression_parser/parser'; import * as i18n from '../../i18n/i18n_ast'; import * as html from '../../ml_parser/ast'; import {HtmlParser} from '../../ml_parser/html_parser'; @@ -2004,7 +2004,8 @@ const elementRegistry = new DomElementSchemaRegistry(); */ export function makeBindingParser( interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): BindingParser { - return new BindingParser(new Parser(new Lexer()), interpolationConfig, elementRegistry, null, []); + return new BindingParser( + new IvyParser(new Lexer()), interpolationConfig, elementRegistry, null, []); } export function resolveSanitizationFn(context: core.SecurityContext, isAttribute?: boolean) {