Skip to content

Commit daa47b4

Browse files
leonsenftatscott
leonsenft
authored and
atscott
committed
refactor(compiler-cli): validate foreign component bindings during analysis
Refactors the unsupported bindings validation for foreign components from the template semantics checker phase (during type-checking) to the component analysis phase. Surfacing this check during component analysis means it will be correctly reported during local compilation (which skips full template type-checking). Specifically: - Creates a new helper `analyzeForeignComponentFeatures` in `foreign_component.ts` that traverses template elements and checks for unsupported outputs, references, and non-property inputs on foreign components. - Removes the legacy validation from `template_semantics_checker.ts`. - Invokes the validation during component analysis in `ComponentDecoratorHandler.analyze()`.
1 parent 5660796 commit daa47b4

4 files changed

Lines changed: 109 additions & 59 deletions

File tree

‎packages/compiler-cli/src/ngtsc/annotations/component/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ ts_project(
3030
"//packages/compiler-cli/src/ngtsc/transform",
3131
"//packages/compiler-cli/src/ngtsc/typecheck",
3232
"//packages/compiler-cli/src/ngtsc/typecheck/api",
33+
"//packages/compiler-cli/src/ngtsc/typecheck/diagnostics",
3334
"//packages/compiler-cli/src/ngtsc/typecheck/extended/api",
3435
"//packages/compiler-cli/src/ngtsc/typecheck/template_semantics/api",
3536
"//packages/compiler-cli/src/ngtsc/util",

‎packages/compiler-cli/src/ngtsc/annotations/component/src/foreign_component.ts‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import {
10+
BindingType,
11+
SelectorlessMatcher,
12+
TmplAstElement,
13+
TmplAstRecursiveVisitor,
14+
tmplAstVisitAll,
15+
TypeCheckId,
16+
} from '@angular/compiler';
17+
import ts from 'typescript';
18+
19+
import {ParsedTemplateWithSource} from './resources';
20+
import {ErrorCode, ngErrorCode} from '../../../diagnostics';
21+
import {ForeignComponentMeta} from '../../../metadata';
22+
import {makeTemplateDiagnostic} from '../../../typecheck/diagnostics';
23+
import {SourceMapping} from '../../../typecheck/api';
24+
25+
/**
26+
* Analyzes the template for invalid use of features relating to foreign components.
27+
*
28+
* @param template The template to analyze.
29+
* @param foreignMatcher A matcher that can be used to identify foreign components.
30+
* @returns A list of diagnostics that should be reported for the template.
31+
*/
32+
export function analyzeForeignComponentFeatures(
33+
template: ParsedTemplateWithSource,
34+
foreignMatcher: SelectorlessMatcher<ForeignComponentMeta> | null,
35+
): ts.Diagnostic[] {
36+
const analyzer = new ForeignComponentFeatureAnalyzer(foreignMatcher, template.sourceMapping);
37+
tmplAstVisitAll(analyzer, template.nodes);
38+
return analyzer.diagnostics;
39+
}
40+
41+
class ForeignComponentFeatureAnalyzer extends TmplAstRecursiveVisitor {
42+
readonly diagnostics: ts.Diagnostic[] = [];
43+
44+
constructor(
45+
private readonly foreignMatcher: SelectorlessMatcher<ForeignComponentMeta> | null,
46+
private readonly sourceMapping: SourceMapping,
47+
) {
48+
super();
49+
}
50+
51+
private elementIsForeignComponent(tagName: string): boolean {
52+
return this.foreignMatcher !== null && this.foreignMatcher.match(tagName).length > 0;
53+
}
54+
55+
override visitElement(element: TmplAstElement): void {
56+
if (this.elementIsForeignComponent(element.name)) {
57+
if (element.outputs.length > 0) {
58+
this.diagnostics.push(
59+
makeTemplateDiagnostic(
60+
'' as TypeCheckId,
61+
this.sourceMapping,
62+
element.sourceSpan,
63+
ts.DiagnosticCategory.Error,
64+
ngErrorCode(ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING),
65+
'Foreign components do not support event bindings.',
66+
),
67+
);
68+
}
69+
if (element.references.length > 0) {
70+
this.diagnostics.push(
71+
makeTemplateDiagnostic(
72+
'' as TypeCheckId,
73+
this.sourceMapping,
74+
element.sourceSpan,
75+
ts.DiagnosticCategory.Error,
76+
ngErrorCode(ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING),
77+
'Foreign components do not support references.',
78+
),
79+
);
80+
}
81+
if (element.inputs.some((input) => input.type !== BindingType.Property)) {
82+
this.diagnostics.push(
83+
makeTemplateDiagnostic(
84+
'' as TypeCheckId,
85+
this.sourceMapping,
86+
element.sourceSpan,
87+
ts.DiagnosticCategory.Error,
88+
ngErrorCode(ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING),
89+
'Foreign components only support static attributes and property bindings.',
90+
),
91+
);
92+
}
93+
}
94+
95+
super.visitElement(element);
96+
}
97+
}

‎packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ import {getTemplateDiagnostics} from '../../../typecheck';
177177
import {getProjectRelativePath} from '../../../util/src/path';
178178
import {JitDeclarationRegistry} from '../../common/src/jit_declaration_registry';
179179
import {analyzeTemplateForAnimations} from './animations';
180+
import {analyzeForeignComponentFeatures} from './foreign_component';
180181
import {checkCustomElementSelectorForErrors, makeCyclicImportInfo} from './diagnostics';
181182
import {
182183
ComponentAnalysisData,
@@ -863,6 +864,14 @@ export class ComponentDecoratorHandler implements DecoratorHandler<
863864
}
864865
}
865866

