Skip to content

Commit ff32301

Browse files
JoostKdylhunn
JoostK
authored and
dylhunn
committed
refactor(compiler-cli): track whether a TcbPosition corresponds with a shim file (#45454)
Extends `TcbPosition` with a field that indicates whether the `tcbPath` is a type-checking shim file, or an original source file with an inline type check block. This field is used in an upcoming commit that fixes an inconsistency with how inline type check blocks are incorrectly interpreted as a type-checking shim file instead. PR Close #45454
1 parent b2758d7 commit ff32301

File tree

8 files changed

+100
-26
lines changed

8 files changed

+100
-26
lines changed

‎packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ export interface TcbLocation {
5050
*/
5151
tcbPath: AbsoluteFsPath;
5252

53+
/**
54+
* Whether the type check block exists in a type-checking shim file or is inline.
55+
*/
56+
isShimFile: boolean;
57+
5358
/** The location in the file where node appears. */
5459
positionInFile: number;
5560
}

‎packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
9797
}
9898

9999
private getLatestComponentState(component: ts.ClassDeclaration):
100-
{data: TemplateData|null, tcb: ts.Node|null, shimPath: AbsoluteFsPath} {
100+
{data: TemplateData|null, tcb: ts.Node|null, tcbPath: AbsoluteFsPath, tcbIsShim: boolean} {
101101
this.ensureShimForComponent(component);
102102

103103
const sf = component.getSourceFile();
@@ -107,7 +107,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
107107
const fileRecord = this.getFileData(sfPath);
108108

109109
if (!fileRecord.shimData.has(shimPath)) {
110-
return {data: null, tcb: null, shimPath};
110+
return {data: null, tcb: null, tcbPath: shimPath, tcbIsShim: true};
111111
}
112112

113113
const templateId = fileRecord.sourceManager.getTemplateId(component);
@@ -123,18 +123,23 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
123123

124124
let tcb: ts.Node|null = findTypeCheckBlock(shimSf, id, /*isDiagnosticsRequest*/ false);
125125

126+
let tcbPath = shimPath;
126127
if (tcb === null) {
127128
// Try for an inline block.
128129
const inlineSf = getSourceFileOrError(program, sfPath);
129130
tcb = findTypeCheckBlock(inlineSf, id, /*isDiagnosticsRequest*/ false);
131+
132+
if (tcb !== null) {
133+
tcbPath = sfPath;
134+
}
130135
}
131136

132137
let data: TemplateData|null = null;
133138
if (shimRecord.templates.has(templateId)) {
134139
data = shimRecord.templates.get(templateId)!;
135140
}
136141

137-
return {data, tcb, shimPath};
142+
return {data, tcb, tcbPath, tcbIsShim: tcbPath === shimPath};
138143
}
139144

140145
isTrackedTypeCheckFile(filePath: AbsoluteFsPath): boolean {
@@ -334,12 +339,12 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
334339
return this.completionCache.get(component)!;
335340
}
336341

337-
const {tcb, data, shimPath} = this.getLatestComponentState(component);
342+
const {tcb, data, tcbPath, tcbIsShim} = this.getLatestComponentState(component);
338343
if (tcb === null || data === null) {
339344
return null;
340345
}
341346

342-
const engine = new CompletionEngine(tcb, data, shimPath);
347+
const engine = new CompletionEngine(tcb, data, tcbPath, tcbIsShim);
343348
this.completionCache.set(component, engine);
344349
return engine;
345350
}
@@ -509,13 +514,13 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
509514
return this.symbolBuilderCache.get(component)!;
510515
}
511516

512-
const {tcb, data, shimPath} = this.getLatestComponentState(component);
517+
const {tcb, data, tcbPath, tcbIsShim} = this.getLatestComponentState(component);
513518
if (tcb === null || data === null) {
514519
return null;
515520
}
516521

517522
const builder = new SymbolBuilder(
518-
shimPath, tcb, data, this.componentScopeReader,
523+
tcbPath, tcbIsShim, tcb, data, this.componentScopeReader,
519524
() => this.programDriver.getProgram().getTypeChecker());
520525
this.symbolBuilderCache.set(component, builder);
521526
return builder;

