Skip to content

Commit 06e161f

Browse files
crisbetothePunderWoman
crisbeto
authored and
thePunderWoman
committed
fix(compiler): incorrect code when non-null assertion is used after a safe access (#48801)
Fixes that the expression converter was producing code that throws a runtime error if a non-null assertion is used as a part of a safe read, write or call. Fixes #48742. PR Close #48801
1 parent fd539a2 commit 06e161f

File tree

6 files changed

+126
-1
lines changed

6 files changed

+126
-1
lines changed

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/GOLDEN_PARTIAL.js‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,3 +165,60 @@ export declare class MyModule {
165165
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
166166
}
167167

168+
/****************************************************************************************************
169+
* PARTIAL FILE: safe_access_non_null.js
170+
****************************************************************************************************/
171+
import { Component, NgModule } from '@angular/core';
172+
import * as i0 from "@angular/core";
173+
export class MyApp {
174+
constructor() {
175+
this.val = null;
176+
}
177+
foo(val) {
178+
return val;
179+
}
180+
}
181+
MyApp.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, deps: [], target: i0.ɵɵFactoryTarget.Component });
182+
MyApp.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyApp, selector: "ng-component", ngImport: i0, template: `
183+
{{ val?.foo!.bar }}
184+
{{ val?.[0].foo!.bar }}
185+
{{ foo(val)?.foo!.bar }}
186+
{{ $any(val)?.foo!.bar }}
187+
`, isInline: true });
188+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyApp, decorators: [{
189+
type: Component,
190+
args: [{
191+
template: `
192+
{{ val?.foo!.bar }}
193+
{{ val?.[0].foo!.bar }}
194+
{{ foo(val)?.foo!.bar }}
195+
{{ $any(val)?.foo!.bar }}
196+
`
197+
}]
198+
}] });
199+
export class MyModule {
200+
}
201+
MyModule.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, deps: [], target: i0.ɵɵFactoryTarget.NgModule });
202+
MyModule.ɵmod = i0.ɵɵngDeclareNgModule({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, declarations: [MyApp] });
203+
MyModule.ɵinj = i0.ɵɵngDeclareInjector({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule });
204+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyModule, decorators: [{
205+
type: NgModule,
206+
args: [{ declarations: [MyApp] }]
207+
}] });
208+
209+
/****************************************************************************************************
210+
* PARTIAL FILE: safe_access_non_null.d.ts
211+
****************************************************************************************************/
212+
import * as i0 from "@angular/core";
213+
export declare class MyApp {
214+
val: any;
215+
foo(val: unknown): unknown;
216+
static ɵfac: i0.ɵɵFactoryDeclaration<MyApp, never>;
217+
static ɵcmp: i0.ɵɵComponentDeclaration<MyApp, "ng-component", never, {}, {}, never, never, false, never>;
218+
}
219+
export declare class MyModule {
220+
static ɵfac: i0.ɵɵFactoryDeclaration<MyModule, never>;
221+
static ɵmod: i0.ɵɵNgModuleDeclaration<MyModule, [typeof MyApp], never, never>;
222+
static ɵinj: i0.ɵɵInjectorDeclaration<MyModule>;
223+
}
224+

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/TEST_CASES.json‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,23 @@
5151
"failureMessage": "Incorrect template"
5252
}
5353
]
54+
},
55+
{
56+
"description": "should handle non-null assertions after a safe access",
57+
"inputFiles": [
58+
"safe_access_non_null.ts"
59+
],
60+
"expectations": [
61+
{
62+
"files": [
63+
{
64+
"expected": "safe_access_non_null_template.js",
65+
"generated": "safe_access_non_null.js"
66+
}
67+
],
68+
"failureMessage": "Incorrect template"
69+
}
70+
]
5471
}
5572
]
5673
}

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/safe_access_non_null.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import {Component, NgModule} from '@angular/core';
2+
3+
@Component({
4+
template: `
5+
{{ val?.foo!.bar }}
6+
{{ val?.[0].foo!.bar }}
7+
{{ foo(val)?.foo!.bar }}
8+
{{ $any(val)?.foo!.bar }}
9+
`
10+
})
11+
export class MyApp {
12+
val: any = null;
13+
14+
foo(val: unknown) {
15+
return val;
16+
}
17+
}
18+
19+
@NgModule({declarations: [MyApp]})
20+
export class MyModule {
21+
}

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler/safe_access/safe_access_non_null_template.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
template: function MyApp_Template(rf, $ctx$) {
2+
if (rf & 1) {
3+
i0.ɵɵtext(0);
4+
}
5+
if (rf & 2) {
6+
let $tmp_0_0$;
7+
i0.ɵɵtextInterpolate4(" ", $ctx$.val == null ? null : $ctx$.val.foo.bar, " ", $ctx$.val == null ? null : $ctx$.val[0].foo.bar, " ", ($tmp_0_0$ = $ctx$.foo($ctx$.val)) == null ? null : $tmp_0_0$.foo.bar, " ", ($tmp_0_0$ = $ctx$.val) == null ? null : $tmp_0_0$.foo.bar, " ");
8+
}
9+
}

‎packages/compiler/src/compiler_util/expression_converter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
717717
return null;
718718
},
719719
visitNonNullAssert(ast: cdAst.NonNullAssert) {
720-
return null;
720+
return visit(this, ast.expression);
721721
},
722722
visitPropertyRead(ast: cdAst.PropertyRead) {
723723
return visit(this, ast.receiver);

‎packages/core/test/acceptance/integration_spec.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2332,6 +2332,27 @@ describe('acceptance integration tests', () => {
23322332
expect(strong.textContent).toBe('Hello Frodo');
23332333
});
23342334

2335+
it('should not throw for a non-null assertion after a safe access', () => {
2336+
@Component({
2337+
template: `
2338+
{{ val?.foo!.bar }}
2339+
{{ val?.[0].foo!.bar }}
2340+
{{ foo(val)?.foo!.bar }}
2341+
{{ $any(val)?.foo!.bar }}
2342+
`,
2343+
})
2344+
class Comp {
2345+
val: any = null;
2346+
2347+
foo(val: unknown) {
2348+
return val;
2349+
}
2350+
}
2351+
2352+
TestBed.configureTestingModule({declarations: [Comp]});
2353+
expect(() => TestBed.createComponent(Comp).detectChanges()).not.toThrow();
2354+
});
2355+
23352356
describe('tView.firstUpdatePass', () => {
23362357
function isFirstUpdatePass() {
23372358
const lView = getLView();

0 commit comments

Comments
 (0)