Skip to content

Commit

Permalink
fix(compiler): Update type check block to fix control flow source map…
Browse files Browse the repository at this point in the history
…pings (#53980)

The source mappings and types for various pieces of the control flow
were not quite right and prevented the language service from providing
accurate information.

fixes angular/vscode-ng-language-service#1988

PR Close #53980
  • Loading branch information
atscott authored and thePunderWoman committed Jan 24, 2024
1 parent 47e6e84 commit eddf5da
Show file tree
Hide file tree
Showing 12 changed files with 429 additions and 89 deletions.
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/src/comments.ts
Expand Up @@ -44,6 +44,7 @@ export enum ExpressionIdentifier {
DIRECTIVE = 'DIR',
COMPONENT_COMPLETION = 'COMPCOMP',
EVENT_PARAMETER = 'EP',
VARIABLE_AS_EXPRESSION = 'VAE',
}

/** Tags the node with the given expression identifier. */
Expand Down
Expand Up @@ -111,7 +111,7 @@ export class SymbolBuilder {
const elementSourceSpan = element.startSourceSpan ?? element.sourceSpan;
const tcbSourceFile = this.typeCheckBlock.getSourceFile();
// directives could be either:
// - var _t1: TestDir /*T:D*/ = (null!);
// - var _t1: TestDir /*T:D*/ = null! as TestDir;
// - var _t1 /*T:D*/ = _ctor1({});
const isDirectiveDeclaration = (node: ts.Node): node is ts.TypeNode|ts.Identifier =>
(ts.isTypeNode(node) || ts.isIdentifier(node)) && ts.isVariableDeclaration(node.parent) &&
Expand Down Expand Up @@ -429,19 +429,25 @@ export class SymbolBuilder {
private getSymbolOfVariable(variable: TmplAstVariable): VariableSymbol|null {
const node = findFirstMatchingNode(
this.typeCheckBlock, {withSpan: variable.sourceSpan, filter: ts.isVariableDeclaration});
if (node === null || node.initializer === undefined) {
if (node === null) {
return null;
}

const expressionSymbol = this.getSymbolOfTsNode(node.initializer);
if (expressionSymbol === null) {
let nodeValueSymbol: TsNodeSymbolInfo|null = null;
if (ts.isForOfStatement(node.parent.parent)) {
nodeValueSymbol = this.getSymbolOfTsNode(node);
} else if (node.initializer !== undefined) {
nodeValueSymbol = this.getSymbolOfTsNode(node.initializer);
}

if (nodeValueSymbol === null) {
return null;
}

return {
tsType: expressionSymbol.tsType,
tsSymbol: expressionSymbol.tsSymbol,
initializerLocation: expressionSymbol.tcbLocation,
tsType: nodeValueSymbol.tsType,
tsSymbol: nodeValueSymbol.tsSymbol,
initializerLocation: nodeValueSymbol.tcbLocation,
kind: SymbolKind.Variable,
declaration: variable,
localVarLocation: {
Expand Down
19 changes: 10 additions & 9 deletions packages/compiler-cli/src/ngtsc/typecheck/src/ts_util.ts
Expand Up @@ -8,6 +8,10 @@

import ts from 'typescript';

import {addExpressionIdentifier, ExpressionIdentifier} from './comments';
import {wrapForTypeChecker} from './diagnostics';



/**
* A `Set` of `ts.SyntaxKind`s of `ts.Expression` which are safe to wrap in a `ts.AsExpression`
Expand Down Expand Up @@ -77,20 +81,17 @@ 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));
}
// to be `never`, instead of a `boolean`. To work around it, we cast the value
// in the initializer, e.g. `var _t1 = null! as boolean;`.
addExpressionIdentifier(type, ExpressionIdentifier.VARIABLE_AS_EXPRESSION);
const initializer: ts.Expression = ts.factory.createAsExpression(
ts.factory.createNonNullExpression(ts.factory.createNull()), type);

const decl = ts.factory.createVariableDeclaration(
/* name */ id,
/* exclamationToken */ undefined,
/* type */ type,
/* type */ undefined,
/* initializer */ initializer);
return ts.factory.createVariableStatement(
/* modifiers */ undefined,
Expand Down
Expand Up @@ -465,8 +465,8 @@ abstract class TcbDirectiveTypeOpBase extends TcbOp {
}

const id = this.tcb.allocateId();
addExpressionIdentifier(type, ExpressionIdentifier.DIRECTIVE);
addParseSpanInfo(type, this.node.startSourceSpan || this.node.sourceSpan);
addExpressionIdentifier(id, ExpressionIdentifier.DIRECTIVE);
addParseSpanInfo(id, this.node.startSourceSpan || this.node.sourceSpan);
this.scope.addStatement(tsDeclareVariable(id, type));
return id;
}
Expand Down Expand Up @@ -1332,7 +1332,9 @@ class TcbBlockVariableOp extends TcbOp {
override execute(): ts.Identifier {
const id = this.tcb.allocateId();
addParseSpanInfo(id, this.variable.keySpan);
this.scope.addStatement(tsCreateVariable(id, this.initializer));
const variable = tsCreateVariable(id, wrapForTypeChecker(this.initializer));
addParseSpanInfo(variable.declarationList.declarations[0], this.variable.sourceSpan);
this.scope.addStatement(variable);
return id;
}
}
Expand All @@ -1355,7 +1357,9 @@ class TcbBlockImplicitVariableOp extends TcbOp {
override execute(): ts.Identifier {
const id = this.tcb.allocateId();
addParseSpanInfo(id, this.variable.keySpan);
this.scope.addStatement(tsDeclareVariable(id, this.type));
const variable = tsDeclareVariable(id, this.type);
addParseSpanInfo(variable.declarationList.declarations[0], this.variable.sourceSpan);
this.scope.addStatement(variable);
return id;
}
}
Expand Down Expand Up @@ -1610,6 +1614,7 @@ class TcbForOfOp extends TcbOp {
}
const initializer = ts.factory.createVariableDeclarationList(
[ts.factory.createVariableDeclaration(initializerId)], ts.NodeFlags.Const);
addParseSpanInfo(initializer, this.block.item.keySpan);
// It's common to have a for loop over a nullable value (e.g. produced by the `async` pipe).
// Add a non-null expression to allow such values to be assigned.
const expression = ts.factory.createNonNullExpression(
Expand Down
Expand Up @@ -173,7 +173,7 @@ describe('type check blocks diagnostics', () => {
pipeName: 'test',
}];
const block = tcbWithSpans(TEMPLATE, PIPES);
expect(block).toContain('var _pipe1: i0.TestPipe = null!');
expect(block).toContain('var _pipe1 = null! as i0.TestPipe');
expect(block).toContain(
'(_pipe1.transform /*7,11*/(((this).a /*3,4*/) /*3,4*/, ((this).b /*12,13*/) /*12,13*/) /*3,13*/);');
});
Expand Down Expand Up @@ -215,6 +215,37 @@ describe('type check blocks diagnostics', () => {
.toContain('_t1.inputA /*9,15*/ = ("" /*18,20*/) /*8,21*/;');
});
});

describe('control flow', () => {
it('@for', () => {
const template = `@for (user of users; track user; let i = $index) { {{i}} }`;
// user variable
expect(tcbWithSpans(template, [])).toContain('const _t1 /*6,10*/');
expect(tcbWithSpans(template, [])).toContain('(this).users /*14,19*/');
// index variable
expect(tcbWithSpans(template, []))
.toContain('var _t2 /*37,38*/ = null! as number /*T:VAE*/ /*37,47*/');
// track expression
expect(tcbWithSpans(template, [])).toContain('_t1 /*27,31*/;');
});

it('@for with comma separated variables', () => {
const template =
`@for (x of y; track x; let i = $index, odd = $odd,e_v_e_n=$even) { {{i + odd + e_v_e_n}} }`;
expect(tcbWithSpans(template, []))
.toContain('var _t2 /*27,28*/ = null! as number /*T:VAE*/ /*27,37*/');
expect(tcbWithSpans(template, []))
.toContain('var _t3 /*39,42*/ = null! as boolean /*T:VAE*/ /*39,54*/');
expect(tcbWithSpans(template, []))
.toContain('var _t4 /*55,62*/ = null! as boolean /*T:VAE*/ /*55,68*/');
});

it('@if', () => {
const template = `@if (x; as alias) { {{alias}} }`;
expect(tcbWithSpans(template, [])).toContain('var _t1 /*11,16*/');
expect(tcbWithSpans(template, [])).toContain('(this).x /*5,6*/');
});
});
});
});

Expand Down

0 comments on commit eddf5da

Please sign in to comment.