‎packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ export class CompletionEngine {
3535
new Map<PropertyRead|SafePropertyRead|LiteralPrimitive|TmplAstTextAttribute, TcbLocation>();
3636

3737

38-
constructor(private tcb: ts.Node, private data: TemplateData, private tcbPath: AbsoluteFsPath) {
38+
constructor(
39+
private tcb: ts.Node, private data: TemplateData, private tcbPath: AbsoluteFsPath,
40+
private tcbIsShim: boolean) {
3941
// Find the component completion expression within the TCB. This looks like: `ctx. /* ... */;`
4042
const globalRead = findFirstMatchingNode(this.tcb, {
4143
filter: ts.isPropertyAccessExpression,
@@ -45,6 +47,7 @@ export class CompletionEngine {
4547
if (globalRead !== null) {
4648
this.componentContext = {
4749
tcbPath: this.tcbPath,
50+
isShimFile: this.tcbIsShim,
4851
// `globalRead.name` is an empty `ts.Identifier`, so its start position immediately follows
4952
// the `.` in `ctx.`. TS autocompletion APIs can then be used to access completion results
5053
// for the component context.
@@ -83,6 +86,7 @@ export class CompletionEngine {
8386
if (nodeLocation !== null) {
8487
nodeContext = {
8588
tcbPath: this.tcbPath,
89+
isShimFile: this.tcbIsShim,
8690
positionInFile: nodeLocation.getStart(),
8791
};
8892
}
@@ -96,6 +100,7 @@ export class CompletionEngine {
96100
if (nodeLocation) {
97101
nodeContext = {
98102
tcbPath: this.tcbPath,
103+
isShimFile: this.tcbIsShim,
99104
positionInFile: nodeLocation.getStart(),
100105
};
101106
}
@@ -148,6 +153,7 @@ export class CompletionEngine {
148153

149154
const res: TcbLocation = {
150155
tcbPath: this.tcbPath,
156+
isShimFile: this.tcbIsShim,
151157
positionInFile: tsExpr.name.getEnd(),
152158
};
153159
this.expressionCompletionCache.set(expr, res);
@@ -188,6 +194,7 @@ export class CompletionEngine {
188194
}
189195
const res: TcbLocation = {
190196
tcbPath: this.tcbPath,
197+
isShimFile: this.tcbIsShim,
191198
positionInFile: positionInShimFile,
192199
};
193200
this.expressionCompletionCache.set(expr, res);

‎packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ export class SymbolBuilder {
2929
private symbolCache = new Map<AST|TmplAstNode, Symbol|null>();
3030

3131
constructor(
32-
private readonly shimPath: AbsoluteFsPath,
32+
private readonly tcbPath: AbsoluteFsPath,
33+
private readonly tcbIsShim: boolean,
3334
private readonly typeCheckBlock: ts.Node,
3435
private readonly templateData: TemplateData,
3536
private readonly componentScopeReader: ComponentScopeReader,
@@ -233,7 +234,7 @@ export class SymbolBuilder {
233234
const addEventListener = outputFieldAccess.name;
234235
const tsSymbol = this.getTypeChecker().getSymbolAtLocation(addEventListener);
235236
const tsType = this.getTypeChecker().getTypeAtLocation(addEventListener);
236-
const positionInShimFile = this.getShimPositionForNode(addEventListener);
237+
const positionInFile = this.getTcbPositionForNode(addEventListener);
237238
const target = this.getSymbol(consumer);
238239

239240
if (target === null || tsSymbol === undefined) {
@@ -245,7 +246,11 @@ export class SymbolBuilder {
245246
tsSymbol,
246247
tsType,
247248
target,
248-
tcbLocation: {tcbPath: this.shimPath, positionInFile: positionInShimFile},
249+
tcbLocation: {
250+
tcbPath: this.tcbPath,
251+
isShimFile: this.tcbIsShim,
252+
positionInFile,
253+
},
249254
});
250255
} else {
251256
if (!ts.isElementAccessExpression(outputFieldAccess)) {
@@ -263,14 +268,18 @@ export class SymbolBuilder {
263268
continue;
264269
}
265270

266-
const positionInShimFile = this.getShimPositionForNode(outputFieldAccess);
271+
const positionInFile = this.getTcbPositionForNode(outputFieldAccess);
267272
const tsType = this.getTypeChecker().getTypeAtLocation(outputFieldAccess);
268273
bindings.push({
269274
kind: SymbolKind.Binding,
270275
tsSymbol,
271276
tsType,
272277
target,
273-
tcbLocation: {tcbPath: this.shimPath, positionInFile: positionInShimFile},
278+
tcbLocation: {
279+
tcbPath: this.tcbPath,
280+
isShimFile: this.tcbIsShim,
281+
positionInFile,
282+
},
274283
});
275284
}
276285
}
@@ -383,8 +392,9 @@ export class SymbolBuilder {
383392
kind: SymbolKind.Variable,
384393
declaration: variable,
385394
localVarLocation: {
386-
tcbPath: this.shimPath,
387-
positionInFile: this.getShimPositionForNode(node.name),
395+
tcbPath: this.tcbPath,
396+
isShimFile: this.tcbIsShim,
397+
positionInFile: this.getTcbPositionForNode(node.name),
388398
}
389399
};
390400
}
@@ -415,8 +425,9 @@ export class SymbolBuilder {
415425
}
416426

417427
const referenceVarTcbLocation: TcbLocation = {
418-
tcbPath: this.shimPath,
419-
positionInFile: this.getShimPositionForNode(node),
428+
tcbPath: this.tcbPath,
429+
isShimFile: this.tcbIsShim,
430+
positionInFile: this.getTcbPositionForNode(node),
420431
};
421432
if (target instanceof TmplAstTemplate || target instanceof TmplAstElement) {
422433
return {
@@ -561,21 +572,25 @@ export class SymbolBuilder {
561572
tsSymbol = this.getTypeChecker().getSymbolAtLocation(node);
562573
}
563574

564-
const positionInShimFile = this.getShimPositionForNode(node);
575+
const positionInFile = this.getTcbPositionForNode(node);
565576
const type = this.getTypeChecker().getTypeAtLocation(node);
566577
return {
567578
// If we could not find a symbol, fall back to the symbol on the type for the node.
568579
// Some nodes won't have a "symbol at location" but will have a symbol for the type.
569580
// Examples of this would be literals and `document.createElement('div')`.
570581
tsSymbol: tsSymbol ?? type.symbol ?? null,
571582
tsType: type,
572-
tcbLocation: {tcbPath: this.shimPath, positionInFile: positionInShimFile},
583+
tcbLocation: {
584+
tcbPath: this.tcbPath,
585+
isShimFile: this.tcbIsShim,
586+
positionInFile,
587+
},
573588
};
574589
}
575590

576-
private getShimPositionForNode(node: ts.Node): number {
591+
private getTcbPositionForNode(node: ts.Node): number {
577592
if (ts.isTypeReferenceNode(node)) {
578-
return this.getShimPositionForNode(node.typeName);
593+
return this.getTcbPositionForNode(node.typeName);
579594
} else if (ts.isQualifiedName(node)) {
580595
return node.right.getStart();
581596
} else if (ts.isPropertyAccessExpression(node)) {

‎packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,6 +1581,48 @@ runInEachFileSystem(() => {
15811581
symbol.directives.map(dir => program.getTypeChecker().typeToString(dir.tsType)).sort();
15821582
expect(actualDirectives).toEqual(['GenericDir<any>']);
15831583
});
1584+
1585+
it('has correct tcb location for components with inline TCBs', () => {
1586+
const fileName = absoluteFrom('/main.ts');
1587+
const {program, templateTypeChecker} = baseTestSetup(
1588+
[
1589+
{
1590+
fileName,
1591+
templates: {'Cmp': '<div></div>'},
1592+
// Force an inline TCB by using a non-exported component class
1593+
source: `class Cmp {}`,
1594+
},
1595+
],
1596+
{inlining: true, config: {enableTemplateTypeChecker: true}});
1597+
const sf = getSourceFileOrError(program, fileName);
1598+
const cmp = getClass(sf, 'Cmp');
1599+
1600+
const nodes = templateTypeChecker.getTemplate(cmp)!;
1601+
1602+
const symbol = templateTypeChecker.getSymbolOfNode(nodes[0] as TmplAstElement, cmp)!;
1603+
assertElementSymbol(symbol);
1604+
expect(symbol.tcbLocation.tcbPath).toBe(sf.fileName);
1605+
expect(symbol.tcbLocation.isShimFile).toBe(false);
1606+
});
1607+
1608+
it('has correct tcb location for components with TCBs in a type-checking shim file', () => {
1609+
const fileName = absoluteFrom('/main.ts');
1610+
const {program, templateTypeChecker} = setup([
1611+
{
1612+
fileName,
1613+
templates: {'Cmp': '<div></div>'},
1614+
},
1615+
]);
1616+
const sf = getSourceFileOrError(program, fileName);
1617+
const cmp = getClass(sf, 'Cmp');
1618+
1619+
const nodes = templateTypeChecker.getTemplate(cmp)!;
1620+
1621+
const symbol = templateTypeChecker.getSymbolOfNode(nodes[0] as TmplAstElement, cmp)!;
1622+
assertElementSymbol(symbol);
1623+
expect(symbol.tcbLocation.tcbPath).not.toBe(sf.fileName);
1624+
expect(symbol.tcbLocation.isShimFile).toBe(true);
1625+
});
15841626
});
15851627
});
15861628

‎packages/language-service/src/definitions.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ export class DefinitionBuilder {
110110
const tcbLocation = symbol.kind === SymbolKind.Variable ? symbol.localVarLocation :
111111
symbol.referenceVarLocation;
112112
const mapping = getTemplateLocationFromTcbLocation(
113-
this.compiler.getTemplateTypeChecker(), tcbLocation.tcbPath,
113+
this.compiler.getTemplateTypeChecker(), tcbLocation.tcbPath, tcbLocation.isShimFile,
114114
tcbLocation.positionInFile);
115115
if (mapping !== null) {
116116
definitions.push({

‎packages/language-service/src/references_and_rename_utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ export function convertToTemplateDocumentSpan<T extends ts.DocumentSpan>(
222222
// serverHost or LSParseConfigHost in the adapter. We should have a better defined way to
223223
// normalize paths.
224224
const mapping = getTemplateLocationFromTcbLocation(
225-
templateTypeChecker, absoluteFrom(shimDocumentSpan.fileName),
225+
templateTypeChecker, absoluteFrom(shimDocumentSpan.fileName), /* tcbIsShim */ true,
226226
shimDocumentSpan.textSpan.start);
227227
if (mapping === null) {
228228
return null;

‎packages/language-service/src/utils.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -363,10 +363,10 @@ export function isWithin(position: number, span: AbsoluteSourceSpan|ParseSourceS
363363
* the span in the template.
364364
*/
365365
export function getTemplateLocationFromTcbLocation(
366-
templateTypeChecker: TemplateTypeChecker, shimPath: AbsoluteFsPath,
367-
positionInShimFile: number): {templateUrl: AbsoluteFsPath, span: ParseSourceSpan}|null {
366+
templateTypeChecker: TemplateTypeChecker, tcbPath: AbsoluteFsPath, tcbIsShim: boolean,
367+
positionInFile: number): {templateUrl: AbsoluteFsPath, span: ParseSourceSpan}|null {
368368
const mapping = templateTypeChecker.getTemplateMappingAtTcbLocation(
369-
{tcbPath: shimPath, positionInFile: positionInShimFile});
369+
{tcbPath, isShimFile: tcbIsShim, positionInFile});
370370
if (mapping === null) {
371371
return null;
372372
}

0 commit comments

Comments
 (0)