From ff32301bf5032191274e70f0ab05d71dc199f7cf Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 26 Mar 2022 22:23:28 +0100 Subject: [PATCH] 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 --- .../src/ngtsc/typecheck/api/symbols.ts | 5 +++ .../src/ngtsc/typecheck/src/checker.ts | 19 +++++---- .../src/ngtsc/typecheck/src/completion.ts | 9 +++- .../typecheck/src/template_symbol_builder.ts | 41 ++++++++++++------ ...ecker__get_symbol_of_template_node_spec.ts | 42 +++++++++++++++++++ packages/language-service/src/definitions.ts | 2 +- .../src/references_and_rename_utils.ts | 2 +- packages/language-service/src/utils.ts | 6 +-- 8 files changed, 100 insertions(+), 26 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts index 3c52f7d87cd..010d8f989f0 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts @@ -50,6 +50,11 @@ export interface TcbLocation { */ tcbPath: AbsoluteFsPath; + /** + * Whether the type check block exists in a type-checking shim file or is inline. + */ + isShimFile: boolean; + /** The location in the file where node appears. */ positionInFile: number; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts index 2a819cd01aa..e755a846350 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts @@ -97,7 +97,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { } private getLatestComponentState(component: ts.ClassDeclaration): - {data: TemplateData|null, tcb: ts.Node|null, shimPath: AbsoluteFsPath} { + {data: TemplateData|null, tcb: ts.Node|null, tcbPath: AbsoluteFsPath, tcbIsShim: boolean} { this.ensureShimForComponent(component); const sf = component.getSourceFile(); @@ -107,7 +107,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { const fileRecord = this.getFileData(sfPath); if (!fileRecord.shimData.has(shimPath)) { - return {data: null, tcb: null, shimPath}; + return {data: null, tcb: null, tcbPath: shimPath, tcbIsShim: true}; } const templateId = fileRecord.sourceManager.getTemplateId(component); @@ -123,10 +123,15 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { let tcb: ts.Node|null = findTypeCheckBlock(shimSf, id, /*isDiagnosticsRequest*/ false); + let tcbPath = shimPath; if (tcb === null) { // Try for an inline block. const inlineSf = getSourceFileOrError(program, sfPath); tcb = findTypeCheckBlock(inlineSf, id, /*isDiagnosticsRequest*/ false); + + if (tcb !== null) { + tcbPath = sfPath; + } } let data: TemplateData|null = null; @@ -134,7 +139,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { data = shimRecord.templates.get(templateId)!; } - return {data, tcb, shimPath}; + return {data, tcb, tcbPath, tcbIsShim: tcbPath === shimPath}; } isTrackedTypeCheckFile(filePath: AbsoluteFsPath): boolean { @@ -334,12 +339,12 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { return this.completionCache.get(component)!; } - const {tcb, data, shimPath} = this.getLatestComponentState(component); + const {tcb, data, tcbPath, tcbIsShim} = this.getLatestComponentState(component); if (tcb === null || data === null) { return null; } - const engine = new CompletionEngine(tcb, data, shimPath); + const engine = new CompletionEngine(tcb, data, tcbPath, tcbIsShim); this.completionCache.set(component, engine); return engine; } @@ -509,13 +514,13 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { return this.symbolBuilderCache.get(component)!; } - const {tcb, data, shimPath} = this.getLatestComponentState(component); + const {tcb, data, tcbPath, tcbIsShim} = this.getLatestComponentState(component); if (tcb === null || data === null) { return null; } const builder = new SymbolBuilder( - shimPath, tcb, data, this.componentScopeReader, + tcbPath, tcbIsShim, tcb, data, this.componentScopeReader, () => this.programDriver.getProgram().getTypeChecker()); this.symbolBuilderCache.set(component, builder); return builder; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts index 2b1c97192f4..9fe421c23dc 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts @@ -35,7 +35,9 @@ export class CompletionEngine { new Map(); - constructor(private tcb: ts.Node, private data: TemplateData, private tcbPath: AbsoluteFsPath) { + constructor( + private tcb: ts.Node, private data: TemplateData, private tcbPath: AbsoluteFsPath, + private tcbIsShim: boolean) { // Find the component completion expression within the TCB. This looks like: `ctx. /* ... */;` const globalRead = findFirstMatchingNode(this.tcb, { filter: ts.isPropertyAccessExpression, @@ -45,6 +47,7 @@ export class CompletionEngine { if (globalRead !== null) { this.componentContext = { tcbPath: this.tcbPath, + isShimFile: this.tcbIsShim, // `globalRead.name` is an empty `ts.Identifier`, so its start position immediately follows // the `.` in `ctx.`. TS autocompletion APIs can then be used to access completion results // for the component context. @@ -83,6 +86,7 @@ export class CompletionEngine { if (nodeLocation !== null) { nodeContext = { tcbPath: this.tcbPath, + isShimFile: this.tcbIsShim, positionInFile: nodeLocation.getStart(), }; } @@ -96,6 +100,7 @@ export class CompletionEngine { if (nodeLocation) { nodeContext = { tcbPath: this.tcbPath, + isShimFile: this.tcbIsShim, positionInFile: nodeLocation.getStart(), }; } @@ -148,6 +153,7 @@ export class CompletionEngine { const res: TcbLocation = { tcbPath: this.tcbPath, + isShimFile: this.tcbIsShim, positionInFile: tsExpr.name.getEnd(), }; this.expressionCompletionCache.set(expr, res); @@ -188,6 +194,7 @@ export class CompletionEngine { } const res: TcbLocation = { tcbPath: this.tcbPath, + isShimFile: this.tcbIsShim, positionInFile: positionInShimFile, }; this.expressionCompletionCache.set(expr, res); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts index c005a6364ab..7f765b45c14 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts @@ -29,7 +29,8 @@ export class SymbolBuilder { private symbolCache = new Map(); constructor( - private readonly shimPath: AbsoluteFsPath, + private readonly tcbPath: AbsoluteFsPath, + private readonly tcbIsShim: boolean, private readonly typeCheckBlock: ts.Node, private readonly templateData: TemplateData, private readonly componentScopeReader: ComponentScopeReader, @@ -233,7 +234,7 @@ export class SymbolBuilder { const addEventListener = outputFieldAccess.name; const tsSymbol = this.getTypeChecker().getSymbolAtLocation(addEventListener); const tsType = this.getTypeChecker().getTypeAtLocation(addEventListener); - const positionInShimFile = this.getShimPositionForNode(addEventListener); + const positionInFile = this.getTcbPositionForNode(addEventListener); const target = this.getSymbol(consumer); if (target === null || tsSymbol === undefined) { @@ -245,7 +246,11 @@ export class SymbolBuilder { tsSymbol, tsType, target, - tcbLocation: {tcbPath: this.shimPath, positionInFile: positionInShimFile}, + tcbLocation: { + tcbPath: this.tcbPath, + isShimFile: this.tcbIsShim, + positionInFile, + }, }); } else { if (!ts.isElementAccessExpression(outputFieldAccess)) { @@ -263,14 +268,18 @@ export class SymbolBuilder { continue; } - const positionInShimFile = this.getShimPositionForNode(outputFieldAccess); + const positionInFile = this.getTcbPositionForNode(outputFieldAccess); const tsType = this.getTypeChecker().getTypeAtLocation(outputFieldAccess); bindings.push({ kind: SymbolKind.Binding, tsSymbol, tsType, target, - tcbLocation: {tcbPath: this.shimPath, positionInFile: positionInShimFile}, + tcbLocation: { + tcbPath: this.tcbPath, + isShimFile: this.tcbIsShim, + positionInFile, + }, }); } } @@ -383,8 +392,9 @@ export class SymbolBuilder { kind: SymbolKind.Variable, declaration: variable, localVarLocation: { - tcbPath: this.shimPath, - positionInFile: this.getShimPositionForNode(node.name), + tcbPath: this.tcbPath, + isShimFile: this.tcbIsShim, + positionInFile: this.getTcbPositionForNode(node.name), } }; } @@ -415,8 +425,9 @@ export class SymbolBuilder { } const referenceVarTcbLocation: TcbLocation = { - tcbPath: this.shimPath, - positionInFile: this.getShimPositionForNode(node), + tcbPath: this.tcbPath, + isShimFile: this.tcbIsShim, + positionInFile: this.getTcbPositionForNode(node), }; if (target instanceof TmplAstTemplate || target instanceof TmplAstElement) { return { @@ -561,7 +572,7 @@ export class SymbolBuilder { tsSymbol = this.getTypeChecker().getSymbolAtLocation(node); } - const positionInShimFile = this.getShimPositionForNode(node); + const positionInFile = this.getTcbPositionForNode(node); const type = this.getTypeChecker().getTypeAtLocation(node); return { // If we could not find a symbol, fall back to the symbol on the type for the node. @@ -569,13 +580,17 @@ export class SymbolBuilder { // Examples of this would be literals and `document.createElement('div')`. tsSymbol: tsSymbol ?? type.symbol ?? null, tsType: type, - tcbLocation: {tcbPath: this.shimPath, positionInFile: positionInShimFile}, + tcbLocation: { + tcbPath: this.tcbPath, + isShimFile: this.tcbIsShim, + positionInFile, + }, }; } - private getShimPositionForNode(node: ts.Node): number { + private getTcbPositionForNode(node: ts.Node): number { if (ts.isTypeReferenceNode(node)) { - return this.getShimPositionForNode(node.typeName); + return this.getTcbPositionForNode(node.typeName); } else if (ts.isQualifiedName(node)) { return node.right.getStart(); } else if (ts.isPropertyAccessExpression(node)) { diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts index 56d67f930e8..60893aff92d 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts @@ -1581,6 +1581,48 @@ runInEachFileSystem(() => { symbol.directives.map(dir => program.getTypeChecker().typeToString(dir.tsType)).sort(); expect(actualDirectives).toEqual(['GenericDir']); }); + + it('has correct tcb location for components with inline TCBs', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = baseTestSetup( + [ + { + fileName, + templates: {'Cmp': '
'}, + // Force an inline TCB by using a non-exported component class + source: `class Cmp {}`, + }, + ], + {inlining: true, config: {enableTemplateTypeChecker: true}}); + const sf = getSourceFileOrError(program, fileName); + const cmp = getClass(sf, 'Cmp'); + + const nodes = templateTypeChecker.getTemplate(cmp)!; + + const symbol = templateTypeChecker.getSymbolOfNode(nodes[0] as TmplAstElement, cmp)!; + assertElementSymbol(symbol); + expect(symbol.tcbLocation.tcbPath).toBe(sf.fileName); + expect(symbol.tcbLocation.isShimFile).toBe(false); + }); + + it('has correct tcb location for components with TCBs in a type-checking shim file', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: {'Cmp': '
'}, + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const cmp = getClass(sf, 'Cmp'); + + const nodes = templateTypeChecker.getTemplate(cmp)!; + + const symbol = templateTypeChecker.getSymbolOfNode(nodes[0] as TmplAstElement, cmp)!; + assertElementSymbol(symbol); + expect(symbol.tcbLocation.tcbPath).not.toBe(sf.fileName); + expect(symbol.tcbLocation.isShimFile).toBe(true); + }); }); }); diff --git a/packages/language-service/src/definitions.ts b/packages/language-service/src/definitions.ts index ab3696cc0c0..76763aebf73 100644 --- a/packages/language-service/src/definitions.ts +++ b/packages/language-service/src/definitions.ts @@ -110,7 +110,7 @@ export class DefinitionBuilder { const tcbLocation = symbol.kind === SymbolKind.Variable ? symbol.localVarLocation : symbol.referenceVarLocation; const mapping = getTemplateLocationFromTcbLocation( - this.compiler.getTemplateTypeChecker(), tcbLocation.tcbPath, + this.compiler.getTemplateTypeChecker(), tcbLocation.tcbPath, tcbLocation.isShimFile, tcbLocation.positionInFile); if (mapping !== null) { definitions.push({ diff --git a/packages/language-service/src/references_and_rename_utils.ts b/packages/language-service/src/references_and_rename_utils.ts index bdbde823225..bd7758fb4a5 100644 --- a/packages/language-service/src/references_and_rename_utils.ts +++ b/packages/language-service/src/references_and_rename_utils.ts @@ -222,7 +222,7 @@ export function convertToTemplateDocumentSpan( // serverHost or LSParseConfigHost in the adapter. We should have a better defined way to // normalize paths. const mapping = getTemplateLocationFromTcbLocation( - templateTypeChecker, absoluteFrom(shimDocumentSpan.fileName), + templateTypeChecker, absoluteFrom(shimDocumentSpan.fileName), /* tcbIsShim */ true, shimDocumentSpan.textSpan.start); if (mapping === null) { return null; diff --git a/packages/language-service/src/utils.ts b/packages/language-service/src/utils.ts index 457aacccdf7..efd2260b4db 100644 --- a/packages/language-service/src/utils.ts +++ b/packages/language-service/src/utils.ts @@ -363,10 +363,10 @@ export function isWithin(position: number, span: AbsoluteSourceSpan|ParseSourceS * the span in the template. */ export function getTemplateLocationFromTcbLocation( - templateTypeChecker: TemplateTypeChecker, shimPath: AbsoluteFsPath, - positionInShimFile: number): {templateUrl: AbsoluteFsPath, span: ParseSourceSpan}|null { + templateTypeChecker: TemplateTypeChecker, tcbPath: AbsoluteFsPath, tcbIsShim: boolean, + positionInFile: number): {templateUrl: AbsoluteFsPath, span: ParseSourceSpan}|null { const mapping = templateTypeChecker.getTemplateMappingAtTcbLocation( - {tcbPath: shimPath, positionInFile: positionInShimFile}); + {tcbPath, isShimFile: tcbIsShim, positionInFile}); if (mapping === null) { return null; }