diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts index 1b57a408f4..f92a01749a 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, ThisReceiver, 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, SafeKeyedRead, SafeMethodCall, SafePropertyRead, ThisReceiver, Unary} from '@angular/compiler'; import * as ts from 'typescript'; import {TypeCheckingConfig} from '../api'; @@ -156,7 +156,7 @@ class AstTranslator implements AstVisitor { } visitKeyedRead(ast: KeyedRead): ts.Expression { - const receiver = wrapForDiagnostics(this.translate(ast.obj)); + const receiver = wrapForDiagnostics(this.translate(ast.receiver)); const key = this.translate(ast.key); const node = ts.createElementAccess(receiver, key); addParseSpanInfo(node, ast.sourceSpan); @@ -164,7 +164,7 @@ class AstTranslator implements AstVisitor { } visitKeyedWrite(ast: KeyedWrite): ts.Expression { - const receiver = wrapForDiagnostics(this.translate(ast.obj)); + const receiver = wrapForDiagnostics(this.translate(ast.receiver)); const left = ts.createElementAccess(receiver, this.translate(ast.key)); // TODO(joost): annotate `left` with the span of the element access, which is not currently // available on `ast`. @@ -330,6 +330,30 @@ class AstTranslator implements AstVisitor { addParseSpanInfo(node, ast.sourceSpan); return node; } + + visitSafeKeyedRead(ast: SafeKeyedRead): ts.Expression { + const receiver = wrapForDiagnostics(this.translate(ast.receiver)); + const key = this.translate(ast.key); + let node: ts.Expression; + + // The form of safe property reads depends on whether strictness is in use. + if (this.config.strictSafeNavigationTypes) { + // "a?.[...]" becomes (null as any ? a![...] : undefined) + const expr = ts.createElementAccess(ts.createNonNullExpression(receiver), key); + addParseSpanInfo(expr, ast.sourceSpan); + node = ts.createParen(ts.createConditional(NULL_AS_ANY, expr, UNDEFINED)); + } else if (VeSafeLhsInferenceBugDetector.veWillInferAnyFor(ast)) { + // "a?.[...]" becomes (a as any)[...] + node = ts.createElementAccess(tsCastToAny(receiver), key); + } else { + // "a?.[...]" becomes (a!.[...] as any) + const expr = ts.createElementAccess(ts.createNonNullExpression(receiver), key); + addParseSpanInfo(expr, ast.sourceSpan); + node = tsCastToAny(expr); + } + addParseSpanInfo(node, ast.sourceSpan); + return node; + } } /** @@ -348,8 +372,9 @@ class AstTranslator implements AstVisitor { class VeSafeLhsInferenceBugDetector implements AstVisitor { private static SINGLETON = new VeSafeLhsInferenceBugDetector(); - static veWillInferAnyFor(ast: SafeMethodCall|SafePropertyRead) { - return ast.receiver.visit(VeSafeLhsInferenceBugDetector.SINGLETON); + static veWillInferAnyFor(ast: SafeMethodCall|SafePropertyRead|SafeKeyedRead) { + const visitor = VeSafeLhsInferenceBugDetector.SINGLETON; + return ast instanceof SafeKeyedRead ? ast.receiver.visit(visitor) : ast.receiver.visit(visitor); } visitUnary(ast: Unary): boolean { @@ -418,4 +443,7 @@ class VeSafeLhsInferenceBugDetector implements AstVisitor { visitSafePropertyRead(ast: SafePropertyRead): boolean { return false; } + visitSafeKeyedRead(ast: SafeKeyedRead): boolean { + return false; + } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts index 718d791615..842cfc5b05 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts @@ -412,6 +412,35 @@ runInEachFileSystem(() => { expect(messages).toEqual([]); }); + + it('does not produce diagnostic for safe keyed access', () => { + const messages = + diagnose(`
`, ` + export class TestComponent { + person: { + favoriteColors?: string[]; + }; + }`); + + expect(messages).toEqual([]); + }); + + it('infers a safe keyed read as undefined', () => { + const messages = diagnose(`
`, ` + export class TestComponent { + person: { + favoriteColors?: string[]; + }; + + log(color: string) { + console.log(color); + } + }`); + + expect(messages).toEqual([ + `TestComponent.html(1, 19): Argument of type 'string | undefined' is not assignable to parameter of type 'string'.` + ]); + }); }); it('computes line and column offsets', () => { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts index ae6545a127..9130af7095 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts @@ -44,6 +44,7 @@ export function typescriptLibDts(): TestFile { call(...args: any[]): any; } declare interface Array { + [index: number]: T; length: number; } declare interface String { 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 ce6ba8fd1b..d55df81b3f 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 @@ -997,12 +997,13 @@ describe('type check blocks', () => { }); describe('config.strictSafeNavigationTypes', () => { - const TEMPLATE = `{{a?.b}} {{a?.method()}}`; + const TEMPLATE = `{{a?.b}} {{a?.method()}} {{a?.[0]}}`; it('should use undefined for safe navigation operations when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); expect(block).toContain('(null as any ? (((ctx).a))!.method() : undefined)'); expect(block).toContain('(null as any ? (((ctx).a))!.b : undefined)'); + expect(block).toContain('(null as any ? (((ctx).a))![0] : undefined)'); }); it('should use an \'any\' type for safe navigation operations when disabled', () => { const DISABLED_CONFIG: @@ -1010,15 +1011,17 @@ describe('type check blocks', () => { const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); expect(block).toContain('((((ctx).a))!.method() as any)'); expect(block).toContain('((((ctx).a))!.b as any)'); + expect(block).toContain('(((((ctx).a))![0] as any)'); }); }); describe('config.strictSafeNavigationTypes (View Engine bug emulation)', () => { - const TEMPLATE = `{{a.method()?.b}} {{a()?.method()}}`; + const TEMPLATE = `{{a.method()?.b}} {{a()?.method()}} {{a.method()?.[0]}}`; it('should check the presence of a property/method on the receiver when enabled', () => { const block = tcb(TEMPLATE, DIRECTIVES); expect(block).toContain('(null as any ? ((((ctx).a)).method())!.b : undefined)'); expect(block).toContain('(null as any ? ((ctx).a())!.method() : undefined)'); + expect(block).toContain('(null as any ? ((((ctx).a)).method())![0] : undefined)'); }); it('should not check the presence of a property/method on the receiver when disabled', () => { const DISABLED_CONFIG: @@ -1026,6 +1029,7 @@ describe('type check blocks', () => { const block = tcb(TEMPLATE, DIRECTIVES, DISABLED_CONFIG); expect(block).toContain('(((((ctx).a)).method()) as any).b'); expect(block).toContain('(((ctx).a()) as any).method()'); + expect(block).toContain('(((((ctx).a)).method()) as any)[0]'); }); }); diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/GOLDEN_PARTIAL.js new file mode 100644 index 0000000000..53a62c187e --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/GOLDEN_PARTIAL.js @@ -0,0 +1,65 @@ +/**************************************************************************************************** + * PARTIAL FILE: safe_keyed_read.js + ****************************************************************************************************/ +import { Component, NgModule } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyApp { + constructor() { + this.unknownNames = null; + this.knownNames = [['Frodo', 'Bilbo']]; + this.species = null; + this.keys = null; + this.speciesMap = { key: 'unknown' }; + } +} +MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: ` + + Hello, {{ knownNames?.[0]?.[1] }}! + You are a Balrog: {{ species?.[0]?.[1]?.[2]?.[3]?.[4]?.[5] || 'unknown' }} + You are an Elf: {{ speciesMap?.[keys?.[0] ?? 'key'] }} + You are an Orc: {{ speciesMap?.['key'] }} + +`, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{ + type: Component, + args: [{ + template: ` + + Hello, {{ knownNames?.[0]?.[1] }}! + You are a Balrog: {{ species?.[0]?.[1]?.[2]?.[3]?.[4]?.[5] || 'unknown' }} + You are an Elf: {{ speciesMap?.[keys?.[0] ?? 'key'] }} + You are an Orc: {{ speciesMap?.['key'] }} + +` + }] + }] }); +export class MyModule { +} +MyModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule }); +MyModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, declarations: [MyApp] }); +MyModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, decorators: [{ + type: NgModule, + args: [{ declarations: [MyApp] }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: safe_keyed_read.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyApp { + unknownNames: string[] | null; + knownNames: string[][]; + species: null; + keys: null; + speciesMap: Record; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} +export declare class MyModule { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵmod: i0.ɵɵNgModuleDeclaration; + static ɵinj: i0.ɵɵInjectorDeclaration; +} + diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/TEST_CASES.json new file mode 100644 index 0000000000..d81f51d15d --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/TEST_CASES.json @@ -0,0 +1,22 @@ +{ + "$schema": "../../test_case_schema.json", + "cases": [ + { + "description": "should handle safe keyed reads inside templates", + "inputFiles": [ + "safe_keyed_read.ts" + ], + "expectations": [ + { + "files": [ + { + "expected": "safe_keyed_read_template.js", + "generated": "safe_keyed_read.js" + } + ], + "failureMessage": "Incorrect template" + } + ] + } + ] +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/safe_keyed_read.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/safe_keyed_read.ts new file mode 100644 index 0000000000..12bfd43564 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/safe_keyed_read.ts @@ -0,0 +1,23 @@ +import {Component, NgModule} from '@angular/core'; + +@Component({ + template: ` + + Hello, {{ knownNames?.[0]?.[1] }}! + You are a Balrog: {{ species?.[0]?.[1]?.[2]?.[3]?.[4]?.[5] || 'unknown' }} + You are an Elf: {{ speciesMap?.[keys?.[0] ?? 'key'] }} + You are an Orc: {{ speciesMap?.['key'] }} + +` +}) +export class MyApp { + unknownNames: string[]|null = null; + knownNames: string[][] = [['Frodo', 'Bilbo']]; + species = null; + keys = null; + speciesMap: Record = {key: 'unknown'}; +} + +@NgModule({declarations: [MyApp]}) +export class MyModule { +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/safe_keyed_read_template.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/safe_keyed_read_template.js new file mode 100644 index 0000000000..b22f78118f --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/safe_keyed_read_template.js @@ -0,0 +1,13 @@ +template: function MyApp_Template(rf, ctx) { + if (rf & 1) { + i0.ɵɵelementStart(0, "span", 0); + i0.ɵɵtext(1); + i0.ɵɵelementEnd(); + } + if (rf & 2) { + let $tmp_0_0$; + i0.ɵɵproperty("title", "Your last name is " + ((ctx.unknownNames == null ? null : ctx.unknownNames[0]) || "unknown")); + i0.ɵɵadvance(1); + i0.ɵɵtextInterpolate4(" Hello, ", ctx.knownNames == null ? null : ctx.knownNames[0] == null ? null : ctx.knownNames[0][1], "! You are a Balrog: ", (ctx.species == null ? null : ctx.species[0] == null ? null : ctx.species[0][1] == null ? null : ctx.species[0][1][2] == null ? null : ctx.species[0][1][2][3] == null ? null : ctx.species[0][1][2][3][4] == null ? null : ctx.species[0][1][2][3][4][5]) || "unknown", " You are an Elf: ", ctx.speciesMap == null ? null : ctx.speciesMap[($tmp_0_0$ = ctx.keys == null ? null : ctx.keys[0]) !== null && $tmp_0_0$ !== undefined ? $tmp_0_0$ : "key"], " You are an Orc: ", ctx.speciesMap == null ? null : ctx.speciesMap["key"], " "); + } +} diff --git a/packages/compiler/src/compiler_util/expression_converter.ts b/packages/compiler/src/compiler_util/expression_converter.ts index eb650f791c..476651660f 100644 --- a/packages/compiler/src/compiler_util/expression_converter.ts +++ b/packages/compiler/src/compiler_util/expression_converter.ts @@ -478,12 +478,13 @@ class _AstToIrVisitor implements cdAst.AstVisitor { return this.convertSafeAccess(ast, leftMostSafe, mode); } else { return convertToStatementIfNeeded( - mode, this._visit(ast.obj, _Mode.Expression).key(this._visit(ast.key, _Mode.Expression))); + mode, + this._visit(ast.receiver, _Mode.Expression).key(this._visit(ast.key, _Mode.Expression))); } } visitKeyedWrite(ast: cdAst.KeyedWrite, mode: _Mode): any { - const obj: o.Expression = this._visit(ast.obj, _Mode.Expression); + const obj: o.Expression = this._visit(ast.receiver, _Mode.Expression); const key: o.Expression = this._visit(ast.key, _Mode.Expression); const value: o.Expression = this._visit(ast.value, _Mode.Expression); return convertToStatementIfNeeded(mode, obj.key(key).set(value)); @@ -627,6 +628,10 @@ class _AstToIrVisitor implements cdAst.AstVisitor { return this.convertSafeAccess(ast, this.leftMostSafeNode(ast), mode); } + visitSafeKeyedRead(ast: cdAst.SafeKeyedRead, mode: _Mode): any { + return this.convertSafeAccess(ast, this.leftMostSafeNode(ast), mode); + } + visitAll(asts: cdAst.AST[], mode: _Mode): any { return asts.map(ast => this._visit(ast, mode)); } @@ -643,7 +648,8 @@ class _AstToIrVisitor implements cdAst.AstVisitor { } private convertSafeAccess( - ast: cdAst.AST, leftMostSafe: cdAst.SafeMethodCall|cdAst.SafePropertyRead, mode: _Mode): any { + ast: cdAst.AST, leftMostSafe: cdAst.SafeMethodCall|cdAst.SafePropertyRead|cdAst.SafeKeyedRead, + mode: _Mode): any { // If the expression contains a safe access node on the left it needs to be converted to // an expression that guards the access to the member by checking the receiver for blank. As // execution proceeds from left to right, the left most part of the expression must be guarded @@ -707,6 +713,11 @@ class _AstToIrVisitor implements cdAst.AstVisitor { leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.nameSpan, leftMostSafe.receiver, leftMostSafe.name, leftMostSafe.args, leftMostSafe.argumentSpan)); + } else if (leftMostSafe instanceof cdAst.SafeKeyedRead) { + this._nodeMap.set( + leftMostSafe, + new cdAst.KeyedRead( + leftMostSafe.span, leftMostSafe.sourceSpan, leftMostSafe.receiver, leftMostSafe.key)); } else { this._nodeMap.set( leftMostSafe, @@ -756,7 +767,8 @@ class _AstToIrVisitor implements cdAst.AstVisitor { // a == null ? null : a.c.b.c?.d.e // then to: // a == null ? null : a.b.c == null ? null : a.b.c.d.e - private leftMostSafeNode(ast: cdAst.AST): cdAst.SafePropertyRead|cdAst.SafeMethodCall { + private leftMostSafeNode(ast: cdAst.AST): cdAst.SafePropertyRead|cdAst.SafeMethodCall + |cdAst.SafeKeyedRead { const visit = (visitor: cdAst.AstVisitor, ast: cdAst.AST): any => { return (this._nodeMap.get(ast) || ast).visit(visitor); }; @@ -786,7 +798,7 @@ class _AstToIrVisitor implements cdAst.AstVisitor { return null; }, visitKeyedRead(ast: cdAst.KeyedRead) { - return visit(this, ast.obj); + return visit(this, ast.receiver); }, visitKeyedWrite(ast: cdAst.KeyedWrite) { return null; @@ -826,6 +838,9 @@ class _AstToIrVisitor implements cdAst.AstVisitor { }, visitSafePropertyRead(ast: cdAst.SafePropertyRead) { return visit(this, ast.receiver) || ast; + }, + visitSafeKeyedRead(ast: cdAst.SafeKeyedRead) { + return visit(this, ast.receiver) || ast; } }); } @@ -906,6 +921,9 @@ class _AstToIrVisitor implements cdAst.AstVisitor { }, visitSafePropertyRead(ast: cdAst.SafePropertyRead) { return false; + }, + visitSafeKeyedRead(ast: cdAst.SafeKeyedRead) { + return false; } }); } diff --git a/packages/compiler/src/expression_parser/ast.ts b/packages/compiler/src/expression_parser/ast.ts index fd0b77584f..51b5ced0e7 100644 --- a/packages/compiler/src/expression_parser/ast.ts +++ b/packages/compiler/src/expression_parser/ast.ts @@ -156,7 +156,8 @@ export class SafePropertyRead extends ASTWithName { } export class KeyedRead extends AST { - constructor(span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public obj: AST, public key: AST) { + constructor( + span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public key: AST) { super(span, sourceSpan); } visit(visitor: AstVisitor, context: any = null): any { @@ -164,9 +165,19 @@ export class KeyedRead extends AST { } } +export class SafeKeyedRead extends AST { + constructor( + span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public key: AST) { + super(span, sourceSpan); + } + visit(visitor: AstVisitor, context: any = null): any { + return visitor.visitSafeKeyedRead(this, context); + } +} + export class KeyedWrite extends AST { constructor( - span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public obj: AST, public key: AST, + span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public receiver: AST, public key: AST, public value: AST) { super(span, sourceSpan); } @@ -453,6 +464,7 @@ export interface AstVisitor { visitQuote(ast: Quote, context: any): any; visitSafeMethodCall(ast: SafeMethodCall, context: any): any; visitSafePropertyRead(ast: SafePropertyRead, context: any): any; + visitSafeKeyedRead(ast: SafeKeyedRead, context: any): any; visitASTWithSource?(ast: ASTWithSource, context: any): any; /** * This function is optionally defined to allow classes that implement this @@ -501,11 +513,11 @@ export class RecursiveAstVisitor implements AstVisitor { this.visitAll(ast.expressions, context); } visitKeyedRead(ast: KeyedRead, context: any): any { - this.visit(ast.obj, context); + this.visit(ast.receiver, context); this.visit(ast.key, context); } visitKeyedWrite(ast: KeyedWrite, context: any): any { - this.visit(ast.obj, context); + this.visit(ast.receiver, context); this.visit(ast.key, context); this.visit(ast.value, context); } @@ -540,6 +552,10 @@ export class RecursiveAstVisitor implements AstVisitor { this.visit(ast.receiver, context); this.visitAll(ast.args, context); } + visitSafeKeyedRead(ast: SafeKeyedRead, context: any): any { + this.visit(ast.receiver, context); + this.visit(ast.key, context); + } visitQuote(ast: Quote, context: any): any {} // This is not part of the AstVisitor interface, just a helper method visitAll(asts: AST[], context: any): any { @@ -644,12 +660,13 @@ export class AstTransformer implements AstVisitor { } visitKeyedRead(ast: KeyedRead, context: any): AST { - return new KeyedRead(ast.span, ast.sourceSpan, ast.obj.visit(this), ast.key.visit(this)); + return new KeyedRead(ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.key.visit(this)); } visitKeyedWrite(ast: KeyedWrite, context: any): AST { return new KeyedWrite( - ast.span, ast.sourceSpan, ast.obj.visit(this), ast.key.visit(this), ast.value.visit(this)); + ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.key.visit(this), + ast.value.visit(this)); } visitAll(asts: any[]): any[] { @@ -668,6 +685,11 @@ export class AstTransformer implements AstVisitor { return new Quote( ast.span, ast.sourceSpan, ast.prefix, ast.uninterpretedExpression, ast.location); } + + visitSafeKeyedRead(ast: SafeKeyedRead, context: any): AST { + return new SafeKeyedRead( + ast.span, ast.sourceSpan, ast.receiver.visit(this), ast.key.visit(this)); + } } // A transformer that only creates new nodes if the transformer makes a change or @@ -822,19 +844,19 @@ export class AstMemoryEfficientTransformer implements AstVisitor { } visitKeyedRead(ast: KeyedRead, context: any): AST { - const obj = ast.obj.visit(this); + const obj = ast.receiver.visit(this); const key = ast.key.visit(this); - if (obj !== ast.obj || key !== ast.key) { + if (obj !== ast.receiver || key !== ast.key) { return new KeyedRead(ast.span, ast.sourceSpan, obj, key); } return ast; } visitKeyedWrite(ast: KeyedWrite, context: any): AST { - const obj = ast.obj.visit(this); + const obj = ast.receiver.visit(this); const key = ast.key.visit(this); const value = ast.value.visit(this); - if (obj !== ast.obj || key !== ast.key || value !== ast.value) { + if (obj !== ast.receiver || key !== ast.key || value !== ast.value) { return new KeyedWrite(ast.span, ast.sourceSpan, obj, key, value); } return ast; @@ -863,6 +885,15 @@ export class AstMemoryEfficientTransformer implements AstVisitor { visitQuote(ast: Quote, context: any): AST { return ast; } + + visitSafeKeyedRead(ast: SafeKeyedRead, context: any): AST { + const obj = ast.receiver.visit(this); + const key = ast.key.visit(this); + if (obj !== ast.receiver || key !== ast.key) { + return new SafeKeyedRead(ast.span, ast.sourceSpan, obj, key); + } + return ast; + } } // Bindings diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index 01dc54a1a9..bf25bf46dd 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -9,7 +9,7 @@ import * as chars from '../chars'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config'; -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 {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, SafeKeyedRead, SafeMethodCall, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, ThisReceiver, Unary, VariableBinding} from './ast'; import {EOF, isIdentifier, isQuote, Lexer, Token, TokenType} from './lexer'; export interface InterpolationPiece { @@ -822,24 +822,11 @@ export class _ParseAST { result = this.parseAccessMemberOrMethodCall(result, start, false); } else if (this.consumeOptionalOperator('?.')) { - result = this.parseAccessMemberOrMethodCall(result, start, true); - + result = this.consumeOptionalCharacter(chars.$LBRACKET) ? + this.parseKeyedReadOrWrite(result, start, true) : + this.parseAccessMemberOrMethodCall(result, start, true); } else if (this.consumeOptionalCharacter(chars.$LBRACKET)) { - this.withContext(ParseContextFlags.Writable, () => { - this.rbracketsExpected++; - const key = this.parsePipe(); - if (key instanceof EmptyExpr) { - this.error(`Key access cannot be empty`); - } - this.rbracketsExpected--; - this.expectCharacter(chars.$RBRACKET); - if (this.consumeOptionalOperator('=')) { - const value = this.parseConditional(); - result = new KeyedWrite(this.span(start), this.sourceSpan(start), result, key, value); - } else { - result = new KeyedRead(this.span(start), this.sourceSpan(start), result, key); - } - }); + result = this.parseKeyedReadOrWrite(result, start, false); } else if (this.consumeOptionalCharacter(chars.$LPAREN)) { this.rparensExpected++; const args = this.parseCallArguments(); @@ -954,7 +941,7 @@ export class _ParseAST { return new LiteralMap(this.span(start), this.sourceSpan(start), keys, values); } - parseAccessMemberOrMethodCall(receiver: AST, start: number, isSafe: boolean = false): AST { + parseAccessMemberOrMethodCall(receiver: AST, start: number, isSafe: boolean): AST { const nameStart = this.inputIndex; const id = this.withContext(ParseContextFlags.Writable, () => { const id = this.expectIdentifierOrKeyword() ?? ''; @@ -1095,6 +1082,31 @@ export class _ParseAST { return new TemplateBindingParseResult(bindings, [] /* warnings */, this.errors); } + parseKeyedReadOrWrite(receiver: AST, start: number, isSafe: boolean): AST { + return this.withContext(ParseContextFlags.Writable, () => { + this.rbracketsExpected++; + const key = this.parsePipe(); + if (key instanceof EmptyExpr) { + this.error(`Key access cannot be empty`); + } + this.rbracketsExpected--; + this.expectCharacter(chars.$RBRACKET); + if (this.consumeOptionalOperator('=')) { + if (isSafe) { + this.error('The \'?.\' operator cannot be used in the assignment'); + } else { + const value = this.parseConditional(); + return new KeyedWrite(this.span(start), this.sourceSpan(start), receiver, key, value); + } + } else { + return isSafe ? new SafeKeyedRead(this.span(start), this.sourceSpan(start), receiver, key) : + new KeyedRead(this.span(start), this.sourceSpan(start), receiver, key); + } + + return new EmptyExpr(this.span(start), this.sourceSpan(start)); + }); + } + /** * Parse a directive keyword, followed by a mandatory expression. * For example, "of items", "trackBy: func". @@ -1333,6 +1345,8 @@ class SimpleExpressionChecker implements AstVisitor { visitChain(ast: Chain, context: any) {} visitQuote(ast: Quote, context: any) {} + + visitSafeKeyedRead(ast: SafeKeyedRead, context: any) {} } /** diff --git a/packages/compiler/test/expression_parser/lexer_spec.ts b/packages/compiler/test/expression_parser/lexer_spec.ts index 6946ff3d84..4bd2a638a4 100644 --- a/packages/compiler/test/expression_parser/lexer_spec.ts +++ b/packages/compiler/test/expression_parser/lexer_spec.ts @@ -127,6 +127,14 @@ function expectErrorToken(token: Token, index: any, end: number, message: string expectCharacterToken(tokens[3], 3, 4, ']'); }); + it('should tokenize a safe indexed operator', () => { + const tokens: number[] = lex('j?.[k]'); + expect(tokens.length).toBe(5); + expectOperatorToken(tokens[1], 1, 3, '?.'); + expectCharacterToken(tokens[2], 3, 4, '['); + expectCharacterToken(tokens[4], 5, 6, ']'); + }); + it('should tokenize numbers', () => { const tokens: number[] = lex('88'); expect(tokens.length).toEqual(1); diff --git a/packages/compiler/test/expression_parser/parser_spec.ts b/packages/compiler/test/expression_parser/parser_spec.ts index f813421059..5d9be92115 100644 --- a/packages/compiler/test/expression_parser/parser_spec.ts +++ b/packages/compiler/test/expression_parser/parser_spec.ts @@ -215,9 +215,16 @@ describe('parser', () => { describe('keyed read', () => { it('should parse keyed reads', () => { - checkAction('a["a"]'); - checkAction('this.a["a"]', 'a["a"]'); - checkAction('a.a["a"]'); + checkBinding('a["a"]'); + checkBinding('this.a["a"]', 'a["a"]'); + checkBinding('a.a["a"]'); + }); + + it('should parse safe keyed reads', () => { + checkBinding('a?.["a"]'); + checkBinding('this.a?.["a"]', 'a?.["a"]'); + checkBinding('a.a?.["a"]'); + checkBinding('a.a?.["a" | foo]', 'a.a?.[("a" | foo)]'); }); describe('malformed keyed reads', () => { @@ -248,6 +255,10 @@ describe('parser', () => { checkAction('a.a["a"] = 1 + 2'); }); + it('should report on safe keyed writes', () => { + expectActionError('a?.["a"] = 123', 'cannot be used in the assignment'); + }); + describe('malformed keyed writes', () => { it('should recover on empty rvalues', () => { checkActionWithError('a["a"] = ', 'a["a"] = ', 'Unexpected end of expression'); diff --git a/packages/compiler/test/expression_parser/utils/unparser.ts b/packages/compiler/test/expression_parser/utils/unparser.ts index 04f96b30b1..ba541ed483 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, ThisReceiver, 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, SafeKeyedRead, SafeMethodCall, SafePropertyRead, ThisReceiver, Unary} from '../../../src/expression_parser/ast'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../../../src/ml_parser/interpolation_config'; class Unparser implements AstVisitor { @@ -101,14 +101,14 @@ class Unparser implements AstVisitor { } visitKeyedRead(ast: KeyedRead, context: any) { - this._visit(ast.obj); + this._visit(ast.receiver); this._expression += '['; this._visit(ast.key); this._expression += ']'; } visitKeyedWrite(ast: KeyedWrite, context: any) { - this._visit(ast.obj); + this._visit(ast.receiver); this._expression += '['; this._visit(ast.key); this._expression += '] = '; @@ -193,6 +193,13 @@ class Unparser implements AstVisitor { this._expression += `${ast.prefix}:${ast.uninterpretedExpression}`; } + visitSafeKeyedRead(ast: SafeKeyedRead, context: any) { + this._visit(ast.receiver); + this._expression += '?.['; + this._visit(ast.key); + this._expression += ']'; + } + private _visit(ast: AST) { ast.visit(this); } diff --git a/packages/compiler/test/expression_parser/utils/validator.ts b/packages/compiler/test/expression_parser/utils/validator.ts index 0023fa77dd..81b330d68c 100644 --- a/packages/compiler/test/expression_parser/utils/validator.ts +++ b/packages/compiler/test/expression_parser/utils/validator.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeMethodCall, SafePropertyRead, Unary} from '../../../src/expression_parser/ast'; +import {AST, Binary, BindingPipe, Chain, Conditional, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralPrimitive, MethodCall, ParseSpan, PrefixNot, PropertyRead, PropertyWrite, Quote, RecursiveAstVisitor, SafeKeyedRead, SafeMethodCall, SafePropertyRead, Unary} from '../../../src/expression_parser/ast'; import {unparse} from './unparser'; @@ -113,6 +113,10 @@ class ASTValidator extends RecursiveAstVisitor { visitSafePropertyRead(ast: SafePropertyRead, context: any): any { this.validate(ast, () => super.visitSafePropertyRead(ast, context)); } + + visitSafeKeyedRead(ast: SafeKeyedRead, context: any): any { + this.validate(ast, () => super.visitSafeKeyedRead(ast, context)); + } } function inSpan(span: ParseSpan, parentSpan: ParseSpan|undefined): parentSpan is ParseSpan { diff --git a/packages/compiler/test/render3/util/expression.ts b/packages/compiler/test/render3/util/expression.ts index a0af48ba91..0eb621d078 100644 --- a/packages/compiler/test/render3/util/expression.ts +++ b/packages/compiler/test/render3/util/expression.ts @@ -114,6 +114,10 @@ class ExpressionSourceHumanizer extends e.RecursiveAstVisitor implements t.Visit this.recordAst(ast); super.visitQuote(ast, null); } + visitSafeKeyedRead(ast: e.SafeKeyedRead) { + this.recordAst(ast); + super.visitSafeKeyedRead(ast, null); + } visitTemplate(ast: t.Template) { t.visitAll(this, ast.children); diff --git a/packages/compiler/test/template_parser/util/expression.ts b/packages/compiler/test/template_parser/util/expression.ts index bad905d9ff..54434e56d8 100644 --- a/packages/compiler/test/template_parser/util/expression.ts +++ b/packages/compiler/test/template_parser/util/expression.ts @@ -114,6 +114,10 @@ class ExpressionSourceHumanizer extends e.RecursiveAstVisitor implements t.Templ this.recordAst(ast); super.visitQuote(ast, null); } + visitSafeKeyedRead(ast: e.SafeKeyedRead) { + this.recordAst(ast); + super.visitSafeKeyedRead(ast, null); + } visitNgContent(ast: t.NgContentAst) {} visitEmbeddedTemplate(ast: t.EmbeddedTemplateAst) { diff --git a/packages/core/test/acceptance/integration_spec.ts b/packages/core/test/acceptance/integration_spec.ts index e01aa0ab36..9ffb95cdd6 100644 --- a/packages/core/test/acceptance/integration_spec.ts +++ b/packages/core/test/acceptance/integration_spec.ts @@ -1955,6 +1955,36 @@ describe('acceptance integration tests', () => { expect(content).toContain(``); }); + it('should handle safe keyed reads inside templates', () => { + @Component({ + template: ` + + Hello, {{ knownNames?.[0]?.[1] }}! + You are a Balrog: {{ species?.[0]?.[1]?.[2]?.[3]?.[4]?.[5] || 'unknown' }} + You are an Elf: {{ speciesMap?.[keys?.[0] ?? 'key'] }} + You are an Orc: {{ speciesMap?.['key'] }} + + ` + }) + class App { + unknownNames: string[]|null = null; + knownNames: string[][] = [['Frodo', 'Bilbo']]; + species = null; + keys = null; + speciesMap: Record = {key: 'unknown'}; + } + + TestBed.configureTestingModule({declarations: [App]}); + const fixture = TestBed.createComponent(App); + fixture.detectChanges(); + const content = fixture.nativeElement.innerHTML; + + expect(content).toContain('Hello, Bilbo!'); + expect(content).toContain('You are a Balrog: unknown'); + expect(content).toContain('You are an Elf: unknown'); + expect(content).toContain(``); + }); + it('should handle nullish coalescing inside host bindings', () => { const logs: string[] = []; diff --git a/packages/language-service/ivy/test/legacy/template_target_spec.ts b/packages/language-service/ivy/test/legacy/template_target_spec.ts index 3099c851a3..6b8e911f57 100644 --- a/packages/language-service/ivy/test/legacy/template_target_spec.ts +++ b/packages/language-service/ivy/test/legacy/template_target_spec.ts @@ -471,6 +471,15 @@ describe('getTargetAtPosition for expression AST', () => { expect(node).toBeInstanceOf(e.KeyedRead); }); + it('should locate safe keyed read', () => { + const {errors, nodes, position} = parse(`{{ foo?.['bar']¦ }}`); + expect(errors).toBe(null); + const {context} = getTargetAtPosition(nodes, position)!; + const {node} = context as SingleNodeTarget; + expect(isExpressionNode(node!)).toBe(true); + expect(node).toBeInstanceOf(e.SafeKeyedRead); + }); + it('should locate property write', () => { const {errors, nodes, position} = parse(`
`); expect(errors).toBe(null); diff --git a/packages/language-service/src/expression_type.ts b/packages/language-service/src/expression_type.ts index 064a2caffd..f4e1972b79 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, ThisReceiver, 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, SafeKeyedRead, SafeMethodCall, SafePropertyRead, ThisReceiver, Unary} from '@angular/compiler'; import {createDiagnostic, Diagnostic} from './diagnostic_messages'; import {BuiltinType, Signature, Symbol, SymbolQuery, SymbolTable} from './symbols'; @@ -271,7 +271,7 @@ export class AstType implements AstVisitor { } visitKeyedRead(ast: KeyedRead): Symbol { - const targetType = this.getType(ast.obj); + const targetType = this.getType(ast.receiver); const keyType = this.getType(ast.key); const result = targetType.indexed( keyType, ast.key instanceof LiteralPrimitive ? ast.key.value : undefined); @@ -379,6 +379,14 @@ export class AstType implements AstVisitor { return this.resolvePropertyRead(this.query.getNonNullableType(this.getType(ast.receiver)), ast); } + visitSafeKeyedRead(ast: SafeKeyedRead): Symbol { + const targetType = this.query.getNonNullableType(this.getType(ast.receiver)); + const keyType = this.getType(ast.key); + const result = targetType.indexed( + keyType, ast.key instanceof LiteralPrimitive ? ast.key.value : undefined); + return result || this.anyType; + } + /** * Gets the source of an expession AST. * The AST's sourceSpan is relative to the start of the template source code, which is contained diff --git a/packages/language-service/src/expressions.ts b/packages/language-service/src/expressions.ts index 5af34f8ba3..be315c9bbc 100644 --- a/packages/language-service/src/expressions.ts +++ b/packages/language-service/src/expressions.ts @@ -74,6 +74,7 @@ export function getExpressionCompletions( result = undefined; }, visitKeyedRead(_ast) {}, + visitSafeKeyedRead(_ast) {}, visitKeyedWrite(_ast) {}, visitLiteralArray(_ast) {}, visitLiteralMap(_ast) {}, @@ -168,6 +169,7 @@ export function getExpressionSymbol( visitThisReceiver(_ast) {}, visitInterpolation(_ast) {}, visitKeyedRead(_ast) {}, + visitSafeKeyedRead(_ast) {}, visitKeyedWrite(_ast) {}, visitLiteralArray(_ast) {}, visitLiteralMap(_ast) {},