Skip to content

Commit 6fee651

Browse files
committed
refactor(compiler-cli): decouple SymbolBuilder from BoundTarget and minimize adapter surface
Decouple `SymbolBuilder` from the full `BoundTarget` interface by introducing a purpose-built `SymbolBoundTarget` interface containing only the 4 methods required for symbol resolution. This eliminates the need for the large, pass-through `BoundTargetAdapter` and further isolates `SymbolBuilder` from compiler-internal implementation details. Also minimize `TypeCheckableDirectiveMetaAdapter` by redefining `SymbolDirectiveMeta` to not extend `DirectiveMeta`, exposing only the properties actually used by `SymbolBuilder`. Removed dead code `getDirectiveMeta` in `template_symbol_builder.ts` which was unused. These changes improve maintainability and ensure a cleaner architecture by strictly defining the boundaries of what `SymbolBuilder` needs from the rest of the system. By limiting the required inputs to only what's necessary for the implementation, we make it easier to re-use the implementation between different compiler architectures
1 parent a24179e commit 6fee651

14 files changed

Lines changed: 367 additions & 144 deletions

File tree

packages/compiler-cli/private/hybrid_analysis.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export {
2020
type SourceMapping,
2121
type OutOfBandDiagnosticRecorder,
2222
type DomSchemaChecker,
23+
type SymbolReference,
2324
OutOfBadDiagnosticCategory,
2425
SymbolKind,
2526
} from '../src/ngtsc/typecheck/api';
@@ -43,4 +44,8 @@ export {
4344
ExpressionIdentifier,
4445
hasExpressionIdentifier,
4546
} from '../src/ngtsc/typecheck/src/comments';
46-
export {SymbolBuilder} from '../src/ngtsc/typecheck/src/template_symbol_builder';
47+
export {
48+
SymbolBuilder,
49+
SymbolBoundTarget,
50+
SymbolDirectiveMeta,
51+
} from '../src/ngtsc/typecheck/src/template_symbol_builder';

packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,21 @@ export interface TsCompletionEntryInfo {
5555
tsCompletionEntryData?: ts.CompletionEntryData;
5656
}
5757

