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
This commit is contained in:
Andrew Kushnir 2020-01-06 17:27:29 -08:00 committed by Matias Niemelä
parent def4127bf1
commit 39ec188003
3 changed files with 82 additions and 21 deletions

View File

@ -2250,30 +2250,68 @@ runInEachFileSystem(os => {
} }
}) })
class FooCmp {} class FooCmp {}
`); `);
const errors = env.driveDiagnostics(); const errors = env.driveDiagnostics();
expect(trim(errors[0].messageText as string)) expect(trim(errors[0].messageText as string))
.toContain('Cannot have a pipe in an action expression'); .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`, ` env.write(`test.ts`, `
import {Component} from '@angular/core'; import {Component} from '@angular/core';
@Component({ @Component({
selector: 'test', selector: 'test',
template: '...', template: '...',
host: { host: {
'[id]': 'id | myPipe' '[id]': 'id | myPipe'
} }
}) })
class FooCmp {} class FooCmp {}
`); `);
const errors = env.driveDiagnostics(); const errors = env.driveDiagnostics();
expect(trim(errors[0].messageText as string)) expect(trim(errors[0].messageText as string))
.toContain('Host binding expression cannot contain pipes'); .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', () => { it('should generate host bindings for directives', () => {
env.write(`test.ts`, ` env.write(`test.ts`, `
import {Component, HostBinding, HostListener, TemplateRef} from '@angular/core'; import {Component, HostBinding, HostListener, TemplateRef} from '@angular/core';

View File

@ -42,6 +42,8 @@ export class Parser {
constructor(private _lexer: Lexer) {} constructor(private _lexer: Lexer) {}
simpleExpressionChecker = SimpleExpressionChecker;
parseAction( parseAction(
input: string, location: any, absoluteOffset: number, input: string, location: any, absoluteOffset: number,
interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource { interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource {
@ -62,11 +64,17 @@ export class Parser {
return new ASTWithSource(ast, input, location, absoluteOffset, this.errors); 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( parseSimpleBinding(
input: string, location: string, absoluteOffset: number, input: string, location: string, absoluteOffset: number,
interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource { interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource {
const ast = this._parseBindingAst(input, location, absoluteOffset, interpolationConfig); const ast = this._parseBindingAst(input, location, absoluteOffset, interpolationConfig);
const errors = SimpleExpressionChecker.check(ast); const errors = this.checkSimpleExpression(ast);
if (errors.length > 0) { if (errors.length > 0) {
this._reportError( this._reportError(
`Host binding expression cannot contain ${errors.join(' ')}`, input, location); `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 { export class _ParseAST {
private rparensExpected = 0; private rparensExpected = 0;
private rbracketsExpected = 0; private rbracketsExpected = 0;
@ -825,12 +837,6 @@ export class _ParseAST {
} }
class SimpleExpressionChecker implements AstVisitor { class SimpleExpressionChecker implements AstVisitor {
static check(ast: AST): string[] {
const s = new SimpleExpressionChecker();
ast.visit(s);
return s.errors;
}
errors: string[] = []; errors: string[] = [];
visitImplicitReceiver(ast: ImplicitReceiver, context: any) {} visitImplicitReceiver(ast: ImplicitReceiver, context: any) {}
@ -875,3 +881,19 @@ class SimpleExpressionChecker implements AstVisitor {
visitQuote(ast: Quote, context: any) {} 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); }
}

View File

@ -12,7 +12,7 @@ import {ConstantPool} from '../../constant_pool';
import * as core from '../../core'; import * as core from '../../core';
import {AST, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, Interpolation, LiteralArray, LiteralMap, LiteralPrimitive, ParsedEventType, PropertyRead} from '../../expression_parser/ast'; import {AST, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, Interpolation, LiteralArray, LiteralMap, LiteralPrimitive, ParsedEventType, PropertyRead} from '../../expression_parser/ast';
import {Lexer} from '../../expression_parser/lexer'; 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 i18n from '../../i18n/i18n_ast';
import * as html from '../../ml_parser/ast'; import * as html from '../../ml_parser/ast';
import {HtmlParser} from '../../ml_parser/html_parser'; import {HtmlParser} from '../../ml_parser/html_parser';
@ -2004,7 +2004,8 @@ const elementRegistry = new DomElementSchemaRegistry();
*/ */
export function makeBindingParser( export function makeBindingParser(
interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): BindingParser { 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) { export function resolveSanitizationFn(context: core.SecurityContext, isAttribute?: boolean) {