From cbc0907bfdbbb82e3827bdc9e17659a8b4cfadf3 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sun, 18 Oct 2020 17:41:29 +0200 Subject: [PATCH] fix(compiler): preserve this.$event and this.$any accesses in expressions (#39323) Currently expressions `$event.foo()` and `this.$event.foo()`, as well as `$any(foo)` and `this.$any(foo)`, are treated as the same expression by the compiler, because `this` is considered the same implicit receiver as when the receiver is omitted. This introduces the following issues: 1. Any time something called `$any` is used, it'll be stripped away, leaving only the first parameter. 2. If something called `$event` is used anywhere in a template, it'll be preserved as `$event`, rather than being rewritten to `ctx.$event`, causing the value to undefined at runtime. This applies to listener, property and text bindings. These changes resolve the first issue and part of the second one by preserving anything that is accessed through `this`, even if it's one of the "special" ones like `$any` or `$event`. Furthermore, these changes only expose the `$event` global variable inside event listeners, whereas previously it was available everywhere. Fixes #30278. PR Close #39323 --- .../src/ngtsc/typecheck/src/expression.ts | 9 +- .../ngtsc/typecheck/src/type_check_block.ts | 8 +- .../typecheck/test/type_check_block_spec.ts | 14 ++ .../r3_view_compiler_listener_spec.ts | 133 ++++++++++++++++++ .../test/compliance/r3_view_compiler_spec.ts | 53 +++++++ .../src/compiler_util/expression_converter.ts | 37 +++-- .../compiler/src/expression_parser/ast.ts | 30 +++- .../compiler/src/expression_parser/parser.ts | 7 +- .../compiler/src/render3/view/template.ts | 26 +++- .../test/expression_parser/utils/unparser.ts | 4 +- .../core/test/acceptance/integration_spec.ts | 14 ++ .../core/test/acceptance/listener_spec.ts | 29 ++++ packages/language-service/ivy/quick_info.ts | 4 +- packages/language-service/ivy/utils.ts | 2 +- .../language-service/src/expression_type.ts | 6 +- packages/language-service/src/expressions.ts | 2 + 16 files changed, 348 insertions(+), 30 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts index f462987f84..f4360a183c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, Unary} from '@angular/compiler'; +import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, EmptyExpr, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, ThisReceiver, Unary} from '@angular/compiler'; import * as ts from 'typescript'; import {TypeCheckingConfig} from '../api'; @@ -137,6 +137,10 @@ class AstTranslator implements AstVisitor { throw new Error('Method not implemented.'); } + visitThisReceiver(ast: ThisReceiver): never { + throw new Error('Method not implemented.'); + } + visitInterpolation(ast: Interpolation): ts.Expression { // Build up a chain of binary + operations to simulate the string concatenation of the // interpolation's expressions. The chain is started using an actual string literal to ensure @@ -363,6 +367,9 @@ class VeSafeLhsInferenceBugDetector implements AstVisitor { visitImplicitReceiver(ast: ImplicitReceiver): boolean { return false; } + visitThisReceiver(ast: ThisReceiver): boolean { + return false; + } visitInterpolation(ast: Interpolation): boolean { return ast.expressions.some(exp => exp.visit(this)); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index 4864dfaec2..73608a6d4e 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiver, MethodCall, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SchemaMetadata, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstIcu, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; +import {AST, BindingPipe, BindingType, BoundTarget, DYNAMIC_TYPE, ImplicitReceiver, MethodCall, ParsedEventType, ParseSourceSpan, PropertyRead, PropertyWrite, SchemaMetadata, ThisReceiver, TmplAstBoundAttribute, TmplAstBoundEvent, TmplAstBoundText, TmplAstElement, TmplAstIcu, TmplAstNode, TmplAstReference, TmplAstTemplate, TmplAstTextAttribute, TmplAstVariable} from '@angular/compiler'; import * as ts from 'typescript'; import {Reference} from '../../imports'; @@ -1586,7 +1586,9 @@ class TcbExpressionTranslator { const result = tsCallMethod(pipe, 'transform', [expr, ...args]); addParseSpanInfo(result, ast.sourceSpan); return result; - } else if (ast instanceof MethodCall && ast.receiver instanceof ImplicitReceiver) { + } else if ( + ast instanceof MethodCall && ast.receiver instanceof ImplicitReceiver && + !(ast.receiver instanceof ThisReceiver)) { // Resolve the special `$any(expr)` syntax to insert a cast of the argument to type `any`. // `$any(expr)` -> `expr as any` if (ast.name === '$any' && ast.args.length === 1) { @@ -1843,7 +1845,7 @@ class TcbEventHandlerTranslator extends TcbExpressionTranslator { // function that the converted expression becomes a child of, just create a reference to the // parameter by its name. if (ast instanceof PropertyRead && ast.receiver instanceof ImplicitReceiver && - ast.name === EVENT_PARAMETER) { + !(ast.receiver instanceof ThisReceiver) && ast.name === EVENT_PARAMETER) { const event = ts.createIdentifier(EVENT_PARAMETER); addParseSpanInfo(event, ast.nameSpan); return event; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index 886a78f958..ed60b9d31f 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -584,6 +584,12 @@ describe('type check blocks', () => { expect(block).toContain('(((ctx).a) as any)'); }); + it('should handle $any accessed through `this`', () => { + const TEMPLATE = `{{this.$any(a)}}`; + const block = tcb(TEMPLATE); + expect(block).toContain('((ctx).$any(((ctx).a)))'); + }); + describe('experimental DOM checking via lib.dom.d.ts', () => { it('should translate unclaimed bindings to their property equivalent', () => { const TEMPLATE = ``; @@ -684,6 +690,14 @@ describe('type check blocks', () => { expect(block).toContain( '_t3.addEventListener("event", function ($event): any { (_t2 = 3); });'); }); + + it('should ignore accesses to $event through `this`', () => { + const TEMPLATE = `
`; + const block = tcb(TEMPLATE); + + expect(block).toContain( + '_t1.addEventListener("event", function ($event): any { (ctx).foo(((ctx).$event)); });'); + }); }); describe('config', () => { diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_listener_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_listener_spec.ts index 01d8d7880b..0173f6f0cd 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_listener_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_listener_spec.ts @@ -447,4 +447,137 @@ describe('compiler compliance: listen()', () => { const result = compile(files, angularFiles); expectEmit(result.source, template, 'Incorrect host bindings'); }); + + it('should assume $event is referring to the event variable in a listener by default', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: '
' + }) + class Comp { + c(event: MouseEvent) {} + } + ` + } + }; + + const template = ` + … + i0.ɵɵlistener("click", function Comp_Template_div_click_0_listener($event) { return ctx.c($event); }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect event listener'); + }); + + it('should preserve accesses to $event if it is done through `this` in a listener', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: '
' + }) + class Comp { + $event = {}; + c(value: {}) {} + } + ` + } + }; + + const template = ` + … + i0.ɵɵlistener("click", function Comp_Template_div_click_0_listener() { return ctx.c(ctx.$event); }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect event listener'); + }); + + it('should not assume that $event is referring to an event object inside a property', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: '
' + }) + class Comp { + $event = 1; + } + ` + } + }; + + const template = ` + … + i0.ɵɵproperty("event", ctx.$event); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect property binding'); + }); + + it('should assume $event is referring to the event variable in a listener by default inside a host binding', + () => { + const files = { + app: { + 'spec.ts': ` + import {Directive} from '@angular/core'; + + @Directive({ + host: { + '(click)': 'c($event)' + } + }) + class Dir { + c(event: MouseEvent) {} + } + ` + } + }; + + const template = ` + … + i0.ɵɵlistener("click", function Dir_click_HostBindingHandler($event) { return ctx.c($event); }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect event listener'); + }); + + it('should preserve accesses to $event if it is done through `this` in a listener inside a host binding', + () => { + const files = { + app: { + 'spec.ts': ` + import {Directive} from '@angular/core'; + + @Directive({ + host: { + '(click)': 'c(this.$event)' + } + }) + class Dir { + $event = {}; + c(value: {}) {} + } + ` + } + }; + + const template = ` + … + i0.ɵɵlistener("click", function Dir_click_HostBindingHandler() { return ctx.c(ctx.$event); }); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect event listener'); + }); }); diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_spec.ts index b2e19b02b4..bdfbd1762d 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_spec.ts @@ -191,4 +191,57 @@ describe('r3_view_compiler', () => { expectEmit(result.source, template, 'Incorrect initialization attributes'); }); }); + + describe('$any', () => { + it('should strip out $any wrappers', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: '
' + }) + class Comp { + } + ` + } + }; + + const template = ` + … + i0.ɵɵproperty("tabIndex", 10); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + + it('should preserve $any if it is accessed through `this`', () => { + const files = { + app: { + 'spec.ts': ` + import {Component} from '@angular/core'; + + @Component({ + template: '
' + }) + class Comp { + $any(value: null): any { + return value as any; + } + } + ` + } + }; + + const template = ` + … + i0.ɵɵproperty("tabIndex", ctx.$any(null)); + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + }); }); diff --git a/packages/compiler/src/compiler_util/expression_converter.ts b/packages/compiler/src/compiler_util/expression_converter.ts index 1bae07ce32..58017f5c73 100644 --- a/packages/compiler/src/compiler_util/expression_converter.ts +++ b/packages/compiler/src/compiler_util/expression_converter.ts @@ -18,6 +18,7 @@ export class EventHandlerVars { export interface LocalResolver { getLocal(name: string): o.Expression|null; notifyImplicitReceiverUse(): void; + globals?: Set; } export class ConvertActionBindingResult { @@ -72,10 +73,10 @@ export type InterpolationFunction = (args: o.Expression[]) => o.Expression; export function convertActionBinding( localResolver: LocalResolver|null, implicitReceiver: o.Expression, action: cdAst.AST, bindingId: string, interpolationFunction?: InterpolationFunction, - baseSourceSpan?: ParseSourceSpan, - implicitReceiverAccesses?: Set): ConvertActionBindingResult { + baseSourceSpan?: ParseSourceSpan, implicitReceiverAccesses?: Set, + globals?: Set): ConvertActionBindingResult { if (!localResolver) { - localResolver = new DefaultLocalResolver(); + localResolver = new DefaultLocalResolver(globals); } const actionWithoutBuiltins = convertPropertyBindingBuiltins( { @@ -446,6 +447,10 @@ class _AstToIrVisitor implements cdAst.AstVisitor { return this._implicitReceiver; } + visitThisReceiver(ast: cdAst.ThisReceiver, mode: _Mode): any { + return this.visitImplicitReceiver(ast, mode); + } + visitInterpolation(ast: cdAst.Interpolation, mode: _Mode): any { ensureExpressionMode(mode, ast); const args = [o.literal(ast.expressions.length)]; @@ -501,12 +506,17 @@ class _AstToIrVisitor implements cdAst.AstVisitor { mode, o.literal(ast.value, type, this.convertSourceSpan(ast.span))); } - private _getLocal(name: string): o.Expression|null { + private _getLocal(name: string, receiver: cdAst.AST): o.Expression|null { + if (this._localResolver.globals?.has(name) && receiver instanceof cdAst.ThisReceiver) { + return null; + } + return this._localResolver.getLocal(name); } visitMethodCall(ast: cdAst.MethodCall, mode: _Mode): any { - if (ast.receiver instanceof cdAst.ImplicitReceiver && ast.name == '$any') { + if (ast.receiver instanceof cdAst.ImplicitReceiver && + !(ast.receiver instanceof cdAst.ThisReceiver) && ast.name === '$any') { const args = this.visitAll(ast.args, _Mode.Expression) as any[]; if (args.length != 1) { throw new Error( @@ -524,14 +534,14 @@ class _AstToIrVisitor implements cdAst.AstVisitor { let result: any = null; const receiver = this._visit(ast.receiver, _Mode.Expression); if (receiver === this._implicitReceiver) { - const varExpr = this._getLocal(ast.name); + const varExpr = this._getLocal(ast.name, ast.receiver); if (varExpr) { // Restore the previous "usesImplicitReceiver" state since the implicit // receiver has been replaced with a resolved local expression. this.usesImplicitReceiver = prevUsesImplicitReceiver; result = varExpr.callFn(args); + this.addImplicitReceiverAccess(ast.name); } - this.addImplicitReceiverAccess(ast.name); } if (result == null) { result = receiver.callMethod(ast.name, args, this.convertSourceSpan(ast.span)); @@ -558,13 +568,13 @@ class _AstToIrVisitor implements cdAst.AstVisitor { const prevUsesImplicitReceiver = this.usesImplicitReceiver; const receiver = this._visit(ast.receiver, _Mode.Expression); if (receiver === this._implicitReceiver) { - result = this._getLocal(ast.name); + result = this._getLocal(ast.name, ast.receiver); if (result) { // Restore the previous "usesImplicitReceiver" state since the implicit // receiver has been replaced with a resolved local expression. this.usesImplicitReceiver = prevUsesImplicitReceiver; + this.addImplicitReceiverAccess(ast.name); } - this.addImplicitReceiverAccess(ast.name); } if (result == null) { result = receiver.prop(ast.name); @@ -579,7 +589,7 @@ class _AstToIrVisitor implements cdAst.AstVisitor { let varExpr: o.ReadPropExpr|null = null; if (receiver === this._implicitReceiver) { - const localExpr = this._getLocal(ast.name); + const localExpr = this._getLocal(ast.name, ast.receiver); if (localExpr) { if (localExpr instanceof o.ReadPropExpr) { // If the local variable is a property read expression, it's a reference @@ -748,6 +758,9 @@ class _AstToIrVisitor implements cdAst.AstVisitor { visitImplicitReceiver(ast: cdAst.ImplicitReceiver) { return null; }, + visitThisReceiver(ast: cdAst.ThisReceiver) { + return null; + }, visitInterpolation(ast: cdAst.Interpolation) { return null; }, @@ -825,6 +838,9 @@ class _AstToIrVisitor implements cdAst.AstVisitor { visitImplicitReceiver(ast: cdAst.ImplicitReceiver) { return false; }, + visitThisReceiver(ast: cdAst.ThisReceiver) { + return false; + }, visitInterpolation(ast: cdAst.Interpolation) { return visitSome(this, ast.expressions); }, @@ -924,6 +940,7 @@ function flattenStatements(arg: any, output: o.Statement[]) { } class DefaultLocalResolver implements LocalResolver { + constructor(public globals?: Set) {} notifyImplicitReceiverUse(): void {} getLocal(name: string): o.Expression|null { if (name === EventHandlerVars.event.name) { diff --git a/packages/compiler/src/expression_parser/ast.ts b/packages/compiler/src/expression_parser/ast.ts index 2773a5458d..08589c143c 100644 --- a/packages/compiler/src/expression_parser/ast.ts +++ b/packages/compiler/src/expression_parser/ast.ts @@ -85,6 +85,20 @@ export class ImplicitReceiver extends AST { } } +/** + * Receiver when something is accessed through `this` (e.g. `this.foo`). Note that this class + * inherits from `ImplicitReceiver`, because accessing something through `this` is treated the + * same as accessing it implicitly inside of an Angular template (e.g. `[attr.title]="this.title"` + * is the same as `[attr.title]="title"`.). Inheriting allows for the `this` accesses to be treated + * the same as implicit ones, except for a couple of exceptions like `$event` and `$any`. + * TODO: we should find a way for this class not to extend from `ImplicitReceiver` in the future. + */ +export class ThisReceiver extends ImplicitReceiver { + visit(visitor: AstVisitor, context: any = null): any { + return visitor.visitThisReceiver?.(this, context); + } +} + /** * Multiple expressions separated by a semicolon. */ @@ -416,6 +430,11 @@ export interface AstVisitor { visitChain(ast: Chain, context: any): any; visitConditional(ast: Conditional, context: any): any; visitFunctionCall(ast: FunctionCall, context: any): any; + /** + * The `visitThisReceiver` method is declared as optional for backwards compatibility. + * In an upcoming major release, this method will be made required. + */ + visitThisReceiver?(ast: ThisReceiver, context: any): any; visitImplicitReceiver(ast: ImplicitReceiver, context: any): any; visitInterpolation(ast: Interpolation, context: any): any; visitKeyedRead(ast: KeyedRead, context: any): any; @@ -474,7 +493,8 @@ export class RecursiveAstVisitor implements AstVisitor { } this.visitAll(ast.args, context); } - visitImplicitReceiver(ast: ImplicitReceiver, context: any): any {} + visitImplicitReceiver(ast: ThisReceiver, context: any): any {} + visitThisReceiver(ast: ThisReceiver, context: any): any {} visitInterpolation(ast: Interpolation, context: any): any { this.visitAll(ast.expressions, context); } @@ -532,6 +552,10 @@ export class AstTransformer implements AstVisitor { return ast; } + visitThisReceiver(ast: ThisReceiver, context: any): AST { + return ast; + } + visitInterpolation(ast: Interpolation, context: any): AST { return new Interpolation(ast.span, ast.sourceSpan, ast.strings, this.visitAll(ast.expressions)); } @@ -651,6 +675,10 @@ export class AstMemoryEfficientTransformer implements AstVisitor { return ast; } + visitThisReceiver(ast: ThisReceiver, context: any): AST { + return ast; + } + visitInterpolation(ast: Interpolation, context: any): Interpolation { const expressions = this.visitAll(ast.expressions); if (expressions !== ast.expressions) diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index 7d1cc1f4a4..2f488d2931 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -10,7 +10,7 @@ import * as chars from '../chars'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config'; 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, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, Unary, 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, ThisReceiver, Unary, VariableBinding} from './ast'; import {EOF, isIdentifier, isQuote, Lexer, Token, TokenType} from './lexer'; export class SplitInterpolation { @@ -784,8 +784,7 @@ export class _ParseAST { } else if (this.next.isKeywordThis()) { this.advance(); - return new ImplicitReceiver(this.span(start), this.sourceSpan(start)); - + return new ThisReceiver(this.span(start), this.sourceSpan(start)); } else if (this.consumeOptionalCharacter(chars.$LBRACKET)) { this.rbracketsExpected++; const elements = this.parseExpressionList(chars.$RBRACKET); @@ -1158,6 +1157,8 @@ class SimpleExpressionChecker implements AstVisitor { visitImplicitReceiver(ast: ImplicitReceiver, context: any) {} + visitThisReceiver(ast: ThisReceiver, context: any) {} + visitInterpolation(ast: Interpolation, context: any) {} visitLiteralPrimitive(ast: LiteralPrimitive, context: any) {} diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 4fc8c24bb9..f6e8707313 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -10,7 +10,7 @@ import {flatten, sanitizeIdentifier} from '../../compile_metadata'; import {BindingForm, BuiltinFunctionCall, convertActionBinding, convertPropertyBinding, convertUpdateArguments, LocalResolver} from '../../compiler_util/expression_converter'; 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 {AST, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, Interpolation, LiteralArray, LiteralMap, LiteralPrimitive, ParsedEventType, PropertyRead, ThisReceiver} from '../../expression_parser/ast'; import {Lexer} from '../../expression_parser/lexer'; import {IvyParser} from '../../expression_parser/parser'; import * as i18n from '../../i18n/i18n_ast'; @@ -48,6 +48,9 @@ const NG_CONTENT_SELECT_ATTR = 'select'; // Attribute name of `ngProjectAs`. const NG_PROJECT_AS_ATTR_NAME = 'ngProjectAs'; +// Global symbols available only inside event bindings. +const EVENT_BINDING_SCOPE_GLOBALS = new Set(['$event']); + // List of supported global targets for event listeners const GLOBAL_TARGET_RESOLVERS = new Map( [['window', R3.resolveWindow], ['document', R3.resolveDocument], ['body', R3.resolveBody]]); @@ -76,7 +79,7 @@ export function prepareEventListenerParameters( scope.getOrCreateSharedContextVar(0); const bindingExpr = convertActionBinding( scope, implicitReceiverExpr, handler, 'b', () => error('Unexpected interpolation'), - eventAst.handlerSpan, implicitReceiverAccesses); + eventAst.handlerSpan, implicitReceiverAccesses, EVENT_BINDING_SCOPE_GLOBALS); const statements = []; if (scope) { statements.push(...scope.restoreViewStatement()); @@ -1427,7 +1430,8 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver prepareSyntheticListenerFunctionName(eventName, outputAst.phase!) : sanitizeIdentifier(eventName); const handlerName = `${this.templateName}_${tagName}_${bindingFnName}_${index}_listener`; - const scope = this._bindingScope.nestedScope(this._bindingScope.bindingLevel); + const scope = this._bindingScope.nestedScope( + this._bindingScope.bindingLevel, EVENT_BINDING_SCOPE_GLOBALS); return prepareEventListenerParameters(outputAst, handlerName, scope); }; } @@ -1625,10 +1629,18 @@ export class BindingScope implements LocalResolver { private referenceNameIndex = 0; private restoreViewVariable: o.ReadVarExpr|null = null; static createRootScope(): BindingScope { - return new BindingScope().set(0, '$event', o.variable('$event')); + return new BindingScope(); } - private constructor(public bindingLevel: number = 0, private parent: BindingScope|null = null) {} + private constructor( + public bindingLevel: number = 0, private parent: BindingScope|null = null, + public globals?: Set) { + if (globals !== undefined) { + for (const name of globals) { + this.set(0, name, o.variable(name)); + } + } + } get(name: string): o.Expression|null { let current: BindingScope|null = this; @@ -1715,8 +1727,8 @@ export class BindingScope implements LocalResolver { } } - nestedScope(level: number): BindingScope { - const newScope = new BindingScope(level, this); + nestedScope(level: number, globals?: Set): BindingScope { + const newScope = new BindingScope(level, this, globals); if (level > 0) newScope.generateSharedContextVar(0); return newScope; } diff --git a/packages/compiler/test/expression_parser/utils/unparser.ts b/packages/compiler/test/expression_parser/utils/unparser.ts index c16dae0d51..cb165b246b 100644 --- a/packages/compiler/test/expression_parser/utils/unparser.ts +++ b/packages/compiler/test/expression_parser/utils/unparser.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead, Unary} from '../../../src/expression_parser/ast'; +import {AST, AstVisitor, ASTWithSource, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead, ThisReceiver, Unary} from '../../../src/expression_parser/ast'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../src/ml_parser/interpolation_config'; class Unparser implements AstVisitor { @@ -87,6 +87,8 @@ class Unparser implements AstVisitor { visitImplicitReceiver(ast: ImplicitReceiver, context: any) {} + visitThisReceiver(ast: ThisReceiver, context: any) {} + visitInterpolation(ast: Interpolation, context: any) { for (let i = 0; i < ast.strings.length; i++) { this._expression += ast.strings[i]; diff --git a/packages/core/test/acceptance/integration_spec.ts b/packages/core/test/acceptance/integration_spec.ts index 5d07cbcefa..21e0c0000a 100644 --- a/packages/core/test/acceptance/integration_spec.ts +++ b/packages/core/test/acceptance/integration_spec.ts @@ -384,6 +384,20 @@ describe('acceptance integration tests', () => { expect(fixture.nativeElement.innerHTML).toEqual(''); }); + + it('should be able to render the result of a function called $any by using this', () => { + @Component({template: '{{this.$any(1, 2)}}'}) + class App { + $any(value: number, multiplier: number) { + return value * multiplier; + } + } + + TestBed.configureTestingModule({declarations: [App]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toBe('2'); + }); }); describe('ngNonBindable handling', () => { diff --git a/packages/core/test/acceptance/listener_spec.ts b/packages/core/test/acceptance/listener_spec.ts index 661006dfb6..671a8baf9b 100644 --- a/packages/core/test/acceptance/listener_spec.ts +++ b/packages/core/test/acceptance/listener_spec.ts @@ -396,4 +396,33 @@ describe('event listeners', () => { button.click(); expect(comp.counter).toBe(1); }); + + onlyInIvy('issue has only been resolved for Ivy') + .it('should be able to access a property called $event using `this`', () => { + let eventVariable: number|undefined; + let eventObject: MouseEvent|undefined; + + @Component({ + template: ` + + `, + }) + class MyComp { + $event = 10; + + clicked(value: number, event: MouseEvent) { + eventVariable = value; + eventObject = event; + } + } + + TestBed.configureTestingModule({declarations: [MyComp]}); + const fixture = TestBed.createComponent(MyComp); + fixture.detectChanges(); + fixture.nativeElement.querySelector('button').click(); + fixture.detectChanges(); + + expect(eventVariable).toBe(10); + expect(eventObject?.type).toBe('click'); + }); }); diff --git a/packages/language-service/ivy/quick_info.ts b/packages/language-service/ivy/quick_info.ts index bafb2c814c..d1e9123e5a 100644 --- a/packages/language-service/ivy/quick_info.ts +++ b/packages/language-service/ivy/quick_info.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {AST, BindingPipe, ImplicitReceiver, MethodCall, TmplAstBoundAttribute, TmplAstNode, TmplAstTextAttribute} from '@angular/compiler'; +import {AST, BindingPipe, ImplicitReceiver, MethodCall, ThisReceiver, TmplAstBoundAttribute, TmplAstNode, TmplAstTextAttribute} from '@angular/compiler'; import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core'; import {DirectiveSymbol, DomBindingSymbol, ElementSymbol, ExpressionSymbol, InputBindingSymbol, OutputBindingSymbol, ReferenceSymbol, ShimLocation, Symbol, SymbolKind, VariableSymbol} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; import * as ts from 'typescript'; @@ -208,7 +208,7 @@ function displayPartsEqual(a: {text: string, kind: string}, b: {text: string, ki function isDollarAny(node: TmplAstNode|AST): node is MethodCall { return node instanceof MethodCall && node.receiver instanceof ImplicitReceiver && - node.name === '$any' && node.args.length === 1; + !(node.receiver instanceof ThisReceiver) && node.name === '$any' && node.args.length === 1; } function createDollarAnyQuickInfo(node: MethodCall): ts.QuickInfo { diff --git a/packages/language-service/ivy/utils.ts b/packages/language-service/ivy/utils.ts index 6888f2d32d..e2481dde42 100644 --- a/packages/language-service/ivy/utils.ts +++ b/packages/language-service/ivy/utils.ts @@ -279,7 +279,7 @@ export function filterAliasImports(displayParts: ts.SymbolDisplayPart[]): ts.Sym export function isDollarEvent(n: t.Node|e.AST): n is e.PropertyRead { return n instanceof e.PropertyRead && n.name === '$event' && - n.receiver instanceof e.ImplicitReceiver; + n.receiver instanceof e.ImplicitReceiver && !(n.receiver instanceof e.ThisReceiver); } /** diff --git a/packages/language-service/src/expression_type.ts b/packages/language-service/src/expression_type.ts index 099ff99fcc..064a2caffd 100644 --- a/packages/language-service/src/expression_type.ts +++ b/packages/language-service/src/expression_type.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, AstVisitor, ASTWithName, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, Unary} from '@angular/compiler'; +import {AST, AstVisitor, ASTWithName, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, NonNullAssert, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, ThisReceiver, Unary} from '@angular/compiler'; import {createDiagnostic, Diagnostic} from './diagnostic_messages'; import {BuiltinType, Signature, Symbol, SymbolQuery, SymbolTable} from './symbols'; @@ -258,6 +258,10 @@ export class AstType implements AstVisitor { }; } + visitThisReceiver(_ast: ThisReceiver): Symbol { + return this.visitImplicitReceiver(_ast); + } + visitInterpolation(ast: Interpolation): Symbol { // If we are producing diagnostics, visit the children. for (const expr of ast.expressions) { diff --git a/packages/language-service/src/expressions.ts b/packages/language-service/src/expressions.ts index ac0eb18ab4..5af34f8ba3 100644 --- a/packages/language-service/src/expressions.ts +++ b/packages/language-service/src/expressions.ts @@ -69,6 +69,7 @@ export function getExpressionCompletions( visitConditional(_ast) {}, visitFunctionCall(_ast) {}, visitImplicitReceiver(_ast) {}, + visitThisReceiver(_ast) {}, visitInterpolation(_ast) { result = undefined; }, @@ -164,6 +165,7 @@ export function getExpressionSymbol( visitConditional(_ast) {}, visitFunctionCall(_ast) {}, visitImplicitReceiver(_ast) {}, + visitThisReceiver(_ast) {}, visitInterpolation(_ast) {}, visitKeyedRead(_ast) {}, visitKeyedWrite(_ast) {},