fix(compiler): check more cases for pipe usage inside host bindings (#37883)

Builds on top of #34655 to support more cases that could be using a pipe inside host bindings (e.g. ternary expressions or function calls).

Fixes #37610.

PR Close #37883
This commit is contained in:
crisbeto 2020-07-04 09:21:40 +02:00 committed by Andrew Scott
parent 9bd4b74b06
commit 9322b9a060
3 changed files with 83 additions and 53 deletions

View File

@ -3071,44 +3071,6 @@ runInEachFileSystem(os => {
.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

@ -10,7 +10,7 @@ import * as chars from '../chars';
import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config';
import {escapeRegExp} from '../util'; import {escapeRegExp} from '../util';
import {AbsoluteSourceSpan, AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, ExpressionBinding, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralMapKey, LiteralPrimitive, MethodCall, NonNullAssert, ParserError, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, VariableBinding} from './ast'; import {AbsoluteSourceSpan, AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, ExpressionBinding, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralMapKey, LiteralPrimitive, MethodCall, NonNullAssert, ParserError, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, VariableBinding} from './ast';
import {EOF, isIdentifier, isQuote, Lexer, Token, TokenType} from './lexer'; import {EOF, isIdentifier, isQuote, Lexer, Token, TokenType} from './lexer';
export class SplitInterpolation { export class SplitInterpolation {
@ -1052,11 +1052,11 @@ class SimpleExpressionChecker implements AstVisitor {
visitFunctionCall(ast: FunctionCall, context: any) {} visitFunctionCall(ast: FunctionCall, context: any) {}
visitLiteralArray(ast: LiteralArray, context: any) { visitLiteralArray(ast: LiteralArray, context: any) {
this.visitAll(ast.expressions); this.visitAll(ast.expressions, context);
} }
visitLiteralMap(ast: LiteralMap, context: any) { visitLiteralMap(ast: LiteralMap, context: any) {
this.visitAll(ast.values); this.visitAll(ast.values, context);
} }
visitBinary(ast: Binary, context: any) {} visitBinary(ast: Binary, context: any) {}
@ -1075,8 +1075,8 @@ class SimpleExpressionChecker implements AstVisitor {
visitKeyedWrite(ast: KeyedWrite, context: any) {} visitKeyedWrite(ast: KeyedWrite, context: any) {}
visitAll(asts: any[]): any[] { visitAll(asts: any[], context: any): any[] {
return asts.map(node => node.visit(this)); return asts.map(node => node.visit(this, context));
} }
visitChain(ast: Chain, context: any) {} visitChain(ast: Chain, context: any) {}
@ -1085,19 +1085,16 @@ class SimpleExpressionChecker implements AstVisitor {
} }
/** /**
* This class extends SimpleExpressionChecker used in View Engine and performs more strict checks to * This class implements SimpleExpressionChecker used in View Engine and performs more strict checks
* make sure host bindings do not contain pipes. In View Engine, having pipes in host bindings is * 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 * 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 * compile time. In order to preserve View Engine behavior, more strict checks are introduced for
* Ivy mode only. * Ivy mode only.
*/ */
class IvySimpleExpressionChecker extends SimpleExpressionChecker { class IvySimpleExpressionChecker extends RecursiveAstVisitor implements SimpleExpressionChecker {
visitBinary(ast: Binary, context: any) { errors: string[] = [];
ast.left.visit(this);
ast.right.visit(this);
}
visitPrefixNot(ast: PrefixNot, context: any) { visitPipe() {
ast.expression.visit(this); this.errors.push('pipes');
} }
} }

View File

@ -8,7 +8,7 @@
import {ASTWithSource, BindingPipe, Interpolation, ParserError, TemplateBinding, VariableBinding} from '@angular/compiler/src/expression_parser/ast'; import {ASTWithSource, BindingPipe, Interpolation, ParserError, TemplateBinding, VariableBinding} from '@angular/compiler/src/expression_parser/ast';
import {Lexer} from '@angular/compiler/src/expression_parser/lexer'; import {Lexer} from '@angular/compiler/src/expression_parser/lexer';
import {Parser, SplitInterpolation} from '@angular/compiler/src/expression_parser/parser'; import {IvyParser, Parser, SplitInterpolation} from '@angular/compiler/src/expression_parser/parser';
import {expect} from '@angular/platform-browser/testing/src/matchers'; import {expect} from '@angular/platform-browser/testing/src/matchers';
@ -740,6 +740,68 @@ describe('parser', () => {
it('should report when encountering field write', () => { it('should report when encountering field write', () => {
expectError(validate(parseSimpleBinding('a = b')), 'Bindings cannot contain assignments'); expectError(validate(parseSimpleBinding('a = b')), 'Bindings cannot contain assignments');
}); });
describe('Ivy-only validations', () => {
it('should throw if a pipe is used inside a conditional', () => {
expectError(
validate(parseSimpleBindingIvy('(hasId | myPipe) ? "my-id" : ""')),
'Host binding expression cannot contain pipes');
});
it('should throw if a pipe is used inside a function call', () => {
expectError(
validate(parseSimpleBindingIvy('getId(true, id | myPipe)')),
'Host binding expression cannot contain pipes');
});
it('should throw if a pipe is used inside a method call', () => {
expectError(
validate(parseSimpleBindingIvy('idService.getId(true, id | myPipe)')),
'Host binding expression cannot contain pipes');
});
it('should throw if a pipe is used inside a safe method call', () => {
expectError(
validate(parseSimpleBindingIvy('idService?.getId(true, id | myPipe)')),
'Host binding expression cannot contain pipes');
});
it('should throw if a pipe is used inside a property access', () => {
expectError(
validate(parseSimpleBindingIvy('a[id | myPipe]')),
'Host binding expression cannot contain pipes');
});
it('should throw if a pipe is used inside a keyed read expression', () => {
expectError(
validate(parseSimpleBindingIvy('a[id | myPipe].b')),
'Host binding expression cannot contain pipes');
});
it('should throw if a pipe is used inside a safe property read', () => {
expectError(
validate(parseSimpleBindingIvy('(id | myPipe)?.id')),
'Host binding expression cannot contain pipes');
});
it('should throw if a pipe is used inside a non-null assertion', () => {
expectError(
validate(parseSimpleBindingIvy('[id | myPipe]!')),
'Host binding expression cannot contain pipes');
});
it('should throw if a pipe is used inside a prefix not expression', () => {
expectError(
validate(parseSimpleBindingIvy('!(id | myPipe)')),
'Host binding expression cannot contain pipes');
});
it('should throw if a pipe is used inside a binary expression', () => {
expectError(
validate(parseSimpleBindingIvy('(id | myPipe) === true')),
'Host binding expression cannot contain pipes');
});
});
}); });
describe('wrapLiteralPrimitive', () => { describe('wrapLiteralPrimitive', () => {
@ -781,6 +843,10 @@ function createParser() {
return new Parser(new Lexer()); return new Parser(new Lexer());
} }
function createIvyParser() {
return new IvyParser(new Lexer());
}
function parseAction(text: string, location: any = null, offset: number = 0): ASTWithSource { function parseAction(text: string, location: any = null, offset: number = 0): ASTWithSource {
return createParser().parseAction(text, location, offset); return createParser().parseAction(text, location, offset);
} }
@ -816,6 +882,11 @@ function parseSimpleBinding(text: string, location: any = null, offset: number =
return createParser().parseSimpleBinding(text, location, offset); return createParser().parseSimpleBinding(text, location, offset);
} }
function parseSimpleBindingIvy(
text: string, location: any = null, offset: number = 0): ASTWithSource {
return createIvyParser().parseSimpleBinding(text, location, offset);
}
function checkInterpolation(exp: string, expected?: string) { function checkInterpolation(exp: string, expected?: string) {
const ast = parseInterpolation(exp)!; const ast = parseInterpolation(exp)!;
if (expected == null) expected = exp; if (expected == null) expected = exp;