Skip to content

Commit ca55b3d

Browse files
crisbetoAndrewKushnir
authored andcommitted
fix(compiler-cli): generate valid TS 5.6 type checking code (angular#57303)
Currently in some scenarios the compiler generates code like `null as any ? foo : bar` which will be invalid with [an upcoming TypeScript change](https://devblogs.microsoft.com/typescript/announcing-typescript-5-6-beta/#disallowed-nullish-and-truthy-checks). These changes switch to generating `0 as any` which is exempt from the change. **Note:** I'm not starting the work to fully get us on TS 5.6 until the 18.2 release comes out, but this change is necessary to unblock an internal team. PR Close angular#57303
1 parent 0761e9a commit ca55b3d

File tree

4 files changed

+55
-26
lines changed

4 files changed

+55
-26
lines changed

packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,19 @@ import {TypeCheckingConfig} from '../api';
4040
import {addParseSpanInfo, wrapForDiagnostics, wrapForTypeChecker} from './diagnostics';
4141
import {tsCastToAny, tsNumericExpression} from './ts_util';
4242

43-
export const NULL_AS_ANY = ts.factory.createAsExpression(
44-
ts.factory.createNull(),
43+
/**
44+
* Expression that is cast to any. Currently represented as `0 as any`.
45+
*
46+
* Historically this expression was using `null as any`, but a newly-added check in TypeScript 5.6
47+
* (https://devblogs.microsoft.com/typescript/announcing-typescript-5-6-beta/#disallowed-nullish-and-truthy-checks)
48+
* started flagging it as always being nullish. Other options that were considered:
49+
* - `NaN as any` or `Infinity as any` - not used, because they don't work if the `noLib` compiler
50+
* option is enabled. Also they require more characters.
51+
* - Some flavor of function call, like `isNan(0) as any` - requires even more characters than the
52+
* NaN option and has the same issue with `noLib`.
53+
*/
54+
export const ANY_EXPRESSION = ts.factory.createAsExpression(
55+
ts.factory.createNumericLiteral('0'),
4556
ts.factory.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword),
4657
);
4758
const UNDEFINED = ts.factory.createIdentifier('undefined');
@@ -306,15 +317,21 @@ class AstTranslator implements AstVisitor {
306317
if (this.config.strictSafeNavigationTypes) {
307318
// Basically, the return here is either the type of the complete expression with a null-safe
308319
// property read, or `undefined`. So a ternary is used to create an "or" type:
309-
// "a?.b" becomes (null as any ? a!.b : undefined)
320+
// "a?.b" becomes (0 as any ? a!.b : undefined)
310321
// The type of this expression is (typeof a!.b) | undefined, which is exactly as desired.
311322
const expr = ts.factory.createPropertyAccessExpression(
312323
ts.factory.createNonNullExpression(receiver),
313324
ast.name,
314325
);
315326
addParseSpanInfo(expr, ast.nameSpan);
316327
node = ts.factory.createParenthesizedExpression(
317-
ts.factory.createConditionalExpression(NULL_AS_ANY, undefined, expr, undefined, UNDEFINED),
328+
ts.factory.createConditionalExpression(
329+
ANY_EXPRESSION,
330+
undefined,
331+
expr,
332+
undefined,
333+
UNDEFINED,
334+
),
318335
);
319336
} else if (VeSafeLhsInferenceBugDetector.veWillInferAnyFor(ast)) {
320337
// Emulate a View Engine bug where 'any' is inferred for the left-hand side of the safe
@@ -345,14 +362,20 @@ class AstTranslator implements AstVisitor {
345362

346363
// The form of safe property reads depends on whether strictness is in use.
347364
if (this.config.strictSafeNavigationTypes) {
348-
// "a?.[...]" becomes (null as any ? a![...] : undefined)
365+
// "a?.[...]" becomes (0 as any ? a![...] : undefined)
349366
const expr = ts.factory.createElementAccessExpression(
350367
ts.factory.createNonNullExpression(receiver),
351368
key,
352369
);
353370
addParseSpanInfo(expr, ast.sourceSpan);
354371
node = ts.factory.createParenthesizedExpression(
355-
ts.factory.createConditionalExpression(NULL_AS_ANY, undefined, expr, undefined, UNDEFINED),
372+
ts.factory.createConditionalExpression(
373+
ANY_EXPRESSION,
374+
undefined,
375+
expr,
376+
undefined,
377+
UNDEFINED,
378+
),
356379
);
357380
} else if (VeSafeLhsInferenceBugDetector.veWillInferAnyFor(ast)) {
358381
// "a?.[...]" becomes (a as any)[...]
@@ -420,14 +443,20 @@ class AstTranslator implements AstVisitor {
420443
args: ts.Expression[],
421444
): ts.Expression {
422445
if (this.config.strictSafeNavigationTypes) {
423-
// "a?.method(...)" becomes (null as any ? a!.method(...) : undefined)
446+
// "a?.method(...)" becomes (0 as any ? a!.method(...) : undefined)
424447
const call = ts.factory.createCallExpression(
425448
ts.factory.createNonNullExpression(expr),
426449
undefined,
427450
args,
428451
);
429452
return ts.factory.createParenthesizedExpression(
430-
ts.factory.createConditionalExpression(NULL_AS_ANY, undefined, call, undefined, UNDEFINED),
453+
ts.factory.createConditionalExpression(
454+
ANY_EXPRESSION,
455+
undefined,
456+
call,
457+
undefined,
458+
UNDEFINED,
459+
),
431460
);
432461
}
433462

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ import {
6969
} from './diagnostics';
7070
import {DomSchemaChecker} from './dom';
7171
import {Environment} from './environment';
72-
import {astToTypescript, NULL_AS_ANY} from './expression';
72+
import {astToTypescript, ANY_EXPRESSION} from './expression';
7373
import {OutOfBandDiagnosticRecorder} from './oob';
7474
import {
7575
tsCallMethod,
@@ -763,7 +763,7 @@ class TcbInvalidReferenceOp extends TcbOp {
763763

764764
override execute(): ts.Identifier {
765765
const id = this.tcb.allocateId();
766-
this.scope.addStatement(tsCreateVariable(id, NULL_AS_ANY));
766+
this.scope.addStatement(tsCreateVariable(id, ANY_EXPRESSION));
767767
return id;
768768
}
769769
}
@@ -2785,7 +2785,7 @@ class TcbExpressionTranslator {
27852785
this.tcb.oobRecorder.missingPipe(this.tcb.id, ast);
27862786

27872787
// Use an 'any' value to at least allow the rest of the expression to be checked.
2788-
pipe = NULL_AS_ANY;
2788+
pipe = ANY_EXPRESSION;
27892789
} else if (
27902790
pipeMeta.isExplicitlyDeferred &&
27912791
this.tcb.boundTarget.getEagerlyUsedPipes().includes(ast.name)
@@ -2795,7 +2795,7 @@ class TcbExpressionTranslator {
27952795
this.tcb.oobRecorder.deferredPipeUsedEagerly(this.tcb.id, ast);
27962796

27972797
// Use an 'any' value to at least allow the rest of the expression to be checked.
2798-
pipe = NULL_AS_ANY;
2798+
pipe = ANY_EXPRESSION;
27992799
} else {
28002800
// Use a variable declared as the pipe's type.
28012801
pipe = this.tcb.env.pipeInst(
@@ -2916,7 +2916,7 @@ function tcbCallTypeCtor(
29162916
} else {
29172917
// A type constructor is required to be called with all input properties, so any unset
29182918
// inputs are simply assigned a value of type `any` to ignore them.
2919-
return ts.factory.createPropertyAssignment(propertyName, NULL_AS_ANY);
2919+
return ts.factory.createPropertyAssignment(propertyName, ANY_EXPRESSION);
29202920
}
29212921
});
29222922

packages/compiler-cli/src/ngtsc/typecheck/test/span_comments_spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ describe('type check blocks diagnostics', () => {
8585
it('should annotate safe calls', () => {
8686
const TEMPLATE = `{{ method?.(a, b) }}`;
8787
expect(tcbWithSpans(TEMPLATE)).toContain(
88-
'((null as any ? (((this).method /*3,9*/) /*3,9*/)!(((this).a /*12,13*/) /*12,13*/, ((this).b /*15,16*/) /*15,16*/) : undefined) /*3,17*/)',
88+
'((0 as any ? (((this).method /*3,9*/) /*3,9*/)!(((this).a /*12,13*/) /*12,13*/, ((this).b /*15,16*/) /*15,16*/) : undefined) /*3,17*/)',
8989
);
9090
});
9191

@@ -141,21 +141,21 @@ describe('type check blocks diagnostics', () => {
141141
it('should annotate safe property access', () => {
142142
const TEMPLATE = `{{ a?.b }}`;
143143
expect(tcbWithSpans(TEMPLATE)).toContain(
144-
'(null as any ? (((this).a /*3,4*/) /*3,4*/)!.b /*6,7*/ : undefined) /*3,7*/',
144+
'(0 as any ? (((this).a /*3,4*/) /*3,4*/)!.b /*6,7*/ : undefined) /*3,7*/',
145145
);
146146
});
147147

148148
it('should annotate safe method calls', () => {
149149
const TEMPLATE = `{{ a?.method(b) }}`;
150150
expect(tcbWithSpans(TEMPLATE)).toContain(
151-
'((null as any ? (null as any ? (((this).a /*3,4*/) /*3,4*/)!.method /*6,12*/ : undefined) /*3,12*/!(((this).b /*13,14*/) /*13,14*/) : undefined) /*3,15*/)',
151+
'((0 as any ? (0 as any ? (((this).a /*3,4*/) /*3,4*/)!.method /*6,12*/ : undefined) /*3,12*/!(((this).b /*13,14*/) /*13,14*/) : undefined) /*3,15*/)',
152152
);
153153
});
154154

155155
it('should annotate safe keyed reads', () => {
156156
const TEMPLATE = `{{ a?.[0] }}`;
157157
expect(tcbWithSpans(TEMPLATE)).toContain(
158-
'(null as any ? (((this).a /*3,4*/) /*3,4*/)![0 /*7,8*/] /*3,9*/ : undefined) /*3,9*/',
158+
'(0 as any ? (((this).a /*3,4*/) /*3,4*/)![0 /*7,8*/] /*3,9*/ : undefined) /*3,9*/',
159159
);
160160
});
161161

packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ describe('type check blocks', () => {
157157
'const _ctor1: <T extends string = any>(init: Pick<i0.Dir<T>, "fieldA" | "fieldB">) => i0.Dir<T> = null!;',
158158
);
159159
expect(actual).toContain(
160-
'var _t1 = _ctor1({ "fieldA": (((this).foo)), "fieldB": null as any });',
160+
'var _t1 = _ctor1({ "fieldA": (((this).foo)), "fieldB": 0 as any });',
161161
);
162162
});
163163

@@ -1235,11 +1235,11 @@ describe('type check blocks', () => {
12351235
it('should use undefined for safe navigation operations when enabled', () => {
12361236
const block = tcb(TEMPLATE, DIRECTIVES);
12371237
expect(block).toContain(
1238-
'(null as any ? (null as any ? (((this).a))!.method : undefined)!() : undefined)',
1238+
'(0 as any ? (0 as any ? (((this).a))!.method : undefined)!() : undefined)',
12391239
);
1240-
expect(block).toContain('(null as any ? (((this).a))!.b : undefined)');
1241-
expect(block).toContain('(null as any ? (((this).a))![0] : undefined)');
1242-
expect(block).toContain('(null as any ? (((((this).a)).optionalMethod))!() : undefined)');
1240+
expect(block).toContain('(0 as any ? (((this).a))!.b : undefined)');
1241+
expect(block).toContain('(0 as any ? (((this).a))![0] : undefined)');
1242+
expect(block).toContain('(0 as any ? (((((this).a)).optionalMethod))!() : undefined)');
12431243
});
12441244
it("should use an 'any' type for safe navigation operations when disabled", () => {
12451245
const DISABLED_CONFIG: TypeCheckingConfig = {
@@ -1258,13 +1258,13 @@ describe('type check blocks', () => {
12581258
const TEMPLATE = `{{a.method()?.b}} {{a()?.method()}} {{a.method()?.[0]}} {{a.method()?.otherMethod?.()}}`;
12591259
it('should check the presence of a property/method on the receiver when enabled', () => {
12601260
const block = tcb(TEMPLATE, DIRECTIVES);
1261-
expect(block).toContain('(null as any ? ((((this).a)).method())!.b : undefined)');
1261+
expect(block).toContain('(0 as any ? ((((this).a)).method())!.b : undefined)');
12621262
expect(block).toContain(
1263-
'(null as any ? (null as any ? ((this).a())!.method : undefined)!() : undefined)',
1263+
'(0 as any ? (0 as any ? ((this).a())!.method : undefined)!() : undefined)',
12641264
);
1265-
expect(block).toContain('(null as any ? ((((this).a)).method())![0] : undefined)');
1265+
expect(block).toContain('(0 as any ? ((((this).a)).method())![0] : undefined)');
12661266
expect(block).toContain(
1267-
'(null as any ? ((null as any ? ((((this).a)).method())!.otherMethod : undefined))!() : undefined)',
1267+
'(0 as any ? ((0 as any ? ((((this).a)).method())!.otherMethod : undefined))!() : undefined)',
12681268
);
12691269
});
12701270
it('should not check the presence of a property/method on the receiver when disabled', () => {

0 commit comments

Comments
 (0)