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({