Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(compiler-cli): incorrect inferred type of for loop implicit variables #52732

Closed
wants to merge 1 commit into from
Closed

fix(compiler-cli): incorrect inferred type of for loop implicit variables #52732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 11 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts
View file
Open in desktop
Expand Up @@ -77,11 +77,21 @@ export function tsCreateElement(tagName: string): ts.Expression {
* Unlike with `tsCreateVariable`, the type of the variable is explicitly specified.
*/
export function tsDeclareVariable(id: ts.Identifier, type: ts.TypeNode): ts.VariableStatement {
let initializer: ts.Expression = ts.factory.createNonNullExpression(ts.factory.createNull());

// When we create a variable like `var _t1: boolean = null!`, TypeScript actually infers `_t1`
// to be `never`, instead of a `boolean`. To work around it, we cast the value to boolean again
// in the initializer, e.g. `var _t1: boolean = null! as boolean;`.
if (type.kind === ts.SyntaxKind.BooleanKeyword) {
initializer = ts.factory.createAsExpression(
initializer, ts.factory.createKeywordTypeNode(ts.SyntaxKind.BooleanKeyword));
}

const decl = ts.factory.createVariableDeclaration(
/* name */ id,
/* exclamationToken */ undefined,
/* type */ type,
/* initializer */ ts.factory.createNonNullExpression(ts.factory.createNull()));
/* initializer */ initializer);
return ts.factory.createVariableStatement(
/* modifiers */ undefined,
/* declarationList */[decl]);
Expand Down
21 changes: 18 additions & 3 deletions packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
View file
Open in desktop
Expand Up @@ -1572,6 +1572,18 @@ class Scope {
*/
private statements: ts.Statement[] = [];

/**
* Names of the for loop context variables and their types.
*/
private static readonly forLoopContextVariableTypes = new Map<string, ts.KeywordTypeSyntaxKind>([
['$first', ts.SyntaxKind.BooleanKeyword],
['$last', ts.SyntaxKind.BooleanKeyword],
['$even', ts.SyntaxKind.BooleanKeyword],
['$odd', ts.SyntaxKind.BooleanKeyword],
['$index', ts.SyntaxKind.NumberKeyword],
['$count', ts.SyntaxKind.NumberKeyword],
]);

private constructor(
private tcb: Context, private parent: Scope|null = null,
private guard: ts.Expression|null = null) {}
Expand Down Expand Up @@ -1628,9 +1640,12 @@ class Scope {
new TcbBlockVariableOp(
tcb, scope, ts.factory.createIdentifier(scopedNode.item.name), scopedNode.item));

for (const variable of Object.values(scopedNode.contextVariables)) {
// Note: currently all context variables are assumed to be number types.
const type = ts.factory.createKeywordTypeNode(ts.SyntaxKind.NumberKeyword);
for (const [name, variable] of Object.entries(scopedNode.contextVariables)) {
if (!this.forLoopContextVariableTypes.has(name)) {
throw new Error(`Unrecognized for loop context variable ${name}`);
}

const type = ts.factory.createKeywordTypeNode(this.forLoopContextVariableTypes.get(name)!);
this.registerVariable(
scope, variable, new TcbBlockImplicitVariableOp(tcb, scope, type, variable));
}
Expand Down
16 changes: 8 additions & 8 deletions packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
View file
Open in desktop
Expand Up @@ -1560,10 +1560,10 @@ describe('type check blocks', () => {
const result = tcb(TEMPLATE);
expect(result).toContain('for (const item of ((this).items)!) { var _t1 = item;');
expect(result).toContain('var _t2: number = null!;');
expect(result).toContain('var _t3: number = null!;');
expect(result).toContain('var _t4: number = null!;');
expect(result).toContain('var _t5: number = null!;');
expect(result).toContain('var _t6: number = null!;');
expect(result).toContain('var _t3: boolean = null! as boolean;');
expect(result).toContain('var _t4: boolean = null! as boolean;');
expect(result).toContain('var _t5: boolean = null! as boolean;');
expect(result).toContain('var _t6: boolean = null! as boolean;');
expect(result).toContain('var _t7: number = null!;');
expect(result).toContain('"" + (_t2) + (_t3) + (_t4) + (_t5) + (_t6) + (_t7)');
});
Expand All @@ -1578,10 +1578,10 @@ describe('type check blocks', () => {
const result = tcb(TEMPLATE);
expect(result).toContain('for (const item of ((this).items)!) { var _t1 = item;');
expect(result).toContain('var _t2: number = null!;');
expect(result).toContain('var _t3: number = null!;');
expect(result).toContain('var _t4: number = null!;');
expect(result).toContain('var _t5: number = null!;');
expect(result).toContain('var _t6: number = null!;');
expect(result).toContain('var _t3: boolean = null! as boolean;');
expect(result).toContain('var _t4: boolean = null! as boolean;');
expect(result).toContain('var _t5: boolean = null! as boolean;');
expect(result).toContain('var _t6: boolean = null! as boolean;');
expect(result).toContain('var _t7: number = null!;');
expect(result).toContain('"" + (_t2) + (_t3) + (_t4) + (_t5) + (_t6) + (_t7)');
});
Expand Down
16 changes: 8 additions & 8 deletions packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
View file
Open in desktop
Expand Up @@ -4477,10 +4477,10 @@ suppress
const diags = env.driveDiagnostics();
expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
`Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
`Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
`Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
`Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
]);
});
Expand Down Expand Up @@ -4513,10 +4513,10 @@ suppress
const diags = env.driveDiagnostics();
expect(diags.map(d => ts.flattenDiagnosticMessageText(d.messageText, ''))).toEqual([
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
`Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
`Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
`Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
`Argument of type 'boolean' is not assignable to parameter of type 'string'.`,
`Argument of type 'number' is not assignable to parameter of type 'string'.`,
]);
});
Expand Down