diff --git a/adev/src/content/reference/extended-diagnostics/NG8116.md b/adev/src/content/reference/extended-diagnostics/NG8116.md new file mode 100644 index 000000000000..a95210a97240 --- /dev/null +++ b/adev/src/content/reference/extended-diagnostics/NG8116.md @@ -0,0 +1,61 @@ +# Missing structural directive + +This diagnostic ensures that a standalone component using custom structural directives (e.g., `*select` or `*featureFlag`) in a template has the necessary imports for those directives. + + + +import {Component} from '@angular/core'; + +@Component({ + // Template uses `*select`, but no corresponding directive imported. + imports: [], + template: `

{{data}}

`, +}) +class MyComponent {} + + + +## What's wrong with that? + +Using a structural directive without importing it will fail at runtime, as Angular attempts to bind to a `select` property of the HTML element, which does not exist. + +## What should I do instead? + +Make sure that the corresponding structural directive is imported into the component: + + + +import {Component} from '@angular/core'; +import {SelectDirective} from 'my-directives'; + +@Component({ + // Add `SelectDirective` to the `imports` array to make it available in the template. + imports: [SelectDirective], + template: `

{{data}}

`, +}) +class MyComponent {} + + + +## Configuration requirements + +[`strictTemplates`](tools/cli/template-typecheck#strict-mode) must be enabled for any extended diagnostic to emit. +`missingStructuralDirective` has no additional requirements beyond `strictTemplates`. + +## What if I can't avoid this? + +This diagnostic can be disabled by editing the project's `tsconfig.json` file: + + +{ + "angularCompilerOptions": { + "extendedDiagnostics": { + "checks": { + "missingStructuralDirective": "suppress" + } + } + } +} + + +See [extended diagnostic configuration](extended-diagnostics#configuration) for more info. \ No newline at end of file diff --git a/adev/src/content/reference/extended-diagnostics/overview.md b/adev/src/content/reference/extended-diagnostics/overview.md index a4d8f8bcfc44..92829d8094be 100644 --- a/adev/src/content/reference/extended-diagnostics/overview.md +++ b/adev/src/content/reference/extended-diagnostics/overview.md @@ -22,6 +22,7 @@ Currently, Angular supports the following extended diagnostics: | `NG8111` | [`uninvokedFunctionInEventBinding`](extended-diagnostics/NG8111) | | `NG8113` | [`unusedStandaloneImports`](extended-diagnostics/NG8113) | | `NG8114` | [`unparenthesizedNullishCoalescing`](extended-diagnostics/NG8114) | +| `NG8116` | [`missingStructuralDirective`](extended-diagnostics/NG8116) | ## Configuration diff --git a/goldens/public-api/compiler-cli/error_code.api.md b/goldens/public-api/compiler-cli/error_code.api.md index b5cb5c508ca2..25baef6ecde6 100644 --- a/goldens/public-api/compiler-cli/error_code.api.md +++ b/goldens/public-api/compiler-cli/error_code.api.md @@ -79,6 +79,7 @@ export enum ErrorCode { MISSING_PIPE = 8004, MISSING_REFERENCE_TARGET = 8003, MISSING_REQUIRED_INPUTS = 8008, + MISSING_STRUCTURAL_DIRECTIVE = 8116, NGMODULE_BOOTSTRAP_IS_STANDALONE = 6009, NGMODULE_DECLARATION_IS_STANDALONE = 6008, NGMODULE_DECLARATION_NOT_UNIQUE = 6007, diff --git a/goldens/public-api/compiler-cli/extended_template_diagnostic_name.api.md b/goldens/public-api/compiler-cli/extended_template_diagnostic_name.api.md index bf905da4198d..e6bc95240dff 100644 --- a/goldens/public-api/compiler-cli/extended_template_diagnostic_name.api.md +++ b/goldens/public-api/compiler-cli/extended_template_diagnostic_name.api.md @@ -17,6 +17,8 @@ export enum ExtendedTemplateDiagnosticName { // (undocumented) MISSING_NGFOROF_LET = "missingNgForOfLet", // (undocumented) + MISSING_STRUCTURAL_DIRECTIVE = "missingStructuralDirective", + // (undocumented) NULLISH_COALESCING_NOT_NULLABLE = "nullishCoalescingNotNullable", // (undocumented) OPTIONAL_CHAIN_NOT_NULLABLE = "optionalChainNotNullable", diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts index 07a8c3dd3d39..7b8aa08e20c0 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts @@ -559,6 +559,11 @@ export enum ErrorCode { */ UNINVOKED_TRACK_FUNCTION = 8115, + /** + * A structural directive is used in a template, but the directive is not imported. + */ + MISSING_STRUCTURAL_DIRECTIVE = 8116, + /** * The template type-checking engine would need to generate an inline type check block for a * component, but the current type-checking environment doesn't support it. diff --git a/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts b/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts index 600a4a4e4816..c0e8c08e213d 100644 --- a/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts +++ b/packages/compiler-cli/src/ngtsc/diagnostics/src/extended_template_diagnostic_name.ts @@ -20,6 +20,7 @@ export enum ExtendedTemplateDiagnosticName { NULLISH_COALESCING_NOT_NULLABLE = 'nullishCoalescingNotNullable', OPTIONAL_CHAIN_NOT_NULLABLE = 'optionalChainNotNullable', MISSING_CONTROL_FLOW_DIRECTIVE = 'missingControlFlowDirective', + MISSING_STRUCTURAL_DIRECTIVE = 'missingStructuralDirective', TEXT_ATTRIBUTE_NOT_BINDING = 'textAttributeNotBinding', UNINVOKED_FUNCTION_IN_EVENT_BINDING = 'uninvokedFunctionInEventBinding', MISSING_NGFOROF_LET = 'missingNgForOfLet', diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel index 24ab4b81b4e9..32228438a17d 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/BUILD.bazel @@ -16,6 +16,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/invalid_banana_in_box", "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_control_flow_directive", "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_ngforof_let", + "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_structural_directive", "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/nullish_coalescing_not_nullable", "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/optional_chain_not_nullable", "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/skip_hydration_not_static", diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_structural_directive/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_structural_directive/BUILD.bazel new file mode 100644 index 000000000000..a8e517f1d7c2 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_structural_directive/BUILD.bazel @@ -0,0 +1,15 @@ +load("//tools:defaults.bzl", "ts_library") + +ts_library( + name = "missing_structural_directive", + srcs = ["index.ts"], + visibility = ["//packages/compiler-cli/src/ngtsc:__subpackages__"], + deps = [ + "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/core:api", + "//packages/compiler-cli/src/ngtsc/diagnostics", + "//packages/compiler-cli/src/ngtsc/typecheck/api", + "//packages/compiler-cli/src/ngtsc/typecheck/extended/api", + "@npm//typescript", + ], +) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_structural_directive/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_structural_directive/index.ts new file mode 100644 index 000000000000..26d5a4d31af0 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_structural_directive/index.ts @@ -0,0 +1,88 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import {AST, TmplAstNode, TmplAstTemplate} from '@angular/compiler'; +import ts from 'typescript'; + +import {ErrorCode, ExtendedTemplateDiagnosticName} from '../../../../diagnostics'; +import {NgTemplateDiagnostic} from '../../../api'; +import {TemplateCheckFactory, TemplateCheckWithVisitor, TemplateContext} from '../../api'; + +/** + * The list of known control flow directives present in the `CommonModule`. + * + * If these control flow directives are missing they will be reported by a separate diagnostic. + */ +export const KNOWN_CONTROL_FLOW_DIRECTIVES = new Set([ + 'ngIf', + 'ngFor', + 'ngForOf', + 'ngForTrackBy', + 'ngSwitchCase', + 'ngSwitchDefault', +]); + +/** + * Ensures that there are no structural directives (something like *select or *featureFlag) + * used in a template of a *standalone* component without importing the directive. Returns + * diagnostics in case such a directive is detected. + * + * Note: this check only handles the cases when structural directive syntax is used (e.g. `*featureFlag`). + * Regular binding syntax (e.g. `[featureFlag]`) is handled separately in type checker and treated as a + * hard error instead of a warning. + */ +class MissingStructuralDirectiveCheck extends TemplateCheckWithVisitor { + override code = ErrorCode.MISSING_STRUCTURAL_DIRECTIVE as const; + + override run( + ctx: TemplateContext, + component: ts.ClassDeclaration, + template: TmplAstNode[], + ) { + const componentMetadata = ctx.templateTypeChecker.getDirectiveMetadata(component); + // Avoid running this check for non-standalone components. + if (!componentMetadata || !componentMetadata.isStandalone) { + return []; + } + return super.run(ctx, component, template); + } + + override visitNode( + ctx: TemplateContext, + component: ts.ClassDeclaration, + node: TmplAstNode | AST, + ): NgTemplateDiagnostic[] { + if (!(node instanceof TmplAstTemplate)) return []; + + const customStructuralDirective = node.templateAttrs.find( + (attr) => !KNOWN_CONTROL_FLOW_DIRECTIVES.has(attr.name), + ); + if (!customStructuralDirective) return []; + + const symbol = ctx.templateTypeChecker.getSymbolOfNode(node, component); + if (symbol?.directives.length) { + return []; + } + + const sourceSpan = customStructuralDirective.keySpan || customStructuralDirective.sourceSpan; + const errorMessage = + `A structural directive \`${customStructuralDirective.name}\` was used in the template ` + + `without a corresponding import in the component. ` + + `Make sure that the directive is included in the \`@Component.imports\` array of this component.`; + return [ctx.makeTemplateDiagnostic(sourceSpan, errorMessage)]; + } +} + +export const factory: TemplateCheckFactory< + ErrorCode.MISSING_STRUCTURAL_DIRECTIVE, + ExtendedTemplateDiagnosticName.MISSING_STRUCTURAL_DIRECTIVE +> = { + code: ErrorCode.MISSING_STRUCTURAL_DIRECTIVE, + name: ExtendedTemplateDiagnosticName.MISSING_STRUCTURAL_DIRECTIVE, + create: () => new MissingStructuralDirectiveCheck(), +}; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts index d1223e3043f1..f03074edeaa0 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts @@ -13,6 +13,7 @@ import {factory as interpolatedSignalNotInvoked} from './checks/interpolated_sig import {factory as invalidBananaInBoxFactory} from './checks/invalid_banana_in_box'; import {factory as missingControlFlowDirectiveFactory} from './checks/missing_control_flow_directive'; import {factory as missingNgForOfLetFactory} from './checks/missing_ngforof_let'; +import {factory as missingStructuralDirectiveFactory} from './checks/missing_structural_directive'; import {factory as nullishCoalescingNotNullableFactory} from './checks/nullish_coalescing_not_nullable'; import {factory as optionalChainNotNullableFactory} from './checks/optional_chain_not_nullable'; import {factory as skipHydrationNotStaticFactory} from './checks/skip_hydration_not_static'; @@ -35,6 +36,7 @@ export const ALL_DIAGNOSTIC_FACTORIES: readonly TemplateCheckFactory< missingControlFlowDirectiveFactory, textAttributeNotBindingFactory, missingNgForOfLetFactory, + missingStructuralDirectiveFactory, suffixNotSupportedFactory, interpolatedSignalNotInvoked, uninvokedFunctionInEventBindingFactory, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_structural_directive/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_structural_directive/BUILD.bazel new file mode 100644 index 000000000000..ee8b71e077cd --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_structural_directive/BUILD.bazel @@ -0,0 +1,30 @@ +load("//tools:defaults.bzl", "jasmine_node_test", "ts_library") + +ts_library( + name = "test_lib", + testonly = True, + srcs = ["missing_structural_directive_spec.ts"], + deps = [ + "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/diagnostics", + "//packages/compiler-cli/src/ngtsc/file_system", + "//packages/compiler-cli/src/ngtsc/file_system/testing", + "//packages/compiler-cli/src/ngtsc/testing", + "//packages/compiler-cli/src/ngtsc/typecheck/api", + "//packages/compiler-cli/src/ngtsc/typecheck/extended", + "//packages/compiler-cli/src/ngtsc/typecheck/extended/checks/missing_structural_directive", + "//packages/compiler-cli/src/ngtsc/typecheck/testing", + "@npm//typescript", + ], +) + +jasmine_node_test( + name = "test", + bootstrap = ["//tools/testing:node_no_angular"], + data = [ + "//packages/core:npm_package", + ], + deps = [ + ":test_lib", + ], +) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_structural_directive/missing_structural_directive_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_structural_directive/missing_structural_directive_spec.ts new file mode 100644 index 000000000000..fa9968846176 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/missing_structural_directive/missing_structural_directive_spec.ts @@ -0,0 +1,341 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import ts from 'typescript'; + +import {ErrorCode, ngErrorCode} from '../../../../../diagnostics'; +import {absoluteFrom, getSourceFileOrError} from '../../../../../file_system'; +import {runInEachFileSystem} from '../../../../../file_system/testing'; +import {createNgCompilerForFile, getClass, setup} from '../../../../testing'; +import {factory as missingStructuralDirectiveCheck} from '../../../checks/missing_structural_directive'; +import {ExtendedTemplateCheckerImpl} from '../../../src/extended_template_checker'; +import {OptimizeFor} from '../../../../api'; + +runInEachFileSystem(() => { + describe('missingStructuralDirectiveCheck', () => { + it('should produce a warning for missing unknown structural directives in standalone components', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': `
`, + }, + declarations: [ + { + name: 'TestCmp', + type: 'directive', + selector: `[test-cmp]`, + isStandalone: true, + }, + ], + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [missingStructuralDirectiveCheck], + {strictNullChecks: true} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + + expect(diags.length).toBe(1); + expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.MISSING_STRUCTURAL_DIRECTIVE)); + }); + + it('should produce a warning if ngTemplateOutlet is used without importing the directive', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': ` + Ahoj {{ person }}!`, + }, + declarations: [ + { + name: 'TestCmp', + type: 'directive', + selector: `[test-cmp]`, + isStandalone: true, + }, + ], + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [missingStructuralDirectiveCheck], + {strictNullChecks: true} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + + expect(diags.length).toBe(1); + expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning); + expect(diags[0].code).toBe(ngErrorCode(ErrorCode.MISSING_STRUCTURAL_DIRECTIVE)); + }); + + it('should *not* produce a warning for custom structural directives that are imported when there is a non-exported class', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': `
`, + 'BarCmp': '', + }, + source: ` + class BarCmp{} + + export class TestCmp {} + export class Foo {} + + `, + declarations: [ + { + type: 'directive', + name: 'Foo', + selector: `[foo]`, + }, + { + name: 'TestCmp', + type: 'directive', + selector: `[test-cmp]`, + isStandalone: true, + }, + { + name: 'BarCmp', + type: 'directive', + selector: `[bar-cmp]`, + isStandalone: true, + }, + ], + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [missingStructuralDirectiveCheck], + {strictNullChecks: true} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + // No diagnostic messages are expected. + expect(diags.length).toBe(0); + }); + + it('should *not* produce a warning for custom structural directives that are imported', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': `
`, + }, + source: ` + export class TestCmp {} + export class Foo {} + `, + declarations: [ + { + type: 'directive', + name: 'Foo', + selector: `[foo]`, + }, + { + name: 'TestCmp', + type: 'directive', + selector: `[test-cmp]`, + isStandalone: true, + }, + ], + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [missingStructuralDirectiveCheck], + {strictNullChecks: true} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + // No diagnostic messages are expected. + expect(diags.length).toBe(0); + }); + + it('should *not* produce a warning for non-standalone components', () => { + const fileName = absoluteFrom('/main.ts'); + + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': `
`, + }, + declarations: [ + { + name: 'TestCmp', + type: 'directive', + selector: `[test-cmp]`, + isStandalone: false, + }, + ], + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [missingStructuralDirectiveCheck], + {strictNullChecks: true} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + // No diagnostic messages are expected. + expect(diags.length).toBe(0); + }); + + it('should *not* produce a warning for non-structural directives in standalone components', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': `
`, + }, + declarations: [ + { + name: 'TestCmp', + type: 'directive', + selector: `[test-cmp]`, + isStandalone: true, + }, + ], + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [missingStructuralDirectiveCheck], + {strictNullChecks: true} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + // No diagnostic messages are expected. + expect(diags.length).toBe(0); + }); + + it('should *not* produce a warning when known control flow directives are used', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': `
+
  • + {{ item.name }} +
    • + +
    +
    +     +
    `, + }, + declarations: [ + { + name: 'TestCmp', + type: 'directive', + selector: `[test-cmp]`, + isStandalone: true, + }, + ], + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [missingStructuralDirectiveCheck], + {strictNullChecks: true} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + // No diagnostic messages are expected. + expect(diags.length).toBe(0); + }); + + it('should *not* produce a warning for templates with no structural directives', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = setup([ + { + fileName, + templates: { + 'TestCmp': `
    `, + }, + declarations: [ + { + name: 'TestCmp', + type: 'directive', + selector: `[test-cmp]`, + isStandalone: true, + }, + ], + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const component = getClass(sf, 'TestCmp'); + const extendedTemplateChecker = new ExtendedTemplateCheckerImpl( + templateTypeChecker, + program.getTypeChecker(), + [missingStructuralDirectiveCheck], + {strictNullChecks: true} /* options */, + ); + const diags = extendedTemplateChecker.getDiagnosticsForComponent(component); + // No diagnostic messages are expected. + expect(diags.length).toBe(0); + }); + + it('should trigger diagnostic for nested component in function', () => { + // This test is more complex as we're testing the diagnostic against a component + // that can't be referenced because it's nested in a function. + + const {compiler, sourceFile} = createNgCompilerForFile( + `import {Component, Directive} from '@angular/core'; + + @Directive({ selector: '[foo]' }) + export class FooDir {} + + export function foo() { + @Component({ + imports: [FooDir], + template: '
    ', + }) + class MyCmp {} + }`, + ); + + const diags = compiler.getDiagnosticsForFile(sourceFile, OptimizeFor.SingleFile); + // Note sure why we get this diagnostic + // It is still raised if we pass `lib: ['dom']` to the compiler options + expect(diags.length).toBe(1); + expect(diags[0].messageText).toContain( + `Cannot find name 'document'. Do you need to change your target library? `, + ); + + // What matters is that we don't get the missing structural directive diagnostic. + }); + }); +}); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/symbol_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/symbol_util.ts index 6d318b9fe9fd..8420b752d4d0 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/symbol_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/symbol_util.ts @@ -44,7 +44,9 @@ function isSignalSymbol(symbol: ts.Symbol): boolean { return ( (ts.isInterfaceDeclaration(decl) || ts.isTypeAliasDeclaration(decl)) && SIGNAL_FNS.has(decl.name.text) && - (fileName.includes('@angular/core') || fileName.includes('angular2/rc/packages/core')) + (fileName.includes('@angular/core') || + fileName.includes('angular2/rc/packages/core') || + fileName.includes('bin/packages/core')) // for local usage in some tests ); }) ); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts index 1dd33905c001..c876571e22f0 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts @@ -168,12 +168,21 @@ export function findTypeCheckBlock( id: TypeCheckId, isDiagnosticRequest: boolean, ): ts.Node | null { + // This prioritised-level statements using a breadth-first search + // This is usually sufficient to find the TCB we're looking for for (const stmt of file.statements) { if (ts.isFunctionDeclaration(stmt) && getTypeCheckId(stmt, file, isDiagnosticRequest) === id) { return stmt; } } - return null; + + // In case the TCB we're looking for is nested (which is not common) + // eg: when a directive is declared inside a function, as it can happen in test files + return findNodeInFile( + file, + (node) => + ts.isFunctionDeclaration(node) && getTypeCheckId(node, file, isDiagnosticRequest) === id, + ); } /** @@ -266,3 +275,24 @@ export function checkIfGenericTypeBoundsCanBeEmitted( const emitter = new TypeParameterEmitter(node.typeParameters, reflector); return emitter.canEmit((ref) => env.canReferenceType(ref)); } + +export function findNodeInFile( + file: ts.SourceFile, + predicate: (node: ts.Node) => node is T, +): T | null; +export function findNodeInFile( + file: ts.SourceFile, + predicate: (node: ts.Node) => boolean, +): ts.Node | null; +export function findNodeInFile( + file: ts.SourceFile, + predicate: (node: ts.Node) => boolean, +): ts.Node | null { + const visit = (node: ts.Node): ts.Node | null => { + if (predicate(node)) { + return node; + } + return ts.forEachChild(node, visit) ?? null; + }; + return ts.forEachChild(file, visit) ?? null; +} 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 e2dd791444ac..2e146b27beb7 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 @@ -62,6 +62,7 @@ import { } from './comments'; import {TypeCheckData} from './context'; import {isAccessExpression} from './ts_util'; +import {MaybeSourceFileWithOriginalFile, NgOriginalFile} from '../../program_driver'; /** * Generates and caches `Symbol`s for various template structures for a given component. @@ -255,7 +256,7 @@ export class SymbolBuilder { private getDirectiveMeta( host: TmplAstTemplate | TmplAstElement, - directiveDeclaration: ts.Declaration, + directiveDeclaration: ts.ClassDeclaration, ): TypeCheckableDirectiveMeta | null { let directives = this.typeCheckData.boundTarget.getDirectivesOfNode(host); @@ -279,7 +280,34 @@ export class SymbolBuilder { return null; } - return directives.find((m) => m.ref.node === directiveDeclaration) ?? null; + const directive = directives.find((m) => m.ref.node === directiveDeclaration); + if (directive) { + return directive; + } + + const originalFile = (directiveDeclaration.getSourceFile() as MaybeSourceFileWithOriginalFile)[ + NgOriginalFile + ]; + + if (originalFile !== undefined) { + // This is a preliminary check ahead of a more expensive search + const hasPotentialCandidate = directives.find( + (m) => m.ref.node.name.text === directiveDeclaration.name?.text, + ); + + if (hasPotentialCandidate) { + // In case the TCB has been inlined, + // We will look for a matching class + // If we find one, we look for it in the directives array + const classWithSameName = findMatchingDirective(originalFile, directiveDeclaration); + if (classWithSameName !== null) { + return directives.find((m) => m.ref.node === classWithSameName) ?? null; + } + } + } + + // Really nothing was found + return null; } private getDirectiveModule(declaration: ts.ClassDeclaration): ClassDeclaration | null { @@ -862,3 +890,47 @@ function unwrapSignalInputWriteTAccessor(expr: ts.LeftHandSideExpression): null typeExpr: expr, }; } + +/** + * Looks for a class declaration in the original source file that matches a given directive + * from the type check source file. + * + * @param originalSourceFile The original source where the runtime code resides + * @param directiveDeclarationInTypeCheckSourceFile The directive from the type check source file + */ +function findMatchingDirective( + originalSourceFile: ts.SourceFile, + directiveDeclarationInTypeCheckSourceFile: ts.ClassDeclaration, +): ts.ClassDeclaration | null { + const className = directiveDeclarationInTypeCheckSourceFile.name?.text ?? ''; + // We build an index of the class declarations with the same name + // To then compare the indexes to confirm we found the right class declaration + const ogClasses = collectClassesWithName(originalSourceFile, className); + const typecheckClasses = collectClassesWithName( + directiveDeclarationInTypeCheckSourceFile.getSourceFile(), + className, + ); + + return ogClasses[typecheckClasses.indexOf(directiveDeclarationInTypeCheckSourceFile)] ?? null; +} + +/** + * Builds a list of class declarations of a given name + * Is used as a index based reference to compare class declarations + * between the typecheck source file and the original source file + */ +function collectClassesWithName( + sourceFile: ts.SourceFile, + className: string, +): ts.ClassDeclaration[] { + const classes: ts.ClassDeclaration[] = []; + function visit(node: ts.Node) { + if (ts.isClassDeclaration(node) && node.name?.text === className) { + classes.push(node); + } + ts.forEachChild(node, visit); + } + sourceFile.forEachChild(visit); + + return classes; +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel index c06cf6251dc9..25ba8d6b88ae 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/BUILD.bazel @@ -11,6 +11,7 @@ ts_library( deps = [ "//packages:types", "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/core", "//packages/compiler-cli/src/ngtsc/diagnostics", "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/file_system/testing", 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 27b812924dcc..32ea8a1d8a41 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 @@ -55,7 +55,10 @@ import { ngForTypeCheckTarget, setup as baseTestSetup, TypeCheckingTarget, + createNgCompilerForFile, } from '../testing'; +import {TsCreateProgramDriver} from '../../program_driver'; +import {findNodeInFile} from '../src/tcb_util'; runInEachFileSystem(() => { describe('TemplateTypeChecker.getSymbolOfNode', () => { @@ -2107,12 +2110,110 @@ runInEachFileSystem(() => { const nodes = templateTypeChecker.getTemplate(cmp)!; - const symbol = templateTypeChecker.getSymbolOfNode(nodes[0] as TmplAstElement, cmp)!; + const symbol = templateTypeChecker.getSymbolOfNode(nodes[0], cmp)!; assertElementSymbol(symbol); expect(symbol.tcbLocation.tcbPath).toBe(sf.fileName); expect(symbol.tcbLocation.isShimFile).toBe(false); }); + it('finds the directive when relying on inline TCB', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = baseTestSetup([ + { + fileName, + templates: { + 'TestCmp': `
    `, + 'BarCmp': '', + }, + source: ` + /* Declare a non-exported component to force using an inline TCB */ + class BarCmp{} + + export class TestCmp {} + export class Foo {} + + `, + declarations: [ + { + type: 'directive', + name: 'Foo', + selector: `[foo]`, + }, + { + name: 'TestCmp', + type: 'directive', + selector: `[test-cmp]`, + }, + { + name: 'BarCmp', + type: 'directive', + selector: `[bar-cmp]`, + }, + ], + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const testCmp = getClass(sf, 'TestCmp'); + const nodes = templateTypeChecker.getTemplate(testCmp)!; + + const symbol = templateTypeChecker.getSymbolOfNode(nodes[0], testCmp)!; + assertTemplateSymbol(symbol); + expect(symbol.directives.length).toBe(1); + expect(symbol.directives[0].selector).toBe('[foo]'); + }); + + it('finds the right directive when relying on inline TCB and having multiple classes with the same name in the scope', () => { + const fileName = absoluteFrom('/main.ts'); + const {program, templateTypeChecker} = baseTestSetup([ + { + fileName, + templates: { + 'TestCmp': `
    `, + 'BarCmp': '', + }, + source: ` + class BarCmp{} + + export class Foo {} + export class TestCmp { + foo() { + // The test should not match this class + class Foo { + ThisIsNotTheClassYoureLookingFor = true; + } + return Foo; + } + } + `, + declarations: [ + { + type: 'directive', + name: 'Foo', + selector: `[foo]`, + }, + { + name: 'TestCmp', + type: 'directive', + selector: `[test-cmp]`, + }, + { + name: 'BarCmp', + type: 'directive', + selector: `[bar-cmp]`, + }, + ], + }, + ]); + const sf = getSourceFileOrError(program, fileName); + const testCmp = getClass(sf, 'TestCmp'); + const nodes = templateTypeChecker.getTemplate(testCmp)!; + + const symbol = templateTypeChecker.getSymbolOfNode(nodes[0], testCmp)!; + assertTemplateSymbol(symbol); + expect(symbol.directives.length).toBe(1); + expect(symbol.directives[0].selector).toBe('[foo]'); + }); + it('has correct tcb location for components with TCBs in a type-checking shim file', () => { const fileName = absoluteFrom('/main.ts'); const {program, templateTypeChecker} = setup([ @@ -2131,6 +2232,124 @@ runInEachFileSystem(() => { expect(symbol.tcbLocation.tcbPath).not.toBe(sf.fileName); expect(symbol.tcbLocation.isShimFile).toBe(true); }); + + it('find the directive when the class is nested in a function', () => { + // This test is more complex as we're testing the diagnostic against a component + // that can't be referenced because it's nested in a function. + + const {compiler, sourceFile} = createNgCompilerForFile(` + import {Component, Directive} from '@angular/core'; + + @Directive({ selector: '[foo]' }) + export class FooDir {} + + export function foo() { + @Component({ + imports: [FooDir], + template: '
    ', + }) + class MyCmp {} + } + `); + + const templateTypeChecker = compiler.getTemplateTypeChecker(); + + const myCmpClass = findNodeInFile( + sourceFile, + (node): node is ts.ClassDeclaration => + ts.isClassDeclaration(node) && node.name?.text === 'MyCmp', + )!; + + const nodes = templateTypeChecker.getTemplate(myCmpClass)!; + const symbol = templateTypeChecker.getSymbolOfNode(nodes[0], myCmpClass)!; + + assertTemplateSymbol(symbol); + expect(symbol.kind).toBe(SymbolKind.Template); + expect(symbol.directives.length).toBe(1); + expect(symbol.directives[0].selector).toBe('[foo]'); + }); + + it('find the directive when the class is nested in a function and has other pention candidates', () => { + // This test is more complex as we're testing the diagnostic against a component + // that can't be referenced because it's nested in a function. + + const {compiler, sourceFile} = createNgCompilerForFile(` + import {Component, Directive} from '@angular/core'; + + if(true) { + @Directive({ selector: '[foo]' }) + export class FooDir {} + + export function foo() { + @Component({ + imports: [FooDir], + template: '
    ', + }) + class MyCmp {} + } + } + + if(true) { + @Directive({ selector: '[foo]' }) + export class FooDir { + /* we should not match this directive */ + } + } + `); + + const templateTypeChecker = compiler.getTemplateTypeChecker(); + + const myCmpClass = findNodeInFile( + sourceFile, + (node): node is ts.ClassDeclaration => + ts.isClassDeclaration(node) && node.name?.text === 'MyCmp', + )!; + + const nodes = templateTypeChecker.getTemplate(myCmpClass)!; + const symbol = templateTypeChecker.getSymbolOfNode(nodes[0], myCmpClass)!; + assertTemplateSymbol(symbol); + expect(symbol.kind).toBe(SymbolKind.Template); + expect(symbol.directives.length).toBe(1); + expect(symbol.directives[0].selector).toBe('[foo]'); + }); + + it('find the directive when it is nested inside a class of the same name', () => { + // This test is more complex as we're testing the diagnostic against a component + // that can't be referenced because it's nested in a function. + + const {compiler, sourceFile} = createNgCompilerForFile(` + import {Component, Directive} from '@angular/core'; + + /* We name this class with the same name as the directive */ + class FooDir { + foo() { + @Directive({ selector: '[foo]' }) + export class FooDir {} + + @Component({ + imports: [FooDir], + template: '
    ', + }) + class MyCmp {} + } + } + `); + + const templateTypeChecker = compiler.getTemplateTypeChecker(); + + const myCmpClass = findNodeInFile( + sourceFile, + (node): node is ts.ClassDeclaration => + ts.isClassDeclaration(node) && node.name?.text === 'MyCmp', + )!; + + const nodes = templateTypeChecker.getTemplate(myCmpClass)!; + const symbol = templateTypeChecker.getSymbolOfNode(nodes[0], myCmpClass)!; + assertTemplateSymbol(symbol); + expect(symbol.kind).toBe(SymbolKind.Template); + expect(symbol.directives.length).toBe(1); + expect(symbol.directives[0].selector).toBe('[foo]'); + }); }); }); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/BUILD.bazel b/packages/compiler-cli/src/ngtsc/typecheck/testing/BUILD.bazel index f1eb155145b7..71ec9e658b3e 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/BUILD.bazel @@ -9,6 +9,7 @@ ts_library( visibility = ["//packages/compiler-cli/src/ngtsc:__subpackages__"], deps = [ "//packages/compiler", + "//packages/compiler-cli/src/ngtsc/core", "//packages/compiler-cli/src/ngtsc/file_system", "//packages/compiler-cli/src/ngtsc/file_system/testing", "//packages/compiler-cli/src/ngtsc/imports", diff --git a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts index 4186a042b540..a095360107e7 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/testing/index.ts @@ -33,8 +33,10 @@ import {globSync} from 'tinyglobby'; import { absoluteFrom, AbsoluteFsPath, + getFileSystem, getSourceFileOrError, LogicalFileSystem, + NgtscCompilerHost, } from '../../file_system'; import {TestFile} from '../../file_system/testing'; import { @@ -46,7 +48,7 @@ import { ReferenceEmitter, RelativePathStrategy, } from '../../imports'; -import {NOOP_INCREMENTAL_BUILD} from '../../incremental'; +import {NOOP_INCREMENTAL_BUILD, NoopIncrementalBuildStrategy} from '../../incremental'; import { ClassPropertyMapping, CompoundMetadataReader, @@ -97,6 +99,7 @@ import {TypeCheckShimGenerator} from '../src/shim'; import {TcbGenericContextBehavior} from '../src/type_check_block'; import {TypeCheckFile} from '../src/type_check_file'; import {sfExtensionData} from '../../shims'; +import {freshCompilationTicket, NgCompiler, NgCompilerHost} from '../../core'; export function typescriptLibDts(): TestFile { return { @@ -1042,3 +1045,31 @@ export class NoopOobRecorder implements OutOfBandDiagnosticRecorder { node: TmplAstComponent | TmplAstDirective, ): void {} } + +export function createNgCompilerForFile(fileContent: string) { + const fs = getFileSystem(); + fs.ensureDir(absoluteFrom('/node_modules/@angular/core')); + const FILE = absoluteFrom('/main.ts'); + + fs.writeFile(FILE, fileContent); + + const options: ts.CompilerOptions = { + strictTemplates: true, + lib: ['dom', 'dom.iterable', 'esnext'], + }; + const baseHost = new NgtscCompilerHost(getFileSystem(), options); + const host = NgCompilerHost.wrap(baseHost, [FILE], options, /* oldProgram */ null); + const program = ts.createProgram({host, options, rootNames: host.inputFiles}); + + const ticket = freshCompilationTicket( + program, + options, + new NoopIncrementalBuildStrategy(), + new TsCreateProgramDriver(program, host, options, []), + /* perfRecorder */ null, + /*enableTemplateTypeChecker*/ true, + /*usePoisonedData*/ false, + ); + const compiler = NgCompiler.fromTicket(ticket, host); + return {compiler, sourceFile: program.getSourceFile(FILE)!}; +}