867+
const foreignMatcher = createForeignComponentMatcher(foreignImports);
868+
const foreignComponentDiagnostics = analyzeForeignComponentFeatures(template, foreignMatcher);
869+
if (foreignComponentDiagnostics.length > 0) {
870+
isPoisoned = true;
871+
diagnostics ??= [];
872+
diagnostics.push(...foreignComponentDiagnostics);
873+
}
874+
866875
// Figure out the set of styles. The ordering here is important: external resources (styleUrls)
867876
// precede inline styles, and styles defined in the template override styles defined in the
868877
// component.

‎packages/compiler-cli/src/ngtsc/typecheck/template_semantics/src/template_semantics_checker.ts‎

Lines changed: 2 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,12 @@
99
import {
1010
AST,
1111
ASTWithSource,
12-
BindingType,
1312
ImplicitReceiver,
1413
ParsedEventType,
1514
PropertyRead,
1615
Binary,
1716
RecursiveAstVisitor,
1817
TmplAstBoundEvent,
19-
TmplAstElement,
2018
TmplAstLetDeclaration,
2119
TmplAstNode,
2220
TmplAstRecursiveVisitor,
@@ -43,12 +41,7 @@ export class TemplateSemanticsCheckerImpl implements TemplateSemanticsChecker {
4341

4442
/** Visitor that verifies the semantics of a template. */
4543
class TemplateSemanticsVisitor extends TmplAstRecursiveVisitor {
46-
private constructor(
47-
private expressionVisitor: ExpressionsSemanticsVisitor,
48-
private templateTypeChecker: TemplateTypeChecker,
49-
private component: ts.ClassDeclaration,
50-
private diagnostics: TemplateDiagnostic[],
51-
) {
44+
private constructor(private expressionVisitor: ExpressionsSemanticsVisitor) {
5245
super();
5346
}
5447

@@ -63,12 +56,7 @@ class TemplateSemanticsVisitor extends TmplAstRecursiveVisitor {
6356
component,
6457
diagnostics,
6558
);
66-
const templateVisitor = new TemplateSemanticsVisitor(
67-
expressionVisitor,
68-
templateTypeChecker,
69-
component,
70-
diagnostics,
71-
);
59+
const templateVisitor = new TemplateSemanticsVisitor(expressionVisitor);
7260
nodes.forEach((node) => node.visit(templateVisitor));
7361
return diagnostics;
7462
}
@@ -77,51 +65,6 @@ class TemplateSemanticsVisitor extends TmplAstRecursiveVisitor {
7765
super.visitBoundEvent(event);
7866
event.handler.visit(this.expressionVisitor, event);
7967
}
80-
81-
override visitElement(element: TmplAstElement): void {
82-
super.visitElement(element);
83-
84-
const foreignMeta = this.templateTypeChecker.getForeignComponent(this.component, element);
85-
if (foreignMeta !== null) {
86-
this.validateForeignComponent(element);
87-
}
88-
}
89-
90-
private validateForeignComponent(element: TmplAstElement) {
91-
if (element.outputs.length > 0) {
92-
this.diagnostics.push(
93-
this.templateTypeChecker.makeTemplateDiagnostic(
94-
this.component,
95-
element.sourceSpan,
96-
ts.DiagnosticCategory.Error,
97-
ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING,
98-
`Foreign components do not support event bindings.`,
99-
),
100-
);
101-
}
102-
if (element.references.length > 0) {
103-
this.diagnostics.push(
104-
this.templateTypeChecker.makeTemplateDiagnostic(
105-
this.component,
106-
element.sourceSpan,
107-
ts.DiagnosticCategory.Error,
108-
ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING,
109-
`Foreign components do not support references.`,
110-
),
111-
);
112-
}
113-
if (element.inputs.some((input) => input.type !== BindingType.Property)) {
114-
this.diagnostics.push(
115-
this.templateTypeChecker.makeTemplateDiagnostic(
116-
this.component,
117-
element.sourceSpan,
118-
ts.DiagnosticCategory.Error,
119-
ErrorCode.FOREIGN_COMPONENT_UNSUPPORTED_BINDING,
120-
`Foreign components only support static attributes and property bindings.`,
121-
),
122-
);
123-
}
124-
}
12568
}
12669

12770
/** Visitor that verifies the semantics of the expressions within a template. */

0 commit comments

Comments
 (0)