Skip to content

Commit

Permalink
feat(compiler-cli): detect missing structural directive imports (angu…
Browse files Browse the repository at this point in the history
…lar#59443)

Adds a new diagnostic that ensures that a standalone component using custom structural directives in a template has the necessary imports for those directives.

Fixes angular#37322

PR Close angular#59443
  • Loading branch information
manbearwiz authored and kirjs committed Jan 15, 2025
1 parent 3b3040d commit ed705a8
Show file tree
Hide file tree
Showing 12 changed files with 429 additions and 1 deletion.
61 changes: 61 additions & 0 deletions adev/src/content/reference/extended-diagnostics/NG8114.md
Original file line number Diff line number Diff line change
@@ -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.

<docs-code language="typescript">

import {Component} from '@angular/core';

@Component({
// Template uses `*select`, but no corresponding directive imported.
imports: [],
template: `<p *select="let data from source">{{data}}</p>`,
})
class MyComponent {}

</docs-code>

## 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:

<docs-code language="typescript">

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: `<p *select="let data from source">{{data}}</p>`,
})
class MyComponent {}

</docs-code>

## 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:

<docs-code language="json">
{
"angularCompilerOptions": {
"extendedDiagnostics": {
"checks": {
"missingStructuralDirective": "suppress"
}
}
}
}
</docs-code>

See [extended diagnostic configuration](extended-diagnostics#configuration) for more info.
3 changes: 2 additions & 1 deletion adev/src/content/reference/extended-diagnostics/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ Currently, Angular supports the following extended diagnostics:
| `NG8108` | [`skipHydrationNotStatic`](extended-diagnostics/NG8108) |
| `NG8109` | [`interpolatedSignalNotInvoked`](extended-diagnostics/NG8109) |
| `NG8111` | [`uninvokedFunctionInEventBinding`](extended-diagnostics/NG8111) |
| `NG8113` | [`unusedStandaloneImports`](extended-diagnostics/NG8113) |
| `NG8113` | [`unusedStandaloneImports`](extended-diagnostics/NG8113) |
| `NG8114` | [`missingStructuralDirective`](extended-diagnostics/NG8114) |

## Configuration

Expand Down
1 change: 1 addition & 0 deletions goldens/public-api/compiler-cli/error_code.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export enum ErrorCode {
MISSING_PIPE = 8004,
MISSING_REFERENCE_TARGET = 8003,
MISSING_REQUIRED_INPUTS = 8008,
MISSING_STRUCTURAL_DIRECTIVE = 8114,
NGMODULE_BOOTSTRAP_IS_STANDALONE = 6009,
NGMODULE_DECLARATION_IS_STANDALONE = 6008,
NGMODULE_DECLARATION_NOT_UNIQUE = 6007,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
5 changes: 5 additions & 0 deletions packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,11 @@ export enum ErrorCode {
*/
UNUSED_STANDALONE_IMPORTS = 8113,

/**
* A structural directive is used in a template, but the directive is not imported.
*/
MISSING_STRUCTURAL_DIRECTIVE = 8114,

/**
* 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* @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 {NgCompilerOptions} from '../../../../core/api';
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',
'ngSwitch',
'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<ErrorCode.MISSING_STRUCTURAL_DIRECTIVE> {
override code = ErrorCode.MISSING_STRUCTURAL_DIRECTIVE as const;

override run(
ctx: TemplateContext<ErrorCode.MISSING_STRUCTURAL_DIRECTIVE>,
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<ErrorCode.MISSING_STRUCTURAL_DIRECTIVE>,
component: ts.ClassDeclaration,
node: TmplAstNode | AST,
): NgTemplateDiagnostic<ErrorCode.MISSING_STRUCTURAL_DIRECTIVE>[] {
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 === null || symbol.directives.length > 0) {
return [];
}

const sourceSpan = customStructuralDirective.keySpan || customStructuralDirective.sourceSpan;
const errorMessage =
`An unknown 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.`;
const diagnostic = ctx.makeTemplateDiagnostic(sourceSpan, errorMessage);
return [diagnostic];
}
}

export const factory: TemplateCheckFactory<
ErrorCode.MISSING_STRUCTURAL_DIRECTIVE,
ExtendedTemplateDiagnosticName.MISSING_STRUCTURAL_DIRECTIVE
> = {
code: ErrorCode.MISSING_STRUCTURAL_DIRECTIVE,
name: ExtendedTemplateDiagnosticName.MISSING_STRUCTURAL_DIRECTIVE,
create: (options: NgCompilerOptions) => {
return new MissingStructuralDirectiveCheck();
},
};
2 changes: 2 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/extended/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 suffixNotSupportedFactory} from './checks/suffix_not_supported';
Expand All @@ -32,6 +33,7 @@ export const ALL_DIAGNOSTIC_FACTORIES: readonly TemplateCheckFactory<
missingControlFlowDirectiveFactory,
textAttributeNotBindingFactory,
missingNgForOfLetFactory,
missingStructuralDirectiveFactory,
suffixNotSupportedFactory,
interpolatedSignalNotInvoked,
uninvokedFunctionInEventBindingFactory,
Expand Down
Original file line number Diff line number Diff line change
@@ -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/core:api",
"//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/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",
],
)
Loading

0 comments on commit ed705a8

Please sign in to comment.