58+
/**
59+
* A reference to a symbol in a source file, without holding heavy AST nodes.
60+
*/
61+
export interface SymbolReference {
62+
filePath: string;
63+
position: number;
64+
name: string;
65+
moduleSpecifier?: string;
66+
}
67+
5868
/**
5969
* Metadata on a directive which is available in a template.
6070
*/
6171
export interface PotentialDirective {
62-
ref: Reference<ClassDeclaration>;
72+
ref: SymbolReference;
6373

6474
/**
6575
* The module which declares the directive.
@@ -99,7 +109,7 @@ export interface PotentialDirective {
99109
* Metadata for a pipe which is available in a template.
100110
*/
101111
export interface PotentialPipe {
102-
ref: Reference<ClassDeclaration>;
112+
ref: SymbolReference;
103113

104114
/**
105115
* Name of the pipe.

packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,4 +333,7 @@ export interface PipeSymbol {
333333
export interface ClassSymbol {
334334
/** The position for the variable declaration for the class instance. */
335335
tcbLocation: TcbLocation;
336+
337+
/** Whether this class symbol represents a pipe. */
338+
isPipeClassSymbol?: boolean;
336339
}

packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts

Lines changed: 148 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,26 @@
88

99
import {
1010
AST,
11+
BoundTarget,
1112
CssSelector,
1213
DomElementSchemaRegistry,
1314
ExternalExpr,
1415
LiteralPrimitive,
1516
ParseSourceSpan,
1617
PropertyRead,
18+
ReferenceTarget,
1719
SafePropertyRead,
20+
ScopedNode,
21+
Target,
1822
TemplateEntity,
23+
TmplAstBoundAttribute,
24+
TmplAstBoundEvent,
1925
TmplAstComponent,
2026
TmplAstDirective,
2127
TmplAstElement,
2228
TmplAstHostElement,
2329
TmplAstNode,
30+
TmplAstReference,
2431
TmplAstTemplate,
2532
TmplAstTextAttribute,
2633
WrappedNodeExpr,
@@ -86,6 +93,7 @@ import {
8693
SelectorlessDirectiveSymbol,
8794
Symbol,
8895
SymbolKind,
96+
SymbolReference,
8997
TcbLocation,
9098
TemplateDiagnostic,
9199
TemplateSymbol,
@@ -108,9 +116,109 @@ import {shouldReportDiagnostic, translateDiagnostic} from './diagnostics';
108116
import {TypeCheckShimGenerator} from './shim';
109117
import {DirectiveSourceManager} from './source';
110118
import {findTypeCheckBlock, getSourceMapping, TypeCheckSourceResolver} from './tcb_util';
111-
import {SymbolBuilder} from './template_symbol_builder';
119+
import {SymbolBuilder, SymbolDirectiveMeta, SymbolBoundTarget} from './template_symbol_builder';
112120
import {findAllMatchingNodes} from './comments';
113121

122+
export class TypeCheckableDirectiveMetaAdapter implements SymbolDirectiveMeta {
123+
constructor(
124+
private meta: TypeCheckableDirectiveMeta,
125+
private componentScopeReader: ComponentScopeReader,
126+
) {}
127+
128+
getSymbolReference(): SymbolReference {
129+
return {
130+
filePath: this.meta.ref.node.getSourceFile().fileName,
131+
position: this.meta.ref.node.name.getStart(),
132+
name: this.meta.ref.node.name.text,
133+
moduleSpecifier: this.meta.ref.bestGuessOwningModule?.specifier,
134+
};
135+
}
136+
137+
getNgModule(): ClassDeclaration | null {
138+
if (ts.isClassDeclaration(this.meta.ref.node)) {
139+
const scope = this.componentScopeReader.getScopeForComponent(this.meta.ref.node);
140+
if (scope === null || scope.kind !== ComponentScopeKind.NgModule) {
141+
return null;
142+
}
143+
return scope.ngModule;
144+
}
145+
return null;
146+
}
147+
148+
getReferenceTargetNode(): ts.ClassDeclaration | null {
149+
return ts.isClassDeclaration(this.meta.ref.node) ? this.meta.ref.node : null;
150+
}
151+
152+
get selector() {
153+
return this.meta.selector;
154+
}
155+
get isComponent() {
156+
return this.meta.isComponent;
157+
}
158+
get inputs() {
159+
return this.meta.inputs;
160+
}
161+
get outputs() {
162+
return this.meta.outputs;
163+
}
164+
get isStructural() {
165+
return this.meta.isStructural;
166+
}
167+
get hostDirectives() {
168+
return this.meta.hostDirectives;
169+
}
170+
get matchSource() {
171+
return this.meta.matchSource;
172+
}
173+
}
174+
175+
export class BoundTargetAdapter implements SymbolBoundTarget {
176+
constructor(
177+
private delegate: BoundTarget<TypeCheckableDirectiveMeta>,
178+
private componentScopeReader: ComponentScopeReader,
179+
) {}
180+
181+
getDirectivesOfNode(node: TmplAstNode): SymbolDirectiveMeta[] | null {
182+
const dirs = this.delegate.getDirectivesOfNode(node as TmplAstElement | TmplAstTemplate);
183+
return dirs
184+
? dirs.map((d) => new TypeCheckableDirectiveMetaAdapter(d, this.componentScopeReader))
185+
: null;
186+
}
187+
188+
getReferenceTarget(ref: TmplAstReference): ReferenceTarget<SymbolDirectiveMeta> | null {
189+
const target = this.delegate.getReferenceTarget(ref);
190+
if (target === null) return null;
191+
if ('directive' in target) {
192+
return {
193+
directive: new TypeCheckableDirectiveMetaAdapter(
194+
target.directive,
195+
this.componentScopeReader,
196+
),
197+
node: target.node,
198+
};
199+
}
200+
return target;
201+
}
202+
203+
getConsumerOfBinding(
204+
binding: TmplAstBoundAttribute | TmplAstBoundEvent | TmplAstTextAttribute,
205+
): SymbolDirectiveMeta | TmplAstElement | TmplAstTemplate | null {
206+
const consumer = this.delegate.getConsumerOfBinding(binding);
207+
if (consumer === null) return null;
208+
if (typeof consumer === 'object' && 'ref' in consumer) {
209+
return new TypeCheckableDirectiveMetaAdapter(
210+
consumer as TypeCheckableDirectiveMeta,
211+
this.componentScopeReader,
212+
);
213+
}
214+
return consumer;
215+
}
216+
217+
getExpressionTarget(expr: AST) {
218+
return this.delegate.getExpressionTarget(expr);
219+
}
220+
}
221+
114222
function getTcbLocationForSymbol(symbol: Symbol | BindingSymbol | ClassSymbol): TcbLocation | null {
115223
if ('tcbLocation' in symbol && symbol.tcbLocation !== undefined) {
116224
return symbol.tcbLocation as TcbLocation;
@@ -255,21 +363,8 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
255363

256364
const typeChecker = this.programDriver.getProgram().getTypeChecker();
257365

258-
if (
259-
'kind' in symbol &&
260-
(symbol.kind === SymbolKind.Directive ||
261-
symbol.kind === SymbolKind.SelectorlessDirective ||
262-
symbol.kind === SymbolKind.SelectorlessComponent)
263-
) {
264-
const refNode = (symbol as any).ref?.node;
265-
if (refNode) {
266-
const tsSymbol = typeChecker.getSymbolAtLocation(refNode.name ?? refNode);
267-
if (tsSymbol) return tsSymbol;
268-
}
269-
}
270-
271366
if ('kind' in symbol && symbol.kind === SymbolKind.Reference) {
272-
if ((symbol.target as any).kind && ts.isClassDeclaration(symbol.target as ts.Node)) {
367+
if (ts.isClassDeclaration(symbol.target as ts.Node)) {
273368
const targetNode = symbol.target as ts.ClassDeclaration;
274369
const tsSymbol = typeChecker.getSymbolAtLocation(targetNode.name ?? targetNode);
275370
if (tsSymbol) return tsSymbol;
@@ -281,7 +376,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
281376
// which will get the symbol of the local variable (e.g. _t4).
282377
}
283378

284-
if ('isPipeClassSymbol' in symbol && (symbol as any).isPipeClassSymbol) {
379+
if ('isPipeClassSymbol' in symbol && symbol.isPipeClassSymbol) {
285380
const type = typeChecker.getTypeAtLocation(node);
286381
if (type && type.getSymbol()) return type.getSymbol() || null;
287382
}
@@ -955,8 +1050,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
9551050
tcbPath,
9561051
tcbIsShim,
9571052
tcb,
958-
data,
959-
this.componentScopeReader,
1053+
new BoundTargetAdapter(data.boundTarget, this.componentScopeReader),
9601054
this.config,
9611055
);
9621056
this.symbolBuilderCache.set(component, builder);
@@ -971,6 +1065,14 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
9711065
return engine.getGlobalTsContext();
9721066
}
9731067

1068+
private getRefKey(ref: Reference<ClassDeclaration> | SymbolReference): string {
1069+
if ('filePath' in ref) {
1070+
return `${ref.filePath}#${ref.position}`;
1071+
} else {
1072+
return `${ref.node.getSourceFile().fileName}#${ref.node.name!.getStart()}`;
1073+
}
1074+
}
1075+
9741076
getPotentialTemplateDirectives(
9751077
component: ts.ClassDeclaration,
9761078
tsLs: ts.LanguageService,
@@ -983,14 +1085,15 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
9831085
return [];
9841086
}
9851087

986-
const resultingDirectives = new Map<ClassDeclaration<DeclarationNode>, PotentialDirective>();
1088+
const resultingDirectives = new Map<string, PotentialDirective>();
9871089
const directivesInScope = this.getTemplateDirectiveInScope(component);
9881090
const directiveInGlobal = this.getElementsInGlobal(component, tsLs, options);
9891091
for (const directive of [...directivesInScope, ...directiveInGlobal]) {
990-
if (resultingDirectives.has(directive.ref.node)) {
1092+
const key = this.getRefKey(directive.ref);
1093+
if (resultingDirectives.has(key)) {
9911094
continue;
9921095
}
993-
resultingDirectives.set(directive.ref.node, directive);
1096+
resultingDirectives.set(key, directive);
9941097
}
9951098
return Array.from(resultingDirectives.values());
9961099
}
@@ -1005,20 +1108,20 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
10051108

10061109
// Very similar to the above `getPotentialTemplateDirectives`, but on pipes.
10071110
const typeChecker = this.programDriver.getProgram().getTypeChecker();
1008-
const resultingPipes = new Map<ClassDeclaration<DeclarationNode>, PotentialPipe>();
1111+
const resultingPipes = new Map<string, PotentialPipe>();
10091112
if (scope !== null) {
10101113
const inScopePipes = this.getScopeData(component, scope)?.pipes ?? [];
10111114
for (const p of inScopePipes) {
1012-
resultingPipes.set(p.ref.node, p);
1115+
resultingPipes.set(this.getRefKey(p.ref), p);
10131116
}
10141117
}
10151118
for (const pipeClass of this.localMetaReader.getKnown(MetaKind.Pipe)) {
10161119
const pipeMeta = this.metaReader.getPipeMetadata(new Reference(pipeClass));
10171120
if (pipeMeta === null) continue;
1018-
if (resultingPipes.has(pipeClass)) continue;
1121+
if (resultingPipes.has(this.getRefKey(new Reference(pipeClass)))) continue;
10191122
const withScope = this.scopeDataOfPipeMeta(typeChecker, pipeMeta);
10201123
if (withScope === null) continue;
1021-
resultingPipes.set(pipeClass, {...withScope, isInScope: false});
1124+
resultingPipes.set(this.getRefKey(withScope.ref), {...withScope, isInScope: false});
10221125
}
10231126
return Array.from(resultingPipes.values());
10241127
}
@@ -1045,7 +1148,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
10451148
}
10461149

10471150
getTemplateDirectiveInScope(component: ts.ClassDeclaration): PotentialDirective[] {
1048-
const resultingDirectives = new Map<ClassDeclaration<DeclarationNode>, PotentialDirective>();
1151+
const resultingDirectives = new Map<string, PotentialDirective>();
10491152

10501153
const scope = this.getComponentScope(component);
10511154

@@ -1058,7 +1161,7 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
10581161
const inScopeDirectives = this.getScopeData(component, scope)?.directives ?? [];
10591162
// First, all in scope directives can be used.
10601163
for (const d of inScopeDirectives) {
1061-
resultingDirectives.set(d.ref.node, d);
1164+
resultingDirectives.set(this.getRefKey(d.ref), d);
10621165
}
10631166
}
10641167

@@ -1073,12 +1176,14 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
10731176
if (directiveClass.getSourceFile().fileName !== currentComponentFileName) {
10741177
continue;
10751178
}
1076-
const directiveMeta = this.metaReader.getDirectiveMetadata(new Reference(directiveClass));
1179+
const ref = new Reference(directiveClass);
1180+
const directiveMeta = this.metaReader.getDirectiveMetadata(ref);
10771181
if (directiveMeta === null) continue;
1078-
if (resultingDirectives.has(directiveClass)) continue;
1182+
const key = this.getRefKey(ref);
1183+
if (resultingDirectives.has(key)) continue;
10791184
const withScope = this.scopeDataOfDirectiveMeta(typeChecker, directiveMeta);
10801185
if (withScope === null) continue;
1081-
resultingDirectives.set(directiveClass, {...withScope, isInScope: false});
1186+
resultingDirectives.set(key, {...withScope, isInScope: false});
10821187
}
10831188

10841189
return Array.from(resultingDirectives.values());
@@ -1548,7 +1653,12 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
15481653
}
15491654

15501655
return {
1551-
ref: dep.ref,
1656+
ref: {
1657+
filePath: dep.ref.node.getSourceFile().fileName,
1658+
position: dep.ref.node.name!.getStart(),
1659+
name: dep.ref.node.name!.text,
1660+
moduleSpecifier: dep.ref.bestGuessOwningModule?.specifier,
1661+
},
15521662
isComponent: dep.isComponent,
15531663
isStructural: dep.isStructural,
15541664
selector: dep.selector,
@@ -1561,12 +1671,16 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
15611671
typeChecker: ts.TypeChecker,
15621672
dep: PipeMeta,
15631673
): Omit<PotentialPipe, 'isInScope'> | null {
1564-
const tsSymbol = typeChecker.getSymbolAtLocation(dep.ref.node.name);
1565-
if (tsSymbol === undefined) {
1674+
if (!dep.ref.node.name) {
15661675
return null;
15671676
}
15681677
return {
1569-
ref: dep.ref,
1678+
ref: {
1679+
filePath: dep.ref.node.getSourceFile().fileName,
1680+
position: dep.ref.node.name!.getStart(),
1681+
name: dep.ref.node.name!.text,
1682+
moduleSpecifier: dep.ref.bestGuessOwningModule?.specifier,
1683+
},
15701684
name: dep.name,
15711685
tsCompletionEntryInfos: null,
15721686
};

0 commit comments

Comments
 (0)