From 4f9c824dd9ec4462d29ed07b5e7916be86c19e84 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 5 May 2026 11:24:06 -0700 Subject: [PATCH] feat(language-service): Typecheck templates which would require inline typecheck blocks (#68454) This change updates the language service to generate TCBs for templates that would previously have required inlining. The new strategy is to copy the original source and then do inlining in the external TCB. This allows language features and type-checking in templates of non-exported classes (such as test components) or classes with local, non exported dependencies. PR Close #68454 --- .../src/ngtsc/typecheck/src/checker.ts | 67 +++++++-- .../src/ngtsc/typecheck/src/context.ts | 8 +- .../ngtsc/typecheck/src/type_check_file.ts | 4 +- .../ngtsc/typecheck/test/type_checker_spec.ts | 76 +++++++++- .../language-service/src/language_service.ts | 3 +- .../language-service/test/quick_info_spec.ts | 142 ++++++++++++++++++ 6 files changed, 283 insertions(+), 17 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts index bed86ece944..215c9e99ec4 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts @@ -52,7 +52,7 @@ import { PipeMeta, } from '../../metadata'; import {PerfCheckpoint, PerfEvent, PerfPhase, PerfRecorder} from '../../perf'; -import {ProgramDriver, UpdateMode} from '../../program_driver'; +import {ProgramDriver, UpdateMode, InliningMode} from '../../program_driver'; import { ClassDeclaration, DeclarationNode, @@ -118,6 +118,7 @@ import {DirectiveSourceManager} from './source'; import {findTypeCheckBlock, getSourceMapping, TypeCheckSourceResolver} from './tcb_util'; import {SymbolBuilder, SymbolDirectiveMeta, SymbolBoundTarget} from './template_symbol_builder'; import {findAllMatchingNodes} from './comments'; +import {TCB_FUNCTION_PREFIX} from './type_check_file'; export class TypeCheckableDirectiveMetaAdapter implements SymbolDirectiveMeta { constructor( @@ -285,6 +286,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { * destroyed and replaced. */ private elementTagCache = new Map>(); + private generatedRangeCache = new WeakMap(); private isComplete = false; private priorResultsAdopted = false; @@ -595,6 +597,51 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { this.ensureAllShimsForAllFiles(); } + private getGeneratedCodeRanges(sf: ts.SourceFile): ts.TextRange[] { + if (this.generatedRangeCache.has(sf)) { + return this.generatedRangeCache.get(sf)!; + } + + const ranges: ts.TextRange[] = []; + const visit = (node: ts.Node) => { + if (ts.isInterfaceDeclaration(node)) { + return; + } + if (ts.isFunctionDeclaration(node)) { + if (node.name !== undefined) { + const name = node.name.text; + if (name.startsWith(TCB_FUNCTION_PREFIX)) { + ranges.push({pos: node.getStart(), end: node.getEnd()}); + // TCBs never contain other TCBs or generated utilities, so we can skip traversing inside them. + return; + } + } + } + ts.forEachChild(node, visit); + }; + + // We do a full AST traversal because TCBs can be generated inside closures (e.g. `it` blocks in tests) + // so a shallow scan of top-level statements is insufficient. + ts.forEachChild(sf, visit); + + this.generatedRangeCache.set(sf, ranges); + return ranges; + } + + private filterShimDiagnostics( + shimSf: ts.SourceFile, + semanticDiagnostics: readonly ts.Diagnostic[], + ): readonly ts.Diagnostic[] { + if (this.programDriver.inliningMode !== InliningMode.CopySourceToTcb) { + return semanticDiagnostics; + } + const ranges = this.getGeneratedCodeRanges(shimSf); + return semanticDiagnostics.filter((diag) => { + if (diag.start === undefined) return true; + return ranges.some((range) => diag.start! >= range.pos && diag.start! < range.end); + }); + } + /** * Retrieve type-checking and template parse diagnostics from the given `ts.SourceFile` using the * most recent type-checking program. @@ -626,14 +673,13 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { } for (const [shimPath, shimRecord] of fileRecord.shimData) { - // TODO(atscott): Filter out diagnostics from original source in CopySourceToTcb - // We don't want to duplicate diagnostics from original source when copying to tcb - const shimSf = getSourceFileOrError(typeCheckProgram, shimPath); + const semanticDiagnostics = typeCheckProgram.getSemanticDiagnostics(shimSf); + + const filteredDiagnostics = this.filterShimDiagnostics(shimSf, semanticDiagnostics); + diagnostics.push( - ...typeCheckProgram - .getSemanticDiagnostics(shimSf) - .map((diag) => convertDiagnostic(diag, fileRecord.sourceManager)), + ...filteredDiagnostics.map((diag) => convertDiagnostic(diag, fileRecord.sourceManager)), ); diagnostics.push(...shimRecord.genesisDiagnostics); @@ -715,10 +761,11 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { } const shimSf = getSourceFileOrError(typeCheckProgram, shimPath); + const semanticDiagnostics = typeCheckProgram.getSemanticDiagnostics(shimSf); + const filteredDiagnostics = this.filterShimDiagnostics(shimSf, semanticDiagnostics); + diagnostics.push( - ...typeCheckProgram - .getSemanticDiagnostics(shimSf) - .map((diag) => convertDiagnostic(diag, fileRecord.sourceManager)), + ...filteredDiagnostics.map((diag) => convertDiagnostic(diag, fileRecord.sourceManager)), ); diagnostics.push(...shimRecord.genesisDiagnostics); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index 0cb5e8fadb1..0656d3b4e4e 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -52,7 +52,7 @@ import {ReferenceEmitEnvironment} from './reference_emit_environment'; import {TypeCheckShimGenerator} from './shim'; import {DirectiveSourceManager} from './source'; import {requiresInlineTypeCheckBlock, TcbInliningRequirement} from './tcb_util'; -import {TypeCheckFile} from './type_check_file'; +import {TCB_FUNCTION_PREFIX, TypeCheckFile} from './type_check_file'; import {generateInlineTypeCtor, requiresInlineTypeCtor} from './type_constructor'; export interface ShimTypeCheckingData { @@ -725,7 +725,7 @@ class InlineTcbOp implements Op { if (tcbSf !== originalSf) { env.copiedSourceOriginPath = absoluteFromSourceFile(originalSf); } - const fnName = `_tcb_${this.ref.node.pos}`; + const fnName = `${TCB_FUNCTION_PREFIX}_${this.ref.node.pos}`; const {tcbMeta, component} = adaptTypeCheckBlockMetadata( this.ref, @@ -746,7 +746,9 @@ class InlineTcbOp implements Op { this.oobRecorder, ); - return fn; + // A leading newline is required so that the generated TCB isn't accidentally + // appended as part of a trailing single-line comment (e.g. `// comment`). + return `\n${fn}`; } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts index edb8dc7d4da..9db033ec180 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts @@ -25,6 +25,8 @@ import {Environment} from './environment'; import {ensureTypeCheckFilePreparationImports} from './tcb_util'; import {adaptTypeCheckBlockMetadata} from './tcb_adapter'; +export const TCB_FUNCTION_PREFIX = '_tcb'; + /** * An `Environment` representing the single type-checking file into which most (if not all) Type * Check Blocks (TCBs) will be generated. @@ -79,7 +81,7 @@ export class TypeCheckFile extends Environment { genericContextBehavior: TcbGenericContextBehavior, reflector: ReflectionHost, ): void { - const fnId = `_tcb${this.nextTcbId++}`; + const fnId = `${TCB_FUNCTION_PREFIX}${this.nextTcbId++}`; const {tcbMeta, component} = adaptTypeCheckBlockMetadata( ref, meta, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts index a18758823bc..ef585c39d03 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts @@ -9,6 +9,7 @@ import {ErrorCode, ngErrorCode} from '../../diagnostics'; import {absoluteFrom, getSourceFileOrError} from '../../file_system'; import {runInEachFileSystem} from '../../file_system/testing'; +import {InliningMode} from '../../program_driver'; import {OptimizeFor} from '../api'; import {getClass, setup, TestDeclaration} from '../testing'; @@ -171,7 +172,11 @@ runInEachFileSystem(() => { [ { fileName, - source: `abstract class Cmp {} // not exported, so requires inline`, + source: ` + abstract class Cmp { + value: string = 'hello'; + } + `, templates: {'Cmp': '
'}, }, ], @@ -182,6 +187,75 @@ runInEachFileSystem(() => { expect(diags.length).toBe(1); expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INLINE_TCB_REQUIRED)); }); + + it('should generate TCB in shim file when inlining is unsupported but required', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup( + [ + { + fileName, + source: `abstract class Cmp {} // not exported, so requires inline`, + templates: {'Cmp': '
'}, + }, + ], + {inliningMode: InliningMode.CopySourceToTcb}, + ); + const sf = getSourceFileOrError(program, fileName); + const diags = templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram); + expect(diags.length).toBe(0); + + const cmp = getClass(sf, 'Cmp'); + const block = templateTypeChecker.getTypeCheckBlock(cmp); + expect(block).not.toBeNull(); + expect(block!.getText()).toContain(`Cmp`); + }); + + it('should filter out diagnostics from copied source in CopySourceToTcb mode', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup( + [ + { + fileName, + source: ` + abstract class Cmp { + value: string = 1; // Semantic error + } + `, + templates: {'Cmp': '
'}, + }, + ], + {inliningMode: InliningMode.CopySourceToTcb}, + ); + const sf = getSourceFileOrError(program, fileName); + const diags = templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram); + // The semantic error in the copied source should be filtered out. + expect(diags.length).toBe(0); + }); + + it('should not filter out template errors in CopySourceToTcb mode', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup( + [ + { + fileName, + source: ` + abstract class Cmp { + value: string = 'hello'; + } + `, + templates: {'Cmp': '
{{nonExisting}}
'}, + }, + ], + {inliningMode: InliningMode.CopySourceToTcb}, + ); + const sf = getSourceFileOrError(program, fileName); + const diags = templateTypeChecker.getDiagnosticsForFile(sf, OptimizeFor.WholeProgram); + // The template error should NOT be filtered out. + expect(diags.length).toBe(1); + expect(diags[0].messageText).toContain( + "Property 'nonExisting' does not exist on type 'Cmp'", + ); + }); }); describe('getTemplateOfComponent()', () => { diff --git a/packages/language-service/src/language_service.ts b/packages/language-service/src/language_service.ts index 0ae2452b0e2..33387bac84d 100644 --- a/packages/language-service/src/language_service.ts +++ b/packages/language-service/src/language_service.ts @@ -1036,8 +1036,7 @@ function detectAngularCoreVersion( function createProgramDriver(project: ts.server.Project): ProgramDriver { return { - // TODO: switch to CopySourceToTcb - inliningMode: InliningMode.Error, + inliningMode: InliningMode.CopySourceToTcb, supportsInlineOperations: false, getProgram(): ts.Program { const program = project.getLanguageService().getProgram(); diff --git a/packages/language-service/test/quick_info_spec.ts b/packages/language-service/test/quick_info_spec.ts index 286b7fb905c..15630dff82b 100644 --- a/packages/language-service/test/quick_info_spec.ts +++ b/packages/language-service/test/quick_info_spec.ts @@ -1264,6 +1264,148 @@ describe('quick info', () => { expectedDisplayString: '(reference) ref: TestDirective', }); }); + + it('should get quick info for a component with non-exported generic bound requiring external copy', () => { + project = env.addProject('test', { + 'app.ts': ` + import {Component, NgModule} from '@angular/core'; + + interface PrivateInterface { + title: string; + } + + @Component({ + selector: 'some-cmp', + templateUrl: './app.html', + standalone: false, + }) + export class SomeCmp { + title = 'Hello'; + } + + @NgModule({ + declarations: [SomeCmp], + }) + export class AppModule {} + `, + 'app.html': ``, + }); + + expectQuickInfo({ + templateOverride: `
{{tit¦le}}
`, + expectedSpanText: 'title', + expectedDisplayString: '(property) SomeCmp.title: string', + }); + }); + + it('should get quick info for a non-exported standalone component', () => { + project = env.addProject('test', { + 'app.ts': ` + import {Component} from '@angular/core'; + + @Component({ + selector: 'some-cmp', + templateUrl: './app.html', + standalone: true, + }) + class SomeCmp { + title = 'Hello'; + } + `, + 'app.html': ``, + }); + + expectQuickInfo({ + templateOverride: `
{{tit¦le}}
`, + expectedSpanText: 'title', + expectedDisplayString: '(property) SomeCmp.title: string', + }); + }); + + it('should get quick info for a standalone component defined in a closure', () => { + project = env.addProject('test', { + 'app.ts': ` + import {Component} from '@angular/core'; + + (function() { + @Component({ + selector: 'some-cmp', + templateUrl: './app.html', + standalone: true, + }) + class TestComponent { + value = 0; + } + })(); + `, + 'app.html': ``, + }); + + expectQuickInfo({ + templateOverride: `
{{val¦ue}}
`, + expectedSpanText: 'value', + expectedDisplayString: '(property) TestComponent.value: number', + }); + }); + + it('should get quick info for a component with constrained generic types requiring inline TCB', () => { + project = env.addProject('test', { + 'app.ts': ` + import {Component} from '@angular/core'; + + interface InternalBound {} + + @Component({ + selector: 'some-cmp', + templateUrl: './app.html', + standalone: true, + }) + export class SomeCmp { + title = 'Hello'; + } + `, + 'app.html': ``, + }); + + expectQuickInfo({ + templateOverride: `
{{tit¦le}}
`, + expectedSpanText: 'title', + expectedDisplayString: '(property) SomeCmp.title: string', + }); + }); + + it('should get quick info when using a non-exported pipe requiring inline TCB', () => { + project = env.addProject('test', { + 'app.ts': ` + import {Component, Pipe, PipeTransform} from '@angular/core'; + + @Pipe({ + name: 'internalPipe', + standalone: true, + }) + class InternalPipe implements PipeTransform { + transform(value: string): string { return value; } + } + + @Component({ + selector: 'some-cmp', + templateUrl: './app.html', + standalone: true, + imports: [InternalPipe], + }) + export class SomeCmp { + title = 'Hello'; + } + `, + 'app.html': ``, + }); + + expectQuickInfo({ + templateOverride: `
{{tit¦le | internalPipe}}
`, + expectedSpanText: 'title', + expectedDisplayString: '(property) SomeCmp.title: string', + }); + }); }); function expectQuickInfo({